Merge "Move RegexListSearcher out of giant server library into ioutil"
diff --git a/Documentation/config-labels.txt b/Documentation/config-labels.txt
index cf78c6d..3db621d 100644
--- a/Documentation/config-labels.txt
+++ b/Documentation/config-labels.txt
@@ -368,6 +368,9 @@
ignored if the label doesn't apply for that branch.
Additionally, the `branch` modifier has no effect when the submit rule
is customized in the rules.pl of the project or inherited from parent projects.
+Branch can be a ref pattern similar to what is documented
+link:access-control.html#reference[here], but must not contain `${username}` or
+`${shardeduserid}`.
[[label_example]]
=== Example
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 54f8022..792cca8 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1357,8 +1357,8 @@
=== Index project
Adds or updates the current project (and children, if specified) in the secondary index.
-The indexing task is executed asynchronously in background, so this command
-returns immediately.
+The indexing task is executed asynchronously in background and this command returns
+immediately if `async` is specified in the input.
As an input, a link:#index-project-input[IndexProjectInput] entity can be provided.
@@ -1369,6 +1369,7 @@
{
"index_children": "true"
+ "async": "true"
}
----
@@ -3162,6 +3163,8 @@
|Field Name ||Description
|`index_children` ||
If children should be indexed recursively.
+|`async` ||
+If projects should be indexed asynchronously.
|================================
[[inherited-boolean-info]]
diff --git a/java/com/google/gerrit/common/data/PermissionRule.java b/java/com/google/gerrit/common/data/PermissionRule.java
index c50af5c..8ab0a55 100644
--- a/java/com/google/gerrit/common/data/PermissionRule.java
+++ b/java/com/google/gerrit/common/data/PermissionRule.java
@@ -66,27 +66,27 @@
action = Action.BLOCK;
}
- public Boolean getForce() {
+ public boolean getForce() {
return force;
}
- public void setForce(Boolean newForce) {
+ public void setForce(boolean newForce) {
force = newForce;
}
- public Integer getMin() {
+ public int getMin() {
return min;
}
- public void setMin(Integer min) {
+ public void setMin(int min) {
this.min = min;
}
- public void setMax(Integer max) {
+ public void setMax(int max) {
this.max = max;
}
- public Integer getMax() {
+ public int getMax() {
return max;
}
@@ -266,7 +266,7 @@
}
public boolean hasRange() {
- return (!(getMin() == null || getMin() == 0)) || (!(getMax() == null || getMax() == 0));
+ return getMin() != 0 || getMax() != 0;
}
public static int parseInt(String value) {
diff --git a/java/com/google/gerrit/extensions/api/projects/IndexProjectInput.java b/java/com/google/gerrit/extensions/api/projects/IndexProjectInput.java
index a41f227..1b962c1 100644
--- a/java/com/google/gerrit/extensions/api/projects/IndexProjectInput.java
+++ b/java/com/google/gerrit/extensions/api/projects/IndexProjectInput.java
@@ -16,4 +16,5 @@
public class IndexProjectInput {
public Boolean indexChildren;
+ public Boolean async;
}
diff --git a/java/com/google/gerrit/extensions/restapi/AcceptsPost.java b/java/com/google/gerrit/extensions/restapi/AcceptsPost.java
deleted file mode 100644
index da87d32..0000000
--- a/java/com/google/gerrit/extensions/restapi/AcceptsPost.java
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright (C) 2012 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.extensions.restapi;
-
-/**
- * Optional interface for {@link RestCollection}.
- *
- * <p>Collections that implement this interface can accept a {@code POST} directly on the collection
- * itself when no id was given in the path. This interface is intended to be used with
- * TopLevelResource collections. Nested collections often bind POST on the parent collection to the
- * view implementation handling the insertion of a new member.
- */
-public interface AcceptsPost<P extends RestResource> {
- /**
- * Handle creation of a child resource by POST on the collection.
- *
- * @param parent parent collection handle.
- * @return a view to perform the creation. The id of the newly created resource should be
- * determined from the input body.
- * @throws RestApiException the view cannot be constructed.
- */
- RestModifyView<P, ?> post(P parent) throws RestApiException;
-}
diff --git a/java/com/google/gerrit/extensions/restapi/RestApiModule.java b/java/com/google/gerrit/extensions/restapi/RestApiModule.java
index ee31113..d47b094 100644
--- a/java/com/google/gerrit/extensions/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/extensions/restapi/RestApiModule.java
@@ -29,6 +29,7 @@
protected static final String DELETE = "DELETE";
protected static final String POST = "POST";
protected static final String CREATE = "CREATE";
+ protected static final String POST_ON_COLLECTION = "POST_ON_COLLECTION";
protected <R extends RestResource> ReadViewBinder<R> get(TypeLiteral<RestView<R>> viewType) {
return get(viewType, "/");
@@ -46,6 +47,12 @@
return delete(viewType, "/");
}
+ protected <R extends RestResource> RestCollectionViewBinder<R> postOnCollection(
+ TypeLiteral<RestView<R>> viewType) {
+ return new RestCollectionViewBinder<>(
+ bind(viewType).annotatedWith(export(POST_ON_COLLECTION, "/")));
+ }
+
protected <R extends RestResource> CreateViewBinder<R> create(TypeLiteral<RestView<R>> viewType) {
return new CreateViewBinder<>(bind(viewType).annotatedWith(export(CREATE, "/")));
}
@@ -142,6 +149,33 @@
}
}
+ public static class RestCollectionViewBinder<C extends RestResource> {
+ private final LinkedBindingBuilder<RestView<C>> binder;
+
+ private RestCollectionViewBinder(LinkedBindingBuilder<RestView<C>> binder) {
+ this.binder = binder;
+ }
+
+ public <P extends RestResource, T extends RestCollectionView<P, C, ?>> ScopedBindingBuilder to(
+ Class<T> impl) {
+ return binder.to(impl);
+ }
+
+ public <P extends RestResource, T extends RestCollectionView<P, C, ?>> void toInstance(T impl) {
+ binder.toInstance(impl);
+ }
+
+ public <P extends RestResource, T extends RestCollectionView<P, C, ?>>
+ ScopedBindingBuilder toProvider(Class<? extends Provider<? extends T>> providerType) {
+ return binder.toProvider(providerType);
+ }
+
+ public <P extends RestResource, T extends RestCollectionView<P, C, ?>>
+ ScopedBindingBuilder toProvider(Provider<? extends T> provider) {
+ return binder.toProvider(provider);
+ }
+ }
+
public static class CreateViewBinder<C extends RestResource> {
private final LinkedBindingBuilder<RestView<C>> binder;
diff --git a/java/com/google/gerrit/extensions/restapi/RestCollectionView.java b/java/com/google/gerrit/extensions/restapi/RestCollectionView.java
new file mode 100644
index 0000000..7e8eef0
--- /dev/null
+++ b/java/com/google/gerrit/extensions/restapi/RestCollectionView.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2018 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.extensions.restapi;
+
+/**
+ * RestView on a RestCollection that supports accepting input.
+ *
+ * <p>The input must be supplied as JSON as the body of the HTTP request. RestCollectionViews can
+ * only be invoked by the HTTP method {@code POST}.
+ *
+ * @param <P> type of the parent resource
+ * @param <C> type of the child resource
+ * @param <I> type of input the JSON parser will parse the input into.
+ */
+public interface RestCollectionView<P extends RestResource, C extends RestResource, I>
+ extends RestView<C> {
+
+ Object apply(P parentResource, I input) throws Exception;
+}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index a454c00..599e98f 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -69,7 +69,6 @@
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AcceptsDelete;
-import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
@@ -87,6 +86,7 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollection;
+import com.google.gerrit.extensions.restapi.RestCollectionView;
import com.google.gerrit.extensions.restapi.RestCreateView;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -308,12 +308,14 @@
if (isRead(req)) {
viewData = new ViewData(null, rc.list());
- } else if (rc instanceof AcceptsPost && isPost(req)) {
- @SuppressWarnings("unchecked")
- AcceptsPost<RestResource> ac = (AcceptsPost<RestResource>) rc;
- viewData = new ViewData(null, ac.post(rsrc));
- } else {
- throw new MethodNotAllowedException();
+ } else if (isPost(req)) {
+ RestView<RestResource> restCollectionView =
+ rc.views().get("gerrit", "POST_ON_COLLECTION./");
+ if (restCollectionView != null) {
+ viewData = new ViewData(null, restCollectionView);
+ } else {
+ throw new MethodNotAllowedException();
+ }
}
} else {
IdString id = path.remove(0);
@@ -350,10 +352,14 @@
if (path.isEmpty()) {
if (isRead(req)) {
viewData = new ViewData(null, c.list());
- } else if (c instanceof AcceptsPost && isPost(req)) {
- @SuppressWarnings("unchecked")
- AcceptsPost<RestResource> ac = (AcceptsPost<RestResource>) c;
- viewData = new ViewData(null, ac.post(rsrc));
+ } else if (isPost(req)) {
+ RestView<RestResource> restCollectionView =
+ c.views().get(viewData.pluginName, "POST_ON_COLLECTION./");
+ if (restCollectionView != null) {
+ viewData = new ViewData(null, restCollectionView);
+ } else {
+ throw new MethodNotAllowedException();
+ }
} else if (c instanceof AcceptsDelete && isDelete(req)) {
@SuppressWarnings("unchecked")
AcceptsDelete<RestResource> ac = (AcceptsDelete<RestResource>) c;
@@ -434,6 +440,19 @@
ServletUtils.consumeRequestBody(is);
}
}
+ } else if (viewData.view instanceof RestCollectionView<?, ?, ?>) {
+ @SuppressWarnings("unchecked")
+ RestCollectionView<RestResource, RestResource, Object> m =
+ (RestCollectionView<RestResource, RestResource, Object>) viewData.view;
+
+ Type type = inputType(m);
+ inputRequestBody = parseRequest(req, type);
+ result = m.apply(rsrc, inputRequestBody);
+ if (inputRequestBody instanceof RawInput) {
+ try (InputStream is = req.getInputStream()) {
+ ServletUtils.consumeRequestBody(is);
+ }
+ }
} else {
throw new ResourceNotFoundException();
}
@@ -778,6 +797,24 @@
return ((ParameterizedType) supertype).getActualTypeArguments()[2];
}
+ private static Type inputType(RestCollectionView<RestResource, RestResource, Object> m) {
+ // MyCollectionView implements RestCollectionView<SomeResource, SomeResource, MyInput>
+ TypeLiteral<?> typeLiteral = TypeLiteral.get(m.getClass());
+
+ // RestCollectionView<SomeResource, SomeResource, MyInput>
+ // This is smart enough to resolve even when there are intervening subclasses, even if they have
+ // reordered type arguments.
+ TypeLiteral<?> supertypeLiteral = typeLiteral.getSupertype(RestCollectionView.class);
+
+ Type supertype = supertypeLiteral.getType();
+ checkState(
+ supertype instanceof ParameterizedType,
+ "supertype of %s is not parameterized: %s",
+ typeLiteral,
+ supertypeLiteral);
+ return ((ParameterizedType) supertype).getActualTypeArguments()[2];
+ }
+
private Object parseRequest(HttpServletRequest req, Type type)
throws IOException, BadRequestException, SecurityException, IllegalArgumentException,
NoSuchMethodException, IllegalAccessException, InstantiationException,
@@ -1142,11 +1179,12 @@
String name = method + "." + p.get(0);
RestView<RestResource> core = views.get("gerrit", name);
if (core != null) {
- return new ViewData(null, core);
+ return new ViewData("gerrit", core);
}
+
core = views.get("gerrit", "GET." + p.get(0));
if (core != null) {
- return new ViewData(null, core);
+ return new ViewData("gerrit", core);
}
Map<String, RestView<RestResource>> r = new TreeMap<>();
diff --git a/java/com/google/gerrit/server/ApprovalCopier.java b/java/com/google/gerrit/server/ApprovalCopier.java
index 922922c..095263e 100644
--- a/java/com/google/gerrit/server/ApprovalCopier.java
+++ b/java/com/google/gerrit/server/ApprovalCopier.java
@@ -79,7 +79,6 @@
*
* @param db review database.
* @param notes change notes for user uploading PatchSet
- * @param user user uploading PatchSet
* @param ps new PatchSet
* @param rw open walk that can read the patch set commit; null to open the repo on demand.
* @param repoConfig repo config used for change kind detection; null to read from repo on demand.
@@ -88,12 +87,11 @@
public void copyInReviewDb(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
throws OrmException {
- copyInReviewDb(db, notes, user, ps, rw, repoConfig, Collections.emptyList());
+ copyInReviewDb(db, notes, ps, rw, repoConfig, Collections.emptyList());
}
/**
@@ -101,7 +99,6 @@
*
* @param db review database.
* @param notes change notes for user uploading PatchSet
- * @param user user uploading PatchSet
* @param ps new PatchSet
* @param rw open walk that can read the patch set commit; null to open the repo on demand.
* @param repoConfig repo config used for change kind detection; null to read from repo on demand.
@@ -111,33 +108,30 @@
public void copyInReviewDb(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
Iterable<PatchSetApproval> dontCopy)
throws OrmException {
if (PrimaryStorage.of(notes.getChange()) == PrimaryStorage.REVIEW_DB) {
- db.patchSetApprovals().insert(getForPatchSet(db, notes, user, ps, rw, repoConfig, dontCopy));
+ db.patchSetApprovals().insert(getForPatchSet(db, notes, ps, rw, repoConfig, dontCopy));
}
}
Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
throws OrmException {
return getForPatchSet(
- db, notes, user, psId, rw, repoConfig, Collections.<PatchSetApproval>emptyList());
+ db, notes, psId, rw, repoConfig, Collections.<PatchSetApproval>emptyList());
}
Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
@@ -147,13 +141,12 @@
if (ps == null) {
return Collections.emptyList();
}
- return getForPatchSet(db, notes, user, ps, rw, repoConfig, dontCopy);
+ return getForPatchSet(db, notes, ps, rw, repoConfig, dontCopy);
}
private Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
@@ -211,7 +204,7 @@
byUser.put(psa.getLabel(), psa.getAccountId(), copy(psa, ps.getId()));
}
}
- return labelNormalizer.normalize(notes, user, byUser.values()).getNormalized();
+ return labelNormalizer.normalize(notes, byUser.values()).getNormalized();
} catch (IOException e) {
throw new OrmException(e);
}
diff --git a/java/com/google/gerrit/server/ApprovalsUtil.java b/java/com/google/gerrit/server/ApprovalsUtil.java
index 8365ddb..0d12eca 100644
--- a/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -388,7 +388,6 @@
public Iterable<PatchSetApproval> byPatchSet(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
@@ -396,13 +395,12 @@
if (!migration.readChanges()) {
return sortApprovals(db.patchSetApprovals().byPatchSet(psId));
}
- return copier.getForPatchSet(db, notes, user, psId, rw, repoConfig);
+ return copier.getForPatchSet(db, notes, psId, rw, repoConfig);
}
public Iterable<PatchSetApproval> byPatchSetUser(
ReviewDb db,
ChangeNotes notes,
- CurrentUser user,
PatchSet.Id psId,
Account.Id accountId,
@Nullable RevWalk rw,
@@ -411,7 +409,7 @@
if (!migration.readChanges()) {
return sortApprovals(db.patchSetApprovals().byPatchSetUser(psId, accountId));
}
- return filterApprovals(byPatchSet(db, notes, user, psId, rw, repoConfig), accountId);
+ return filterApprovals(byPatchSet(db, notes, psId, rw, repoConfig), accountId);
}
public PatchSetApproval getSubmitter(ReviewDb db, ChangeNotes notes, PatchSet.Id c) {
diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java
index c9ca972..fe42a61 100644
--- a/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/java/com/google/gerrit/server/PatchSetUtil.java
@@ -181,17 +181,16 @@
}
/** Check if the current patch set of the change is locked. */
- public void checkPatchSetNotLocked(ChangeNotes notes, CurrentUser user)
+ public void checkPatchSetNotLocked(ChangeNotes notes)
throws OrmException, IOException, ResourceConflictException {
- if (isPatchSetLocked(notes, user)) {
+ if (isPatchSetLocked(notes)) {
throw new ResourceConflictException(
String.format("The current patch set of change %s is locked", notes.getChangeId()));
}
}
/** Is the current patch set locked against state changes? */
- public boolean isPatchSetLocked(ChangeNotes notes, CurrentUser user)
- throws OrmException, IOException {
+ public boolean isPatchSetLocked(ChangeNotes notes) throws OrmException, IOException {
Change change = notes.getChange();
if (change.getStatus() == Change.Status.MERGED) {
return false;
@@ -202,9 +201,8 @@
ApprovalsUtil approvalsUtil = approvalsUtilProvider.get();
for (PatchSetApproval ap :
- approvalsUtil.byPatchSet(
- dbProvider.get(), notes, user, change.currentPatchSetId(), null, null)) {
- LabelType type = projectState.getLabelTypes(notes, user).byLabel(ap.getLabel());
+ approvalsUtil.byPatchSet(dbProvider.get(), notes, change.currentPatchSetId(), null, null)) {
+ LabelType type = projectState.getLabelTypes(notes).byLabel(ap.getLabel());
if (type != null
&& ap.getValue() == 1
&& type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index c6fe93b..8ca364b 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -521,8 +521,7 @@
if (fireRevisionCreated) {
revisionCreated.fire(change, patchSet, ctx.getAccount(), ctx.getWhen(), notify);
if (approvals != null && !approvals.isEmpty()) {
- List<LabelType> labels =
- projectState.getLabelTypes(change.getDest(), ctx.getUser()).getLabelTypes();
+ List<LabelType> labels = projectState.getLabelTypes(change.getDest()).getLabelTypes();
Map<String, Short> allApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
for (LabelType lt : labels) {
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 2feea47..a5bcf48 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -658,10 +658,9 @@
// list permitted labels, since users can't vote on those patch sets.
if (user.isIdentifiedUser()
&& (!limitToPsId.isPresent() || limitToPsId.get().equals(in.currentPatchSetId()))) {
- PermissionBackend.ForChange perm = permissionBackendForChange(user, cd);
out.permittedLabels =
cd.change().getStatus() != Change.Status.ABANDONED
- ? permittedLabels(perm, cd)
+ ? permittedLabels(user.getAccountId(), cd)
: ImmutableMap.of();
}
@@ -889,7 +888,7 @@
LabelTypes labelTypes = cd.getLabelTypes();
for (Account.Id accountId : allUsers) {
PermissionBackend.ForChange perm = permissionBackendForChange(accountId, cd);
- Map<String, VotingRangeInfo> pvr = getPermittedVotingRanges(permittedLabels(perm, cd));
+ Map<String, VotingRangeInfo> pvr = getPermittedVotingRanges(permittedLabels(accountId, cd));
for (Map.Entry<String, LabelWithStatus> e : labels.entrySet()) {
LabelType lt = labelTypes.byLabel(e.getKey());
if (lt == null) {
@@ -1030,8 +1029,7 @@
Map<String, ApprovalInfo> byLabel = Maps.newHashMapWithExpectedSize(labels.size());
Map<String, VotingRangeInfo> pvr = Collections.emptyMap();
if (detailed) {
- PermissionBackend.ForChange perm = permissionBackendForChange(accountId, cd);
- pvr = getPermittedVotingRanges(permittedLabels(perm, cd));
+ pvr = getPermittedVotingRanges(permittedLabels(accountId, cd));
for (Map.Entry<String, LabelWithStatus> entry : labels.entrySet()) {
ApprovalInfo ai = approvalInfo(accountId, 0, null, null, null);
byLabel.put(entry.getKey(), ai);
@@ -1106,8 +1104,7 @@
}
private Map<String, Collection<String>> permittedLabels(
- PermissionBackend.ForChange perm, ChangeData cd)
- throws OrmException, PermissionBackendException {
+ Account.Id filterApprovalsBy, ChangeData cd) throws OrmException, PermissionBackendException {
boolean isMerged = cd.change().getStatus() == Change.Status.MERGED;
LabelTypes labelTypes = cd.getLabelTypes();
Map<String, LabelType> toCheck = new HashMap<>();
@@ -1123,7 +1120,8 @@
}
Map<String, Short> labels = null;
- Set<LabelPermission.WithValue> can = perm.testLabels(toCheck.values());
+ Set<LabelPermission.WithValue> can =
+ permissionBackendForChange(filterApprovalsBy, cd).testLabels(toCheck.values());
SetMultimap<String, String> permitted = LinkedHashMultimap.create();
for (SubmitRecord rec : submitRecords(cd)) {
if (rec.labels == null) {
@@ -1139,7 +1137,7 @@
boolean ok = can.contains(new LabelPermission.WithValue(type, v));
if (isMerged) {
if (labels == null) {
- labels = currentLabels(perm, cd);
+ labels = currentLabels(filterApprovalsBy, cd);
}
short prev = labels.getOrDefault(type.getName(), (short) 0);
ok &= v.getValue() >= prev;
@@ -1163,17 +1161,15 @@
return permitted.asMap();
}
- private Map<String, Short> currentLabels(PermissionBackend.ForChange perm, ChangeData cd)
+ private Map<String, Short> currentLabels(Account.Id accountId, ChangeData cd)
throws OrmException {
- IdentifiedUser user = perm.user().asIdentifiedUser();
Map<String, Short> result = new HashMap<>();
for (PatchSetApproval psa :
approvalsUtil.byPatchSetUser(
db.get(),
lazyLoad ? cd.notes() : notesFactory.createFromIndexedChange(cd.change()),
- user,
cd.change().currentPatchSetId(),
- user.getAccountId(),
+ accountId,
null,
null)) {
result.put(psa.getLabel(), psa.getValue());
diff --git a/java/com/google/gerrit/server/change/LabelNormalizer.java b/java/com/google/gerrit/server/change/LabelNormalizer.java
index f4dbf21..1ec1717 100644
--- a/java/com/google/gerrit/server/change/LabelNormalizer.java
+++ b/java/com/google/gerrit/server/change/LabelNormalizer.java
@@ -26,11 +26,8 @@
import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectCache;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -71,43 +68,25 @@
}
}
- private final IdentifiedUser.GenericFactory userFactory;
private final ProjectCache projectCache;
@Inject
- LabelNormalizer(IdentifiedUser.GenericFactory userFactory, ProjectCache projectCache) {
- this.userFactory = userFactory;
+ LabelNormalizer(ProjectCache projectCache) {
this.projectCache = projectCache;
}
/**
- * @param notes change containing the given approvals.
+ * @param notes change notes containing the given approvals.
* @param approvals list of approvals.
* @return copies of approvals normalized to the defined ranges for the label type. Approvals for
* unknown labels are not included in the output.
- * @throws OrmException
*/
public Result normalize(ChangeNotes notes, Collection<PatchSetApproval> approvals)
- throws OrmException, IOException {
- IdentifiedUser user = userFactory.create(notes.getChange().getOwner());
- return normalize(notes, user, approvals);
- }
-
- /**
- * @param notes change notes containing the given approvals.
- * @param user current user.
- * @param approvals list of approvals.
- * @return copies of approvals normalized to the defined ranges for the label type. Approvals for
- * unknown labels are not included in the output.
- */
- public Result normalize(
- ChangeNotes notes, CurrentUser user, Collection<PatchSetApproval> approvals)
throws IOException {
List<PatchSetApproval> unchanged = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> updated = Lists.newArrayListWithCapacity(approvals.size());
List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size());
- LabelTypes labelTypes =
- projectCache.checkedGet(notes.getProjectName()).getLabelTypes(notes, user);
+ LabelTypes labelTypes = projectCache.checkedGet(notes.getProjectName()).getLabelTypes(notes);
for (PatchSetApproval psa : approvals) {
Change.Id changeId = psa.getKey().getParentKey().getParentKey();
checkArgument(
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index b979240..d71a93d 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -273,12 +273,7 @@
change.setCurrentPatchSet(patchSetInfo);
if (copyApprovals) {
approvalCopier.copyInReviewDb(
- db,
- ctx.getNotes(),
- ctx.getUser(),
- patchSet,
- ctx.getRevWalk(),
- ctx.getRepoView().getConfig());
+ db, ctx.getNotes(), patchSet, ctx.getRevWalk(), ctx.getRepoView().getConfig());
}
if (changeMessage != null) {
cmUtil.addChangeMessage(db, update, changeMessage);
@@ -314,7 +309,7 @@
throws AuthException, ResourceConflictException, IOException, PermissionBackendException,
OrmException {
// Not allowed to create a new patch set if the current patch set is locked.
- psUtil.checkPatchSetNotLocked(origNotes, ctx.getUser());
+ psUtil.checkPatchSetNotLocked(origNotes);
if (checkAddPatchSetPermission) {
permissionBackend
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 80d1cd1..a8c463d 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -408,7 +408,7 @@
}
// Not allowed to edit if the current patch set is locked.
- patchSetUtil.checkPatchSetNotLocked(notes, currentUser.get());
+ patchSetUtil.checkPatchSetNotLocked(notes);
try {
permissionBackend
.currentUser()
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 637be24..0231378 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -361,7 +361,7 @@
PatchSetApproval submitAudit = null;
- for (PatchSetApproval a : safeGetApprovals(notes, user, psId)) {
+ for (PatchSetApproval a : safeGetApprovals(notes, psId)) {
if (a.getValue() <= 0) {
// Negative votes aren't counted.
continue;
@@ -460,10 +460,9 @@
return "Verified".equalsIgnoreCase(id.get());
}
- private Iterable<PatchSetApproval> safeGetApprovals(
- ChangeNotes notes, CurrentUser user, PatchSet.Id psId) {
+ private Iterable<PatchSetApproval> safeGetApprovals(ChangeNotes notes, PatchSet.Id psId) {
try {
- return approvalsUtil.byPatchSet(db.get(), notes, user, psId, null, null);
+ return approvalsUtil.byPatchSet(db.get(), notes, psId, null, null);
} catch (OrmException e) {
logger.atSevere().withCause(e).log("Can't read approval records for %s", psId);
return Collections.emptyList();
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index dd79f3f..d5b1966 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2444,7 +2444,7 @@
RevCommit priorCommit = revisions.inverse().get(priorPatchSet);
// Not allowed to create a new patch set if the current patch set is locked.
- if (psUtil.isPatchSetLocked(notes, user)) {
+ if (psUtil.isPatchSetLocked(notes)) {
reject(inputCommand, "cannot add patch set to " + ontoChange + ".");
return false;
}
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 36c5005..3e7942f 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -309,7 +309,6 @@
approvalCopier.copyInReviewDb(
ctx.getDb(),
ctx.getNotes(),
- ctx.getUser(),
newPatchSet,
ctx.getRevWalk(),
ctx.getRepoView().getConfig(),
@@ -408,7 +407,6 @@
approvalsUtil.byPatchSetUser(
ctx.getDb(),
ctx.getNotes(),
- ctx.getUser(),
priorPatchSetId,
ctx.getAccountId(),
ctx.getRevWalk(),
@@ -541,10 +539,7 @@
* show a transition from an oldValue of 0 to the new value.
*/
List<LabelType> labels =
- projectCache
- .checkedGet(ctx.getProject())
- .getLabelTypes(notes, ctx.getUser())
- .getLabelTypes();
+ projectCache.checkedGet(ctx.getProject()).getLabelTypes(notes).getLabelTypes();
Map<String, Short> allApprovals = new HashMap<>();
Map<String, Short> oldApprovals = new HashMap<>();
for (LabelType lt : labels) {
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 0e0bca6..bf76bde 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -323,7 +323,6 @@
.byPatchSetUser(
ctx.getDb(),
notes,
- ctx.getUser(),
psId,
ctx.getAccountId(),
ctx.getRevWalk(),
diff --git a/java/com/google/gerrit/server/mail/send/MergedSender.java b/java/com/google/gerrit/server/mail/send/MergedSender.java
index cf9257c..34f959c 100644
--- a/java/com/google/gerrit/server/mail/send/MergedSender.java
+++ b/java/com/google/gerrit/server/mail/send/MergedSender.java
@@ -70,12 +70,7 @@
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
for (PatchSetApproval ca :
args.approvalsUtil.byPatchSet(
- args.db.get(),
- changeData.notes(),
- args.identifiedUserFactory.create(changeData.change().getOwner()),
- patchSet.getId(),
- null,
- null)) {
+ args.db.get(), changeData.notes(), patchSet.getId(), null, null)) {
LabelType lt = labelTypes.byLabel(ca.getLabelId());
if (lt == null) {
continue;
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index cf86c99..5a4d1f5 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -16,12 +16,9 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.GerritServerId;
import com.google.inject.Inject;
import java.util.Date;
-import java.util.Optional;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.FooterKey;
@@ -65,17 +62,13 @@
private final LegacyChangeNoteRead legacyChangeNoteRead;
private final ChangeNoteJson changeNoteJson;
-
- private final AccountCache accountCache;
private final String serverId;
@Inject
public ChangeNoteUtil(
ChangeNoteJson changeNoteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
- AccountCache accountCache,
@GerritServerId String serverId) {
- this.accountCache = accountCache;
this.serverId = serverId;
this.changeNoteJson = changeNoteJson;
this.legacyChangeNoteRead = legacyChangeNoteRead;
@@ -90,9 +83,8 @@
}
public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
- Optional<Account> author = accountCache.get(authorId).map(AccountState::getAccount);
return new PersonIdent(
- author.map(Account::getName).orElseGet(() -> Account.getName(authorId)),
+ "Gerrit User " + authorId.toString(),
authorId.get() + "@" + serverId,
when,
serverIdent.getTimeZone());
@@ -101,6 +93,9 @@
@VisibleForTesting
public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
return new PersonIdent(
- author.getName(), author.getId().get() + "@" + serverId, when, serverIdent.getTimeZone());
+ "Gerrit User " + author.getId(),
+ author.getId().get() + "@" + serverId,
+ when,
+ serverIdent.getTimeZone());
}
}
diff --git a/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java b/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java
index 1cf0c7c..7c946b8 100644
--- a/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java
+++ b/java/com/google/gerrit/server/notedb/LegacyChangeNoteWrite.java
@@ -23,8 +23,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.GerritServerId;
import com.google.inject.Inject;
import java.io.OutputStream;
@@ -35,39 +33,30 @@
import java.util.Collections;
import java.util.Date;
import java.util.List;
-import java.util.Optional;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.util.QuotedString;
public class LegacyChangeNoteWrite {
- private final AccountCache accountCache;
private final PersonIdent serverIdent;
private final String serverId;
@Inject
public LegacyChangeNoteWrite(
- AccountCache accountCache,
- @GerritPersonIdent PersonIdent serverIdent,
- @GerritServerId String serverId) {
- this.accountCache = accountCache;
+ @GerritPersonIdent PersonIdent serverIdent, @GerritServerId String serverId) {
this.serverIdent = serverIdent;
this.serverId = serverId;
}
public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
- Optional<Account> author = accountCache.get(authorId).map(AccountState::getAccount);
return new PersonIdent(
- author.map(Account::getName).orElseGet(() -> Account.getName(authorId)),
- authorId.get() + "@" + serverId,
- when,
- serverIdent.getTimeZone());
+ authorId.toString(), authorId.get() + "@" + serverId, when, serverIdent.getTimeZone());
}
@VisibleForTesting
public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
return new PersonIdent(
- author.getName(), author.getId().get() + "@" + serverId, when, serverIdent.getTimeZone());
+ author.toString(), author.getId().get() + "@" + serverId, when, serverIdent.getTimeZone());
}
public String getServerId() {
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index bb752a0..d4d1e67 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -23,6 +23,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -263,13 +264,8 @@
}
@Override
- public CurrentUser user() {
- return getUser();
- }
-
- @Override
public ForChange user(CurrentUser user) {
- return user().equals(user) ? this : forUser(user).asForChange(cd, db);
+ return getUser().equals(user) ? this : forUser(user).asForChange(cd, db);
}
@Override
@@ -308,6 +304,11 @@
return ok;
}
+ @Override
+ public BooleanCondition testCond(ChangePermissionOrLabel perm) {
+ return new PermissionBackendCondition.ForChange(this, perm, getUser());
+ }
+
private boolean can(ChangePermissionOrLabel perm) throws PermissionBackendException {
if (perm instanceof ChangePermission) {
return can((ChangePermission) perm);
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
index 490b45e..bf68026 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
@@ -23,6 +23,7 @@
import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
import com.google.gerrit.extensions.api.access.PluginPermission;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -32,6 +33,7 @@
import com.google.gerrit.server.PeerDaemonUser;
import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.cache.PerThreadCache;
+import com.google.gerrit.server.permissions.PermissionBackendCondition.WithUser;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
@@ -98,11 +100,6 @@
}
@Override
- public CurrentUser user() {
- return user;
- }
-
- @Override
public ForProject project(Project.NameKey project) {
try {
ProjectState state = projectCache.checkedGet(project);
@@ -138,6 +135,11 @@
return ok;
}
+ @Override
+ public BooleanCondition testCond(GlobalOrPluginPermission perm) {
+ return new PermissionBackendCondition.WithUser(this, perm, user);
+ }
+
private boolean can(GlobalOrPluginPermission perm) throws PermissionBackendException {
if (perm instanceof GlobalPermission) {
return can((GlobalPermission) perm);
diff --git a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
index 431bfd9..6c6f136 100644
--- a/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/FailedPermissionBackend.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.permissions;
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -84,11 +85,6 @@
}
@Override
- public CurrentUser user() {
- throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user");
- }
-
- @Override
public ForProject project(Project.NameKey project) {
return new FailedProject(message, cause);
}
@@ -103,6 +99,12 @@
throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
}
+
+ @Override
+ public BooleanCondition testCond(GlobalOrPluginPermission perm) {
+ throw new UnsupportedOperationException(
+ "FailedPermissionBackend does not support conditions");
+ }
}
private static class FailedProject extends ForProject {
@@ -120,11 +122,6 @@
}
@Override
- public CurrentUser user() {
- throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user");
- }
-
- @Override
public ForProject user(CurrentUser user) {
return this;
}
@@ -157,6 +154,12 @@
}
@Override
+ public BooleanCondition testCond(ProjectPermission perm) {
+ throw new UnsupportedOperationException(
+ "FailedPermissionBackend does not support conditions");
+ }
+
+ @Override
public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
@@ -178,11 +181,6 @@
}
@Override
- public CurrentUser user() {
- throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user");
- }
-
- @Override
public ForRef user(CurrentUser user) {
return this;
}
@@ -223,6 +221,12 @@
throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
}
+
+ @Override
+ public BooleanCondition testCond(RefPermission perm) {
+ throw new UnsupportedOperationException(
+ "FailedPermissionBackend does not support conditions");
+ }
}
private static class FailedChange extends ForChange {
@@ -267,8 +271,9 @@
}
@Override
- public CurrentUser user() {
- throw new UnsupportedOperationException("FailedPermissionBackend is not scoped to user");
+ public BooleanCondition testCond(ChangePermissionOrLabel perm) {
+ throw new UnsupportedOperationException(
+ "FailedPermissionBackend does not support conditions");
}
}
}
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 357770d..0690d6c 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -173,9 +173,6 @@
/** PermissionBackend scoped to a specific user. */
public abstract static class WithUser extends AcceptsReviewDb<WithUser> {
- /** Returns the user this instance is scoped to. */
- public abstract CurrentUser user();
-
/** Returns an instance scoped for the specified project. */
public abstract ForProject project(Project.NameKey project);
@@ -257,9 +254,7 @@
}
}
- public BooleanCondition testCond(GlobalOrPluginPermission perm) {
- return new PermissionBackendCondition.WithUser(this, perm);
- }
+ public abstract BooleanCondition testCond(GlobalOrPluginPermission perm);
/**
* Filter a set of projects using {@code check(perm)}.
@@ -296,9 +291,6 @@
/** PermissionBackend scoped to a user and project. */
public abstract static class ForProject extends AcceptsReviewDb<ForProject> {
- /** Returns the user this instance is scoped to. */
- public abstract CurrentUser user();
-
/** Returns the fully qualified resource path that this instance is scoped to. */
public abstract String resourcePath();
@@ -355,9 +347,7 @@
}
}
- public BooleanCondition testCond(ProjectPermission perm) {
- return new PermissionBackendCondition.ForProject(this, perm);
- }
+ public abstract BooleanCondition testCond(ProjectPermission perm);
/**
* Filter a map of references by visibility.
@@ -407,9 +397,6 @@
/** PermissionBackend scoped to a user, project and reference. */
public abstract static class ForRef extends AcceptsReviewDb<ForRef> {
- /** Returns the user this instance is scoped to. */
- public abstract CurrentUser user();
-
/** Returns a fully qualified resource path that this instance is scoped to. */
public abstract String resourcePath();
@@ -461,16 +448,11 @@
}
}
- public BooleanCondition testCond(RefPermission perm) {
- return new PermissionBackendCondition.ForRef(this, perm);
- }
+ public abstract BooleanCondition testCond(RefPermission perm);
}
/** PermissionBackend scoped to a user, project, reference and change. */
public abstract static class ForChange extends AcceptsReviewDb<ForChange> {
- /** Returns the user this instance is scoped to. */
- public abstract CurrentUser user();
-
/** Returns the fully qualified resource path that this instance is scoped to. */
public abstract String resourcePath();
@@ -511,9 +493,7 @@
}
}
- public BooleanCondition testCond(ChangePermissionOrLabel perm) {
- return new PermissionBackendCondition.ForChange(this, perm);
- }
+ public abstract BooleanCondition testCond(ChangePermissionOrLabel perm);
/**
* Test which values of a label the user may be able to set.
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java b/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java
index 3a661cda..1b6b087 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java
@@ -56,10 +56,13 @@
public static class WithUser extends PermissionBackendCondition {
private final PermissionBackend.WithUser impl;
private final GlobalOrPluginPermission perm;
+ private final CurrentUser user;
- WithUser(PermissionBackend.WithUser impl, GlobalOrPluginPermission perm) {
+ public WithUser(
+ PermissionBackend.WithUser impl, GlobalOrPluginPermission perm, CurrentUser user) {
this.impl = impl;
this.perm = perm;
+ this.user = user;
}
public PermissionBackend.WithUser withUser() {
@@ -82,7 +85,7 @@
@Override
public int hashCode() {
- return Objects.hash(perm, hashForUser(impl.user()));
+ return Objects.hash(perm, hashForUser(user));
}
@Override
@@ -91,17 +94,19 @@
return false;
}
WithUser other = (WithUser) obj;
- return Objects.equals(perm, other.perm) && usersAreEqual(impl.user(), other.impl.user());
+ return Objects.equals(perm, other.perm) && usersAreEqual(user, other.user);
}
}
public static class ForProject extends PermissionBackendCondition {
private final PermissionBackend.ForProject impl;
private final ProjectPermission perm;
+ private final CurrentUser user;
- ForProject(PermissionBackend.ForProject impl, ProjectPermission perm) {
+ public ForProject(PermissionBackend.ForProject impl, ProjectPermission perm, CurrentUser user) {
this.impl = impl;
this.perm = perm;
+ this.user = user;
}
public PermissionBackend.ForProject project() {
@@ -124,7 +129,7 @@
@Override
public int hashCode() {
- return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user()));
+ return Objects.hash(perm, impl.resourcePath(), hashForUser(user));
}
@Override
@@ -135,17 +140,19 @@
ForProject other = (ForProject) obj;
return Objects.equals(perm, other.perm)
&& Objects.equals(impl.resourcePath(), other.impl.resourcePath())
- && usersAreEqual(impl.user(), other.impl.user());
+ && usersAreEqual(user, other.user);
}
}
public static class ForRef extends PermissionBackendCondition {
private final PermissionBackend.ForRef impl;
private final RefPermission perm;
+ private final CurrentUser user;
- ForRef(PermissionBackend.ForRef impl, RefPermission perm) {
+ public ForRef(PermissionBackend.ForRef impl, RefPermission perm, CurrentUser user) {
this.impl = impl;
this.perm = perm;
+ this.user = user;
}
public PermissionBackend.ForRef ref() {
@@ -168,7 +175,7 @@
@Override
public int hashCode() {
- return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user()));
+ return Objects.hash(perm, impl.resourcePath(), hashForUser(user));
}
@Override
@@ -179,17 +186,20 @@
ForRef other = (ForRef) obj;
return Objects.equals(perm, other.perm)
&& Objects.equals(impl.resourcePath(), other.impl.resourcePath())
- && usersAreEqual(impl.user(), other.impl.user());
+ && usersAreEqual(user, other.user);
}
}
public static class ForChange extends PermissionBackendCondition {
private final PermissionBackend.ForChange impl;
private final ChangePermissionOrLabel perm;
+ private final CurrentUser user;
- ForChange(PermissionBackend.ForChange impl, ChangePermissionOrLabel perm) {
+ public ForChange(
+ PermissionBackend.ForChange impl, ChangePermissionOrLabel perm, CurrentUser user) {
this.impl = impl;
this.perm = perm;
+ this.user = user;
}
public PermissionBackend.ForChange change() {
@@ -212,7 +222,7 @@
@Override
public int hashCode() {
- return Objects.hash(perm, impl.resourcePath(), hashForUser(impl.user()));
+ return Objects.hash(perm, impl.resourcePath(), hashForUser(user));
}
@Override
@@ -223,7 +233,7 @@
ForChange other = (ForChange) obj;
return Objects.equals(perm, other.perm)
&& Objects.equals(impl.resourcePath(), other.impl.resourcePath())
- && usersAreEqual(impl.user(), other.impl.user());
+ && usersAreEqual(user, other.user);
}
}
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index 2d2a64d..ed12a2b 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -19,6 +19,7 @@
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -323,11 +324,6 @@
private String resourcePath;
@Override
- public CurrentUser user() {
- return getUser();
- }
-
- @Override
public ForProject user(CurrentUser user) {
return forUser(user).asForProject().database(db);
}
@@ -395,6 +391,11 @@
}
@Override
+ public BooleanCondition testCond(ProjectPermission perm) {
+ return new PermissionBackendCondition.ForProject(this, perm, getUser());
+ }
+
+ @Override
public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
if (refFilter == null) {
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index f68a7a2..012d68a 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -20,6 +20,7 @@
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.data.PermissionRule.Action;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -408,11 +409,6 @@
private String resourcePath;
@Override
- public CurrentUser user() {
- return getUser();
- }
-
- @Override
public ForRef user(CurrentUser user) {
return forUser(user).asForRef().database(db);
}
@@ -480,6 +476,11 @@
return ok;
}
+ @Override
+ public BooleanCondition testCond(RefPermission perm) {
+ return new PermissionBackendCondition.ForRef(this, perm, getUser());
+ }
+
private boolean can(RefPermission perm) throws PermissionBackendException {
switch (perm) {
case READ:
diff --git a/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java b/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
index d840123..c3c76de 100644
--- a/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
+++ b/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
@@ -108,7 +108,11 @@
if (!iUser.getEffectiveGroups().containsAnyOf(okGroupIds)) {
final StringBuilder msg = new StringBuilder();
- msg.append("A Contributor Agreement must be completed before uploading");
+ msg.append("No Contributor Agreement on file for user ")
+ .append(iUser.getNameEmail())
+ .append(" (id=")
+ .append(iUser.getAccountId())
+ .append(")");
if (canonicalWebUrl != null) {
msg.append(":\n\n ");
msg.append(canonicalWebUrl);
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index a490f10..87dbcfc 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -40,7 +40,6 @@
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
@@ -394,13 +393,13 @@
return labelTypes;
}
- /** All available label types for this change and user. */
- public LabelTypes getLabelTypes(ChangeNotes notes, CurrentUser user) {
- return getLabelTypes(notes.getChange().getDest(), user);
+ /** All available label types for this change. */
+ public LabelTypes getLabelTypes(ChangeNotes notes) {
+ return getLabelTypes(notes.getChange().getDest());
}
- /** All available label types for this branch and user. */
- public LabelTypes getLabelTypes(Branch.NameKey destination, CurrentUser user) {
+ /** All available label types for this branch. */
+ public LabelTypes getLabelTypes(Branch.NameKey destination) {
List<LabelType> all = getLabelTypes().getLabelTypes();
List<LabelType> r = Lists.newArrayListWithCapacity(all.size());
@@ -410,7 +409,15 @@
r.add(l);
} else {
for (String refPattern : refs) {
- if (RefConfigSection.isValid(refPattern) && match(destination, refPattern, user)) {
+ if (refPattern.contains("${")) {
+ logger.atWarning().log(
+ "Ref pattern for label %s in project %s contains illegal expanded parameters: %s."
+ + " Ref pattern will be ignored.",
+ l, getName(), refPattern);
+ continue;
+ }
+
+ if (RefConfigSection.isValid(refPattern) && match(destination, refPattern)) {
r.add(l);
break;
}
@@ -558,7 +565,7 @@
return new LabelTypes(Collections.unmodifiableList(all));
}
- private boolean match(Branch.NameKey destination, String refPattern, CurrentUser user) {
- return RefPatternMatcher.getMatcher(refPattern).match(destination.get(), user);
+ private boolean match(Branch.NameKey destination, String refPattern) {
+ return RefPatternMatcher.getMatcher(refPattern).match(destination.get(), null);
}
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 1a336fc..1813211 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -51,7 +51,6 @@
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
@@ -325,7 +324,7 @@
ChangeData cd =
new ChangeData(
null, null, null, null, null, null, null, null, null, null, null, null, null, null,
- null, null, null, null, project, id, null, null);
+ null, null, null, project, id, null, null);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd;
}
@@ -338,7 +337,6 @@
private final ChangeNotes.Factory notesFactory;
private final CommentsUtil commentsUtil;
private final GitRepositoryManager repoManager;
- private final IdentifiedUser.GenericFactory userFactory;
private final MergeUtil.Factory mergeUtilFactory;
private final MergeabilityCache mergeabilityCache;
private final NotesMigration notesMigration;
@@ -407,7 +405,6 @@
ChangeNotes.Factory notesFactory,
CommentsUtil commentsUtil,
GitRepositoryManager repoManager,
- IdentifiedUser.GenericFactory userFactory,
MergeUtil.Factory mergeUtilFactory,
MergeabilityCache mergeabilityCache,
NotesMigration notesMigration,
@@ -428,7 +425,6 @@
this.notesFactory = notesFactory;
this.commentsUtil = commentsUtil;
this.repoManager = repoManager;
- this.userFactory = userFactory;
this.mergeUtilFactory = mergeUtilFactory;
this.mergeabilityCache = mergeabilityCache;
this.notesMigration = notesMigration;
@@ -590,7 +586,7 @@
} catch (IOException e) {
throw new OrmException("project state not available", e);
}
- labelTypes = state.getLabelTypes(change().getDest(), userFactory.create(change().getOwner()));
+ labelTypes = state.getLabelTypes(change().getDest());
}
return labelTypes;
}
@@ -633,13 +629,7 @@
try {
currentApprovals =
ImmutableList.copyOf(
- approvalsUtil.byPatchSet(
- db,
- notes(),
- userFactory.create(c.getOwner()),
- c.currentPatchSetId(),
- null,
- null));
+ approvalsUtil.byPatchSet(db, notes(), c.currentPatchSetId(), null, null));
} catch (OrmException e) {
if (e.getCause() instanceof NoSuchChangeException) {
currentApprovals = Collections.emptyList();
diff --git a/java/com/google/gerrit/server/restapi/account/AddSshKey.java b/java/com/google/gerrit/server/restapi/account/AddSshKey.java
index ab06e25..9d9c1e3 100644
--- a/java/com/google/gerrit/server/restapi/account/AddSshKey.java
+++ b/java/com/google/gerrit/server/restapi/account/AddSshKey.java
@@ -26,7 +26,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.RestCollectionView;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResource;
@@ -46,7 +46,8 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
-public class AddSshKey implements RestModifyView<AccountResource, SshKeyInput> {
+public class AddSshKey
+ implements RestCollectionView<AccountResource, AccountResource.SshKey, SshKeyInput> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final Provider<CurrentUser> self;
diff --git a/java/com/google/gerrit/server/restapi/account/Module.java b/java/com/google/gerrit/server/restapi/account/Module.java
index f37687f..be0924a 100644
--- a/java/com/google/gerrit/server/restapi/account/Module.java
+++ b/java/com/google/gerrit/server/restapi/account/Module.java
@@ -66,12 +66,12 @@
put(EMAIL_KIND, "preferred").to(PutPreferred.class);
put(ACCOUNT_KIND, "password.http").to(PutHttpPassword.class);
delete(ACCOUNT_KIND, "password.http").to(PutHttpPassword.class);
- child(ACCOUNT_KIND, "sshkeys").to(SshKeys.class);
- post(ACCOUNT_KIND, "sshkeys").to(AddSshKey.class);
get(ACCOUNT_KIND, "watched.projects").to(GetWatchedProjects.class);
post(ACCOUNT_KIND, "watched.projects").to(PostWatchedProjects.class);
post(ACCOUNT_KIND, "watched.projects:delete").to(DeleteWatchedProjects.class);
+ child(ACCOUNT_KIND, "sshkeys").to(SshKeys.class);
+ postOnCollection(SSH_KEY_KIND).to(AddSshKey.class);
get(SSH_KEY_KIND).to(GetSshKey.class);
delete(SSH_KEY_KIND).to(DeleteSshKey.class);
diff --git a/java/com/google/gerrit/server/restapi/change/Abandon.java b/java/com/google/gerrit/server/restapi/change/Abandon.java
index 7978990..c739e54 100644
--- a/java/com/google/gerrit/server/restapi/change/Abandon.java
+++ b/java/com/google/gerrit/server/restapi/change/Abandon.java
@@ -81,7 +81,7 @@
throws RestApiException, UpdateException, OrmException, PermissionBackendException,
IOException, ConfigInvalidException {
// Not allowed to abandon if the current patch set is locked.
- patchSetUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+ patchSetUtil.checkPatchSetNotLocked(rsrc.getNotes());
rsrc.permissions().database(dbProvider).check(ChangePermission.ABANDON);
@@ -155,7 +155,7 @@
}
try {
- if (patchSetUtil.isPatchSetLocked(rsrc.getNotes(), rsrc.getUser())) {
+ if (patchSetUtil.isPatchSetLocked(rsrc.getNotes())) {
return description;
}
} catch (OrmException | IOException e) {
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index 826c792..efc2712 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -20,7 +20,6 @@
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AcceptsDelete;
-import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
@@ -32,6 +31,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestCollectionView;
import com.google.gerrit.extensions.restapi.RestCreateView;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -71,26 +71,21 @@
@Singleton
public class ChangeEdits
- implements ChildCollection<ChangeResource, ChangeEditResource>,
- AcceptsPost<ChangeResource>,
- AcceptsDelete<ChangeResource> {
+ implements ChildCollection<ChangeResource, ChangeEditResource>, AcceptsDelete<ChangeResource> {
private final DynamicMap<RestView<ChangeEditResource>> views;
private final DeleteFile.Factory deleteFileFactory;
private final Provider<Detail> detail;
private final ChangeEditUtil editUtil;
- private final Post post;
@Inject
ChangeEdits(
DynamicMap<RestView<ChangeEditResource>> views,
Provider<Detail> detail,
ChangeEditUtil editUtil,
- Post post,
DeleteFile.Factory deleteFileFactory) {
this.views = views;
this.detail = detail;
this.editUtil = editUtil;
- this.post = post;
this.deleteFileFactory = deleteFileFactory;
}
@@ -114,11 +109,6 @@
return new ChangeEditResource(rsrc, edit.get(), id.get());
}
- @Override
- public Post post(ChangeResource parent) throws RestApiException {
- return post;
- }
-
/**
* This method is invoked if a DELETE request on a non-existing member is done. For change edits
* this is the case if a DELETE request for a file in a change edit is done and the change edit
@@ -247,7 +237,8 @@
* The combination of two operations in one request is supported.
*/
@Singleton
- public static class Post implements RestModifyView<ChangeResource, Post.Input> {
+ public static class Post
+ implements RestCollectionView<ChangeResource, ChangeEditResource, Post.Input> {
public static class Input {
public String restorePath;
public String oldPath;
diff --git a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
index 58ea185..e9b4d87 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangesCollection.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.restapi.change;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -43,14 +42,12 @@
import java.util.List;
@Singleton
-public class ChangesCollection
- implements RestCollection<TopLevelResource, ChangeResource>, AcceptsPost<TopLevelResource> {
+public class ChangesCollection implements RestCollection<TopLevelResource, ChangeResource> {
private final Provider<ReviewDb> db;
private final Provider<CurrentUser> user;
private final Provider<QueryChanges> queryFactory;
private final DynamicMap<RestView<ChangeResource>> views;
private final ChangeFinder changeFinder;
- private final CreateChange createChange;
private final ChangeResource.Factory changeResourceFactory;
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
@@ -62,7 +59,6 @@
Provider<QueryChanges> queryFactory,
DynamicMap<RestView<ChangeResource>> views,
ChangeFinder changeFinder,
- CreateChange createChange,
ChangeResource.Factory changeResourceFactory,
PermissionBackend permissionBackend,
ProjectCache projectCache) {
@@ -71,7 +67,6 @@
this.queryFactory = queryFactory;
this.views = views;
this.changeFinder = changeFinder;
- this.createChange = createChange;
this.changeResourceFactory = changeResourceFactory;
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
@@ -130,11 +125,6 @@
return changeResourceFactory.create(notes, user);
}
- @Override
- public CreateChange post(TopLevelResource parent) throws RestApiException {
- return createChange;
- }
-
private boolean canRead(ChangeNotes notes) throws PermissionBackendException, IOException {
try {
permissionBackend.currentUser().change(notes).database(db).check(ChangePermission.READ);
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index 73669a1..0dcbeb0 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -50,6 +50,7 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyUtil;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -68,7 +69,7 @@
import com.google.gerrit.server.restapi.project.ProjectsCollection;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryingRestModifyView;
+import com.google.gerrit.server.update.RetryingRestCollectionView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -97,7 +98,8 @@
@Singleton
public class CreateChange
- extends RetryingRestModifyView<TopLevelResource, ChangeInput, Response<ChangeInfo>> {
+ extends RetryingRestCollectionView<
+ TopLevelResource, ChangeResource, ChangeInput, Response<ChangeInfo>> {
private final String anonymousCowardName;
private final Provider<ReviewDb> db;
private final GitRepositoryManager gitManager;
diff --git a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
index ff85880..a5698b6 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
@@ -128,7 +128,7 @@
throws OrmException, IOException, RestApiException, UpdateException,
PermissionBackendException {
// Not allowed to create a new patch set if the current patch set is locked.
- psUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+ psUtil.checkPatchSetNotLocked(rsrc.getNotes());
rsrc.permissions().database(db).check(ChangePermission.ADD_PATCH_SET);
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
index 91f5d15..2cc4ce4 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteReviewerOp.java
@@ -131,8 +131,7 @@
currChange = ctx.getChange();
currPs = psUtil.current(ctx.getDb(), ctx.getNotes());
- LabelTypes labelTypes =
- projectCache.checkedGet(ctx.getProject()).getLabelTypes(ctx.getNotes(), ctx.getUser());
+ LabelTypes labelTypes = projectCache.checkedGet(ctx.getProject()).getLabelTypes(ctx.getNotes());
// removing a reviewer will remove all her votes
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
index f268a30..dc44e65 100644
--- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java
+++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java
@@ -172,7 +172,7 @@
ps = psUtil.current(db.get(), ctx.getNotes());
boolean found = false;
- LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes());
Account.Id accountId = accountState.getAccount().getId();
@@ -180,7 +180,6 @@
approvalsUtil.byPatchSetUser(
ctx.getDb(),
ctx.getNotes(),
- ctx.getUser(),
psId,
accountId,
ctx.getRevWalk(),
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index ef0ba2d..dca64fd 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -72,6 +72,7 @@
DynamicMap.mapOf(binder(), VOTE_KIND);
DynamicMap.mapOf(binder(), CHANGE_MESSAGE_KIND);
+ postOnCollection(CHANGE_KIND).to(CreateChange.class);
get(CHANGE_KIND).to(GetChange.class);
post(CHANGE_KIND, "merge").to(CreateMergePatchSet.class);
get(CHANGE_KIND, "detail").to(GetDetail.class);
@@ -112,9 +113,9 @@
post(CHANGE_KIND, "ready").to(SetReadyForReview.class);
put(CHANGE_KIND, "message").to(PutMessage.class);
- post(CHANGE_KIND, "reviewers").to(PostReviewers.class);
get(CHANGE_KIND, "suggest_reviewers").to(SuggestChangeReviewers.class);
child(CHANGE_KIND, "reviewers").to(Reviewers.class);
+ postOnCollection(REVIEWER_KIND).to(PostReviewers.class);
get(REVIEWER_KIND).to(GetReviewer.class);
delete(REVIEWER_KIND).to(DeleteReviewer.class);
post(REVIEWER_KIND, "delete").to(DeleteReviewer.class);
@@ -170,9 +171,12 @@
child(CHANGE_KIND, "edit").to(ChangeEdits.class);
create(CHANGE_EDIT_KIND).to(ChangeEdits.Create.class);
+ postOnCollection(CHANGE_EDIT_KIND).to(ChangeEdits.Post.class);
delete(CHANGE_KIND, "edit").to(DeleteChangeEdit.class);
child(CHANGE_KIND, "edit:publish").to(PublishChangeEdit.class);
+ postOnCollection(CHANGE_EDIT_PUBLISH_KIND).to(PublishChangeEdit.Publish.class);
child(CHANGE_KIND, "edit:rebase").to(RebaseChangeEdit.class);
+ postOnCollection(CHANGE_EDIT_REBASE_KIND).to(RebaseChangeEdit.Rebase.class);
put(CHANGE_KIND, "edit:message").to(ChangeEdits.EditMessage.class);
get(CHANGE_KIND, "edit:message").to(ChangeEdits.GetMessage.class);
put(CHANGE_EDIT_KIND, "/").to(ChangeEdits.Put.class);
diff --git a/java/com/google/gerrit/server/restapi/change/Move.java b/java/com/google/gerrit/server/restapi/change/Move.java
index 42531c5..3833050 100644
--- a/java/com/google/gerrit/server/restapi/change/Move.java
+++ b/java/com/google/gerrit/server/restapi/change/Move.java
@@ -44,7 +44,6 @@
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeJson;
@@ -88,7 +87,6 @@
private final PatchSetUtil psUtil;
private final ApprovalsUtil approvalsUtil;
private final ProjectCache projectCache;
- private final Provider<CurrentUser> userProvider;
@Inject
Move(
@@ -101,8 +99,7 @@
RetryHelper retryHelper,
PatchSetUtil psUtil,
ApprovalsUtil approvalsUtil,
- ProjectCache projectCache,
- Provider<CurrentUser> userProvider) {
+ ProjectCache projectCache) {
super(retryHelper);
this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
@@ -113,7 +110,6 @@
this.psUtil = psUtil;
this.approvalsUtil = approvalsUtil;
this.projectCache = projectCache;
- this.userProvider = userProvider;
}
@Override
@@ -139,7 +135,7 @@
}
// Not allowed to move if the current patch set is locked.
- psUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+ psUtil.checkPatchSetNotLocked(rsrc.getNotes());
// Move requires abandoning this change, and creating a new change.
try {
@@ -261,15 +257,9 @@
List<PatchSetApproval> approvals = new ArrayList<>();
for (PatchSetApproval psa :
approvalsUtil.byPatchSet(
- ctx.getDb(),
- ctx.getNotes(),
- userProvider.get(),
- psId,
- ctx.getRevWalk(),
- ctx.getRepoView().getConfig())) {
+ ctx.getDb(), ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
ProjectState projectState = projectCache.checkedGet(project);
- LabelType type =
- projectState.getLabelTypes(ctx.getNotes(), ctx.getUser()).byLabel(psa.getLabelId());
+ LabelType type = projectState.getLabelTypes(ctx.getNotes()).byLabel(psa.getLabelId());
// Only keep veto votes, defined as votes where:
// 1- the label function allows minimum values to block submission.
// 2- the vote holds the minimum value.
@@ -314,7 +304,7 @@
}
try {
- if (psUtil.isPatchSetLocked(rsrc.getNotes(), rsrc.getUser())) {
+ if (psUtil.isPatchSetLocked(rsrc.getNotes())) {
return description;
}
} catch (OrmException | IOException e) {
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index e6f4f69..c6dbda3 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -237,7 +237,7 @@
throw new ResourceConflictException("cannot post review on edit");
}
ProjectState projectState = projectCache.checkedGet(revision.getProject());
- LabelTypes labelTypes = projectState.getLabelTypes(revision.getNotes(), revision.getUser());
+ LabelTypes labelTypes = projectState.getLabelTypes(revision.getNotes());
input.drafts = firstNonNull(input.drafts, DraftHandling.KEEP);
if (input.onBehalfOf != null) {
revision = onBehalfOf(revision, labelTypes, input);
@@ -1148,7 +1148,7 @@
List<PatchSetApproval> del = new ArrayList<>();
List<PatchSetApproval> ups = new ArrayList<>();
Map<String, PatchSetApproval> current = scanLabels(projectState, ctx, del);
- LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes());
Map<String, Short> allApprovals =
getAllApprovals(labelTypes, approvalsByKey(current.values()), inLabels);
Map<String, Short> previous =
@@ -1297,11 +1297,7 @@
// If no existing label is being set to 0, hack in the caller
// as a reviewer by picking the first server-wide LabelType.
LabelId labelId =
- projectState
- .getLabelTypes(ctx.getNotes(), ctx.getUser())
- .getLabelTypes()
- .get(0)
- .getLabelId();
+ projectState.getLabelTypes(ctx.getNotes()).getLabelTypes().get(0).getLabelId();
PatchSetApproval c = ApprovalsUtil.newApproval(psId, user, labelId, 0, ctx.getWhen());
c.setTag(in.tag);
c.setGranted(ctx.getWhen());
@@ -1322,14 +1318,13 @@
private Map<String, PatchSetApproval> scanLabels(
ProjectState projectState, ChangeContext ctx, List<PatchSetApproval> del)
throws OrmException, IOException {
- LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes(), ctx.getUser());
+ LabelTypes labelTypes = projectState.getLabelTypes(ctx.getNotes());
Map<String, PatchSetApproval> current = new HashMap<>();
for (PatchSetApproval a :
approvalsUtil.byPatchSetUser(
ctx.getDb(),
ctx.getNotes(),
- ctx.getUser(),
psId,
user.getAccountId(),
ctx.getRevWalk(),
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
index 65c7db7..4991c18 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
@@ -51,6 +51,7 @@
import com.google.gerrit.server.change.ChangeMessages;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyUtil;
+import com.google.gerrit.server.change.ReviewerResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
@@ -69,7 +70,7 @@
import com.google.gerrit.server.restapi.group.GroupsCollection;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryingRestModifyView;
+import com.google.gerrit.server.update.RetryingRestCollectionView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -85,7 +86,8 @@
@Singleton
public class PostReviewers
- extends RetryingRestModifyView<ChangeResource, AddReviewerInput, AddReviewerResult> {
+ extends RetryingRestCollectionView<
+ ChangeResource, ReviewerResource, AddReviewerInput, AddReviewerResult> {
public static final int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10;
public static final int DEFAULT_MAX_REVIEWERS = 20;
@@ -452,17 +454,13 @@
}
ChangeData cd = changeDataFactory.create(dbProvider.get(), notes);
- PermissionBackend.ForChange perm =
- permissionBackend.user(caller).database(dbProvider).change(cd);
-
// Generate result details and fill AccountLoader. This occurs outside
// the Op because the accounts are in a different table.
PostReviewersOp.Result opResult = op.getResult();
if (migration.readChanges() && state == CC) {
result.ccs = Lists.newArrayListWithCapacity(opResult.addedCCs().size());
for (Account.Id accountId : opResult.addedCCs()) {
- result.ccs.add(
- json.format(new ReviewerInfo(accountId.get()), perm.absentUser(accountId), cd));
+ result.ccs.add(json.format(new ReviewerInfo(accountId.get()), accountId, cd));
}
accountLoaderFactory.create(true).fill(result.ccs);
for (Address a : reviewersByEmail) {
@@ -475,7 +473,7 @@
result.reviewers.add(
json.format(
new ReviewerInfo(psa.getAccountId().get()),
- perm.absentUser(psa.getAccountId()),
+ psa.getAccountId(),
cd,
ImmutableList.of(psa)));
}
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewersOp.java b/java/com/google/gerrit/server/restapi/change/PostReviewersOp.java
index 0502e91..3cb6136 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewersOp.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewersOp.java
@@ -169,7 +169,7 @@
ctx.getUpdate(ctx.getChange().currentPatchSetId()),
projectCache
.checkedGet(rsrc.getProject())
- .getLabelTypes(rsrc.getChange().getDest(), ctx.getUser()),
+ .getLabelTypes(rsrc.getChange().getDest()),
rsrc.getChange(),
reviewers);
if (addedReviewers.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java b/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
index 36a0561..7f4844e 100644
--- a/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
+++ b/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
@@ -16,7 +16,6 @@
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -33,7 +32,7 @@
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryingRestModifyView;
+import com.google.gerrit.server.update.RetryingRestCollectionView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -44,16 +43,13 @@
@Singleton
public class PublishChangeEdit
- implements ChildCollection<ChangeResource, ChangeEditResource.Publish>,
- AcceptsPost<ChangeResource> {
+ implements ChildCollection<ChangeResource, ChangeEditResource.Publish> {
private final DynamicMap<RestView<ChangeEditResource.Publish>> views;
- private final Publish publish;
@Inject
- PublishChangeEdit(DynamicMap<RestView<ChangeEditResource.Publish>> views, Publish publish) {
+ PublishChangeEdit(DynamicMap<RestView<ChangeEditResource.Publish>> views) {
this.views = views;
- this.publish = publish;
}
@Override
@@ -72,14 +68,10 @@
throw new ResourceNotFoundException();
}
- @Override
- public Publish post(ChangeResource parent) throws RestApiException {
- return publish;
- }
-
@Singleton
public static class Publish
- extends RetryingRestModifyView<ChangeResource, PublishChangeEditInput, Response<?>> {
+ extends RetryingRestCollectionView<
+ ChangeResource, ChangeEditResource.Publish, PublishChangeEditInput, Response<?>> {
private final ChangeEditUtil editUtil;
private final NotifyUtil notifyUtil;
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index eb46521..bee0ed7 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -184,7 +184,7 @@
}
// Not allowed to put message if the current patch set is locked.
- psUtil.checkPatchSetNotLocked(changeNotes, userProvider.get());
+ psUtil.checkPatchSetNotLocked(changeNotes);
try {
permissionBackend
.user(userProvider.get())
diff --git a/java/com/google/gerrit/server/restapi/change/Rebase.java b/java/com/google/gerrit/server/restapi/change/Rebase.java
index 99a755ae..0fffb58 100644
--- a/java/com/google/gerrit/server/restapi/change/Rebase.java
+++ b/java/com/google/gerrit/server/restapi/change/Rebase.java
@@ -107,7 +107,7 @@
throws OrmException, UpdateException, RestApiException, IOException,
PermissionBackendException {
// Not allowed to rebase if the current patch set is locked.
- patchSetUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+ patchSetUtil.checkPatchSetNotLocked(rsrc.getNotes());
rsrc.permissions().database(dbProvider).check(ChangePermission.REBASE);
projectCache.checkedGet(rsrc.getProject()).checkStatePermitsWrite();
@@ -228,7 +228,7 @@
}
try {
- if (patchSetUtil.isPatchSetLocked(rsrc.getNotes(), rsrc.getUser())) {
+ if (patchSetUtil.isPatchSetLocked(rsrc.getNotes())) {
return description;
}
} catch (OrmException | IOException e) {
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
index 47af894..8868e0e 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
@@ -16,14 +16,12 @@
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.change.ChangeEditResource;
@@ -34,7 +32,7 @@
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryingRestModifyView;
+import com.google.gerrit.server.update.RetryingRestCollectionView;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -43,16 +41,13 @@
@Singleton
public class RebaseChangeEdit
- implements ChildCollection<ChangeResource, ChangeEditResource.Rebase>,
- AcceptsPost<ChangeResource> {
+ implements ChildCollection<ChangeResource, ChangeEditResource.Rebase> {
private final DynamicMap<RestView<ChangeEditResource.Rebase>> views;
- private final Rebase rebase;
@Inject
- RebaseChangeEdit(DynamicMap<RestView<ChangeEditResource.Rebase>> views, Rebase rebase) {
+ RebaseChangeEdit(DynamicMap<RestView<ChangeEditResource.Rebase>> views) {
this.views = views;
- this.rebase = rebase;
}
@Override
@@ -71,13 +66,10 @@
throw new ResourceNotFoundException();
}
- @Override
- public Rebase post(ChangeResource parent) throws RestApiException {
- return rebase;
- }
-
@Singleton
- public static class Rebase extends RetryingRestModifyView<ChangeResource, Input, Response<?>> {
+ public static class Rebase
+ extends RetryingRestCollectionView<
+ ChangeResource, ChangeEditResource.Rebase, Input, Response<?>> {
private final GitRepositoryManager repositoryManager;
private final ChangeEditModifier editModifier;
diff --git a/java/com/google/gerrit/server/restapi/change/Restore.java b/java/com/google/gerrit/server/restapi/change/Restore.java
index 5e4ede3..a29347f 100644
--- a/java/com/google/gerrit/server/restapi/change/Restore.java
+++ b/java/com/google/gerrit/server/restapi/change/Restore.java
@@ -91,7 +91,7 @@
throws RestApiException, UpdateException, OrmException, PermissionBackendException,
IOException {
// Not allowed to restore if the current patch set is locked.
- psUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+ psUtil.checkPatchSetNotLocked(rsrc.getNotes());
rsrc.permissions().database(dbProvider).check(ChangePermission.RESTORE);
projectCache.checkedGet(rsrc.getProject()).checkStatePermitsWrite();
@@ -183,7 +183,7 @@
}
try {
- if (psUtil.isPatchSetLocked(rsrc.getNotes(), rsrc.getUser())) {
+ if (psUtil.isPatchSetLocked(rsrc.getNotes())) {
return description;
}
} catch (OrmException | IOException e) {
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerJson.java b/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
index cfd20c2..c891a65 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
@@ -80,10 +80,7 @@
ReviewerInfo info =
format(
new ReviewerInfo(rsrc.getReviewerUser().getAccountId().get()),
- permissionBackend
- .absentUser(rsrc.getReviewerUser().getAccountId())
- .database(db)
- .change(cd),
+ rsrc.getReviewerUser().getAccountId(),
cd);
loader.put(info);
infos.add(info);
@@ -97,22 +94,19 @@
return format(ImmutableList.<ReviewerResource>of(rsrc));
}
- public ReviewerInfo format(ReviewerInfo out, PermissionBackend.ForChange perm, ChangeData cd)
+ public ReviewerInfo format(ReviewerInfo out, Account.Id reviewer, ChangeData cd)
throws OrmException, PermissionBackendException {
PatchSet.Id psId = cd.change().currentPatchSetId();
return format(
out,
- perm,
+ reviewer,
cd,
approvalsUtil.byPatchSetUser(
- db.get(), cd.notes(), perm.user(), psId, new Account.Id(out._accountId), null, null));
+ db.get(), cd.notes(), psId, new Account.Id(out._accountId), null, null));
}
public ReviewerInfo format(
- ReviewerInfo out,
- PermissionBackend.ForChange perm,
- ChangeData cd,
- Iterable<PatchSetApproval> approvals)
+ ReviewerInfo out, Account.Id reviewer, ChangeData cd, Iterable<PatchSetApproval> approvals)
throws OrmException, PermissionBackendException {
LabelTypes labelTypes = cd.getLabelTypes();
@@ -128,6 +122,9 @@
// do not exist in the DB.
PatchSet ps = cd.currentPatchSet();
if (ps != null) {
+ PermissionBackend.ForChange perm =
+ permissionBackend.absentUser(reviewer).database(db).change(cd);
+
for (SubmitRecord rec : submitRuleEvaluator.evaluate(cd)) {
if (rec.labels == null) {
continue;
diff --git a/java/com/google/gerrit/server/restapi/change/Votes.java b/java/com/google/gerrit/server/restapi/change/Votes.java
index b931c7e..3b2548c 100644
--- a/java/com/google/gerrit/server/restapi/change/Votes.java
+++ b/java/com/google/gerrit/server/restapi/change/Votes.java
@@ -87,7 +87,6 @@
approvalsUtil.byPatchSetUser(
db.get(),
rsrc.getChangeResource().getNotes(),
- rsrc.getChangeResource().getUser(),
rsrc.getChange().currentPatchSetId(),
rsrc.getReviewerUser().getAccountId(),
null,
diff --git a/java/com/google/gerrit/server/restapi/config/CachesCollection.java b/java/com/google/gerrit/server/restapi/config/CachesCollection.java
index e3d9e3c..152fef9 100644
--- a/java/com/google/gerrit/server/restapi/config/CachesCollection.java
+++ b/java/com/google/gerrit/server/restapi/config/CachesCollection.java
@@ -20,12 +20,10 @@
import com.google.common.cache.Cache;
import com.google.gerrit.extensions.annotations.RequiresAnyCapability;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
-import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.config.CacheResource;
import com.google.gerrit.server.config.ConfigResource;
@@ -38,27 +36,23 @@
@RequiresAnyCapability({VIEW_CACHES, MAINTAIN_SERVER})
@Singleton
-public class CachesCollection
- implements ChildCollection<ConfigResource, CacheResource>, AcceptsPost<ConfigResource> {
+public class CachesCollection implements ChildCollection<ConfigResource, CacheResource> {
private final DynamicMap<RestView<CacheResource>> views;
private final Provider<ListCaches> list;
private final PermissionBackend permissionBackend;
private final DynamicMap<Cache<?, ?>> cacheMap;
- private final PostCaches postCaches;
@Inject
CachesCollection(
DynamicMap<RestView<CacheResource>> views,
Provider<ListCaches> list,
PermissionBackend permissionBackend,
- DynamicMap<Cache<?, ?>> cacheMap,
- PostCaches postCaches) {
+ DynamicMap<Cache<?, ?>> cacheMap) {
this.views = views;
this.list = list;
this.permissionBackend = permissionBackend;
this.cacheMap = cacheMap;
- this.postCaches = postCaches;
}
@Override
@@ -90,9 +84,4 @@
public DynamicMap<RestView<CacheResource>> views() {
return views;
}
-
- @Override
- public PostCaches post(ConfigResource parent) throws RestApiException {
- return postCaches;
- }
}
diff --git a/java/com/google/gerrit/server/restapi/config/PostCaches.java b/java/com/google/gerrit/server/restapi/config/PostCaches.java
index f21672c..e20e477 100644
--- a/java/com/google/gerrit/server/restapi/config/PostCaches.java
+++ b/java/com/google/gerrit/server/restapi/config/PostCaches.java
@@ -23,7 +23,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
-import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.RestCollectionView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.config.CacheResource;
import com.google.gerrit.server.config.ConfigResource;
@@ -36,7 +36,7 @@
@RequiresAnyCapability({FLUSH_CACHES, MAINTAIN_SERVER})
@Singleton
-public class PostCaches implements RestModifyView<ConfigResource, Input> {
+public class PostCaches implements RestCollectionView<ConfigResource, CacheResource, Input> {
public static class Input {
public Operation operation;
public List<String> caches;
diff --git a/java/com/google/gerrit/server/restapi/config/RestCacheAdminModule.java b/java/com/google/gerrit/server/restapi/config/RestCacheAdminModule.java
index 7283033..c929bc6 100644
--- a/java/com/google/gerrit/server/restapi/config/RestCacheAdminModule.java
+++ b/java/com/google/gerrit/server/restapi/config/RestCacheAdminModule.java
@@ -26,6 +26,7 @@
protected void configure() {
DynamicMap.mapOf(binder(), CACHE_KIND);
child(CONFIG_KIND, "caches").to(CachesCollection.class);
+ postOnCollection(CACHE_KIND).to(PostCaches.class);
get(CACHE_KIND).to(GetCache.class);
post(CACHE_KIND, "flush").to(FlushCache.class);
get(CONFIG_KIND, "summary").to(GetSummary.class);
diff --git a/java/com/google/gerrit/server/restapi/project/Index.java b/java/com/google/gerrit/server/restapi/project/Index.java
index 5547864..1b2a523 100644
--- a/java/com/google/gerrit/server/restapi/project/Index.java
+++ b/java/com/google/gerrit/server/restapi/project/Index.java
@@ -21,6 +21,7 @@
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.projects.IndexProjectInput;
+import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
@@ -62,27 +63,33 @@
ResourceConflictException {
String response = "Project " + rsrc.getName() + " submitted for reindexing";
- reindexAsync(rsrc.getNameKey());
+ reindex(rsrc.getNameKey(), input.async);
if (Boolean.TRUE.equals(input.indexChildren)) {
ListChildProjects listChildProjects = listChildProjectsProvider.get();
listChildProjects.setRecursive(true);
- listChildProjects.apply(rsrc).forEach(p -> reindexAsync(new Project.NameKey(p.name)));
+ for (ProjectInfo child : listChildProjects.apply(rsrc)) {
+ reindex(new Project.NameKey(child.name), input.async);
+ }
response += " (indexing children recursively)";
}
return Response.accepted(response);
}
- private void reindexAsync(Project.NameKey project) {
- @SuppressWarnings("unused")
- Future<?> possiblyIgnoredError =
- executor.submit(
- () -> {
- try {
- indexer.index(project);
- } catch (IOException e) {
- logger.atWarning().withCause(e).log("reindexing project %s failed", project);
- }
- });
+ private void reindex(Project.NameKey project, Boolean async) throws IOException {
+ if (Boolean.TRUE.equals(async)) {
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError =
+ executor.submit(
+ () -> {
+ try {
+ indexer.index(project);
+ } catch (IOException e) {
+ logger.atWarning().withCause(e).log("reindexing project %s failed", project);
+ }
+ });
+ } else {
+ indexer.index(project);
+ }
}
}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index a39e4e9..cb2e306 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -787,19 +787,33 @@
}
if (!revisions.containsEntry(id, ps.getId())) {
- // TODO this is actually an error, the branch is gone but we
- // want to merge the issue. We can't safely do that if the
- // tip is not reachable.
- //
+ if (revisions.containsValue(ps.getId())) {
+ // TODO This is actually an error, the patch set ref exists but points to a revision that
+ // is different from the revision that we have stored for the patch set in the change
+ // meta data.
+ commitStatus.logProblem(
+ changeId,
+ "Revision "
+ + idstr
+ + " of patch set "
+ + ps.getPatchSetId()
+ + " does not match the revision of the patch set ref "
+ + ps.getId().toRefName());
+ continue;
+ }
+
+ // The patch set ref is not found but we want to merge the change. We can't safely do that
+ // if the patch set ref is missing. In a multi-master setup this can indicate a replication
+ // lag (e.g. the change meta data was already replicated, but the replication of the patch
+ // set ref is still pending).
commitStatus.logProblem(
changeId,
- "Revision "
- + idstr
- + " of patch set "
- + ps.getPatchSetId()
- + " does not match "
+ "Patch set ref "
+ ps.getId().toRefName()
- + " for change");
+ + " not found. Expected patch set ref of "
+ + ps.getPatchSetId()
+ + " to point to revision "
+ + idstr);
continue;
}
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
index 62dabae..abe3632 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
@@ -357,12 +357,7 @@
Map<PatchSetApproval.Key, PatchSetApproval> byKey = new HashMap<>();
for (PatchSetApproval psa :
args.approvalsUtil.byPatchSet(
- ctx.getDb(),
- ctx.getNotes(),
- ctx.getUser(),
- psId,
- ctx.getRevWalk(),
- ctx.getRepoView().getConfig())) {
+ ctx.getDb(), ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
byKey.put(psa.getKey(), psa);
}
@@ -376,7 +371,7 @@
// was added. So we need to make sure votes are accurate now. This way if
// permissions get modified in the future, historical records stay accurate.
LabelNormalizer.Result normalized =
- args.labelNormalizer.normalize(ctx.getNotes(), ctx.getUser(), byKey.values());
+ args.labelNormalizer.normalize(ctx.getNotes(), byKey.values());
update.putApproval(submitter.getLabel(), submitter.getValue());
saveApprovals(normalized, ctx, update, false);
return normalized;
diff --git a/java/com/google/gerrit/server/update/RetryingRestCollectionView.java b/java/com/google/gerrit/server/update/RetryingRestCollectionView.java
new file mode 100644
index 0000000..e0e3148
--- /dev/null
+++ b/java/com/google/gerrit/server/update/RetryingRestCollectionView.java
@@ -0,0 +1,40 @@
+// Copyright (C) 2018 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.update;
+
+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.extensions.restapi.RestCollectionView;
+import com.google.gerrit.extensions.restapi.RestResource;
+
+public abstract class RetryingRestCollectionView<
+ P extends RestResource, C extends RestResource, I, O>
+ implements RestCollectionView<P, C, I> {
+ private final RetryHelper retryHelper;
+
+ protected RetryingRestCollectionView(RetryHelper retryHelper) {
+ this.retryHelper = retryHelper;
+ }
+
+ @Override
+ public final O apply(P parentResource, I input)
+ throws AuthException, BadRequestException, ResourceConflictException, Exception {
+ return retryHelper.execute((updateFactory) -> applyImpl(updateFactory, parentResource, input));
+ }
+
+ protected abstract O applyImpl(BatchUpdate.Factory updateFactory, P parentResource, I input)
+ throws Exception;
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index c3a0dc9..7a4a901 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -177,7 +177,7 @@
setApiUser(user);
setUseContributorAgreements(InheritableBoolean.TRUE);
exception.expect(AuthException.class);
- exception.expectMessage("A Contributor Agreement must be completed");
+ exception.expectMessage("Contributor Agreement");
gApi.changes().id(change.changeId).revert();
}
@@ -209,7 +209,7 @@
in.destination = dest.ref;
in.message = change.subject;
exception.expect(AuthException.class);
- exception.expectMessage("A Contributor Agreement must be completed");
+ exception.expectMessage("Contributor Agreement");
gApi.changes().id(change.changeId).current().cherryPick(in);
}
@@ -227,7 +227,7 @@
gApi.changes().create(newChangeInput());
fail("Expected AuthException");
} catch (AuthException e) {
- assertThat(e.getMessage()).contains("A Contributor Agreement must be completed");
+ assertThat(e.getMessage()).contains("Contributor Agreement");
}
// Sign the agreement
diff --git a/javatests/com/google/gerrit/common/data/PermissionRuleTest.java b/javatests/com/google/gerrit/common/data/PermissionRuleTest.java
new file mode 100644
index 0000000..14c47b4
--- /dev/null
+++ b/javatests/com/google/gerrit/common/data/PermissionRuleTest.java
@@ -0,0 +1,399 @@
+// Copyright (C) 2018 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 static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.common.data.PermissionRule.Action;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+public class PermissionRuleTest {
+ @Rule public ExpectedException exception = ExpectedException.none();
+
+ private GroupReference groupReference;
+ private PermissionRule permissionRule;
+
+ @Before
+ public void setup() {
+ this.groupReference = new GroupReference(new AccountGroup.UUID("uuid"), "group");
+ this.permissionRule = new PermissionRule(groupReference);
+ }
+
+ @Test
+ public void getAndSetAction() {
+ assertThat(permissionRule.getAction()).isEqualTo(Action.ALLOW);
+
+ permissionRule.setAction(Action.DENY);
+ assertThat(permissionRule.getAction()).isEqualTo(Action.DENY);
+ }
+
+ @Test
+ public void cannotSetActionToNull() {
+ exception.expect(NullPointerException.class);
+ permissionRule.setAction(null);
+ }
+
+ @Test
+ public void setDeny() {
+ assertThat(permissionRule.isDeny()).isFalse();
+
+ permissionRule.setDeny();
+ assertThat(permissionRule.isDeny()).isTrue();
+ }
+
+ @Test
+ public void setBlock() {
+ assertThat(permissionRule.isBlock()).isFalse();
+
+ permissionRule.setBlock();
+ assertThat(permissionRule.isBlock()).isTrue();
+ }
+
+ @Test
+ public void setForce() {
+ assertThat(permissionRule.getForce()).isFalse();
+
+ permissionRule.setForce(true);
+ assertThat(permissionRule.getForce()).isTrue();
+
+ permissionRule.setForce(false);
+ assertThat(permissionRule.getForce()).isFalse();
+ }
+
+ @Test
+ public void setMin() {
+ assertThat(permissionRule.getMin()).isEqualTo(0);
+
+ permissionRule.setMin(-2);
+ assertThat(permissionRule.getMin()).isEqualTo(-2);
+
+ permissionRule.setMin(2);
+ assertThat(permissionRule.getMin()).isEqualTo(2);
+ }
+
+ @Test
+ public void setMax() {
+ assertThat(permissionRule.getMax()).isEqualTo(0);
+
+ permissionRule.setMax(2);
+ assertThat(permissionRule.getMax()).isEqualTo(2);
+
+ permissionRule.setMax(-2);
+ assertThat(permissionRule.getMax()).isEqualTo(-2);
+ }
+
+ @Test
+ public void setRange() {
+ assertThat(permissionRule.getMin()).isEqualTo(0);
+ assertThat(permissionRule.getMax()).isEqualTo(0);
+
+ permissionRule.setRange(-2, 2);
+ assertThat(permissionRule.getMin()).isEqualTo(-2);
+ assertThat(permissionRule.getMax()).isEqualTo(2);
+
+ permissionRule.setRange(2, -2);
+ assertThat(permissionRule.getMin()).isEqualTo(-2);
+ assertThat(permissionRule.getMax()).isEqualTo(2);
+
+ permissionRule.setRange(1, 1);
+ assertThat(permissionRule.getMin()).isEqualTo(1);
+ assertThat(permissionRule.getMax()).isEqualTo(1);
+ }
+
+ @Test
+ public void hasRange() {
+ assertThat(permissionRule.hasRange()).isFalse();
+
+ permissionRule.setMin(-1);
+ assertThat(permissionRule.hasRange()).isTrue();
+
+ permissionRule.setMax(1);
+ assertThat(permissionRule.hasRange()).isTrue();
+ }
+
+ @Test
+ public void getGroup() {
+ assertThat(permissionRule.getGroup()).isEqualTo(groupReference);
+ }
+
+ @Test
+ public void setGroup() {
+ GroupReference groupReference2 = new GroupReference(new AccountGroup.UUID("uuid2"), "group2");
+ assertThat(groupReference2).isNotEqualTo(groupReference);
+
+ assertThat(permissionRule.getGroup()).isEqualTo(groupReference);
+
+ permissionRule.setGroup(groupReference2);
+ assertThat(permissionRule.getGroup()).isEqualTo(groupReference2);
+ }
+
+ @Test
+ public void mergeFromAnyBlock() {
+ GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid1"), "group1");
+ PermissionRule permissionRule1 = new PermissionRule(groupReference1);
+
+ GroupReference groupReference2 = new GroupReference(new AccountGroup.UUID("uuid2"), "group2");
+ PermissionRule permissionRule2 = new PermissionRule(groupReference2);
+
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.isBlock()).isFalse();
+ assertThat(permissionRule2.isBlock()).isFalse();
+
+ permissionRule2.setBlock();
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.isBlock()).isTrue();
+ assertThat(permissionRule2.isBlock()).isTrue();
+
+ permissionRule2.setDeny();
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.isBlock()).isTrue();
+ assertThat(permissionRule2.isBlock()).isFalse();
+
+ permissionRule2.setAction(Action.BATCH);
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.isBlock()).isTrue();
+ assertThat(permissionRule2.isBlock()).isFalse();
+ }
+
+ @Test
+ public void mergeFromAnyDeny() {
+ GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid1"), "group1");
+ PermissionRule permissionRule1 = new PermissionRule(groupReference1);
+
+ GroupReference groupReference2 = new GroupReference(new AccountGroup.UUID("uuid2"), "group2");
+ PermissionRule permissionRule2 = new PermissionRule(groupReference2);
+
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.isDeny()).isFalse();
+ assertThat(permissionRule2.isDeny()).isFalse();
+
+ permissionRule2.setDeny();
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.isDeny()).isTrue();
+ assertThat(permissionRule2.isDeny()).isTrue();
+
+ permissionRule2.setAction(Action.BATCH);
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.isDeny()).isTrue();
+ assertThat(permissionRule2.isDeny()).isFalse();
+ }
+
+ @Test
+ public void mergeFromAnyBatch() {
+ GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid1"), "group1");
+ PermissionRule permissionRule1 = new PermissionRule(groupReference1);
+
+ GroupReference groupReference2 = new GroupReference(new AccountGroup.UUID("uuid2"), "group2");
+ PermissionRule permissionRule2 = new PermissionRule(groupReference2);
+
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.getAction()).isNotEqualTo(Action.BATCH);
+ assertThat(permissionRule2.getAction()).isNotEqualTo(Action.BATCH);
+
+ permissionRule2.setAction(Action.BATCH);
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.getAction()).isEqualTo(Action.BATCH);
+ assertThat(permissionRule2.getAction()).isEqualTo(Action.BATCH);
+
+ permissionRule2.setAction(Action.ALLOW);
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.getAction()).isEqualTo(Action.BATCH);
+ assertThat(permissionRule2.getAction()).isNotEqualTo(Action.BATCH);
+ }
+
+ @Test
+ public void mergeFromAnyForce() {
+ GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid1"), "group1");
+ PermissionRule permissionRule1 = new PermissionRule(groupReference1);
+
+ GroupReference groupReference2 = new GroupReference(new AccountGroup.UUID("uuid2"), "group2");
+ PermissionRule permissionRule2 = new PermissionRule(groupReference2);
+
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.getForce()).isFalse();
+ assertThat(permissionRule2.getForce()).isFalse();
+
+ permissionRule2.setForce(true);
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.getForce()).isTrue();
+ assertThat(permissionRule2.getForce()).isTrue();
+
+ permissionRule2.setForce(false);
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.getForce()).isTrue();
+ assertThat(permissionRule2.getForce()).isFalse();
+ }
+
+ @Test
+ public void mergeFromMergeRange() {
+ GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid1"), "group1");
+ PermissionRule permissionRule1 = new PermissionRule(groupReference1);
+ permissionRule1.setRange(-1, 2);
+
+ GroupReference groupReference2 = new GroupReference(new AccountGroup.UUID("uuid2"), "group2");
+ PermissionRule permissionRule2 = new PermissionRule(groupReference2);
+ permissionRule2.setRange(-2, 1);
+
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.getMin()).isEqualTo(-2);
+ assertThat(permissionRule1.getMax()).isEqualTo(2);
+ assertThat(permissionRule2.getMin()).isEqualTo(-2);
+ assertThat(permissionRule2.getMax()).isEqualTo(1);
+ }
+
+ @Test
+ public void mergeFromGroupNotChanged() {
+ GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid1"), "group1");
+ PermissionRule permissionRule1 = new PermissionRule(groupReference1);
+
+ GroupReference groupReference2 = new GroupReference(new AccountGroup.UUID("uuid2"), "group2");
+ PermissionRule permissionRule2 = new PermissionRule(groupReference2);
+
+ permissionRule1.mergeFrom(permissionRule2);
+ assertThat(permissionRule1.getGroup()).isEqualTo(groupReference1);
+ assertThat(permissionRule2.getGroup()).isEqualTo(groupReference2);
+ }
+
+ @Test
+ public void asString() {
+ assertThat(permissionRule.asString(true)).isEqualTo("group " + groupReference.getName());
+
+ permissionRule.setDeny();
+ assertThat(permissionRule.asString(true)).isEqualTo("deny group " + groupReference.getName());
+
+ permissionRule.setBlock();
+ assertThat(permissionRule.asString(true)).isEqualTo("block group " + groupReference.getName());
+
+ permissionRule.setAction(Action.BATCH);
+ assertThat(permissionRule.asString(true)).isEqualTo("batch group " + groupReference.getName());
+
+ permissionRule.setAction(Action.INTERACTIVE);
+ assertThat(permissionRule.asString(true))
+ .isEqualTo("interactive group " + groupReference.getName());
+
+ permissionRule.setForce(true);
+ assertThat(permissionRule.asString(true))
+ .isEqualTo("interactive +force group " + groupReference.getName());
+
+ permissionRule.setAction(Action.ALLOW);
+ assertThat(permissionRule.asString(true)).isEqualTo("+force group " + groupReference.getName());
+
+ permissionRule.setMax(1);
+ assertThat(permissionRule.asString(true))
+ .isEqualTo("+force +0..+1 group " + groupReference.getName());
+
+ permissionRule.setMin(-1);
+ assertThat(permissionRule.asString(true))
+ .isEqualTo("+force -1..+1 group " + groupReference.getName());
+
+ assertThat(permissionRule.asString(false))
+ .isEqualTo("+force group " + groupReference.getName());
+ }
+
+ @Test
+ public void fromString() {
+ PermissionRule permissionRule = PermissionRule.fromString("group A", true);
+ assertPermissionRule(permissionRule, "A", Action.ALLOW, false, 0, 0);
+
+ permissionRule = PermissionRule.fromString("deny group A", true);
+ assertPermissionRule(permissionRule, "A", Action.DENY, false, 0, 0);
+
+ permissionRule = PermissionRule.fromString("block group A", true);
+ assertPermissionRule(permissionRule, "A", Action.BLOCK, false, 0, 0);
+
+ permissionRule = PermissionRule.fromString("batch group A", true);
+ assertPermissionRule(permissionRule, "A", Action.BATCH, false, 0, 0);
+
+ permissionRule = PermissionRule.fromString("interactive group A", true);
+ assertPermissionRule(permissionRule, "A", Action.INTERACTIVE, false, 0, 0);
+
+ permissionRule = PermissionRule.fromString("interactive +force group A", true);
+ assertPermissionRule(permissionRule, "A", Action.INTERACTIVE, true, 0, 0);
+
+ permissionRule = PermissionRule.fromString("+force group A", true);
+ assertPermissionRule(permissionRule, "A", Action.ALLOW, true, 0, 0);
+
+ permissionRule = PermissionRule.fromString("+force +0..+1 group A", true);
+ assertPermissionRule(permissionRule, "A", Action.ALLOW, true, 0, 1);
+
+ permissionRule = PermissionRule.fromString("+force -1..+1 group A", true);
+ assertPermissionRule(permissionRule, "A", Action.ALLOW, true, -1, 1);
+
+ permissionRule = PermissionRule.fromString("+force group A", false);
+ assertPermissionRule(permissionRule, "A", Action.ALLOW, true, 0, 0);
+ }
+
+ @Test
+ public void parseInt() {
+ assertThat(PermissionRule.parseInt("0")).isEqualTo(0);
+ assertThat(PermissionRule.parseInt("+0")).isEqualTo(0);
+ assertThat(PermissionRule.parseInt("-0")).isEqualTo(0);
+ assertThat(PermissionRule.parseInt("1")).isEqualTo(1);
+ assertThat(PermissionRule.parseInt("+1")).isEqualTo(1);
+ assertThat(PermissionRule.parseInt("-1")).isEqualTo(-1);
+ }
+
+ @Test
+ public void testEquals() {
+ GroupReference groupReference2 = new GroupReference(new AccountGroup.UUID("uuid2"), "group2");
+ PermissionRule permissionRuleOther = new PermissionRule(groupReference2);
+ assertThat(permissionRule.equals(permissionRuleOther)).isFalse();
+
+ permissionRuleOther.setGroup(groupReference);
+ assertThat(permissionRule.equals(permissionRuleOther)).isTrue();
+
+ permissionRule.setDeny();
+ assertThat(permissionRule.equals(permissionRuleOther)).isFalse();
+
+ permissionRuleOther.setDeny();
+ assertThat(permissionRule.equals(permissionRuleOther)).isTrue();
+
+ permissionRule.setForce(true);
+ assertThat(permissionRule.equals(permissionRuleOther)).isFalse();
+
+ permissionRuleOther.setForce(true);
+ assertThat(permissionRule.equals(permissionRuleOther)).isTrue();
+
+ permissionRule.setMin(-1);
+ assertThat(permissionRule.equals(permissionRuleOther)).isFalse();
+
+ permissionRuleOther.setMin(-1);
+ assertThat(permissionRule.equals(permissionRuleOther)).isTrue();
+
+ permissionRule.setMax(1);
+ assertThat(permissionRule.equals(permissionRuleOther)).isFalse();
+
+ permissionRuleOther.setMax(1);
+ assertThat(permissionRule.equals(permissionRuleOther)).isTrue();
+ }
+
+ private void assertPermissionRule(
+ PermissionRule permissionRule,
+ String expectedGroupName,
+ Action expectedAction,
+ boolean expectedForce,
+ int expectedMin,
+ int expectedMax) {
+ assertThat(permissionRule.getGroup().getName()).isEqualTo(expectedGroupName);
+ assertThat(permissionRule.getAction()).isEqualTo(expectedAction);
+ assertThat(permissionRule.getForce()).isEqualTo(expectedForce);
+ assertThat(permissionRule.getMin()).isEqualTo(expectedMin);
+ assertThat(permissionRule.getMax()).isEqualTo(expectedMax);
+ }
+}
diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD
index 1249909..595c887 100644
--- a/javatests/com/google/gerrit/elasticsearch/BUILD
+++ b/javatests/com/google/gerrit/elasticsearch/BUILD
@@ -10,49 +10,84 @@
visibility = ["//visibility:public"],
deps = [
"//java/com/google/gerrit/elasticsearch",
- "//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/index",
- "//java/com/google/gerrit/index/project",
- "//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
- "//lib:gson",
"//lib:guava",
"//lib:junit",
"//lib/guice",
"//lib/httpcomponents:httpcore",
"//lib/jgit/org.eclipse.jgit:jgit",
- "//lib/jgit/org.eclipse.jgit.junit:junit",
"//lib/testcontainers",
- "//lib/truth",
],
)
-ELASTICSEARCH_TESTS = {i: "ElasticQuery" + i.capitalize() + "sTest.java" for i in [
+ELASTICSEARCH_DEPS = [
+ ":elasticsearch_test_utils",
+ "//java/com/google/gerrit/elasticsearch",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib/guice",
+ "//lib/jgit/org.eclipse.jgit:jgit",
+]
+
+QUERY_TESTS_DEP = "//javatests/com/google/gerrit/server/query/%s:abstract_query_tests"
+
+TYPES = [
"account",
"change",
"group",
"project",
-]}
+]
+
+SUFFIX = "sTest.java"
+
+ELASTICSEARCH_TESTS = {i: "ElasticQuery" + i.capitalize() + SUFFIX for i in TYPES}
+
+ELASTICSEARCH_TESTS_V5 = {i: "ElasticV5Query" + i.capitalize() + SUFFIX for i in TYPES}
+
+ELASTICSEARCH_TESTS_V6 = {i: "ElasticV6Query" + i.capitalize() + SUFFIX for i in TYPES}
+
+ELASTICSEARCH_TAGS = [
+ "docker",
+ "elastic",
+]
[junit_tests(
name = "elasticsearch_%ss_test" % name,
size = "large",
srcs = [src],
- tags = [
- "docker",
- "elastic",
- ],
- deps = [
- ":elasticsearch_test_utils",
- "//java/com/google/gerrit/elasticsearch",
- "//java/com/google/gerrit/server",
- "//java/com/google/gerrit/server/project/testing:project-test-util",
- "//java/com/google/gerrit/testing:gerrit-test-util",
- "//javatests/com/google/gerrit/server/query/%s:abstract_query_tests" % name,
- "//lib/guice",
- "//lib/httpcomponents:httpcore",
- "//lib/jgit/org.eclipse.jgit:jgit",
- "//lib/jgit/org.eclipse.jgit.junit:junit",
- "//lib/testcontainers",
- ],
+ tags = ELASTICSEARCH_TAGS,
+ deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name],
) for name, src in ELASTICSEARCH_TESTS.items()]
+
+[junit_tests(
+ name = "elasticsearch_%ss_test_V5" % name,
+ size = "large",
+ srcs = [src],
+ tags = ELASTICSEARCH_TAGS,
+ deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name],
+) for name, src in ELASTICSEARCH_TESTS_V5.items()]
+
+[junit_tests(
+ name = "elasticsearch_%ss_test_V6" % name,
+ size = "large",
+ srcs = [src],
+ tags = ELASTICSEARCH_TAGS + ["flaky"],
+ deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name],
+) for name, src in ELASTICSEARCH_TESTS_V6.items()]
+
+[junit_tests(
+ name = "elasticsearch_tests",
+ size = "small",
+ srcs = glob(
+ ["*Test.java"],
+ exclude = ["Elastic*Query*" + SUFFIX],
+ ),
+ tags = ["elastic"],
+ deps = [
+ "//java/com/google/gerrit/elasticsearch",
+ "//lib:guava",
+ "//lib/guice",
+ "//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
+ ],
+)]
diff --git a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
index 834f658..f63b181 100644
--- a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
+++ b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
@@ -43,31 +44,6 @@
private boolean allowValueQueries = true;
@Override
- public CurrentUser user() {
- return new CurrentUser() {
- @Override
- public GroupMembership getEffectiveGroups() {
- throw new UnsupportedOperationException("not implemented");
- }
-
- @Override
- public Object getCacheKey() {
- return new Object();
- }
-
- @Override
- public boolean isIdentifiedUser() {
- return true;
- }
-
- @Override
- public Account.Id getAccountId() {
- return new Account.Id(1);
- }
- };
- }
-
- @Override
public String resourcePath() {
return "/projects/test-project";
}
@@ -100,6 +76,11 @@
}
@Override
+ public BooleanCondition testCond(ProjectPermission perm) {
+ return new PermissionBackendCondition.ForProject(this, perm, fakeUser());
+ }
+
+ @Override
public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
throw new UnsupportedOperationException("not implemented");
@@ -108,6 +89,30 @@
private void disallowValueQueries() {
allowValueQueries = false;
}
+
+ private static CurrentUser fakeUser() {
+ return new CurrentUser() {
+ @Override
+ public GroupMembership getEffectiveGroups() {
+ throw new UnsupportedOperationException("not implemented");
+ }
+
+ @Override
+ public Object getCacheKey() {
+ return new Object();
+ }
+
+ @Override
+ public boolean isIdentifiedUser() {
+ return true;
+ }
+
+ @Override
+ public Account.Id getAccountId() {
+ return new Account.Id(1);
+ }
+ };
+ }
}
@Test
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index bea61ed..3a98127 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -691,7 +691,7 @@
try (RevWalk rw = new RevWalk(repo)) {
RevCommit commit = rw.parseCommit(update.getResult());
rw.parseBody(commit);
- String strIdent = otherUser.getName() + " <" + otherUserId + "@" + serverId + ">";
+ String strIdent = "Gerrit User " + otherUserId + " <" + otherUserId + "@" + serverId + ">";
assertThat(commit.getFullMessage()).contains("Assignee: " + strIdent);
}
}
diff --git a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
index f826fec..db7035b 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
@@ -62,14 +62,14 @@
+ "Commit: "
+ update.getCommit().name()
+ "\n"
- + "Reviewer: Change Owner <1@gerrit>\n"
- + "CC: Other Account <2@gerrit>\n"
+ + "Reviewer: Gerrit User 1 <1@gerrit>\n"
+ + "CC: Gerrit User 2 <2@gerrit>\n"
+ "Label: Code-Review=-1\n"
+ "Label: Verified=+1\n",
commit);
PersonIdent author = commit.getAuthorIdent();
- assertThat(author.getName()).isEqualTo("Change Owner");
+ assertThat(author.getName()).isEqualTo("Gerrit User 1");
assertThat(author.getEmailAddress()).isEqualTo("1@gerrit");
assertThat(author.getWhen()).isEqualTo(new Date(c.getCreatedOn().getTime() + 1000));
assertThat(author.getTimeZone()).isEqualTo(TimeZone.getTimeZone("GMT-7:00"));
@@ -177,15 +177,15 @@
+ submissionId.toStringForStorage()
+ "\n"
+ "Submitted-with: NOT_READY\n"
- + "Submitted-with: OK: Verified: Change Owner <1@gerrit>\n"
+ + "Submitted-with: OK: Verified: Gerrit User 1 <1@gerrit>\n"
+ "Submitted-with: NEED: Code-Review\n"
+ "Submitted-with: NOT_READY\n"
- + "Submitted-with: OK: Verified: Change Owner <1@gerrit>\n"
+ + "Submitted-with: OK: Verified: Gerrit User 1 <1@gerrit>\n"
+ "Submitted-with: NEED: Alternative-Code-Review\n",
commit);
PersonIdent author = commit.getAuthorIdent();
- assertThat(author.getName()).isEqualTo("Change Owner");
+ assertThat(author.getName()).isEqualTo("Gerrit User 1");
assertThat(author.getEmailAddress()).isEqualTo("1@gerrit");
assertThat(author.getWhen()).isEqualTo(new Date(c.getCreatedOn().getTime() + 2000));
assertThat(author.getTimeZone()).isEqualTo(TimeZone.getTimeZone("GMT-7:00"));
@@ -210,7 +210,7 @@
assertBodyEquals("Update patch set 1\n\nComment on the change.\n\nPatch-set: 1\n", commit);
PersonIdent author = commit.getAuthorIdent();
- assertThat(author.getName()).isEqualTo("GerritAccount #3");
+ assertThat(author.getName()).isEqualTo("Gerrit User 3");
assertThat(author.getEmailAddress()).isEqualTo("3@gerrit");
}
@@ -245,7 +245,7 @@
update.commit();
assertBodyEquals(
- "Update patch set 1\n\nPatch-set: 1\nReviewer: Change Owner <1@gerrit>\n",
+ "Update patch set 1\n\nPatch-set: 1\nReviewer: Gerrit User 1 <1@gerrit>\n",
update.getResult());
}
@@ -360,7 +360,7 @@
RevCommit commit = parseCommit(update.getResult());
PersonIdent author = commit.getAuthorIdent();
- assertThat(author.getName()).isEqualTo("Other Account");
+ assertThat(author.getName()).isEqualTo("Gerrit User 2");
assertThat(author.getEmailAddress()).isEqualTo("2@gerrit");
assertBodyEquals(
@@ -369,7 +369,7 @@
+ "Message on behalf of other user\n"
+ "\n"
+ "Patch-set: 1\n"
- + "Real-user: Change Owner <1@gerrit>\n",
+ + "Real-user: Gerrit User 1 <1@gerrit>\n",
commit);
}
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index c73171e..920f28b 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit c73171ea9abbeb765d585a92753ce01151355a5c
+Subproject commit 920f28b46021d9c49fac09d869aa4040d13796e7
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
index 396fed8..0682ab2 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.js
@@ -49,12 +49,12 @@
},
get selectedItem() {
- if (!this._ironSelector) { return; }
+ if (!this._ironSelector) { return undefined; }
return this._ironSelector.selectedItem;
},
get selectedValue() {
- if (!this._ironSelector) { return; }
+ if (!this._ironSelector) { return undefined; }
return this._ironSelector.selected;
},
