Merge "Document adding and removal of included groups via REST"
diff --git a/Documentation/database-setup.txt b/Documentation/database-setup.txt
index 2652952..0465d62 100644
--- a/Documentation/database-setup.txt
+++ b/Documentation/database-setup.txt
@@ -33,7 +33,7 @@
full rights on the newly created database:
----
- $ createuser --username=postgres -A -D -P -E gerrit2
+ $ createuser --username=postgres -S -R -D -P -E gerrit2
$ createdb --username=postgres -E UTF-8 -O gerrit2 reviewdb
----
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 3695d6a..6634915 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -140,6 +140,10 @@
* `LABELS`: a summary of each label required for submit, and
approvers that have granted (or rejected) with that label.
+* `DETAILED_LABELS`: detailed label information, including numeric
+ values of all existing approvals, recognized label values, and
+ values permitted to be set by the current user.
+
* `CURRENT_REVISION`: describe the current revision (patch set)
of the change, including the commit SHA-1 and URLs to fetch from.
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java b/gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java
index 5072b76..e3f9236 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java
@@ -19,6 +19,7 @@
/** Output options available when using {@code /changes/} RPCs. */
public enum ListChangesOption {
LABELS(0),
+ DETAILED_LABELS(8),
/** Return information on the current patch set of the change. */
CURRENT_REVISION(1),
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java
index cf4b28b..7637f75 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java
@@ -14,7 +14,6 @@
package com.google.gerrit.common.data;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -123,9 +122,9 @@
return null;
}
- public PatchSetApproval getChangeApproval(ApprovalCategory.Id id) {
+ public PatchSetApproval getChangeApproval(String label) {
for (PatchSetApproval a : given) {
- if (a.getCategoryId().equals(id)) {
+ if (a.getLabel() != null && a.getLabel().equals(label)) {
return a;
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
index 5341d9f..115c0c1 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java
@@ -16,6 +16,7 @@
import com.google.gerrit.client.rpc.NativeString;
import com.google.gerrit.client.rpc.RestApi;
+import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.user.client.rpc.AsyncCallback;
@@ -60,6 +61,10 @@
}
}
+ public static RestApi revision(PatchSet.Id id) {
+ return change(id.getParentKey().get()).view("revisions").id(id.get());
+ }
+
/** Submit a specific revision of a change. */
public static void submit(int id, String commit, AsyncCallback<SubmitInfo> cb) {
SubmitInput in = SubmitInput.create();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
index 112c42f..f857169 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
@@ -14,17 +14,27 @@
package com.google.gerrit.client.changes;
+import com.google.gerrit.client.rpc.NativeMap;
+import com.google.gerrit.client.rpc.NativeString;
import com.google.gerrit.client.rpc.Natives;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwt.core.client.JsArray;
+import com.google.gwt.core.client.JsArrayString;
import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer;
import java.sql.Timestamp;
import java.util.Set;
public class ChangeInfo extends JavaScriptObject {
+ public final void init() {
+ if (labels0() != null) {
+ labels0().copyKeysIntoChildren("_name");
+ }
+ }
+
public final Project.NameKey project_name_key() {
return new Project.NameKey(project());
}
@@ -58,7 +68,7 @@
}
public final Set<String> labels() {
- return Natives.keys(labels0());
+ return labels0().keySet();
}
public final native String id() /*-{ return this.id; }-*/;
@@ -74,8 +84,17 @@
public final native boolean starred() /*-{ return this.starred ? true : false; }-*/;
public final native boolean reviewed() /*-{ return this.reviewed ? true : false; }-*/;
public final native String _sortkey() /*-{ return this._sortkey; }-*/;
- private final native JavaScriptObject labels0() /*-{ return this.labels; }-*/;
+ private final native NativeMap<LabelInfo> labels0() /*-{ return this.labels; }-*/;
public final native LabelInfo label(String n) /*-{ return this.labels[n]; }-*/;
+
+ private final native NativeMap<JavaScriptObject> _permitted_labels()
+ /*-{ return this.permitted_labels; }-*/;
+ public final Set<String> permitted_labels() {
+ return Natives.keys(_permitted_labels());
+ }
+ public final native JsArrayString permitted_values(String n)
+ /*-{ return this.permitted_labels[n]; }-*/;
+
final native int _number() /*-{ return this._number; }-*/;
final native boolean _more_changes()
/*-{ return this._more_changes ? true : false; }-*/;
@@ -109,6 +128,15 @@
public final native AccountInfo recommended() /*-{ return this.recommended; }-*/;
public final native AccountInfo disliked() /*-{ return this.disliked; }-*/;
+
+ public final native JsArray<ApprovalInfo> all() /*-{ return this.all; }-*/;
+
+ private final native NativeMap<NativeString> _values() /*-{ return this.values; }-*/;
+ public final Set<String> values() {
+ return Natives.keys(_values());
+ }
+ public final native String value_text(String n) /*-{ return this.values[n]; }-*/;
+
public final native boolean optional() /*-{ return this.optional ? true : false; }-*/;
final native short _value()
/*-{
@@ -121,4 +149,14 @@
protected LabelInfo() {
}
}
+
+ public static class ApprovalInfo extends JavaScriptObject {
+ public final native int _account_id() /*-{ return this._account_id || 0; }-*/;
+ public final native String name() /*-{ return this.name; }-*/;
+ public final native String email() /*-{ return this.email; }-*/;
+ public final native short value() /*-{ return this.value; }-*/;
+
+ protected ApprovalInfo() {
+ }
+ }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
index 0b2a148..ecb9a04 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
@@ -15,11 +15,14 @@
package com.google.gerrit.client.changes;
import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.client.changes.ChangeInfo.ApprovalInfo;
+import com.google.gerrit.client.changes.ChangeInfo.LabelInfo;
import com.google.gerrit.client.patches.AbstractPatchContentTable;
import com.google.gerrit.client.patches.CommentEditorContainer;
import com.google.gerrit.client.patches.CommentEditorPanel;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.GerritCallback;
+import com.google.gerrit.client.rpc.Natives;
import com.google.gerrit.client.rpc.RestApi;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
import com.google.gerrit.client.ui.AccountScreen;
@@ -27,20 +30,16 @@
import com.google.gerrit.client.ui.PatchLink;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.common.PageLinks;
-import com.google.gerrit.common.data.ApprovalType;
-import com.google.gerrit.common.data.ApprovalTypes;
import com.google.gerrit.common.data.PatchSetPublishDetail;
-import com.google.gerrit.common.data.PermissionRange;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
-import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwt.core.client.JsArrayString;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
+import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.FormPanel;
@@ -78,6 +77,7 @@
private Button cancel;
private boolean saveStateOnUnload = true;
private List<CommentEditorPanel> commentEditors;
+ private ChangeInfo change;
public PublishCommentScreen(final PatchSet.Id psi) {
patchSetId = psi;
@@ -148,6 +148,19 @@
super.onLoad();
CallbackGroup cbs = new CallbackGroup();
+ ChangeApi.revision(patchSetId).view("review").get(cbs.add(
+ new AsyncCallback<ChangeInfo>() {
+ @Override
+ public void onSuccess(ChangeInfo result) {
+ result.init();
+ change = result;
+ }
+
+ @Override
+ public void onFailure(Throwable caught) {
+ // Handled by ScreenLoadCallback.onFailure().
+ }
+ }));
Util.DETAIL_SVC.patchSetPublishDetail(patchSetId, cbs.addGwtjsonrpc(
new ScreenLoadCallback<PatchSetPublishDetail>(this) {
@Override
@@ -230,47 +243,48 @@
}
private void initApprovals(final PatchSetPublishDetail r, final Panel body) {
- ApprovalTypes types = Gerrit.getConfig().getApprovalTypes();
- for (PermissionRange range : r.getLabels()) {
- ApprovalType type = types.byLabel(range.getLabel());
- if (type != null) {
- // Legacy type, use radio buttons.
- initApprovalType(r, body, type, range);
- } else {
- // TODO Newer style label.
- }
+ for (String labelName : change.labels()) {
+ initLabel(r, labelName, body);
}
}
- private void initApprovalType(final PatchSetPublishDetail r,
- final Panel body, final ApprovalType ct, final PermissionRange range) {
- body.add(new SmallHeading(ct.getCategory().getName() + ":"));
+ private void initLabel(PatchSetPublishDetail r, String labelName,
+ Panel body) {
+ JsArrayString nativeValues = change.permitted_values(labelName);
+ if (nativeValues == null || nativeValues.length() == 0) {
+ return;
+ }
+ List<String> values = new ArrayList<String>(nativeValues.length());
+ for (int i = 0; i < nativeValues.length(); i++) {
+ values.add(nativeValues.get(i));
+ }
+ Collections.reverse(values);
+ LabelInfo label = change.label(labelName);
- final VerticalPanel vp = new VerticalPanel();
+ body.add(new SmallHeading(label.name() + ":"));
+
+ VerticalPanel vp = new VerticalPanel();
vp.setStyleName(Gerrit.RESOURCES.css().approvalCategoryList());
- final List<ApprovalCategoryValue> lst =
- new ArrayList<ApprovalCategoryValue>(ct.getValues());
- Collections.reverse(lst);
- final ApprovalCategory.Id catId = ct.getCategory().getId();
- final PatchSetApproval prior = r.getChangeApproval(catId);
- for (final ApprovalCategoryValue buttonValue : lst) {
- if (!range.contains(buttonValue.getValue())) {
- continue;
+ Short prior = null;
+ for (ApprovalInfo app : Natives.asList(label.all())) {
+ if (app._account_id() == Gerrit.getUserAccount().getId().get()) {
+ prior = app.value();
+ break;
}
+ }
- ValueRadioButton b = new ValueRadioButton(ct.getCategory(), buttonValue);
- SafeHtml buf = new SafeHtmlBuilder().append(buttonValue.format());
+ for (String value : values) {
+ ValueRadioButton b = new ValueRadioButton(label, value);
+ SafeHtml buf = new SafeHtmlBuilder().append(b.format());
buf = CommentLinkProcessor.apply(buf);
SafeHtml.set(b, buf);
if (lastState != null && patchSetId.equals(lastState.patchSetId)
- && lastState.approvals.containsKey(buttonValue.getCategoryId())) {
- b.setValue(lastState.approvals.get(buttonValue.getCategoryId()).equals(
- buttonValue));
+ && lastState.approvals.containsKey(label.name())) {
+ b.setValue(lastState.approvals.get(label.name()) == value);
} else {
- b.setValue(prior != null ? buttonValue.getValue() == prior.getValue()
- : buttonValue.getValue() == 0);
+ b.setValue(b.parseValue() == (prior != null ? prior : 0));
}
approvalButtons.add(b);
@@ -366,7 +380,7 @@
data.init();
for (final ValueRadioButton b : approvalButtons) {
if (b.getValue()) {
- data.label(b.category.getLabelName(), b.value.getValue());
+ data.label(b.label.name(), b.parseValue());
}
}
@@ -435,29 +449,45 @@
Gerrit.display(PageLinks.toChange(ck), new ChangeScreen(ck));
}
- private static class ValueRadioButton extends RadioButton {
- final ApprovalCategory category;
- final ApprovalCategoryValue value;
+ private static short parseLabelValue(String value) {
+ if (value.charAt(0) == ' ' || value.charAt(0) == '+') {
+ value = value.substring(1);
+ }
+ return Short.parseShort(value);
+ }
- ValueRadioButton(ApprovalCategory c, ApprovalCategoryValue v) {
- super(c.getLabelName());
- category = c;
- value = v;
+ private static class ValueRadioButton extends RadioButton {
+ final LabelInfo label;
+ final String value;
+
+ ValueRadioButton(LabelInfo label, String value) {
+ super(label.name());
+ this.label = label;
+ this.value = value;
+ }
+
+ String format() {
+ return new StringBuilder().append(value).append(' ')
+ .append(label.value_text(value)).toString();
+ }
+
+ short parseValue() {
+ return parseLabelValue(value);
}
}
private static class SavedState {
final PatchSet.Id patchSetId;
final String message;
- final Map<ApprovalCategory.Id, ApprovalCategoryValue> approvals;
+ final Map<String, String> approvals;
SavedState(final PublishCommentScreen p) {
patchSetId = p.patchSetId;
message = p.message.getText();
- approvals = new HashMap<ApprovalCategory.Id, ApprovalCategoryValue>();
+ approvals = new HashMap<String, String>();
for (final ValueRadioButton b : p.approvalButtons) {
if (b.getValue()) {
- approvals.put(b.value.getCategoryId(), b.value);
+ approvals.put(b.label.name(), b.value);
}
}
}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java
index b7761c5..46e9892 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/ApprovalCategoryValue.java
@@ -88,21 +88,26 @@
name = n;
}
- public String formatValue() {
- if (getValue() < 0) {
- return Short.toString(getValue());
- } else if (getValue() == 0) {
+ public static String formatValue(short value) {
+ if (value < 0) {
+ return Short.toString(value);
+ } else if (value == 0) {
return " 0";
} else {
- return "+" + Short.toString(getValue());
+ return "+" + Short.toString(value);
}
}
+ public String formatValue() {
+ return formatValue(getValue());
+ }
+
+ public static String format(String name, short value) {
+ return new StringBuilder().append(formatValue(value))
+ .append(' ').append(name).toString();
+ }
+
public String format() {
- final StringBuilder m = new StringBuilder();
- m.append(formatValue());
- m.append(' ');
- m.append(getName());
- return m.toString();
+ return format(getName(), getValue());
}
}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
index 1c1cb35..e230880 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSetApproval.java
@@ -96,6 +96,9 @@
@Column(id = 5, length = 16, notNull = false)
protected String changeSortKey;
+ /** Label name copied from corresponding {@link ApprovalCategory}. */
+ protected String label;
+
protected PatchSetApproval() {
}
@@ -154,4 +157,12 @@
changeOpen = c.open;
changeSortKey = c.sortKey;
}
+
+ public String getLabel() {
+ return label;
+ }
+
+ public void setLabel(String label) {
+ this.label = label;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index b0e59390..f9f35ba 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -21,19 +21,25 @@
import static com.google.gerrit.common.changes.ListChangesOption.CURRENT_FILES;
import static com.google.gerrit.common.changes.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.common.changes.ListChangesOption.DETAILED_ACCOUNTS;
+import static com.google.gerrit.common.changes.ListChangesOption.DETAILED_LABELS;
import static com.google.gerrit.common.changes.ListChangesOption.LABELS;
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.LinkedListMultimap;
+import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gerrit.common.changes.ListChangesOption;
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ApprovalTypes;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Patch;
@@ -179,6 +185,12 @@
return formatList2(ImmutableList.of(tmp)).get(0).get(0);
}
+ public ChangeInfo format(RevisionResource rsrc) throws OrmException {
+ ChangeData cd = new ChangeData(rsrc.getControl());
+ cd.limitToPatchSets(ImmutableList.of(rsrc.getPatchSet().getId()));
+ return format(cd);
+ }
+
public List<List<ChangeInfo>> formatList2(List<List<ChangeData>> in)
throws OrmException {
accountLoader =
@@ -219,10 +231,15 @@
out._sortkey = in.getSortKey();
out.starred = user.getStarredChanges().contains(in.getId()) ? true : null;
out.reviewed = in.getStatus().isOpen() && isChangeReviewed(cd) ? true : null;
- out.labels = options.contains(LABELS) ? labelsFor(cd) : null;
+ out.labels = labelsFor(cd, options.contains(LABELS),
+ options.contains(DETAILED_LABELS));
+ if (options.contains(DETAILED_LABELS)) {
+ out.permitted_labels = permittedLabels(cd);
+ }
out.finish();
- if (options.contains(ALL_REVISIONS) || options.contains(CURRENT_REVISION)) {
+ if (options.contains(ALL_REVISIONS) || options.contains(CURRENT_REVISION)
+ || cd.getLimitedPatchSets() != null) {
out.revisions = revisions(cd);
for (String commit : out.revisions.keySet()) {
if (out.revisions.get(commit).isCurrent) {
@@ -259,7 +276,24 @@
return ctrl;
}
- private Map<String, LabelInfo> labelsFor(ChangeData cd) throws OrmException {
+ private List<SubmitRecord> submitRecords(ChangeData cd) throws OrmException {
+ if (cd.getSubmitRecords() != null) {
+ return cd.getSubmitRecords();
+ }
+ ChangeControl ctl = control(cd);
+ if (ctl == null) {
+ return ImmutableList.of();
+ }
+ PatchSet ps = cd.currentPatchSet(db);
+ if (ps == null) {
+ return ImmutableList.of();
+ }
+ cd.setSubmitRecords(ctl.canSubmit(db.get(), ps, cd, true, false));
+ return cd.getSubmitRecords();
+ }
+
+ private Map<String, LabelInfo> labelsFor(ChangeData cd, boolean standard,
+ boolean detailed) throws OrmException {
ChangeControl ctl = control(cd);
if (ctl == null) {
return Collections.emptyMap();
@@ -271,7 +305,31 @@
}
Map<String, LabelInfo> labels = Maps.newLinkedHashMap();
- for (SubmitRecord rec : ctl.canSubmit(db.get(), ps, cd, true, false)) {
+ initLabels(cd, labels, standard);
+
+ Collection<PatchSetApproval> approvals = null;
+ if (detailed) {
+ approvals = cd.currentApprovals(db);
+ setAllApprovals(labels, approvals);
+ }
+ for (Map.Entry<String, LabelInfo> e : labels.entrySet()) {
+ ApprovalType type = approvalTypes.byLabel(e.getKey());
+ if (type == null) {
+ continue; // TODO: Support arbitrary labels.
+ }
+ if (standard) {
+ approvals = setRecommendedAndDisliked(cd, approvals, type, e.getValue());
+ }
+ if (detailed) {
+ setLabelValues(type, e.getValue());
+ }
+ }
+ return labels;
+ }
+
+ private void initLabels(ChangeData cd, Map<String, LabelInfo> labels,
+ boolean standard) throws OrmException {
+ for (SubmitRecord rec : submitRecords(cd)) {
if (rec.labels == null) {
continue;
}
@@ -280,59 +338,127 @@
if (p == null || p._status.compareTo(r.status) < 0) {
LabelInfo n = new LabelInfo();
n._status = r.status;
- switch (r.status) {
- case OK:
- n.approved = accountLoader.get(r.appliedBy);
- break;
- case REJECT:
- n.rejected = accountLoader.get(r.appliedBy);
- break;
- default:
- break;
+ if (standard) {
+ switch (r.status) {
+ case OK:
+ n.approved = accountLoader.get(r.appliedBy);
+ break;
+ case REJECT:
+ n.rejected = accountLoader.get(r.appliedBy);
+ break;
+ default:
+ break;
+ }
}
+
n.optional = n._status == SubmitRecord.Label.Status.MAY ? true : null;
labels.put(r.label, n);
}
}
}
+ }
- Collection<PatchSetApproval> approvals = null;
- for (Map.Entry<String, LabelInfo> e : labels.entrySet()) {
- if (e.getValue().approved != null || e.getValue().rejected != null) {
+ private Collection<PatchSetApproval> setRecommendedAndDisliked(ChangeData cd,
+ Collection<PatchSetApproval> approvals, ApprovalType type,
+ LabelInfo label) throws OrmException {
+ if (label.approved != null || label.rejected != null) {
+ return approvals;
+ }
+
+ if (type.getMin() == null || type.getMax() == null) {
+ // Unknown or misconfigured type can't have intermediate scores.
+ return approvals;
+ }
+
+ short min = type.getMin().getValue();
+ short max = type.getMax().getValue();
+ if (-1 <= min && max <= 1) {
+ // Types with a range of -1..+1 can't have intermediate scores.
+ return approvals;
+ }
+
+ if (approvals == null) {
+ approvals = cd.currentApprovals(db);
+ }
+ for (PatchSetApproval psa : approvals) {
+ short val = psa.getValue();
+ if (val != 0 && min < val && val < max
+ && psa.getCategoryId().equals(type.getCategory().getId())) {
+ if (0 < val) {
+ label.recommended = accountLoader.get(psa.getAccountId());
+ label.value = val != 1 ? val : null;
+ } else {
+ label.disliked = accountLoader.get(psa.getAccountId());
+ label.value = val != -1 ? val : null;
+ }
+ }
+ }
+ return approvals;
+ }
+
+ private void setAllApprovals(Map<String, LabelInfo> labels,
+ Collection<PatchSetApproval> approvals) {
+ for (PatchSetApproval psa : approvals) {
+ ApprovalType at = approvalTypes.byId(psa.getCategoryId());
+ if (at == null) {
continue;
}
+ LabelInfo p = labels.get(at.getCategory().getLabelName());
+ if (p == null) {
+ continue; // TODO: support arbitrary labels.
+ }
+ if (p.all == null) {
+ p.all = Lists.newArrayList();
+ }
+ ApprovalInfo ai = new ApprovalInfo(psa.getAccountId());
+ accountLoader.put(ai);
+ ai.value = psa.getValue();
+ p.all.add(ai);
+ }
+ }
- ApprovalType type = approvalTypes.byLabel(e.getKey());
- if (type == null || type.getMin() == null || type.getMax() == null) {
- // Unknown or misconfigured type can't have intermediate scores.
+ private static boolean isOnlyZero(Collection<String> values) {
+ return values.isEmpty() || (values.size() == 1 && values.contains(" 0"));
+ }
+
+ private void setLabelValues(ApprovalType type, LabelInfo label) {
+ label.values = Maps.newLinkedHashMap();
+ for (ApprovalCategoryValue acv : type.getValues()) {
+ label.values.put(acv.formatValue(), acv.getName());
+ }
+ if (isOnlyZero(label.values.keySet())) {
+ label.values = null;
+ }
+ }
+
+
+ private Map<String, Collection<String>> permittedLabels(ChangeData cd)
+ throws OrmException {
+ ChangeControl ctl = control(cd);
+ ListMultimap<String, String> permitted = LinkedListMultimap.create();
+ for (SubmitRecord rec : submitRecords(cd)) {
+ if (rec.labels == null) {
continue;
}
-
- short min = type.getMin().getValue();
- short max = type.getMax().getValue();
- if (-1 <= min && max <= 1) {
- // Types with a range of -1..+1 can't have intermediate scores.
- continue;
- }
-
- if (approvals == null) {
- approvals = cd.currentApprovals(db);
- }
- for (PatchSetApproval psa : approvals) {
- short val = psa.getValue();
- if (val != 0 && min < val && val < max
- && psa.getCategoryId().equals(type.getCategory().getId())) {
- if (0 < val) {
- e.getValue().recommended = accountLoader.get(psa.getAccountId());
- e.getValue().value = val != 1 ? val : null;
- } else {
- e.getValue().disliked = accountLoader.get(psa.getAccountId());
- e.getValue().value = val != -1 ? val : null;
+ for (SubmitRecord.Label r : rec.labels) {
+ ApprovalType type = approvalTypes.byLabel(r.label);
+ if (type == null) {
+ continue; // TODO: Support arbitrary labels.
+ }
+ PermissionRange range = ctl.getRange(Permission.forLabel(r.label));
+ for (ApprovalCategoryValue acv : type.getValues()) {
+ if (range.contains(acv.getValue())) {
+ permitted.put(r.label, acv.formatValue());
}
}
}
}
- return labels;
+ for (Collection<String> values : permitted.asMap().values()) {
+ if (isOnlyZero(values)) {
+ values.clear();
+ }
+ }
+ return permitted.asMap();
}
private boolean isChangeReviewed(ChangeData cd) throws OrmException {
@@ -377,7 +503,7 @@
}
Collection<PatchSet> src;
- if (options.contains(ALL_REVISIONS)) {
+ if (cd.getLimitedPatchSets() != null || options.contains(ALL_REVISIONS)) {
src = cd.patches(db);
} else {
src = Collections.singletonList(cd.currentPatchSet(db));
@@ -531,6 +657,7 @@
AccountInfo owner;
Map<String, LabelInfo> labels;
+ Map<String, Collection<String>> permitted_labels;
String current_revision;
Map<String, RevisionInfo> revisions;
public Boolean _moreChanges;
@@ -588,12 +715,24 @@
static class LabelInfo {
transient SubmitRecord.Label.Status _status;
+
AccountInfo approved;
AccountInfo rejected;
-
AccountInfo recommended;
AccountInfo disliked;
+ List<ApprovalInfo> all;
+
+ Map<String, String> values;
+
Short value;
Boolean optional;
}
+
+ static class ApprovalInfo extends AccountInfo {
+ short value;
+
+ ApprovalInfo(Account.Id id) {
+ super(id);
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java
new file mode 100644
index 0000000..997f5e7
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReview.java
@@ -0,0 +1,35 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import com.google.gerrit.common.changes.ListChangesOption;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+
+public class GetReview implements RestReadView<RevisionResource> {
+ private final ChangeJson json;
+
+ @Inject
+ GetReview(ChangeJson json) {
+ this.json = json.addOption(ListChangesOption.DETAILED_LABELS)
+ .addOption(ListChangesOption.DETAILED_ACCOUNTS);
+ }
+
+ @Override
+ public Object apply(RevisionResource resource) throws OrmException {
+ return json.format(resource);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
index 571439d..6586725 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
@@ -54,6 +54,7 @@
delete(REVIEWER_KIND).to(DeleteReviewer.class);
child(CHANGE_KIND, "revisions").to(Revisions.class);
+ get(REVISION_KIND, "review").to(GetReview.class);
post(REVISION_KIND, "review").to(PostReview.class);
post(REVISION_KIND, "submit").to(Submit.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index afde702..07222fa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -14,14 +14,17 @@
package com.google.gerrit.server.query.change;
+import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Sets;
+import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.TrackingId;
@@ -47,9 +50,9 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
public class ChangeData {
public static void ensureChangeLoaded(
@@ -79,7 +82,9 @@
for (PatchSet ps : db.get().patchSets().get(missing.keySet())) {
ChangeData cd = missing.get(ps.getId());
cd.currentPatchSet = ps;
- cd.patches = Lists.newArrayList(ps);
+ if (cd.limitedIds == null) {
+ cd.patches = Lists.newArrayList(ps);
+ }
}
}
}
@@ -107,9 +112,9 @@
private Change change;
private String commitMessage;
private PatchSet currentPatchSet;
+ private Set<PatchSet.Id> limitedIds;
private Collection<PatchSet> patches;
- private Collection<PatchSetApproval> approvals;
- private Map<PatchSet.Id,Collection<PatchSetApproval>> approvalsMap;
+ private Multimap<PatchSet.Id, PatchSetApproval> approvals;
private Collection<PatchSetApproval> currentApprovals;
private String[] currentFiles;
private Collection<PatchLineComment> comments;
@@ -117,6 +122,7 @@
private CurrentUser visibleTo;
private ChangeControl changeControl;
private List<ChangeMessage> messages;
+ private List<SubmitRecord> submitRecords;
public ChangeData(final Change.Id id) {
legacyId = id;
@@ -133,6 +139,21 @@
changeControl = c;
}
+ public void limitToPatchSets(Collection<PatchSet.Id> ids) {
+ limitedIds = Sets.newLinkedHashSetWithExpectedSize(ids.size());
+ for (PatchSet.Id id : ids) {
+ if (!id.getParentKey().equals(legacyId)) {
+ throw new IllegalArgumentException(String.format(
+ "invalid patch set %s for change %s", id, legacyId));
+ }
+ limitedIds.add(id);
+ }
+ }
+
+ public Collection<PatchSet.Id> getLimitedPatchSets() {
+ return limitedIds;
+ }
+
public void setCurrentFilePaths(String[] filePaths) {
currentFiles = filePaths;
}
@@ -239,13 +260,9 @@
Change c = change(db);
if (c == null) {
currentApprovals = Collections.emptyList();
- } else if (approvals != null) {
- Map<Id, Collection<PatchSetApproval>> map = approvalsMap(db);
- currentApprovals = map.get(c.currentPatchSetId());
- if (currentApprovals == null) {
- currentApprovals = Collections.emptyList();
- map.put(c.currentPatchSetId(), currentApprovals);
- }
+ } else if (approvals != null &&
+ (limitedIds == null || limitedIds.contains(c.currentPatchSetId()))) {
+ return approvals.get(c.currentPatchSetId());
} else {
currentApprovals = db.get().patchSetApprovals()
.byPatchSet(c.currentPatchSetId()).toList();
@@ -276,37 +293,60 @@
return commitMessage;
}
+ /**
+ * @param db review database.
+ * @return patches for the change. If {@link #limitToPatchSets(Collection)}
+ * was previously called, only contains patches with the specified IDs.
+ * @throws OrmException an error occurred reading the database.
+ */
public Collection<PatchSet> patches(Provider<ReviewDb> db)
throws OrmException {
if (patches == null) {
- patches = db.get().patchSets().byChange(legacyId).toList();
+ if (limitedIds != null) {
+ patches = Lists.newArrayList();
+ for (PatchSet ps : db.get().patchSets().byChange(legacyId)) {
+ if (limitedIds.contains(ps.getId())) {
+ patches.add(ps);
+ }
+ }
+ } else {
+ patches = db.get().patchSets().byChange(legacyId).toList();
+ }
}
return patches;
}
+ /**
+ * @param db review database.
+ * @return patch set approvals for the change. If
+ * {@link #limitToPatchSets(Collection)} was previously called, only contains
+ * approvals for the patches with the specified IDs.
+ * @throws OrmException an error occurred reading the database.
+ */
public Collection<PatchSetApproval> approvals(Provider<ReviewDb> db)
throws OrmException {
- if (approvals == null) {
- approvals = db.get().patchSetApprovals().byChange(legacyId).toList();
- }
- return approvals;
+ return approvalsMap(db).values();
}
- public Map<PatchSet.Id,Collection<PatchSetApproval>> approvalsMap(Provider<ReviewDb> db)
- throws OrmException {
- if (approvalsMap == null) {
- Collection<PatchSetApproval> all = approvals(db);
- approvalsMap = new HashMap<PatchSet.Id,Collection<PatchSetApproval>>(all.size());
- for (PatchSetApproval psa : all) {
- Collection<PatchSetApproval> c = approvalsMap.get(psa.getPatchSetId());
- if (c == null) {
- c = new ArrayList<PatchSetApproval>();
- approvalsMap.put(psa.getPatchSetId(), c);
+ /**
+ * @param db review database.
+ * @return patch set approvals for the change, keyed by ID. If
+ * {@link #limitToPatchSets(Collection)} was previously called, only
+ * contains approvals for the patches with the specified IDs.
+ * @throws OrmException an error occurred reading the database.
+ */
+ public Multimap<PatchSet.Id,PatchSetApproval> approvalsMap(
+ Provider<ReviewDb> db) throws OrmException {
+ if (approvals == null) {
+ approvals = ArrayListMultimap.create();
+ for (PatchSetApproval psa :
+ db.get().patchSetApprovals().byChange(legacyId)) {
+ if (limitedIds == null || limitedIds.contains(legacyId)) {
+ approvals.put(psa.getPatchSetId(), psa);
}
- c.add(psa);
}
}
- return approvalsMap;
+ return approvals;
}
public Collection<PatchLineComment> comments(Provider<ReviewDb> db)
@@ -332,4 +372,12 @@
}
return messages;
}
+
+ public void setSubmitRecords(List<SubmitRecord> records) {
+ submitRecords = records;
+ }
+
+ public List<SubmitRecord> getSubmitRecords() {
+ return submitRecords;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
index 8cc9e78..8ca0548 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
@@ -286,11 +286,11 @@
if (includePatchSets) {
if (includeFiles) {
eventFactory.addPatchSets(c, d.patches(db),
- includeApprovals ? d.approvalsMap(db) : null,
+ includeApprovals ? d.approvalsMap(db).asMap() : null,
includeFiles, d.change(db));
} else {
eventFactory.addPatchSets(c, d.patches(db),
- includeApprovals ? d.approvalsMap(db) : null);
+ includeApprovals ? d.approvalsMap(db).asMap() : null);
}
}
