Refactor and add new data for jobs
This change fixes stuff, refactors others and adds some new data columns.
Basically this change is for backend improvements.
This change does the following:
1. Change `categoryId` to `jobId`
2. Re-purpose `category` to represent a job category to help organize
jobs.
3. Changed `verifier` column to `reporter` since i feel the word
`verify` and it's derivitives
(verified, verification, verifier etc..) is overly used.
4. Add the following columns:
duration - the time for the job run
abstain - whether pass/fail result should count as a vote
5. Fix up some REST endpoint bugs
6. Update docs to reflect this refactoring
Change-Id: Ibbfb059bbca5144d67fd00143e9a7846eed442af
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/client/BuildsDropDownPanel.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/client/BuildsDropDownPanel.java
index 3536dfa..d967ba3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/client/BuildsDropDownPanel.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/client/BuildsDropDownPanel.java
@@ -48,7 +48,7 @@
.get(new AsyncCallback<NativeMap<VerificationInfo>>() {
@Override
public void onSuccess(NativeMap<VerificationInfo> map) {
- map.copyKeysIntoChildren("category");
+ map.copyKeysIntoChildren("job");
// TODO only rendern when not empty
panel.setWidget(new BuildsDropDownPanel());
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/client/VerificationInfo.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/client/VerificationInfo.java
index 1fd7d67..9c3ed37 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/client/VerificationInfo.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/client/VerificationInfo.java
@@ -1,13 +1,34 @@
package com.googlesource.gerrit.plugins.verifystatus.client;
import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer;
+
+import java.sql.Timestamp;
public class VerificationInfo extends JavaScriptObject {
- public final native String category() /*-{ return this.category; }-*/;
public final native String url() /*-{ return this.url; }-*/;
- public final native String granted() /*-{ return this.granted; }-*/;
- public final native int value() /*-{ return this.value; }-*/;
+ public final native String comment() /*-{ return this.comment; }-*/;
+ public final native short value() /*-{ return this.value; }-*/;
+ public final native boolean abstain() /*-{ return this.abstain || false; }-*/;
+ public final native String category() /*-{ return this.category; }-*/;
+ public final native String duration() /*-{ return this.duration; }-*/;
+ public final native String reporter() /*-{ return this.reporter; }-*/;
+
+ public final Timestamp granted() {
+ Timestamp r = grantedTimestamp();
+ if (r == null) {
+ String s = grantedRaw();
+ if (s != null) {
+ r = JavaSqlTimestamp_JsonSerializer.parseTimestamp(s);
+ grantedTimestamp(r);
+ }
+ }
+ return r;
+ }
+ private final native String grantedRaw() /*-{ return this.granted; }-*/;
+ private final native Timestamp grantedTimestamp() /*-{ return this._ts }-*/;
+ private final native void grantedTimestamp(Timestamp t) /*-{ this._ts = t }-*/;
protected VerificationInfo() {
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/commands/SaveCommand.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/commands/SaveCommand.java
index bda9b8f..fc694fb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/commands/SaveCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/commands/SaveCommand.java
@@ -94,16 +94,20 @@
throw new IllegalArgumentException(String.valueOf("Invalid verification parameters"));
}
- String category = params.get("category");
- checkArgument(category != null, "Verification is missing a category");
+ String job = params.get("job");
+ checkArgument(job != null, "Verification is missing a job");
String value = params.get("value");
checkArgument(value != null, "Verification is missing a value");
+ String abstain = params.get("abstain");
VerificationInfo data = new VerificationInfo();
data.value = Short.parseShort(value);
+ data.abstain = Boolean.valueOf(abstain);
data.url = params.get("url");
- data.verifier = params.get("verifier");
+ data.reporter = params.get("reporter");
data.comment = params.get("comment");
- jobResult.put(category, data);
+ data.category = params.get("category");
+ data.duration = params.get("duration");
+ jobResult.put(job, data);
}
@Inject
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/common/VerificationInfo.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/common/VerificationInfo.java
index b0ee136..b9b563f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/common/VerificationInfo.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/common/VerificationInfo.java
@@ -14,10 +14,15 @@
package com.googlesource.gerrit.plugins.verifystatus.common;
+import java.sql.Timestamp;
+
public class VerificationInfo {
public String url;
public Short value;
- public String verifier;
+ public boolean abstain;
+ public String reporter;
public String comment;
- public String granted;
+ public Timestamp granted;
+ public String category;
+ public String duration;
}
\ No newline at end of file
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/GetVerifications.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/GetVerifications.java
index 661e337..4c4d2ee 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/GetVerifications.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/GetVerifications.java
@@ -25,14 +25,10 @@
import com.googlesource.gerrit.plugins.verifystatus.common.VerificationInfo;
import java.io.IOException;
-import java.text.SimpleDateFormat;
-import java.util.Locale;
import java.util.Map;
@Singleton
public class GetVerifications implements RestReadView<RevisionResource> {
- private static final SimpleDateFormat DATE_FORMAT =
- new SimpleDateFormat("d MMM yyyy HH:mm:ss", Locale.US);
private final SchemaFactory<CiDb> schemaFactory;
@Inject
@@ -49,11 +45,14 @@
.byPatchSet(rsrc.getPatchSet().getId())) {
VerificationInfo info = new VerificationInfo();
info.value = v.getValue();
+ info.abstain = v.getAbstain();
info.url = v.getUrl();
- info.verifier = v.getVerifier();
+ info.reporter = v.getReporter();
info.comment = v.getComment();
- info.granted = DATE_FORMAT.format(v.getGranted());
- out.put(v.getLabelId().get(), info);
+ info.granted = v.getGranted();
+ info.category = v.getCategory();
+ info.duration = v.getDuration();
+ out.put(v.getJobId().get(), info);
}
}
return out;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/PatchSetVerification.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/PatchSetVerification.java
index fe368f7..b4b3f38 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/PatchSetVerification.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/PatchSetVerification.java
@@ -31,16 +31,16 @@
protected PatchSet.Id patchSetId;
@Column(id = 2)
- protected LabelId categoryId;
+ protected LabelId jobId;
protected Key() {
patchSetId = new PatchSet.Id();
- categoryId = new LabelId();
+ jobId = new LabelId();
}
public Key(PatchSet.Id ps, LabelId c) {
this.patchSetId = ps;
- this.categoryId = c;
+ this.jobId = c;
}
@Override
@@ -49,12 +49,12 @@
}
public LabelId getLabelId() {
- return categoryId;
+ return jobId;
}
@Override
public com.google.gwtorm.client.Key<?>[] members() {
- return new com.google.gwtorm.client.Key<?>[] {categoryId};
+ return new com.google.gwtorm.client.Key<?>[] {jobId};
}
}
@@ -71,11 +71,20 @@
protected String url;
@Column(id = 5, notNull = false, length = 255)
- protected String verifier;
+ protected String reporter;
@Column(id = 6, notNull = false, length = 255)
protected String comment;
+ @Column(id = 7, notNull = false, length = 255)
+ protected String category;
+
+ @Column(id = 8, notNull = false, length = 255)
+ protected String duration;
+
+ @Column(id = 9)
+ protected boolean abstain;
+
protected PatchSetVerification() {
}
@@ -94,8 +103,8 @@
return key.patchSetId;
}
- public LabelId getLabelId() {
- return key.categoryId;
+ public LabelId getJobId() {
+ return key.jobId;
}
public short getValue() {
@@ -106,6 +115,14 @@
value = v;
}
+ public boolean getAbstain() {
+ return abstain;
+ }
+
+ public void setAbstain(boolean a) {
+ abstain = a;
+ }
+
public Timestamp getGranted() {
return granted;
}
@@ -114,8 +131,8 @@
granted = ts;
}
- public String getLabel() {
- return getLabelId().get();
+ public String getJob() {
+ return getJobId().get();
}
public String getUrl() {
@@ -126,12 +143,12 @@
this.url = url;
}
- public String getVerifier() {
- return verifier;
+ public String getReporter() {
+ return reporter;
}
- public void setVerifier(String reporter) {
- this.verifier = reporter;
+ public void setReporter(String reporter) {
+ this.reporter = reporter;
}
public String getComment() {
@@ -142,6 +159,22 @@
this.comment = comment;
}
+ public String getCategory() {
+ return category;
+ }
+
+ public void setCategory(String category) {
+ this.category = category;
+ }
+
+ public String getDuration() {
+ return duration;
+ }
+
+ public void setDuration(String duration) {
+ this.duration = duration;
+ }
+
@Override
public String toString() {
return new StringBuilder().append('[').append(key).append(": ")
@@ -152,8 +185,7 @@
public boolean equals(Object o) {
if (o instanceof PatchSetVerification) {
PatchSetVerification p = (PatchSetVerification) o;
- return Objects.equals(key, p.key)
- && Objects.equals(value, p.value)
+ return Objects.equals(key, p.key) && Objects.equals(value, p.value)
&& Objects.equals(granted, p.granted);
}
return false;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/PostVerification.java b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/PostVerification.java
index 82a669f..d10c3d0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/PostVerification.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/verifystatus/server/PostVerification.java
@@ -96,31 +96,44 @@
if (c != null) {
c.setGranted(ts);
c.setValue(value);
+ if (Boolean.TRUE.equals(ent.getValue().abstain)) {
+ c.setAbstain(true);
+ }
String url = ent.getValue().url;
if (url != null) {
c.setUrl(url);
}
- String verifier = ent.getValue().verifier;
- if (verifier != null) {
- c.setVerifier(verifier);
+ String reporter = ent.getValue().reporter;
+ if (reporter != null) {
+ c.setReporter(reporter);
}
String comment = ent.getValue().comment;
if (comment != null) {
c.setComment(comment);
}
- log.info("Updating job " + c.getLabel() + " for change "
+ String category = ent.getValue().category;
+ if (category != null) {
+ c.setCategory(category);
+ }
+ String duration = ent.getValue().duration;
+ if (duration != null) {
+ c.setDuration(duration);
+ }
+ log.info("Updating job " + c.getJob() + " for change "
+ c.getPatchSetId());
ups.add(c);
} else {
c = new PatchSetVerification(new PatchSetVerification.Key(
resource.getPatchSet().getId(),
new LabelId(name)),
- value, TimeUtil.nowTs());
- c.setGranted(ts);
+ value, ts);
+ c.setAbstain(ent.getValue().abstain);
c.setUrl(ent.getValue().url);
- c.setVerifier(ent.getValue().verifier);
+ c.setReporter(ent.getValue().reporter);
c.setComment(ent.getValue().comment);
- log.info("Adding job " + c.getLabel() + " for change "
+ c.setCategory(ent.getValue().category);
+ c.setDuration(ent.getValue().duration);
+ log.info("Adding job " + c.getJob() + " for change "
+ c.getPatchSetId());
ups.add(c);
}
@@ -136,7 +149,7 @@
Map<String, PatchSetVerification> current = Maps.newHashMap();
for (PatchSetVerification v : db.patchSetVerifications()
.byPatchSet(resource.getPatchSet().getId())) {
- current.put(v.getLabelId().get(), v);
+ current.put(v.getJobId().get(), v);
}
return current;
}
diff --git a/src/main/resources/Documentation/cmd-save.md b/src/main/resources/Documentation/cmd-save.md
index 7bf1eae..cc13ec3 100644
--- a/src/main/resources/Documentation/cmd-save.md
+++ b/src/main/resources/Documentation/cmd-save.md
@@ -77,11 +77,13 @@
> $ ssh -p 29418 review.example.com @PLUGIN@ save --verification
-> "'category=gate-horizon-pep8
+> "'job=gate-horizon-pep8
> |value=1
> |url=https://ci.host.com/jobs/pep8/4711
-> |verifier=Jenkins
-> |comment=Non Voting'"
+> |reporter=Jenkins CI
+> |comment=Experimental
+> |category=gate
+> |duration=1m 30s'"
> 14a95001c
diff --git a/src/main/resources/Documentation/rest-api-changes.md b/src/main/resources/Documentation/rest-api-changes.md
index db5410e..44917de 100644
--- a/src/main/resources/Documentation/rest-api-changes.md
+++ b/src/main/resources/Documentation/rest-api-changes.md
@@ -49,26 +49,33 @@
)]}'
{
- "gate-horizon-pep8": {
+ "check-horizon-pep8": {
"url": "https://ci.host.com/jobs/gate-horizon-pep8/1711",
"value": 1,
- "verifier": "Jenkins",
- "comment": "Non Voting",
- "granted": "15 Mar 2016 08:10:41"
+ "reporter": "Jenkins CI",
+ "comment": "Experimental",
+ "granted": "15 Mar 2016 08:10:41",
+ "category": "check",
+ "duration": "1m 30s"
},
"gate-horizon-python27": {
"url": "https://ci.host.com/jobs/gate-horizon-python27/1711",
"value": 1,
- "verifier": "Jenkins",
- "comment": "Passed",
+ "abstain": true,
+ "reporter": "Acme CI",
+ "comment": "Informational only",
"granted": "15 Mar 2016 08:30:16"
+ "category": "gate",
+ "duration": "2m 40s",
}
"gate-horizon-python34": {
"url": "https://ci.host.com/jobs/gate-horizon-python34/1711",
"value": -1,
- "verifier": "Jenkins",
- "comment": "Failed",
- "granted": "15 Mar 2016 08:40:23"
+ "reporter": "Drone CI",
+ "comment": "RuntimeError: File was not found",
+ "granted": "15 Mar 2016 08:40:23",
+ "category": "gate",
+ "duration": "12m 20s"
}
}
```
@@ -107,14 +114,20 @@
"gate-horizon-python27": {
"url": "https://ci.host.com/jobs/gate-horizon-python27/1711",
"value": 1,
- "verifier": "Jenkins",
- "comment": "Passed"
+ "abstain": true,
+ "reporter": "Acme CI",
+ "comment": "Informational only",
+ "category": "gate",
+ "duration": "2m 40s"
},
"gate-horizon-python34": {
"url": "https://ci.host.com/jobs/gate-horizon-python34/1711",
"value": -1,
- "verifier": "Jenkins",
- "comment": "Failed"
+ "abstain": false,
+ "reporter": "Drone CI",
+ "comment": "RuntimeError: File was not found",
+ "category": "gate",
+ "duration": "12m 20s"
}
}
}
@@ -132,12 +145,13 @@
|Field Name | |Description|
|:-----------|:----|:----------|
-|category |required|The name of the category to be added as a verification|
-|value |required|The value associated with the category|
-|comment |optional|The comment associated with the category|
-|url |optional|The url associated with the category|
-|verifier |optional|The user that verified the revision|
-
+|value |required|The pass/fail result for this job|
+|abstain |optional|Whether the value counts as a vote (defaults to false)|
+|comment |optional|A short comment about this job|
+|url |optional|The url link to more info about this job|
+|reporter |optional|The user that verified this job|
+|category |optional|A category for this job|
+|duration |optional|The time it took to run this job|
### <a id="revision-info"></a>RevisionInfo
@@ -158,10 +172,14 @@
|Field Name |Description|
|:----------|:----------|
-|comment |A short comment about about this verification|
-|url |The URL for this verification|
-|value |The value for this verification|
-|verifier |The user that reported this verification|
+|value |The pass/fail result for this job|
+|abstain |Whether the value counts as a vote|
+|comment |A short comment about this job|
+|url |The url link to more info about this job|
+|reporter |The user that verified this job|
+|category |A category for this job|
+|duration |The time it took to run this job|
+|granted |The date this verification was recorded|
