Merge "Expose index.change.mergeable in ServerInfo"
diff --git a/WORKSPACE b/WORKSPACE
index 7529d8a..e370f69 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -748,7 +748,7 @@
sha1 = "f7be08ec23c21485b9b5a1cf1654c2ec8c58168d",
)
-GITILES_VERS = "0.3-6"
+GITILES_VERS = "0.3-7"
GITILES_REPO = GERRIT
@@ -757,14 +757,14 @@
artifact = "com.google.gitiles:blame-cache:" + GITILES_VERS,
attach_source = False,
repository = GITILES_REPO,
- sha1 = "bd1ec86570b8a6e4b68c5af6311c8cd10aa3f295",
+ sha1 = "af6212a62363906c63d367f8276ae1645f83bf93",
)
maven_jar(
name = "gitiles-servlet",
artifact = "com.google.gitiles:gitiles-servlet:" + GITILES_VERS,
repository = GITILES_REPO,
- sha1 = "98bf06ca9abc871beb3d6c01e6f053243d4e911a",
+ sha1 = "6a53f722f8572a2f1bcb7d86e5692168844bab76",
)
# prettify must match the version used in Gitiles
diff --git a/java/com/google/gerrit/entities/Change.java b/java/com/google/gerrit/entities/Change.java
index 739bd38..04e97dc 100644
--- a/java/com/google/gerrit/entities/Change.java
+++ b/java/com/google/gerrit/entities/Change.java
@@ -26,6 +26,7 @@
import java.security.SecureRandom;
import java.sql.Timestamp;
import java.util.Arrays;
+import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -116,13 +117,30 @@
@AutoValue
public abstract static class Id {
- /** Parse a Change.Id out of a string representation. */
+ /**
+ * Parse a Change.Id out of a string representation.
+ *
+ * @deprecated use {@link #tryParse(String)} instead.
+ */
+ @Deprecated
public static Id parse(String str) {
Integer id = Ints.tryParse(str);
checkArgument(id != null, "invalid change ID: %s", str);
return Change.id(id);
}
+ /**
+ * Parse a Change.Id out of a string representation.
+ *
+ * @param str the string to parse
+ * @return Optional containing the Change.Id, or {@code Optional.empty()} if str does not
+ * represent a valid Change.Id.
+ */
+ public static Optional<Id> tryParse(String str) {
+ Integer id = Ints.tryParse(str);
+ return id != null ? Optional.of(Change.id(id)) : Optional.empty();
+ }
+
public static Id fromRef(String ref) {
if (RefNames.isRefsEdit(ref)) {
return fromEditRefPart(ref);
diff --git a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
index 89ad878..77c5381 100644
--- a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
+++ b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
@@ -24,6 +24,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Optional;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -46,17 +47,15 @@
if (idString.endsWith("/")) {
idString = idString.substring(0, idString.length() - 1);
}
- Change.Id id;
- try {
- id = Change.Id.parse(idString);
- } catch (IllegalArgumentException e) {
+ Optional<Change.Id> id = Change.Id.tryParse(idString);
+ if (!id.isPresent()) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
}
ChangeResource changeResource;
try {
- changeResource = changesCollection.parse(id);
+ changeResource = changesCollection.parse(id.get());
} catch (ResourceConflictException | ResourceNotFoundException e) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 36b3c20..d7f09d2 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -180,6 +180,7 @@
setChangeSubjectHeader();
setHeader(MailHeader.CHANGE_ID.fieldName(), "" + change.getKey().get());
setHeader(MailHeader.CHANGE_NUMBER.fieldName(), "" + change.getChangeId());
+ setHeader(MailHeader.PROJECT.fieldName(), "" + change.getProject());
setChangeUrlHeader();
setCommitIdHeader();
diff --git a/java/com/google/gerrit/server/mail/send/NotificationEmail.java b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
index f928bf0..0fb5c6f 100644
--- a/java/com/google/gerrit/server/mail/send/NotificationEmail.java
+++ b/java/com/google/gerrit/server/mail/send/NotificationEmail.java
@@ -122,7 +122,7 @@
soyContext.put("branch", branchData);
footers.add(MailHeader.PROJECT.withDelimiter() + branch.project().get());
- footers.add("Gerrit-Branch: " + branch.shortName());
+ footers.add(MailHeader.BRANCH.withDelimiter() + branch.shortName());
}
@VisibleForTesting
diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index 7d2fa0a..e81f7f4 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -211,12 +211,12 @@
}
}
- Set<Address> intersection = Sets.intersection(smtpRcptTo, smtpRcptToPlaintextOnly);
+ Set<Address> intersection = Sets.intersection(va.smtpRcptTo, smtpRcptToPlaintextOnly);
if (!intersection.isEmpty()) {
logger.atSevere().log("Email '%s' will be sent twice to %s", messageClass, intersection);
}
- if (!smtpRcptTo.isEmpty()) {
+ if (!va.smtpRcptTo.isEmpty()) {
// Send multipart message
logger.atFine().log(
"Sending multipart '%s' from %s to %s",
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index ce88f07..d301d34 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -51,11 +51,11 @@
private final Change change;
protected final PersonIdent serverIdent;
- protected PatchSet.Id psId;
+ @Nullable protected PatchSet.Id psId;
private ObjectId result;
- protected boolean rootOnly;
+ boolean rootOnly;
- protected AbstractChangeUpdate(
+ AbstractChangeUpdate(
ChangeNotes notes,
CurrentUser user,
PersonIdent serverIdent,
@@ -72,7 +72,7 @@
this.when = when;
}
- protected AbstractChangeUpdate(
+ AbstractChangeUpdate(
ChangeNoteUtil noteUtil,
PersonIdent serverIdent,
@Nullable ChangeNotes notes,
@@ -172,7 +172,7 @@
public abstract boolean isEmpty();
/** Wether this update can only be a root commit. */
- public boolean isRootOnly() {
+ boolean isRootOnly() {
return rootOnly;
}
@@ -256,7 +256,7 @@
protected abstract CommitBuilder applyImpl(RevWalk rw, ObjectInserter ins, ObjectId curr)
throws IOException;
- protected static final CommitBuilder NO_OP_UPDATE = new CommitBuilder();
+ static final CommitBuilder NO_OP_UPDATE = new CommitBuilder();
ObjectId getResult() {
return result;
@@ -270,7 +270,7 @@
return ins.insert(Constants.OBJ_TREE, new byte[] {});
}
- protected void verifyComment(Comment c) {
+ void verifyComment(Comment c) {
checkArgument(c.getCommitId() != null, "commit ID required for comment: %s", c);
checkArgument(
c.author.getId().equals(getAccountId()),
diff --git a/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java b/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
index 5d909d0..030cfb2 100644
--- a/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AllUsersAsyncUpdate.java
@@ -91,7 +91,7 @@
executor.submit(
() -> {
try (OpenRepo allUsersRepo = OpenRepo.open(repoManager, allUsersName)) {
- allUsersRepo.addUpdates(draftUpdates);
+ allUsersRepo.addUpdatesNoLimits(draftUpdates);
allUsersRepo.flush();
BatchRefUpdate bru = allUsersRepo.repo.getRefDatabase().newBatchUpdate();
bru.setPushCertificate(pushCert);
diff --git a/java/com/google/gerrit/server/notedb/LimitExceededException.java b/java/com/google/gerrit/server/notedb/LimitExceededException.java
new file mode 100644
index 0000000..9e0969b
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/LimitExceededException.java
@@ -0,0 +1,32 @@
+// Copyright (C) 2019 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.notedb;
+
+import com.google.gerrit.exceptions.StorageException;
+
+/**
+ * A write operation was rejected because a limit would be exceeded. Limits are currently imposed
+ * on:
+ *
+ * <ul>
+ * <li>The number of NoteDb updates per change.
+ * <li>The number of patch sets per change.
+ * </ul>
+ */
+public class LimitExceededException extends StorageException {
+ LimitExceededException(String message) {
+ super(message);
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index 886e02b..1b92c0e 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -64,6 +64,10 @@
* {@link #stage()}.
*/
public class NoteDbUpdateManager implements AutoCloseable {
+ private static final int MAX_UPDATES_DEFAULT = 1000;
+ /** Limits the number of patch sets that can be created. Can be overridden in the config. */
+ private static final int MAX_PATCH_SETS_DEFAULT = 1500;
+
public interface Factory {
NoteDbUpdateManager create(Project.NameKey projectName);
}
@@ -74,6 +78,7 @@
private final NoteDbMetrics metrics;
private final Project.NameKey projectName;
private final int maxUpdates;
+ private final int maxPatchSets;
private final ListMultimap<String, ChangeUpdate> changeUpdates;
private final ListMultimap<String, ChangeDraftUpdate> draftUpdates;
private final ListMultimap<String, RobotCommentUpdate> robotCommentUpdates;
@@ -103,7 +108,8 @@
this.metrics = metrics;
this.updateAllUsersAsync = updateAllUsersAsync;
this.projectName = projectName;
- maxUpdates = cfg.getInt("change", null, "maxUpdates", 1000);
+ maxUpdates = cfg.getInt("change", null, "maxUpdates", MAX_UPDATES_DEFAULT);
+ maxPatchSets = cfg.getInt("change", null, "maxPatchSets", MAX_PATCH_SETS_DEFAULT);
changeUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
draftUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
robotCommentUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
@@ -351,17 +357,17 @@
}
private void addCommands() throws IOException {
- changeRepo.addUpdates(changeUpdates, Optional.of(maxUpdates));
+ changeRepo.addUpdates(changeUpdates, Optional.of(maxUpdates), Optional.of(maxPatchSets));
if (!draftUpdates.isEmpty()) {
boolean publishOnly = draftUpdates.values().stream().allMatch(ChangeDraftUpdate::canRunAsync);
if (publishOnly) {
updateAllUsersAsync.setDraftUpdates(draftUpdates);
} else {
- allUsersRepo.addUpdates(draftUpdates);
+ allUsersRepo.addUpdatesNoLimits(draftUpdates);
}
}
if (!robotCommentUpdates.isEmpty()) {
- changeRepo.addUpdates(robotCommentUpdates);
+ changeRepo.addUpdatesNoLimits(robotCommentUpdates);
}
if (!rewriters.isEmpty()) {
addRewrites(rewriters, changeRepo);
@@ -375,17 +381,16 @@
private void doDelete(Change.Id id) throws IOException {
String metaRef = RefNames.changeMetaRef(id);
Optional<ObjectId> old = changeRepo.cmds.get(metaRef);
- if (old.isPresent()) {
- changeRepo.cmds.add(new ReceiveCommand(old.get(), ObjectId.zeroId(), metaRef));
- }
+ old.ifPresent(
+ objectId -> changeRepo.cmds.add(new ReceiveCommand(objectId, ObjectId.zeroId(), metaRef)));
// Just scan repo for ref names, but get "old" values from cmds.
for (Ref r :
allUsersRepo.repo.getRefDatabase().getRefsByPrefix(RefNames.refsDraftCommentsPrefix(id))) {
old = allUsersRepo.cmds.get(r.getName());
- if (old.isPresent()) {
- allUsersRepo.cmds.add(new ReceiveCommand(old.get(), ObjectId.zeroId(), r.getName()));
- }
+ old.ifPresent(
+ objectId ->
+ allUsersRepo.cmds.add(new ReceiveCommand(objectId, ObjectId.zeroId(), r.getName())));
}
}
diff --git a/java/com/google/gerrit/server/notedb/OpenRepo.java b/java/com/google/gerrit/server/notedb/OpenRepo.java
index de88684..a60309a 100644
--- a/java/com/google/gerrit/server/notedb/OpenRepo.java
+++ b/java/com/google/gerrit/server/notedb/OpenRepo.java
@@ -43,6 +43,18 @@
* objects that are jointly closed when invoking {@link #close}.
*/
class OpenRepo implements AutoCloseable {
+ private static final Integer UNLIMITED_UPDATES = null;
+ private static final Integer UNLIMITED_PATCH_SETS = null;
+
+ final Repository repo;
+ final RevWalk rw;
+ final ChainedReceiveCommands cmds;
+ final ObjectInserter tempIns;
+
+ private final InMemoryInserter inMemIns;
+ @Nullable private final ObjectInserter finalIns;
+ private final boolean close;
+
/** Returns a {@link OpenRepo} wrapping around an open {@link Repository}. */
static OpenRepo open(GitRepositoryManager repoManager, Project.NameKey project)
throws IOException {
@@ -60,15 +72,6 @@
}
}
- final Repository repo;
- final RevWalk rw;
- final ChainedReceiveCommands cmds;
- final ObjectInserter tempIns;
-
- private final InMemoryInserter inMemIns;
- @Nullable private final ObjectInserter finalIns;
- private final boolean close;
-
OpenRepo(
Repository repo,
RevWalk rw,
@@ -125,12 +128,15 @@
return updates.iterator().next().allowWriteToNewRef();
}
- <U extends AbstractChangeUpdate> void addUpdates(ListMultimap<String, U> all) throws IOException {
- addUpdates(all, Optional.empty());
+ <U extends AbstractChangeUpdate> void addUpdatesNoLimits(ListMultimap<String, U> all)
+ throws IOException {
+ addUpdates(
+ all, Optional.empty() /* unlimited updates */, Optional.empty() /* unlimited patch sets */);
}
<U extends AbstractChangeUpdate> void addUpdates(
- ListMultimap<String, U> all, Optional<Integer> maxUpdates) throws IOException {
+ ListMultimap<String, U> all, Optional<Integer> maxUpdates, Optional<Integer> maxPatchSets)
+ throws IOException {
for (Map.Entry<String, Collection<U>> e : all.asMap().entrySet()) {
String refName = e.getKey();
Collection<U> updates = e.getValue();
@@ -142,29 +148,43 @@
continue;
}
- int updateCount;
+ int updateCount = 0;
U first = updates.iterator().next();
if (maxUpdates.isPresent()) {
checkState(first.getNotes() != null, "expected ChangeNotes on %s", first);
updateCount = first.getNotes().getUpdateCount();
- } else {
- updateCount = 0;
}
ObjectId curr = old;
- for (U u : updates) {
- if (u.isRootOnly() && !old.equals(ObjectId.zeroId())) {
+ for (U update : updates) {
+ if (maxPatchSets.isPresent() && update.psId != null) {
+ // Patch set IDs are assigned consecutively. Patch sets may have been deleted, but the ID
+ // is still a good estimate and an upper bound.
+ if (update.psId.get() > maxPatchSets.get()) {
+ throw new LimitExceededException(
+ String.format(
+ "Change %d may not exceed %d patch sets. To continue working on this change, "
+ + "recreate it with a new Change-Id, then abandon this one.",
+ update.getId().get(), maxPatchSets.get()));
+ }
+ }
+ if (update.isRootOnly() && !old.equals(ObjectId.zeroId())) {
throw new StorageException("Given ChangeUpdate is only allowed on initial commit");
}
- ObjectId next = u.apply(rw, tempIns, curr);
+ ObjectId next = update.apply(rw, tempIns, curr);
if (next == null) {
continue;
}
if (maxUpdates.isPresent()
&& !Objects.equals(next, curr)
&& ++updateCount > maxUpdates.get()
- && !u.bypassMaxUpdates()) {
- throw new TooManyUpdatesException(u.getId(), maxUpdates.get());
+ && !update.bypassMaxUpdates()) {
+ throw new LimitExceededException(
+ String.format(
+ "Change %s may not exceed %d updates. It may still be abandoned or submitted. To"
+ + " continue working on this change, recreate it with a new Change-Id, then"
+ + " abandon this one.",
+ update.getId(), maxUpdates.get()));
}
curr = next;
}
diff --git a/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java b/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java
deleted file mode 100644
index 9c6faaf..0000000
--- a/java/com/google/gerrit/server/notedb/TooManyUpdatesException.java
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright (C) 2019 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.notedb;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.exceptions.StorageException;
-
-/**
- * Exception indicating that the change has received too many updates. Further actions apart from
- * {@code abandon} or {@code submit} are blocked.
- */
-public class TooManyUpdatesException extends StorageException {
- @VisibleForTesting
- public static String message(Change.Id id, int maxUpdates) {
- return "Change "
- + id
- + " may not exceed "
- + maxUpdates
- + " updates. It may still be abandoned or submitted. To continue working on this "
- + "change, recreate it with a new Change-Id, then abandon this one.";
- }
-
- private static final long serialVersionUID = 1L;
-
- TooManyUpdatesException(Change.Id id, int maxUpdates) {
- super(message(id, maxUpdates));
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 61b90f1..47f5ad6 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -1394,7 +1394,11 @@
private List<Change> parseChange(String value) throws QueryParseException {
if (PAT_LEGACY_ID.matcher(value).matches()) {
- return asChanges(args.queryProvider.get().byLegacyChangeId(Change.Id.parse(value)));
+ Optional<Change.Id> id = Change.Id.tryParse(value);
+ if (!id.isPresent()) {
+ throw error("Invalid change id " + value);
+ }
+ return asChanges(args.queryProvider.get().byLegacyChangeId(id.get()));
} else if (PAT_CHANGE_ID.matcher(value).matches()) {
List<Change> changes = asChanges(args.queryProvider.get().byKeyPrefix(parseChangeId(value)));
if (changes.isEmpty()) {
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index e240f6a..1e86d52 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -52,8 +52,8 @@
import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.LimitExceededException;
import com.google.gerrit.server.notedb.NoteDbUpdateManager;
-import com.google.gerrit.server.notedb.TooManyUpdatesException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
@@ -181,7 +181,7 @@
private static void wrapAndThrowException(Exception e) throws UpdateException, RestApiException {
// Convert common non-REST exception types with user-visible messages to corresponding REST
// exception types.
- if (e instanceof InvalidChangeOperationException || e instanceof TooManyUpdatesException) {
+ if (e instanceof InvalidChangeOperationException || e instanceof LimitExceededException) {
throw new ResourceConflictException(e.getMessage(), e);
} else if (e instanceof NoSuchChangeException
|| e instanceof NoSuchRefException
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 65f8aa0..ba41d7e 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -319,8 +319,6 @@
@Test
public void postCommentsUnreachableData() throws Exception {
- requestScopeOperations.setApiUser(admin.id());
-
String file = "file";
PushOneCommit push =
pushFactory.create(admin.newIdent(), testRepo, "first subject", file, "l1\nl2\n");
diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
index a2f800a..5860c48 100644
--- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
+++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
@@ -35,7 +35,6 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.Sequences;
-import com.google.gerrit.server.notedb.TooManyUpdatesException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.InMemoryTestEnvironment;
import com.google.inject.Inject;
@@ -51,6 +50,7 @@
public class BatchUpdateTest {
private static final int MAX_UPDATES = 4;
+ private static final int MAX_PATCH_SETS = 3;
@Rule
public InMemoryTestEnvironment testEnvironment =
@@ -58,6 +58,7 @@
() -> {
Config cfg = new Config();
cfg.setInt("change", null, "maxUpdates", MAX_UPDATES);
+ cfg.setInt("change", null, "maxPatchSets", MAX_PATCH_SETS);
return cfg;
});
@@ -106,11 +107,10 @@
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Excessive update"));
- ResourceConflictException thrown =
- assertThrows(ResourceConflictException.class, () -> bu.execute());
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
assertThat(thrown)
.hasMessageThat()
- .isEqualTo(TooManyUpdatesException.message(id, MAX_UPDATES));
+ .contains("Change " + id + " may not exceed " + MAX_UPDATES);
}
assertThat(getUpdateCount(id)).isEqualTo(MAX_UPDATES);
assertThat(getMetaId(id)).isEqualTo(oldMetaId);
@@ -118,17 +118,16 @@
@Test
public void cannotExceedMaxUpdatesCountingMultipleChangeUpdatesInSingleBatch() throws Exception {
- Change.Id id = createChangeWithTwoPatchSets(MAX_UPDATES - 1);
+ Change.Id id = createChangeWithPatchSets(2);
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Update on PS1", PatchSet.id(id, 1)));
bu.addOp(id, new AddMessageOp("Update on PS2", PatchSet.id(id, 2)));
- ResourceConflictException thrown =
- assertThrows(ResourceConflictException.class, () -> bu.execute());
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
assertThat(thrown)
.hasMessageThat()
- .isEqualTo(TooManyUpdatesException.message(id, MAX_UPDATES));
+ .contains("Change " + id + " may not exceed " + MAX_UPDATES);
}
assertThat(getUpdateCount(id)).isEqualTo(MAX_UPDATES - 1);
assertThat(getMetaId(id)).isEqualTo(oldMetaId);
@@ -187,7 +186,7 @@
@Test
public void exceedingMaxUpdatesAllowedWithSubmitAfterOtherOp() throws Exception {
- Change.Id id = createChangeWithTwoPatchSets(MAX_UPDATES - 1);
+ Change.Id id = createChangeWithPatchSets(2);
ObjectId oldMetaId = getMetaId(id);
try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
bu.addOp(id, new AddMessageOp("Message on PS1", PatchSet.id(id, 1)));
@@ -222,6 +221,28 @@
assertThat(getMetaId(id)).isNotEqualTo(oldMetaId);
}
+ @Test
+ public void limitPatchSetCount_exceed() throws Exception {
+ Change.Id changeId = createChangeWithPatchSets(MAX_PATCH_SETS);
+ ObjectId oldMetaId = getMetaId(changeId);
+ ChangeNotes notes = changeNotesFactory.create(project, changeId);
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId()).message("kaboom").create();
+ bu.addOp(
+ changeId,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(changeId, MAX_PATCH_SETS + 1), commitId)
+ .setMessage("kaboom"));
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Change " + changeId + " may not exceed " + MAX_PATCH_SETS + " patch sets");
+ }
+ assertThat(changeNotesFactory.create(project, changeId).getPatchSets()).hasSize(MAX_PATCH_SETS);
+ assertThat(getMetaId(changeId)).isEqualTo(oldMetaId);
+ }
+
private Change.Id createChangeWithUpdates(int totalUpdates) throws Exception {
checkArgument(totalUpdates > 0);
checkArgument(totalUpdates <= MAX_UPDATES);
@@ -243,21 +264,22 @@
return id;
}
- private Change.Id createChangeWithTwoPatchSets(int totalUpdates) throws Exception {
- Change.Id id = createChangeWithUpdates(totalUpdates - 1);
+ private Change.Id createChangeWithPatchSets(int patchSets) throws Exception {
+ checkArgument(patchSets >= 2);
+ Change.Id id = createChangeWithUpdates(MAX_UPDATES - 2);
ChangeNotes notes = changeNotesFactory.create(project, id);
-
- try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
- ObjectId commitId = repo.amend(notes.getCurrentPatchSet().commitId()).message("PS2").create();
- bu.addOp(
- id,
- patchSetInserterFactory
- .create(notes, PatchSet.id(id, 2), commitId)
- .setMessage("Add PS2"));
- bu.execute();
+ for (int i = 2; i <= patchSets; ++i) {
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId()).message("PS" + i).create();
+ bu.addOp(
+ id,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(id, i), commitId)
+ .setMessage("Add PS" + i));
+ bu.execute();
+ }
}
-
- assertThat(getUpdateCount(id)).isEqualTo(totalUpdates);
return id;
}
@@ -291,7 +313,7 @@
}
}
- private int getUpdateCount(Change.Id changeId) throws Exception {
+ private int getUpdateCount(Change.Id changeId) {
return changeNotesFactory.create(project, changeId).getUpdateCount();
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js b/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
index 3652e70..cbc7bcd 100644
--- a/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
+++ b/polygerrit-ui/app/elements/change-list/gr-create-destination-dialog/gr-create-destination-dialog.js
@@ -50,9 +50,13 @@
this.$.createOverlay.close();
}
- _pickerConfirm() {
+ _pickerConfirm(e) {
this.$.createOverlay.close();
const detail = {repo: this._repo, branch: this._branch};
+ // e is a 'confirm' event from gr-dialog. We want to fire a more detailed
+ // 'confirm' event here, so let's stop propagation of the bare event.
+ e.preventDefault();
+ e.stopPropagation();
this.dispatchEvent(new CustomEvent('confirm', {detail, bubbles: false}));
}
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
index fb5ab15..fb5d64f 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.html
@@ -80,10 +80,10 @@
data-name$="[[item.name]]"
data-url$="[[item.url]]"
on-click="_handleShowAgreement"
- disabled$="[[_disableAggreements(item, _groups, _signedAgreements)]]">
+ disabled$="[[_disableAgreements(item, _groups, _signedAgreements)]]">
<label id="claNewAgreementsLabel">[[item.name]]</label>
</span>
- <div class$="alreadySubmittedText [[_hideAggreements(item, _groups, _signedAgreements)]]">
+ <div class$="alreadySubmittedText [[_hideAgreements(item, _groups, _signedAgreements)]]">
Agreement already submitted.
</div>
<div class="agreementsUrl">
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
index 98f1413..863577c 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.js
@@ -117,7 +117,8 @@
return agreements ? 'show' : '';
}
- _disableAggreements(item, groups, signedAgreements) {
+ _disableAgreements(item, groups, signedAgreements) {
+ if (!groups) return false;
for (const group of groups) {
if ((item && item.auto_verify_group &&
item.auto_verify_group.id === group.id) ||
@@ -128,8 +129,8 @@
return false;
}
- _hideAggreements(item, groups, signedAgreements) {
- return this._disableAggreements(item, groups, signedAgreements) ?
+ _hideAgreements(item, groups, signedAgreements) {
+ return this._disableAgreements(item, groups, signedAgreements) ?
'' : 'hide';
}
@@ -141,6 +142,7 @@
// if specified it returns 'hideAgreementsTextBox' which
// then hides the text box and submit button.
_computeHideAgreementClass(name, config) {
+ if (!config) return '';
for (const key in config) {
if (!config.hasOwnProperty(key)) {
continue;
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
index 833fa39..13c4de4 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view_test.html
@@ -142,28 +142,31 @@
'none');
});
- test('_disableAggreements', () => {
+ test('_disableAgreements', () => {
// In the auto verify group and have not yet signed agreement
assert.isTrue(
- element._disableAggreements(auth, groups, signedAgreements));
+ element._disableAgreements(auth, groups, signedAgreements));
// Not in the auto verify group and have not yet signed agreement
assert.isFalse(
- element._disableAggreements(auth2, groups, signedAgreements));
+ element._disableAgreements(auth2, groups, signedAgreements));
// Not in the auto verify group, have signed agreement
assert.isTrue(
- element._disableAggreements(auth3, groups, signedAgreements));
+ element._disableAgreements(auth3, groups, signedAgreements));
+ // Make sure the undefined check works
+ assert.isFalse(
+ element._disableAgreements(auth, undefined, signedAgreements));
});
- test('_hideAggreements', () => {
+ test('_hideAgreements', () => {
// Not in the auto verify group and have not yet signed agreement
assert.equal(
- element._hideAggreements(auth, groups, signedAgreements), '');
+ element._hideAgreements(auth, groups, signedAgreements), '');
// In the auto verify group
assert.equal(
- element._hideAggreements(auth2, groups, signedAgreements), 'hide');
+ element._hideAgreements(auth2, groups, signedAgreements), 'hide');
// Not in the auto verify group, have signed agreement
assert.equal(
- element._hideAggreements(auth3, groups, signedAgreements), '');
+ element._hideAgreements(auth3, groups, signedAgreements), '');
});
test('_disableAgreementsText', () => {
diff --git a/polygerrit-ui/app/rules.bzl b/polygerrit-ui/app/rules.bzl
index 16c0f29..e1304e6 100644
--- a/polygerrit-ui/app/rules.bzl
+++ b/polygerrit-ui/app/rules.bzl
@@ -89,6 +89,8 @@
# we extract from the zip, but depend on the component for license checking.
"@webcomponentsjs//:zipfile",
"//lib/js:webcomponentsjs",
+ "@font-roboto-local//:zipfile",
+ "//lib/js:font-roboto-local",
],
outs = outs,
cmd = " && ".join([
@@ -100,6 +102,7 @@
"for f in $(locations " + name + "_theme_sources); do cp $$f $$TMP/polygerrit_ui/styles/themes; done",
"for f in $(locations //lib/js:highlightjs_files); do cp $$f $$TMP/polygerrit_ui/bower_components/highlightjs/ ; done",
"unzip -qd $$TMP/polygerrit_ui/bower_components $(location @webcomponentsjs//:zipfile) webcomponentsjs/webcomponents-lite.js",
+ "unzip -qd $$TMP/polygerrit_ui/bower_components $(location @font-roboto-local//:zipfile) font-roboto-local/fonts/\*/\*.ttf",
"cd $$TMP",
"find . -exec touch -t 198001010000 '{}' ';'",
"zip -qr $$ROOT/$@ *",
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 357faa4..1dfc2f4 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -170,18 +170,18 @@
sha1 = "3e83394258ae2089be7219b971ec21a8288528ad",
)
- TESTCONTAINERS_VERSION = "1.12.3"
+ TESTCONTAINERS_VERSION = "1.12.4"
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
- sha1 = "e424a4549640e120acceac641ac909fcda58bf62",
+ sha1 = "456b6facac12c4b67130d9056a43c011679e9f0c",
)
maven_jar(
name = "testcontainers-elasticsearch",
artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
- sha1 = "c0796de5032070b8768ce78c78949b48f13c30db",
+ sha1 = "9e210c277a35a95a76d03a79e2812575bd07391c",
)
maven_jar(
