Merge "Document Prolog predicate classes"
diff --git a/e2e-tests/load-tests/Dockerfile b/e2e-tests/load-tests/Dockerfile
new file mode 100644
index 0000000..ceae672
--- /dev/null
+++ b/e2e-tests/load-tests/Dockerfile
@@ -0,0 +1,32 @@
+FROM denvazh/gatling:3.2.1
+
+ARG gatling_git_version=1.0.9
+RUN apk add --no-cache maven
+RUN mvn dependency:get \
+ -DgroupId=com.gerritforge \
+ -DartifactId=gatling-git_2.12 \
+ -Dversion=$gatling_git_version \
+ -Dtype=pom
+RUN mvn dependency:copy-dependencies \
+ -f /root/.m2/repository/com/gerritforge/gatling-git_2.12/$gatling_git_version/gatling-git_2.12-$gatling_git_version.pom \
+ -DoutputDirectory=/opt/gatling/lib/
+RUN mvn dependency:get \
+ -Dartifact=com.gerritforge:gatling-git_2.12:$gatling_git_version:jar \
+ -Ddest=/opt/gatling/lib/gatling-git.jar
+
+ARG gatling_home=/home/gatling
+RUN addgroup -g 1000 -S appgroup && \
+ adduser -u 1000 -S gatling -G appgroup -h $gatling_home
+RUN cp -R /opt/gatling/* $gatling_home && \
+ chown -R gatling:appgroup $gatling_home
+
+WORKDIR $gatling_home
+USER gatling
+
+COPY ./src/test/scala/com/google/gerrit/scenarios $gatling_home/user-files/simulations
+COPY ./src/test/resources/application.conf $gatling_home/conf
+COPY ./src/test/resources/data $gatling_home/user-files/resources/data
+
+ENV GATLING_HOME=$gatling_home
+
+ENTRYPOINT ["/home/gatling/bin/gatling.sh"]
diff --git a/e2e-tests/load-tests/README.md b/e2e-tests/load-tests/README.md
new file mode 100644
index 0000000..534fde5
--- /dev/null
+++ b/e2e-tests/load-tests/README.md
@@ -0,0 +1,11 @@
+# How to build the Docker image
+
+```$shell
+docker build . -t e2e-tests
+```
+
+# How to run a test
+
+```$shell
+docker run -it e2e-tests -s com.google.gerrit.scenarios.ReplayRecordsFromFeederScenario
+```
diff --git a/java/com/google/gerrit/acceptance/UseLocalDisk.java b/java/com/google/gerrit/acceptance/UseLocalDisk.java
index e177bb4..192caa0 100644
--- a/java/com/google/gerrit/acceptance/UseLocalDisk.java
+++ b/java/com/google/gerrit/acceptance/UseLocalDisk.java
@@ -21,6 +21,15 @@
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
+/**
+ * Annotation to mark tests that require a local disk for the execution.
+ *
+ * <p>Tests that do not have this annotation are executed in memory.
+ *
+ * <p>Using this annotation makes the execution of the test more expensive/slower. This is why it
+ * should only be used if the test requires a local disk (e.g. if the test triggers the Git garbage
+ * collection functionality which only works with a local disk).
+ */
@Target({TYPE, METHOD})
@Retention(RUNTIME)
public @interface UseLocalDisk {}
diff --git a/java/com/google/gerrit/acceptance/UseSsh.java b/java/com/google/gerrit/acceptance/UseSsh.java
index 5509140..12a9977 100644
--- a/java/com/google/gerrit/acceptance/UseSsh.java
+++ b/java/com/google/gerrit/acceptance/UseSsh.java
@@ -21,6 +21,14 @@
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
+/**
+ * Annotation to mark SSH tests.
+ *
+ * <p>When running tests the SSH functionality is disabled unless the {@link UseSsh} annotation is
+ * used.
+ *
+ * <p>SSH tests can be skipped when executing tests (see {@link com.google.gerrit.testing.SshMode}).
+ */
@Target({TYPE, METHOD})
@Retention(RUNTIME)
public @interface UseSsh {}
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index e8f6096..90d6b66 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -44,6 +44,11 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.git.validators.ValidationMessage.Type;
+import com.google.gerrit.server.patch.DiffSummary;
+import com.google.gerrit.server.patch.DiffSummaryKey;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListKey;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
@@ -76,7 +81,8 @@
import org.eclipse.jgit.util.SystemReader;
/**
- * Represents a list of CommitValidationListeners to run for a push to one branch of one project.
+ * Represents a list of {@link CommitValidationListener}s to run for a push to one branch of one
+ * project.
*/
public class CommitValidators {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@@ -94,15 +100,16 @@
private final AllProjectsName allProjects;
private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker;
private final AccountValidator accountValidator;
- private final String installCommitMsgHookCommand;
private final ProjectCache projectCache;
private final ProjectConfig.Factory projectConfigFactory;
+ private final PatchListCache patchListCache;
+ private final Config config;
@Inject
Factory(
@GerritPersonIdent PersonIdent gerritIdent,
DynamicItem<UrlFormatter> urlFormatter,
- @GerritServerConfig Config cfg,
+ @GerritServerConfig Config config,
PluginSetContext<CommitValidationListener> pluginValidators,
GitRepositoryManager repoManager,
AllUsersName allUsers,
@@ -110,19 +117,20 @@
ExternalIdsConsistencyChecker externalIdsConsistencyChecker,
AccountValidator accountValidator,
ProjectCache projectCache,
- ProjectConfig.Factory projectConfigFactory) {
+ ProjectConfig.Factory projectConfigFactory,
+ PatchListCache patchListCache) {
this.gerritIdent = gerritIdent;
this.urlFormatter = urlFormatter;
+ this.config = config;
this.pluginValidators = pluginValidators;
this.repoManager = repoManager;
this.allUsers = allUsers;
this.allProjects = allProjects;
this.externalIdsConsistencyChecker = externalIdsConsistencyChecker;
this.accountValidator = accountValidator;
- this.installCommitMsgHookCommand =
- cfg != null ? cfg.getString("gerrit", null, "installCommitMsgHookCommand") : null;
this.projectCache = projectCache;
this.projectConfigFactory = projectConfigFactory;
+ this.patchListCache = patchListCache;
}
public CommitValidators forReceiveCommits(
@@ -146,12 +154,7 @@
new CommitterUploaderValidator(user, perm, urlFormatter.get()),
new SignedOffByValidator(user, perm, projectState),
new ChangeIdValidator(
- projectState,
- user,
- urlFormatter.get(),
- installCommitMsgHookCommand,
- sshInfo,
- change),
+ projectState, user, urlFormatter.get(), config, sshInfo, change),
new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators, skipValidation),
@@ -176,14 +179,10 @@
new ProjectStateValidationListener(projectState),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
new AuthorUploaderValidator(user, perm, urlFormatter.get()),
+ new FileCountValidator(patchListCache, config),
new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.project())),
new ChangeIdValidator(
- projectState,
- user,
- urlFormatter.get(),
- installCommitMsgHookCommand,
- sshInfo,
- change),
+ projectState, user, urlFormatter.get(), config, sshInfo, change),
new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
@@ -268,14 +267,14 @@
ProjectState projectState,
IdentifiedUser user,
UrlFormatter urlFormatter,
- String installCommitMsgHookCommand,
+ Config config,
SshInfo sshInfo,
Change change) {
this.projectState = projectState;
- this.urlFormatter = urlFormatter;
- this.installCommitMsgHookCommand = installCommitMsgHookCommand;
- this.sshInfo = sshInfo;
this.user = user;
+ this.urlFormatter = urlFormatter;
+ installCommitMsgHookCommand = config.getString("gerrit", null, "installCommitMsgHookCommand");
+ this.sshInfo = sshInfo;
this.change = change;
}
@@ -387,6 +386,40 @@
}
}
+ /** Limits the number of files per change. */
+ private static class FileCountValidator implements CommitValidationListener {
+
+ private final PatchListCache patchListCache;
+ private final int maxFileCount;
+
+ FileCountValidator(PatchListCache patchListCache, Config config) {
+ this.patchListCache = patchListCache;
+ maxFileCount = config.getInt("change", null, "maxFiles", 50_000);
+ }
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
+ throws CommitValidationException {
+ PatchListKey patchListKey =
+ PatchListKey.againstBase(
+ receiveEvent.commit.getId(), receiveEvent.commit.getParentCount());
+ DiffSummaryKey diffSummaryKey = DiffSummaryKey.fromPatchListKey(patchListKey);
+ try {
+ DiffSummary diffSummary =
+ patchListCache.getDiffSummary(diffSummaryKey, receiveEvent.project.getNameKey());
+ if (diffSummary.getPaths().size() > maxFileCount) {
+ throw new CommitValidationException(
+ String.format(
+ "Exceeding maximum number of files per change (%d > %d)",
+ diffSummary.getPaths().size(), maxFileCount));
+ }
+ } catch (PatchListNotAvailableException e) {
+ logger.atWarning().withCause(e).log("Failed to validate file count");
+ }
+ return Collections.emptyList();
+ }
+ }
+
/** If this is the special project configuration branch, validate the config. */
public static class ConfigValidator implements CommitValidationListener {
private final ProjectConfig.Factory projectConfigFactory;
diff --git a/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index 15fa0f4..b663b9d 100644
--- a/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -138,11 +138,10 @@
throws PatchListNotAvailableException {
Project.NameKey project = change.getProject();
ObjectId b = patchSet.commitId();
- Whitespace ws = Whitespace.IGNORE_NONE;
if (parentNum != null) {
- return get(PatchListKey.againstParentNum(parentNum, b, ws), project);
+ return get(PatchListKey.againstParentNum(parentNum, b, Whitespace.IGNORE_NONE), project);
}
- return get(PatchListKey.againstDefaultBase(b, ws), project);
+ return get(PatchListKey.againstDefaultBase(b, Whitespace.IGNORE_NONE), project);
}
@Override
diff --git a/java/com/google/gerrit/server/patch/PatchListKey.java b/java/com/google/gerrit/server/patch/PatchListKey.java
index bf38029..386c50d 100644
--- a/java/com/google/gerrit/server/patch/PatchListKey.java
+++ b/java/com/google/gerrit/server/patch/PatchListKey.java
@@ -59,6 +59,12 @@
return new PatchListKey(otherCommitId, newId, whitespace);
}
+ public static PatchListKey againstBase(ObjectId id, int parentCount) {
+ return parentCount > 1
+ ? PatchListKey.againstParentNum(1, id, Whitespace.IGNORE_NONE)
+ : PatchListKey.againstDefaultBase(id, Whitespace.IGNORE_NONE);
+ }
+
/**
* Old patch-set ID
*
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 78ca0fc..883f550 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -45,7 +45,6 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.RobotComment;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.ApprovalsUtil;
@@ -396,12 +395,7 @@
return Optional.empty();
}
- ObjectId id = ps.commitId();
- Whitespace ws = Whitespace.IGNORE_NONE;
- PatchListKey pk =
- parentCount > 1
- ? PatchListKey.againstParentNum(1, id, ws)
- : PatchListKey.againstDefaultBase(id, ws);
+ PatchListKey pk = PatchListKey.againstBase(ps.commitId(), parentCount);
DiffSummaryKey key = DiffSummaryKey.fromPatchListKey(pk);
try {
diffSummary = Optional.of(patchListCache.getDiffSummary(key, c.getProject()));
diff --git a/java/com/google/gerrit/server/restapi/change/GetDiff.java b/java/com/google/gerrit/server/restapi/change/GetDiff.java
index 82bdc34..c1d286a 100644
--- a/java/com/google/gerrit/server/restapi/change/GetDiff.java
+++ b/java/com/google/gerrit/server/restapi/change/GetDiff.java
@@ -29,6 +29,7 @@
import com.google.gerrit.extensions.common.DiffWebLinkInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.CacheControl;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -102,8 +103,8 @@
@Override
public Response<DiffInfo> apply(FileResource resource)
- throws ResourceConflictException, ResourceNotFoundException, AuthException,
- InvalidChangeOperationException, IOException, PermissionBackendException {
+ throws BadRequestException, ResourceConflictException, ResourceNotFoundException,
+ AuthException, InvalidChangeOperationException, IOException, PermissionBackendException {
DiffPreferencesInfo prefs = new DiffPreferencesInfo();
if (whitespace != null) {
prefs.ignoreWhitespace = whitespace;
@@ -130,6 +131,9 @@
RevisionResource baseResource =
revisions.parse(resource.getRevision().getChangeResource(), IdString.fromDecoded(base));
basePatchSet = baseResource.getPatchSet();
+ if (basePatchSet.id().get() == 0) {
+ throw new BadRequestException("edit not allowed as base");
+ }
psf = patchScriptFactoryFactory.create(notes, fileName, basePatchSet.id(), pId, prefs);
} else if (parentNum > 0) {
psf = patchScriptFactoryFactory.create(notes, fileName, parentNum - 1, pId, prefs);
diff --git a/java/com/google/gerrit/server/schema/Schema_180.java b/java/com/google/gerrit/server/schema/Schema_180.java
index 6912b3e..7e13d0d 100644
--- a/java/com/google/gerrit/server/schema/Schema_180.java
+++ b/java/com/google/gerrit/server/schema/Schema_180.java
@@ -14,6 +14,15 @@
package com.google.gerrit.server.schema;
+/**
+ * Schema 180 for Gerrit metadata.
+ *
+ * <p>180 is the first schema version that is supported by NoteDb. All previous schema versions were
+ * for ReviewDb. Since ReviewDb no longer exists those schema versions have been deleted.
+ *
+ * <p>Upgrading to this schema version creates the {@code refs/meta/version} ref in NoteDb that
+ * stores the number of the current schema version.
+ */
public class Schema_180 implements NoteDbSchemaVersion {
@Override
public void upgrade(Arguments args, UpdateUI ui) {
diff --git a/java/com/google/gerrit/server/schema/Schema_181.java b/java/com/google/gerrit/server/schema/Schema_181.java
index 3054ad3..d357ae7 100644
--- a/java/com/google/gerrit/server/schema/Schema_181.java
+++ b/java/com/google/gerrit/server/schema/Schema_181.java
@@ -17,10 +17,16 @@
import com.google.gerrit.gpg.PublicKeyStore;
import org.eclipse.jgit.lib.Repository;
+/**
+ * Schema 181 for Gerrit metadata.
+ *
+ * <p>Upgrading to this schema version populates the GPG subkey to master key map (see {@link
+ * PublicKeyStore}.
+ */
public class Schema_181 implements NoteDbSchemaVersion {
@Override
public void upgrade(Arguments args, UpdateUI ui) throws Exception {
- ui.message("Rebuild GPGP note map to build subkey to master key map");
+ ui.message("Rebuild GPG note map to build subkey to master key map");
try (Repository repo = args.repoManager.openRepository(args.allUsers);
PublicKeyStore store = new PublicKeyStore(repo)) {
store.rebuildSubkeyMasterKeyMap();
diff --git a/java/com/google/gerrit/server/submit/MergeSuperSet.java b/java/com/google/gerrit/server/submit/MergeSuperSet.java
index b36eb7d..f8540fb 100644
--- a/java/com/google/gerrit/server/submit/MergeSuperSet.java
+++ b/java/com/google/gerrit/server/submit/MergeSuperSet.java
@@ -18,7 +18,6 @@
import static java.util.Objects.requireNonNull;
import com.google.common.base.Strings;
-import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.registration.DynamicItem;
@@ -58,6 +57,7 @@
public class MergeSuperSet {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final ChangeData.Factory changeDataFactory;
private final Provider<InternalChangeQuery> queryProvider;
private final Provider<MergeOpRepoManager> repoManagerProvider;
private final DynamicItem<MergeSuperSetComputation> mergeSuperSetComputation;
@@ -72,6 +72,7 @@
@Inject
MergeSuperSet(
@GerritServerConfig Config cfg,
+ ChangeData.Factory changeDataFactory,
Provider<InternalChangeQuery> queryProvider,
Provider<MergeOpRepoManager> repoManagerProvider,
DynamicItem<MergeSuperSetComputation> mergeSuperSetComputation,
@@ -79,6 +80,7 @@
ProjectCache projectCache,
ChangeNotes.Factory notesFactory) {
this.cfg = cfg;
+ this.changeDataFactory = changeDataFactory;
this.queryProvider = queryProvider;
this.repoManagerProvider = repoManagerProvider;
this.mergeSuperSetComputation = mergeSuperSetComputation;
@@ -105,14 +107,7 @@
orm = repoManagerProvider.get();
closeOrm = true;
}
- List<ChangeData> cds = queryProvider.get().byLegacyChangeId(change.getId());
- checkState(
- cds.size() == 1,
- "Expected exactly one ChangeData for change ID %s, got %s",
- change.getId(),
- cds.size());
- ChangeData cd = Iterables.getFirst(cds, null);
-
+ ChangeData cd = changeDataFactory.create(change.getProject(), change.getId());
boolean visible = false;
if (cd != null) {
ProjectState projectState = projectCache.checkedGet(cd.project());
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index d4a4c45..717d3cc 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -21,6 +21,7 @@
import static com.google.gerrit.extensions.common.testing.DiffInfoSubject.assertThat;
import static com.google.gerrit.extensions.common.testing.FileInfoSubject.assertThat;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toMap;
@@ -41,6 +42,7 @@
import com.google.gerrit.extensions.common.ChangeType;
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.FileInfo;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.testing.ConfigSuite;
@@ -2718,6 +2720,23 @@
assertThat(diffInfo).content().element(0).numberOfSkippedLines().isGreaterThan(0);
}
+ @Test
+ public void editNotAllowedAsBase() throws Exception {
+ gApi.changes().id(changeId).edit().create();
+
+ BadRequestException e =
+ assertThrows(
+ BadRequestException.class,
+ () -> getDiffRequest(changeId, CURRENT, FILE_NAME).withBase("edit").get());
+ assertThat(e).hasMessageThat().isEqualTo("edit not allowed as base");
+
+ e =
+ assertThrows(
+ BadRequestException.class,
+ () -> getDiffRequest(changeId, CURRENT, FILE_NAME).withBase("0").get());
+ assertThat(e).hasMessageThat().isEqualTo("edit not allowed as base");
+ }
+
private static CommentInput createCommentInput(
int startLine, int startCharacter, int endLine, int endCharacter, String message) {
CommentInput comment = new CommentInput();
diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
index c7ca887..083493d 100644
--- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
+++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
@@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord;
@@ -35,10 +36,13 @@
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.patch.DiffSummary;
+import com.google.gerrit.server.patch.DiffSummaryKey;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.InMemoryTestEnvironment;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.google.inject.name.Named;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
@@ -57,8 +61,9 @@
new InMemoryTestEnvironment(
() -> {
Config cfg = new Config();
- cfg.setInt("change", null, "maxUpdates", MAX_UPDATES);
+ cfg.setInt("change", null, "maxFiles", 2);
cfg.setInt("change", null, "maxPatchSets", MAX_PATCH_SETS);
+ cfg.setInt("change", null, "maxUpdates", MAX_UPDATES);
return cfg;
});
@@ -70,6 +75,9 @@
@Inject private Provider<CurrentUser> user;
@Inject private Sequences sequences;
+ @Inject
+ private @Named("diff_summary") Cache<DiffSummaryKey, DiffSummary> diffSummaryCache;
+
private Project.NameKey project;
private TestRepository<Repository> repo;
@@ -243,6 +251,59 @@
assertThat(getMetaId(changeId)).isEqualTo(oldMetaId);
}
+ @Test
+ public void limitFileCount_exceed() throws Exception {
+ Change.Id changeId = createChangeWithUpdates(1);
+ ChangeNotes notes = changeNotesFactory.create(project, changeId);
+
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId())
+ .add("bar.txt", "bar")
+ .add("baz.txt", "baz")
+ .add("boom.txt", "boom")
+ .message("blah")
+ .create();
+ bu.addOp(
+ changeId,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(changeId, 2), commitId)
+ .setMessage("blah"));
+ ResourceConflictException thrown = assertThrows(ResourceConflictException.class, bu::execute);
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Exceeding maximum number of files per change (3 > 2)");
+ }
+ }
+
+ @Test
+ public void limitFileCount_cacheKeyMatches() throws Exception {
+ Change.Id changeId = createChangeWithUpdates(1);
+ ChangeNotes notes = changeNotesFactory.create(project, changeId);
+
+ int cacheSizeBefore = diffSummaryCache.asMap().size();
+
+ // We don't want to depend on the test helper used above so we perform an explicit commit here.
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ ObjectId commitId =
+ repo.amend(notes.getCurrentPatchSet().commitId())
+ .add("bar.txt", "bar")
+ .add("baz.txt", "baz")
+ .message("blah")
+ .create();
+ bu.addOp(
+ changeId,
+ patchSetInserterFactory
+ .create(notes, PatchSet.id(changeId, 3), commitId)
+ .setMessage("blah"));
+ bu.execute();
+ }
+
+ // Assert that we only performed the diff computation once. This would e.g. catch
+ // bugs/deviations in the computation of the cache key.
+ assertThat(diffSummaryCache.asMap()).hasSize(cacheSizeBefore + 1);
+ }
+
private Change.Id createChangeWithUpdates(int totalUpdates) throws Exception {
checkArgument(totalUpdates > 0);
checkArgument(totalUpdates <= MAX_UPDATES);