Migrate ApprovalTypes away from ApprovalCategory(Value)
Many callers of ApprovalTypes depend directly on ApprovalCategory and
ApprovalCategoryValue, which are the types stored in the database. We
want to move away from storing approval categories in the database in
favor of arbitrary labels that can be defined on a per-project level,
including implicitly in submit rules. Leaking the database types
around the code makes this harder.
Replace ApprovalCategoryValue everywhere except the database code with
a similar and more concise LabelValue type. Use Strings instead of the
database Id types when referring to labels in general. In the short
term this may create some confusion in the code (alleviated,
hopefully, by documentation and variable names), since some places use
legacy category IDs as unique keys and others use new style label
names as unique keys. In the long term we will stop using category IDs
except to map them to unique label names.
Unlike ApprovalCategory, the new and improved ApprovalType does not
distinguish between the label name and the human-readable name.
Generally these do not vary significantly other than the label name
having dashes instead of spaces, which is minor enough in the UI. The
other affected functionality here is searching by label; since label
names can no longer contain spaces "label:CodeReview" no longer works,
only "label:Code-Review".
Change-Id: I6854259d60af88b6d9df8d7553120a9af64c74ad
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 3bd437f..c821b00 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -37,7 +37,7 @@
|Full or abbreviated Change-Id | Ic0ff33
|Full or abbreviated commit SHA-1 | d81b32ef
|Email address | user@example.com
-|Approval requirement | CodeReview>=+2, Verified=1
+|Approval requirement | Code-Review>=+2, Verified=1
|=============================================================
@@ -317,16 +317,12 @@
------
Label operators can be used to match approval scores given during
a code review. The specific set of supported labels depends on
-the server configuration, however `CodeReview` and `Verified`
+the server configuration, however `Code-Review` and `Verified`
are the default labels provided out of the box.
A label name is any of the following:
-* The category name. If the category name contains spaces,
- it must be wrapped in double quotes. Example: `label:"Code Review"`.
-
-* The name, without spaces. This avoids needing to use double quotes
- for the common category Code Review. Example: `label:CodeReview`.
+* The label name.
* The internal short name. Example: `label:CRVW`, or `label:VRIF`.
@@ -336,38 +332,38 @@
A label name must be followed by a score, or an operator and a score.
The easiest way to explain this is by example.
-`label:CodeReview=2`::
-`label:CodeReview=+2`::
-`label:CodeReview+2`::
+`label:Code-Review=2`::
+`label:Code-Review=+2`::
+`label:Code-Review+2`::
+
-Matches changes where there is at least one +2 score for Code Review.
+Matches changes where there is at least one +2 score for Code-Review.
The + prefix is optional for positive score values. If the + is used,
the = operator is optional.
-`label:CodeReview=-2`::
-`label:CodeReview-2`::
+`label:Code-Review=-2`::
+`label:Code-Review-2`::
+
-Matches changes where there is at least one -2 score for Code Review.
+Matches changes where there is at least one -2 score for Code-Review.
Because the negative sign is required, the = operator is optional.
-`label:CodeReview=1`::
+`label:Code-Review=1`::
+
-Matches changes where there is at least one +1 score for Code Review.
+Matches changes where there is at least one +1 score for Code-Review.
Scores of +2 are not matched, even though they are higher.
-`label:CodeReview>=1`::
+`label:Code-Review>=1`::
+
Matches changes with either a +1, +2, or any higher score.
-`label:CodeReview<=-1`::
+`label:Code-Review<=-1`::
+
Matches changes with either a -1, -2, or any lower score.
-`is:open CodeReview+2 Verified+1 -Verified-1 -CodeReview-2`::
+`is:open Code-Review+2 Verified+1 -Verified-1 -Code-Review-2`::
+
Matches changes that are ready to be submitted.
-`is:open (Verified-1 OR CodeReview-2)`::
+`is:open (Verified-1 OR Code-Review-2)`::
+
Changes that are blocked from submission due to a blocking score.
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalSummary.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalSummary.java
index 19c9d67..c7e4007 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalSummary.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalSummary.java
@@ -14,7 +14,6 @@
package com.google.gerrit.common.data;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import java.util.Collections;
@@ -26,19 +25,21 @@
* category, with all of the approvals coming from a single patch set.
*/
public class ApprovalSummary {
- protected Map<ApprovalCategory.Id, PatchSetApproval> approvals;
+ protected Map<String, PatchSetApproval> approvals;
protected ApprovalSummary() {
}
public ApprovalSummary(final Iterable<PatchSetApproval> list) {
- approvals = new HashMap<ApprovalCategory.Id, PatchSetApproval>();
+ approvals = new HashMap<String, PatchSetApproval>();
for (final PatchSetApproval a : list) {
- approvals.put(a.getCategoryId(), a);
+ approvals.put(a.getCategoryId().get(), a);
}
}
- public Map<ApprovalCategory.Id, PatchSetApproval> getApprovalMap() {
+ // TODO: Convert keys to label names.
+ /** @return a map of approvals keyed by ID string. */
+ public Map<String, PatchSetApproval> getApprovalMap() {
return Collections.unmodifiableMap(approvals);
}
}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalType.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalType.java
index 333b91c..99c32f5 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalType.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalType.java
@@ -26,27 +26,97 @@
import java.util.Map;
public class ApprovalType {
- protected ApprovalCategory category;
- protected List<ApprovalCategoryValue> values;
+ public static ApprovalType fromApprovalCategory(ApprovalCategory ac,
+ List<ApprovalCategoryValue> acvs) {
+ List<LabelValue> values = new ArrayList<LabelValue>(acvs.size());
+ for (ApprovalCategoryValue acv : acvs) {
+ values.add(
+ new LabelValue(acv.getValue(), acv.getName()));
+ }
+ ApprovalType at =
+ new ApprovalType(ac.getId().get(), ac.getLabelName(), values);
+ at.setAbbreviatedName(ac.getAbbreviatedName());
+ at.setFunctionName(ac.getFunctionName());
+ at.setCopyMinScore(ac.isCopyMinScore());
+ at.setPosition(ac.getPosition());
+ return at;
+ }
+
+ public static ApprovalType withDefaultValues(String id, String name) {
+ checkName(name);
+ List<LabelValue> values = new ArrayList<LabelValue>(2);
+ values.add(new LabelValue((short) 0, "Rejected"));
+ values.add(new LabelValue((short) 1, "Approved"));
+ return new ApprovalType(id, name, values);
+ }
+
+ private static String checkId(String id) {
+ if (id == null || id.length() > 4) {
+ throw new IllegalArgumentException(
+ "Illegal label ID \"" + id + "\"");
+ }
+ return id;
+ }
+
+ private static String checkName(String name) {
+ for (int i = 0; i < name.length(); i++) {
+ char c = name.charAt(i);
+ if (!((c >= 'a' && c <= 'z') ||
+ (c >= 'A' && c <= 'Z') ||
+ (c >= '0' && c <= '9') ||
+ c == '-')) {
+ throw new IllegalArgumentException(
+ "Illegal label name \"" + name + "\"");
+ }
+ }
+ return name;
+ }
+
+ private static String defaultAbbreviation(String name) {
+ StringBuilder abbr = new StringBuilder();
+ for (int i = 0; i < name.length(); i++) {
+ char c = name.charAt(i);
+ if (c >= 'A' && c <= 'Z') {
+ abbr.append(c);
+ }
+ }
+ if (abbr.length() == 0) {
+ abbr.append(Character.toUpperCase(name.charAt(0)));
+ }
+ return abbr.toString();
+ }
+
+ protected String name;
+
+ protected String id;
+ protected String abbreviatedName;
+ protected String functionName;
+ protected boolean copyMinScore;
+ protected short position;
+
+ protected List<LabelValue> values;
protected short maxNegative;
protected short maxPositive;
private transient List<Integer> intList;
- private transient Map<Short, ApprovalCategoryValue> byValue;
+ private transient Map<Short, LabelValue> byValue;
protected ApprovalType() {
}
- public ApprovalType(final ApprovalCategory ac,
- final List<ApprovalCategoryValue> valueList) {
- category = ac;
- values = new ArrayList<ApprovalCategoryValue>(valueList);
- Collections.sort(values, new Comparator<ApprovalCategoryValue>() {
- public int compare(ApprovalCategoryValue o1, ApprovalCategoryValue o2) {
+ public ApprovalType(String id, String name, List<LabelValue> valueList) {
+ this.id = checkId(id);
+ this.name = checkName(name);
+ values = new ArrayList<LabelValue>(valueList);
+ Collections.sort(values, new Comparator<LabelValue>() {
+ public int compare(LabelValue o1, LabelValue o2) {
return o1.getValue() - o2.getValue();
}
});
+ abbreviatedName = defaultAbbreviation(name);
+ functionName = "MaxWithBlock";
+
maxNegative = Short.MIN_VALUE;
maxPositive = Short.MAX_VALUE;
if (values.size() > 0) {
@@ -57,57 +127,90 @@
maxPositive = values.get(values.size() - 1).getValue();
}
}
-
- // Force the label name to pre-compute so we don't have data race conditions.
- getCategory().getLabelName();
}
- public ApprovalCategory getCategory() {
- return category;
+ public String getName() {
+ return name;
}
- public List<ApprovalCategoryValue> getValues() {
+ public String getId() {
+ return id;
+ }
+
+ public String getAbbreviatedName() {
+ return abbreviatedName;
+ }
+
+ public void setAbbreviatedName(String abbreviatedName) {
+ this.abbreviatedName = abbreviatedName;
+ }
+
+ public String getFunctionName() {
+ return functionName;
+ }
+
+ public void setFunctionName(String functionName) {
+ this.functionName = functionName;
+ }
+
+ public short getPosition() {
+ return position;
+ }
+
+ public void setPosition(short position) {
+ this.position = position;
+ }
+
+ public List<LabelValue> getValues() {
return values;
}
- public ApprovalCategoryValue getMin() {
+ public LabelValue getMin() {
if (values.isEmpty()) {
return null;
}
return values.get(0);
}
- public ApprovalCategoryValue getMax() {
+ public LabelValue getMax() {
if (values.isEmpty()) {
return null;
}
- final ApprovalCategoryValue v = values.get(values.size() - 1);
+ final LabelValue v = values.get(values.size() - 1);
return v.getValue() > 0 ? v : null;
}
- public boolean isMaxNegative(final PatchSetApproval ca) {
+ public boolean isCopyMinScore() {
+ return copyMinScore;
+ }
+
+ public void setCopyMinScore(boolean copyMinScore) {
+ this.copyMinScore = copyMinScore;
+ }
+
+ public boolean isMaxNegative(PatchSetApproval ca) {
return maxNegative == ca.getValue();
}
- public boolean isMaxPositive(final PatchSetApproval ca) {
+ public boolean isMaxPositive(PatchSetApproval ca) {
return maxPositive == ca.getValue();
}
- public ApprovalCategoryValue getValue(final short value) {
+ public LabelValue getValue(short value) {
initByValue();
return byValue.get(value);
}
- public ApprovalCategoryValue getValue(final PatchSetApproval ca) {
+ public LabelValue getValue(final PatchSetApproval ca) {
initByValue();
return byValue.get(ca.getValue());
}
private void initByValue() {
if (byValue == null) {
- byValue = new HashMap<Short, ApprovalCategoryValue>();
- for (final ApprovalCategoryValue acv : values) {
- byValue.put(acv.getValue(), acv);
+ byValue = new HashMap<Short, LabelValue>();
+ for (final LabelValue v : values) {
+ byValue.put(v.getValue(), v);
}
}
}
@@ -115,12 +218,22 @@
public List<Integer> getValuesAsList() {
if (intList == null) {
intList = new ArrayList<Integer>(values.size());
- for (ApprovalCategoryValue acv : values) {
- intList.add(Integer.valueOf(acv.getValue()));
+ for (LabelValue v : values) {
+ intList.add(Integer.valueOf(v.getValue()));
}
Collections.sort(intList);
Collections.reverse(intList);
}
return intList;
}
+
+ @Deprecated
+ public ApprovalCategoryValue.Id getApprovalCategoryValueId(short value) {
+ return new ApprovalCategoryValue.Id(getApprovalCategoryId(), value);
+ }
+
+ @Deprecated
+ public ApprovalCategory.Id getApprovalCategoryId() {
+ return new ApprovalCategory.Id(getId());
+ }
}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalTypes.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalTypes.java
index b1e32d1..7461101 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalTypes.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalTypes.java
@@ -14,15 +14,13 @@
package com.google.gerrit.common.data;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
-
import java.util.HashMap;
import java.util.List;
import java.util.Map;
public class ApprovalTypes {
protected List<ApprovalType> approvalTypes;
- private transient Map<ApprovalCategory.Id, ApprovalType> byId;
+ private transient Map<String, ApprovalType> byId;
private transient Map<String, ApprovalType> byLabel;
protected ApprovalTypes() {
@@ -30,23 +28,23 @@
public ApprovalTypes(final List<ApprovalType> approvals) {
approvalTypes = approvals;
- byCategory();
+ byId();
}
public List<ApprovalType> getApprovalTypes() {
return approvalTypes;
}
- public ApprovalType byId(final ApprovalCategory.Id id) {
- return byCategory().get(id);
+ public ApprovalType byId(String id) {
+ return byId().get(id);
}
- private Map<ApprovalCategory.Id, ApprovalType> byCategory() {
+ private Map<String, ApprovalType> byId() {
if (byId == null) {
- byId = new HashMap<ApprovalCategory.Id, ApprovalType>();
+ byId = new HashMap<String, ApprovalType>();
if (approvalTypes != null) {
for (final ApprovalType t : approvalTypes) {
- byId.put(t.getCategory().getId(), t);
+ byId.put(t.getId(), t);
}
}
}
@@ -62,7 +60,7 @@
byLabel = new HashMap<String, ApprovalType>();
if (approvalTypes != null) {
for (ApprovalType t : approvalTypes) {
- byLabel.put(t.getCategory().getLabelName().toLowerCase(), t);
+ byLabel.put(t.getName().toLowerCase(), t);
}
}
}
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelValue.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelValue.java
new file mode 100644
index 0000000..1f80b8c
--- /dev/null
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/LabelValue.java
@@ -0,0 +1,62 @@
+// 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.common.data;
+
+import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
+
+public class LabelValue {
+ @Deprecated
+ public static LabelValue fromApprovalCategoryValue(ApprovalCategoryValue acv) {
+ return new LabelValue(acv.getValue(), acv.getName());
+ }
+
+ public static String formatValue(short value) {
+ if (value < 0) {
+ return Short.toString(value);
+ } else if (value == 0) {
+ return " 0";
+ } else {
+ return "+" + Short.toString(value);
+ }
+ }
+
+ protected short value;
+ protected String text;
+
+ public LabelValue(short value, String text) {
+ this.value = value;
+ this.text = text;
+ }
+
+ protected LabelValue() {
+ }
+
+ public short getValue() {
+ return value;
+ }
+
+ public String getText() {
+ return text;
+ }
+
+ public String formatValue() {
+ return formatValue(value);
+ }
+
+ public String format() {
+ return new StringBuilder().append(formatValue())
+ .append(' ').append(text).toString();
+ }
+}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccessSectionEditor.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccessSectionEditor.java
index a9aa418..577f40b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccessSectionEditor.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccessSectionEditor.java
@@ -228,8 +228,7 @@
} else if (RefConfigSection.isValid(value.getName())) {
for (ApprovalType t : Gerrit.getConfig().getApprovalTypes()
.getApprovalTypes()) {
- String varName = Permission.LABEL + t.getCategory().getLabelName();
- addPermission(varName, perms);
+ addPermission(Permission.LABEL + t.getName(), perms);
}
for (String varName : Util.C.permissionNames().keySet()) {
addPermission(varName, perms);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java
index f98ee93f..393f9ce 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ApprovalTable.java
@@ -14,7 +14,7 @@
package com.google.gerrit.client.changes;
-import static com.google.gerrit.reviewdb.client.ApprovalCategoryValue.formatValue;
+import static com.google.gerrit.common.data.LabelValue.formatValue;
import com.google.gerrit.client.ConfirmationCallback;
import com.google.gerrit.client.ConfirmationDialog;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java
index b28d994..0d7f8db 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeTable.java
@@ -33,10 +33,9 @@
import com.google.gerrit.common.data.ApprovalSummarySet;
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ChangeInfo;
+import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences;
-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.PatchSetApproval;
import com.google.gwt.dom.client.Element;
@@ -98,14 +97,13 @@
table.setText(0, C_LAST_UPDATE, Util.C.changeTableColumnLastUpdate());
for (int i = BASE_COLUMNS; i < columns; i++) {
final ApprovalType type = approvalTypes.get(i - BASE_COLUMNS);
- final ApprovalCategory cat = type.getCategory();
- String text = cat.getAbbreviatedName();
+ String text = type.getAbbreviatedName();
if (text == null) {
- text = cat.getName();
+ text = type.getName();
}
table.setText(0, i, text);
if (text != null) {
- table.getCellFormatter().getElement(0, i).setTitle(cat.getName());
+ table.getCellFormatter().getElement(0, i).setTitle(type.getName());
}
}
@@ -282,8 +280,7 @@
private void displayApprovals(final int row, final ApprovalSummary summary,
final AccountInfoCache aic, final boolean highlightUnreviewed) {
final CellFormatter fmt = table.getCellFormatter();
- final Map<ApprovalCategory.Id, PatchSetApproval> approvals =
- summary.getApprovalMap();
+ final Map<String, PatchSetApproval> approvals = summary.getApprovalMap();
int col = BASE_COLUMNS;
boolean haveReview = false;
@@ -298,7 +295,7 @@
}
for (final ApprovalType type : approvalTypes) {
- final PatchSetApproval ca = approvals.get(type.getCategory().getId());
+ final PatchSetApproval ca = approvals.get(type.getId());
fmt.removeStyleName(row, col, Gerrit.RESOURCES.css().negscore());
fmt.removeStyleName(row, col, Gerrit.RESOURCES.css().posscore());
@@ -310,7 +307,7 @@
} else {
haveReview = true;
- final ApprovalCategoryValue acv = type.getValue(ca);
+ final LabelValue v = type.getValue(ca);
final AccountInfo ai = aic.get(ca.getAccountId());
if (type.isMaxNegative(ca)) {
@@ -355,7 +352,7 @@
// so we include a space before the newline to accommodate both.
//
fmt.getElement(row, col).setTitle(
- acv.getName() + " \nby " + FormatUtil.nameEmail(ai));
+ v.getText() + " \nby " + FormatUtil.nameEmail(ai));
}
col++;
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
index 1ab484f..85088a5 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java
@@ -189,7 +189,7 @@
continue;
}
if (change.getStatus().isOpen()) {
- fs.normalize(approvalTypes.byId(category), ca);
+ fs.normalize(approvalTypes.byId(category.get()), ca);
}
if (ca.getValue() == 0) {
continue;
@@ -235,7 +235,7 @@
continue;
}
if (change.getStatus().isOpen()) {
- fs.normalize(approvalTypes.byId(category), ca);
+ fs.normalize(approvalTypes.byId(category.get()), ca);
}
if (ca.getValue() == 0) {
continue;
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 46e9892..e5fe735 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
@@ -87,27 +87,4 @@
public void setName(final String n) {
name = n;
}
-
- public static String formatValue(short value) {
- if (value < 0) {
- return Short.toString(value);
- } else if (value == 0) {
- return " 0";
- } else {
- 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() {
- return format(getName(), getValue());
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
index a1f63cd..4eb9ae9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
@@ -616,9 +616,9 @@
Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval) {
ApprovalAttribute a = new ApprovalAttribute();
a.type = approval.getKey().get();
- ApprovalType at = approvalTypes.byId(approval.getKey());
+ ApprovalType at = approvalTypes.byId(approval.getKey().get());
if (at != null) {
- a.description = at.getCategory().getName();
+ a.description = at.getName();
}
a.value = Short.toString(approval.getValue().get());
return a;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index 032d486..9665f66 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.common.data.ApprovalType;
@@ -90,9 +91,9 @@
for (PatchSetApproval a : patchSetApprovals) {
// ApprovalCategory.SUBMIT is still in db but not relevant in git-store
if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
- final ApprovalType type = approvalTypes.byId(a.getCategoryId());
+ final ApprovalType type = approvalTypes.byId(a.getCategoryId().get());
if (a.getPatchSetId().equals(source) &&
- type.getCategory().isCopyMinScore() &&
+ type.isCopyMinScore() &&
type.isMaxNegative(a)) {
db.patchSetApprovals().insert(
Collections.singleton(new PatchSetApproval(dest, a)));
@@ -128,7 +129,8 @@
need.removeAll(existingReviewers);
List<PatchSetApproval> cells = Lists.newArrayListWithCapacity(need.size());
- ApprovalCategory.Id catId = allTypes.get(allTypes.size() - 1).getCategory().getId();
+ ApprovalCategory.Id catId =
+ Iterables.getLast(allTypes).getApprovalCategoryId();
for (Account.Id account : need) {
PatchSetApproval psa = new PatchSetApproval(
new PatchSetApproval.Key(ps.getId(), account, catId),
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 e3e5905..46e75db 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
@@ -37,12 +37,12 @@
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.LabelValue;
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.Change.Status;
import com.google.gerrit.reviewdb.client.ChangeMessage;
@@ -408,7 +408,7 @@
for (PatchSetApproval psa : cd.currentApprovals(db)) {
short val = psa.getValue();
if (val != 0 && min < val && val < max
- && psa.getCategoryId().equals(type.getCategory().getId())) {
+ && psa.getCategoryId().get().equals(type.getId())) {
if (0 < val) {
label.recommended = accountLoader.get(psa.getAccountId());
label.value = val != 1 ? val : null;
@@ -429,25 +429,24 @@
FunctionState fs =
functionState.create(ctl, cd.change(db).currentPatchSetId(), approvals);
for (ApprovalType at : approvalTypes.getApprovalTypes()) {
- CategoryFunction.forCategory(at.getCategory()).run(at, fs);
+ CategoryFunction.forType(at).run(at, fs);
}
Multimap<Account.Id, String> existing =
HashMultimap.create(approvals.size(), labels.size());
for (PatchSetApproval psa : approvals) {
- ApprovalType at = approvalTypes.byId(psa.getCategoryId());
+ ApprovalType at = approvalTypes.byId(psa.getCategoryId().get());
if (at == null) {
continue;
}
- String name = at.getCategory().getLabelName();
- LabelInfo p = labels.get(name);
+ LabelInfo p = labels.get(at.getName());
if (p == null) {
continue; // TODO: support arbitrary labels.
}
- if (!getRange(ctl, psa.getAccountId(), name).isEmpty()) {
+ if (!getRange(ctl, psa.getAccountId(), at.getName()).isEmpty()) {
p.addApproval(approvalInfo(psa.getAccountId(), psa.getValue()));
}
- existing.put(psa.getAccountId(), at.getCategory().getLabelName());
+ existing.put(psa.getAccountId(), at.getName());
}
// Add dummy approvals for all permitted labels for each user even if they
@@ -478,11 +477,11 @@
Map<String, LabelInfo> labels =
new TreeMap<String, LabelInfo>(LabelOrdering.create(approvalTypes));
for (PatchSetApproval psa : cd.currentApprovals(db)) {
- ApprovalType type = approvalTypes.byId(psa.getCategoryId());
+ ApprovalType type = approvalTypes.byId(psa.getCategoryId().get());
if (type == null) {
continue;
}
- String label = type.getCategory().getLabelName();
+ String label = type.getName();
LabelInfo li = labels.get(label);
if (li == null) {
li = new LabelInfo();
@@ -540,8 +539,8 @@
private void setLabelValues(ApprovalType type, LabelInfo label) {
label.values = Maps.newLinkedHashMap();
- for (ApprovalCategoryValue acv : type.getValues()) {
- label.values.put(acv.formatValue(), acv.getName());
+ for (LabelValue v : type.getValues()) {
+ label.values.put(v.formatValue(), v.getText());
}
if (isOnlyZero(label.values.keySet())) {
label.values = null;
@@ -562,9 +561,9 @@
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());
+ for (LabelValue v : type.getValues()) {
+ if (range.contains(v.getValue())) {
+ permitted.put(r.label, v.formatValue());
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/LabelOrdering.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/LabelOrdering.java
index d7304ae..1455824 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/LabelOrdering.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/LabelOrdering.java
@@ -26,7 +26,7 @@
@Override
public Short apply(String n) {
ApprovalType at = approvalTypes.byLabel(n);
- return at != null ? at.getCategory().getPosition() : null;
+ return at != null ? at.getPosition() : null;
}
}).compound(Ordering.natural());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index f4edb10..74b7e9e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -209,7 +209,7 @@
}
}
- String name = at.getCategory().getLabelName();
+ String name = at.getName();
PermissionRange range = ctl.getRange(Permission.forLabel(name));
if (range == null || !range.contains(ent.getValue())) {
if (strict) {
@@ -346,7 +346,7 @@
for (Map.Entry<String, Short> ent : labels.entrySet()) {
// TODO Support arbitrary label names.
ApprovalType at = approvalTypes.byLabel(ent.getKey());
- String name = at.getCategory().getLabelName();
+ String name = at.getName();
if (change.getStatus().isClosed()) {
// TODO Allow updating some labels even when closed.
continue;
@@ -368,23 +368,23 @@
upd.add(c);
labelDelta.add(format(name, c.getValue()));
categories.put(
- at.getCategory().getId(),
- at.getValue(c.getValue()).getId());
+ at.getApprovalCategoryId(),
+ at.getApprovalCategoryValueId(c.getValue()));
} else if (c != null && c.getValue() == ent.getValue()) {
current.put(name, c);
} else if (c == null) {
c = new PatchSetApproval(new PatchSetApproval.Key(
rsrc.getPatchSet().getId(),
rsrc.getAccountId(),
- at.getCategory().getId()),
+ at.getApprovalCategoryId()),
ent.getValue());
c.setGranted(timestamp);
c.cache(change);
ins.add(c);
labelDelta.add(format(name, c.getValue()));
categories.put(
- at.getCategory().getId(),
- at.getValue(c.getValue()).getId());
+ at.getApprovalCategoryId(),
+ at.getApprovalCategoryValueId(c.getValue()));
}
}
@@ -406,7 +406,7 @@
PatchSetApproval c = new PatchSetApproval(new PatchSetApproval.Key(
rsrc.getPatchSet().getId(),
rsrc.getAccountId(),
- approvalTypes.getApprovalTypes().get(0).getCategory().getId()),
+ approvalTypes.getApprovalTypes().get(0).getApprovalCategoryId()),
(short) 0);
c.setGranted(timestamp);
c.cache(change);
@@ -433,9 +433,9 @@
continue;
}
- ApprovalType at = approvalTypes.byId(a.getCategoryId());
+ ApprovalType at = approvalTypes.byId(a.getCategoryId().get());
if (at != null) {
- current.put(at.getCategory().getLabelName(), a);
+ current.put(at.getName(), a);
} else {
del.add(a);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index 48eb4c9..d02d11d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -117,8 +117,8 @@
this.accountCache = accountCache;
this.json = json;
- this.addReviewerCategoryId = Iterables.getLast(
- approvalTypes.getApprovalTypes()).getCategory().getId();
+ this.addReviewerCategoryId = Iterables.getLast(approvalTypes.getApprovalTypes())
+ .getApprovalCategoryId();
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
index 42cb757..fc00892 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -14,7 +14,7 @@
package com.google.gerrit.server.change;
-import static com.google.gerrit.reviewdb.client.ApprovalCategoryValue.formatValue;
+import static com.google.gerrit.common.data.LabelValue.formatValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
@@ -85,7 +85,7 @@
FunctionState fs = functionState.create(ctl, psId, approvals);
for (ApprovalType at : approvalTypes.getApprovalTypes()) {
- CategoryFunction.forCategory(at.getCategory()).run(at, fs);
+ CategoryFunction.forType(at).run(at, fs);
}
// Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
@@ -95,10 +95,9 @@
for (PermissionRange pr : ctl.getLabelRanges()) {
if (!pr.isEmpty()) {
// TODO: Support arbitrary labels.
- ApprovalType at = approvalTypes.byId(ca.getCategoryId());
+ ApprovalType at = approvalTypes.byId(ca.getCategoryId().get());
if (at != null) {
- out.approvals.put(at.getCategory().getLabelName(),
- formatValue(ca.getValue())); }
+ out.approvals.put(at.getName(), formatValue(ca.getValue())); }
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ApprovalTypesProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ApprovalTypesProvider.java
index ed9416d..b11e1ba 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/ApprovalTypesProvider.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ApprovalTypesProvider.java
@@ -47,7 +47,7 @@
for (final ApprovalCategory c : db.approvalCategories().all()) {
final List<ApprovalCategoryValue> values =
db.approvalCategoryValues().byCategory(c.getId()).toList();
- types.add(new ApprovalType(c, values));
+ types.add(ApprovalType.fromApprovalCategory(c, values));
}
} finally {
db.close();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
index bbc45e5..e786430 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
@@ -460,9 +460,9 @@
a.by = asAccountAttribute(approval.getAccountId());
a.grantedOn = approval.getGranted().getTime() / 1000L;
- ApprovalType at = approvalTypes.byId(approval.getCategoryId());
+ ApprovalType at = approvalTypes.byId(approval.getCategoryId().get());
if (at != null) {
- a.description = at.getCategory().getName();
+ a.description = at.getName();
}
return a;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index df97c4f..10963f4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -937,7 +937,7 @@
identifiedUserFactory.create(c.getOwner())),
merged, approvals);
for (ApprovalType at : approvalTypes.getApprovalTypes()) {
- CategoryFunction.forCategory(at.getCategory()).run(at, fs);
+ CategoryFunction.forType(at).run(at, fs);
}
for (PatchSetApproval a : approvals) {
if (a.getValue() > 0
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
index 408a6e5..a6e19e3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
@@ -254,12 +254,12 @@
} else if (VRIF.equals(a.getCategoryId())) {
tag = "Tested-by";
} else {
- final ApprovalType at = approvalTypes.byId(a.getCategoryId());
+ final ApprovalType at = approvalTypes.byId(a.getCategoryId().get());
if (at == null) {
- // A deprecated/deleted approval type, ignore it.
+ // TODO: Support arbitrary labels.
continue;
}
- tag = at.getCategory().getName().replace(' ', '-');
+ tag = at.getName();
}
if (!contains(footers, new FooterKey(tag), identbuf.toString())) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java
index 7b669b2..49652b4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java
@@ -14,8 +14,8 @@
package com.google.gerrit.server.git;
+import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
@@ -51,9 +51,8 @@
sb.append("Change-Id: ").append(changeKey.get()).append("\n");
}
- void appendApproval(ApprovalCategory category,
- short value, Account user) {
- sb.append(category.getLabelName());
+ void appendApproval(ApprovalType type, short value, Account user) {
+ sb.append(type.getName());
sb.append(value < 0 ? "-" : "+").append(Math.abs(value)).append(": ");
appendUserData(user);
sb.append("\n");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
index d84deb3..e75f117 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
@@ -14,22 +14,20 @@
package com.google.gerrit.server.mail;
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.Table;
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ApprovalTypes;
+import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
-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.PatchSetApproval;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
-import java.util.HashMap;
-import java.util.Map;
-
/** Send notice about a change successfully merged. */
public class MergedSender extends ReplyToChangeSender {
public static interface Factory {
@@ -61,18 +59,18 @@
public String getApprovals() {
try {
- final Map<Account.Id, Map<ApprovalCategory.Id, PatchSetApproval>> pos =
- new HashMap<Account.Id, Map<ApprovalCategory.Id, PatchSetApproval>>();
-
- final Map<Account.Id, Map<ApprovalCategory.Id, PatchSetApproval>> neg =
- new HashMap<Account.Id, Map<ApprovalCategory.Id, PatchSetApproval>>();
-
+ Table<Account.Id, String, PatchSetApproval> pos = HashBasedTable.create();
+ Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
for (PatchSetApproval ca : args.db.get().patchSetApprovals()
.byPatchSet(patchSet.getId())) {
+ ApprovalType lt = approvalTypes.byId(ca.getCategoryId().get());
+ if (lt == null) {
+ continue;
+ }
if (ca.getValue() > 0) {
- insert(pos, ca);
+ pos.put(ca.getAccountId(), lt.getName(), ca);
} else if (ca.getValue() < 0) {
- insert(neg, ca);
+ neg.put(ca.getAccountId(), lt.getName(), ca);
}
}
@@ -83,22 +81,20 @@
return "";
}
- private String format(final String type,
- final Map<Account.Id, Map<ApprovalCategory.Id, PatchSetApproval>> list) {
+ private String format(String type,
+ Table<Account.Id, String, PatchSetApproval> approvals) {
StringBuilder txt = new StringBuilder();
- if (list.isEmpty()) {
+ if (approvals.isEmpty()) {
return "";
}
- txt.append(type + ":\n");
- for (final Map.Entry<Account.Id, Map<ApprovalCategory.Id, PatchSetApproval>> ent : list
- .entrySet()) {
- final Map<ApprovalCategory.Id, PatchSetApproval> l = ent.getValue();
+ txt.append(type).append(":\n");
+ for (Account.Id id : approvals.rowKeySet()) {
txt.append(" ");
- txt.append(getNameFor(ent.getKey()));
+ txt.append(getNameFor(id));
txt.append(": ");
boolean first = true;
- for (ApprovalType at : approvalTypes.getApprovalTypes()) {
- final PatchSetApproval ca = l.get(at.getCategory().getId());
+ for (ApprovalType lt : approvalTypes.getApprovalTypes()) {
+ PatchSetApproval ca = approvals.get(id, lt.getName());
if (ca == null) {
continue;
}
@@ -109,32 +105,18 @@
txt.append("; ");
}
- final ApprovalCategoryValue v = at.getValue(ca);
+ LabelValue v = lt.getValue(ca);
if (v != null) {
- txt.append(v.getName());
+ txt.append(v.getText());
} else {
- txt.append(at.getCategory().getName());
- txt.append("=");
- if (ca.getValue() > 0) {
- txt.append("+");
- }
- txt.append("" + ca.getValue());
+ txt.append(lt.getName());
+ txt.append('=');
+ txt.append(LabelValue.formatValue(ca.getValue()));
}
}
- txt.append("\n");
+ txt.append('\n');
}
- txt.append("\n");
+ txt.append('\n');
return txt.toString();
}
-
- private void insert(
- final Map<Account.Id, Map<ApprovalCategory.Id, PatchSetApproval>> list,
- final PatchSetApproval ca) {
- Map<ApprovalCategory.Id, PatchSetApproval> m = list.get(ca.getAccountId());
- if (m == null) {
- m = new HashMap<ApprovalCategory.Id, PatchSetApproval>();
- list.put(ca.getAccountId(), m);
- }
- m.put(ca.getCategoryId(), ca);
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
index afec452..d13e7b8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -18,7 +18,6 @@
import com.google.gerrit.common.data.ApprovalTypes;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -58,34 +57,28 @@
abstract boolean match(int psValue, int expValue);
}
- private static ApprovalCategory category(ApprovalTypes types, String toFind) {
+ private static ApprovalType type(ApprovalTypes types, String toFind) {
if (types.byLabel(toFind) != null) {
- return types.byLabel(toFind).getCategory();
+ return types.byLabel(toFind);
}
- if (types.byId(new ApprovalCategory.Id(toFind)) != null) {
- return types.byId(new ApprovalCategory.Id(toFind)).getCategory();
+ if (types.byId(toFind) != null) {
+ return types.byId(toFind);
}
for (ApprovalType at : types.getApprovalTypes()) {
- ApprovalCategory category = at.getCategory();
-
- if (toFind.equalsIgnoreCase(category.getName())) {
- return category;
-
- } else if (toFind.equalsIgnoreCase(category.getName().replace(" ", ""))) {
- return category;
+ if (toFind.equalsIgnoreCase(at.getName())) {
+ return at;
}
}
for (ApprovalType at : types.getApprovalTypes()) {
- ApprovalCategory category = at.getCategory();
- if (toFind.equalsIgnoreCase(category.getAbbreviatedName())) {
- return category;
+ if (toFind.equalsIgnoreCase(at.getAbbreviatedName())) {
+ return at;
}
}
- return new ApprovalCategory(new ApprovalCategory.Id(toFind), toFind);
+ return ApprovalType.withDefaultValues(toFind.substring(0, 4), toFind);
}
private static Test op(String op) {
@@ -114,7 +107,7 @@
private final IdentifiedUser.GenericFactory userFactory;
private final Provider<ReviewDb> dbProvider;
private final Test test;
- private final ApprovalCategory category;
+ private final ApprovalType type;
private final String permissionName;
private final int expVal;
@@ -129,22 +122,22 @@
Matcher m1 = Pattern.compile("(=|>=|<=)([+-]?\\d+)$").matcher(value);
Matcher m2 = Pattern.compile("([+-]\\d+)$").matcher(value);
if (m1.find()) {
- category = category(types, value.substring(0, m1.start()));
+ type = type(types, value.substring(0, m1.start()));
test = op(m1.group(1));
expVal = value(m1.group(2));
} else if (m2.find()) {
- category = category(types, value.substring(0, m2.start()));
+ type = type(types, value.substring(0, m2.start()));
test = Test.EQ;
expVal = value(m2.group(1));
} else {
- category = category(types, value);
+ type = type(types, value);
test = Test.EQ;
expVal = 1;
}
- this.permissionName = Permission.forLabel(category.getLabelName());
+ this.permissionName = Permission.forLabel(type.getName());
}
@Override
@@ -153,7 +146,7 @@
final Set<Account.Id> approversThatVotedInCategory = new HashSet<Account.Id>();
for (PatchSetApproval p : object.currentApprovals(dbProvider)) {
allApprovers.add(p.getAccountId());
- if (p.getCategoryId().equals(category.getId())) {
+ if (p.getCategoryId().get().equals(type.getId())) {
approversThatVotedInCategory.add(p.getAccountId());
if (match(object.change(dbProvider), p.getValue(), p.getAccountId())) {
return true;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java
index f6f1bfb..07591bc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java
@@ -32,6 +32,22 @@
all.put(NoBlock.NAME, new NoBlock());
}
+ public static CategoryFunction forName(String name) {
+ return all.get(name);
+ }
+
+ /**
+ * Locate a function by type.
+ *
+ * @param type the type the function is for.
+ * @return the function implementation; {@link NoOpFunction} if the function
+ * is not known to Gerrit and thus cannot be executed.
+ */
+ public static CategoryFunction forType(ApprovalType type) {
+ CategoryFunction r = all.get(type.getFunctionName());
+ return r != null ? r : new NoOpFunction();
+ }
+
/**
* Locate a function by category.
*
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java
index 74c97f3..0b1d711 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java
@@ -16,14 +16,12 @@
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ApprovalTypes;
+import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.Permission;
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.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.ApprovalCategory.Id;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.ChangeControl;
@@ -47,10 +45,9 @@
private final ApprovalTypes approvalTypes;
private final IdentifiedUser.GenericFactory userFactory;
- private final Map<ApprovalCategory.Id, Collection<PatchSetApproval>> approvals =
- new HashMap<ApprovalCategory.Id, Collection<PatchSetApproval>>();
- private final Map<ApprovalCategory.Id, Boolean> valid =
- new HashMap<ApprovalCategory.Id, Boolean>();
+ private final Map<String, Collection<PatchSetApproval>> approvals =
+ new HashMap<String, Collection<PatchSetApproval>>();
+ private final Map<String, Boolean> valid = new HashMap<String, Boolean>();
private final ChangeControl callerChangeControl;
private final Change change;
@@ -67,10 +64,15 @@
for (final PatchSetApproval ca : all) {
if (psId.equals(ca.getPatchSetId())) {
- Collection<PatchSetApproval> l = approvals.get(ca.getCategoryId());
+ Collection<PatchSetApproval> l =
+ approvals.get(ca.getCategoryId().get());
if (l == null) {
l = new ArrayList<PatchSetApproval>();
- approvals.put(ca.getCategoryId(), l);
+ ApprovalType at = approvalTypes.byId(ca.getCategoryId().get());
+ if (at != null) {
+ // TODO: Support arbitrary labels
+ approvals.put(at.getName(), l);
+ }
}
l.add(ca);
}
@@ -90,20 +92,20 @@
}
public boolean isValid(final ApprovalType at) {
- return isValid(id(at));
+ return isValid(at.getName());
}
- public boolean isValid(final ApprovalCategory.Id id) {
- final Boolean b = valid.get(id);
+ public boolean isValid(final String labelName) {
+ final Boolean b = valid.get(labelName);
return b != null && b;
}
public Collection<PatchSetApproval> getApprovals(final ApprovalType at) {
- return getApprovals(id(at));
+ return getApprovals(at.getName());
}
- public Collection<PatchSetApproval> getApprovals(final ApprovalCategory.Id id) {
- final Collection<PatchSetApproval> l = approvals.get(id);
+ public Collection<PatchSetApproval> getApprovals(final String labelName) {
+ final Collection<PatchSetApproval> l = approvals.get(labelName);
return l != null ? l : Collections.<PatchSetApproval> emptySet();
}
@@ -113,13 +115,13 @@
* <p>
*/
private void applyTypeFloor(final ApprovalType at, final PatchSetApproval a) {
- final ApprovalCategoryValue atMin = at.getMin();
+ final LabelValue atMin = at.getMin();
if (atMin != null && a.getValue() < atMin.getValue()) {
a.setValue(atMin.getValue());
}
- final ApprovalCategoryValue atMax = at.getMax();
+ final LabelValue atMax = at.getMax();
if (atMax != null && a.getValue() > atMax.getValue()) {
a.setValue(atMax.getValue());
}
@@ -135,8 +137,7 @@
* <p>
*/
private void applyRightFloor(final ApprovalType at, final PatchSetApproval a) {
- final ApprovalCategory category = at.getCategory();
- final String permission = Permission.forLabel(category.getLabelName());
+ final String permission = Permission.forLabel(at.getName());
final IdentifiedUser user = userFactory.create(a.getAccountId());
final PermissionRange range = controlFor(user).getRange(permission);
a.setValue((short) range.squash(a.getValue()));
@@ -152,7 +153,7 @@
applyRightFloor(at, ca);
}
- private static Id id(final ApprovalType at) {
- return at.getCategory().getId();
+ private static String id(final ApprovalType at) {
+ return at.getId();
}
}
diff --git a/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java b/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java
index a26a492..8c5aedd 100644
--- a/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED__load_commit_labels_1.java
@@ -60,14 +60,14 @@
continue;
}
- ApprovalType t = types.byId(a.getCategoryId());
+ ApprovalType t = types.byId(a.getCategoryId().get());
if (t == null) {
continue;
}
StructureTerm labelTerm = new StructureTerm(
sym_label,
- SymbolTerm.intern(t.getCategory().getLabelName()),
+ SymbolTerm.intern(t.getName()),
new IntegerTerm(a.getValue()));
StructureTerm userTerm = new StructureTerm(
diff --git a/gerrit-server/src/main/java/gerrit/PRED_get_legacy_approval_types_1.java b/gerrit-server/src/main/java/gerrit/PRED_get_legacy_approval_types_1.java
index cbe0fd8..7da6f1b 100644
--- a/gerrit-server/src/main/java/gerrit/PRED_get_legacy_approval_types_1.java
+++ b/gerrit-server/src/main/java/gerrit/PRED_get_legacy_approval_types_1.java
@@ -73,9 +73,9 @@
static Term export(ApprovalType type) {
return new StructureTerm(symApprovalType,
- SymbolTerm.intern(type.getCategory().getLabelName()),
- SymbolTerm.intern(type.getCategory().getId().get()),
- SymbolTerm.intern(type.getCategory().getFunctionName()),
+ SymbolTerm.intern(type.getName()),
+ SymbolTerm.intern(type.getId()),
+ SymbolTerm.intern(type.getFunctionName()),
new IntegerTerm(type.getMin().getValue()),
new IntegerTerm(type.getMax().getValue()));
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java
index 9f2ccbf..3de1531 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java
@@ -16,13 +16,10 @@
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ApprovalTypes;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
-import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
+import com.google.gerrit.common.data.LabelValue;
import com.google.inject.AbstractModule;
-import java.util.ArrayList;
import java.util.Arrays;
-import java.util.List;
public class GerritCommonTest extends PrologTestCase {
@Override
@@ -30,8 +27,16 @@
super.setUp();
final ApprovalTypes types = new ApprovalTypes(Arrays.asList(
- codeReviewCategory(),
- verifiedCategory()
+ category(0, "CRVW", "Code-Review",
+ value(2, "Looks good to me, approved"),
+ value(1, "Looks good to me, but someone else must approve"),
+ value(0, "No score"),
+ value(-1, "I would prefer that you didn't submit this"),
+ value(-2, "Do not submit")),
+ category(1, "VRIF", "Verified",
+ value(1, "Verified"),
+ value(0, "No score"),
+ value(-1, "Fails"))
));
load("gerrit", "gerrit_common_test.pl", new AbstractModule() {
@@ -42,40 +47,14 @@
});
}
- private static ApprovalType codeReviewCategory() {
- ApprovalCategory cat = category(0, "CRVW", "Code Review");
- List<ApprovalCategoryValue> vals = newList();
- vals.add(value(cat, 2, "Looks good to me, approved"));
- vals.add(value(cat, 1, "Looks good to me, but someone else must approve"));
- vals.add(value(cat, 0, "No score"));
- vals.add(value(cat, -1, "I would prefer that you didn't submit this"));
- vals.add(value(cat, -2, "Do not submit"));
- return new ApprovalType(cat, vals);
+ private static LabelValue value(int value, String text) {
+ return new LabelValue((short) value, text);
}
- private static ApprovalType verifiedCategory() {
- ApprovalCategory cat = category(1, "VRIF", "Verified");
- List<ApprovalCategoryValue> vals = newList();
- vals.add(value(cat, 1, "Verified"));
- vals.add(value(cat, 0, "No score"));
- vals.add(value(cat, -1, "Fails"));
- return new ApprovalType(cat, vals);
- }
-
- private static ApprovalCategory category(int pos, String id, String name) {
- ApprovalCategory cat;
- cat = new ApprovalCategory(new ApprovalCategory.Id(id), name);
- cat.setPosition((short) pos);
- return cat;
- }
-
- private static ArrayList<ApprovalCategoryValue> newList() {
- return new ArrayList<ApprovalCategoryValue>();
- }
-
- private static ApprovalCategoryValue value(ApprovalCategory c, int v, String n) {
- return new ApprovalCategoryValue(
- new ApprovalCategoryValue.Id(c.getId(), (short) v),
- n);
+ private static ApprovalType category(int pos, String id, String name,
+ LabelValue... values) {
+ ApprovalType type = new ApprovalType(id, name, Arrays.asList(values));
+ type.setPosition((short) pos);
+ return type;
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveOption.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveOption.java
index b631553..ee7bcd2 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveOption.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveOption.java
@@ -15,7 +15,7 @@
package com.google.gerrit.sshd.commands;
import com.google.gerrit.common.data.ApprovalType;
-import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
+import com.google.gerrit.common.data.LabelValue;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
@@ -100,7 +100,7 @@
}
String getLabelName() {
- return type.getCategory().getLabelName();
+ return type.getName();
}
public static class Handler extends OneArgumentOptionHandler<Short> {
@@ -121,8 +121,8 @@
}
final short value = Short.parseShort(argument);
- final ApprovalCategoryValue min = cmdOption.type.getMin();
- final ApprovalCategoryValue max = cmdOption.type.getMax();
+ final LabelValue min = cmdOption.type.getMin();
+ final LabelValue max = cmdOption.type.getMax();
if (value < min.getValue() || value > max.getValue()) {
final String name = cmdOption.name();
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
index e6b376a4b..3412507 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
@@ -18,13 +18,12 @@
import com.google.common.collect.Maps;
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ApprovalTypes;
+import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.common.data.ReviewResult.Error.Type;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
-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.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
@@ -32,11 +31,11 @@
import com.google.gerrit.server.change.Abandon;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.PostReview;
-import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Restore;
+import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
import com.google.gerrit.server.changedetail.PublishDraft;
-import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -403,15 +402,13 @@
for (ApprovalType type : approvalTypes.getApprovalTypes()) {
String usage = "";
- final ApprovalCategory category = type.getCategory();
- usage = "score for " + category.getName() + "\n";
+ usage = "score for " + type.getName() + "\n";
- for (ApprovalCategoryValue v : type.getValues()) {
+ for (LabelValue v : type.getValues()) {
usage += v.format() + "\n";
}
- final String name =
- "--" + category.getName().toLowerCase().replace(' ', '-');
+ final String name = "--" + type.getName().toLowerCase();
optionList.add(new ApproveOption(name, usage, type));
}
