Merge "Delete unused VersionedAccountPreferences class"
diff --git a/.zuul.yaml b/.zuul.yaml
index d6dbc34..e0e92fa 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -7,6 +7,7 @@
This adds required projects needed for all Gerrit-related builds
(i.e., builds of Gerrit itself or plugins) on this branch.
required-projects:
+ - java-prettify
- jgit
- job:
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index fff9d0b..fa30c87 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -2467,6 +2467,53 @@
]
----
+DescendingOrder(d)::
+Sort the returned tags in descending order.
++
+.Request
+----
+ GET /projects/work%2Fmy-project/tags?d HTTP/1.0
+----
++
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ [
+ {
+ "ref": "refs/tags/v3.0",
+ "revision": "c628685b3c5a3614571ecb5c8fceb85db9112313",
+ "object": "1624f5af8ae89148d1a3730df8c290413e3dcf30",
+ "message": "Signed tag\n-----BEGIN PGP SIGNATURE-----\nVersion: GnuPG v1.4.11 (GNU/Linux)\n\niQEcBAABAgAGBQJUMlqYAAoJEPI2qVPgglptp7MH/j+KFcittFbxfSnZjUl8n5IZ\nveZo7wE+syjD9sUbMH4EGv0WYeVjphNTyViBof+stGTNkB0VQzLWI8+uWmOeiJ4a\nzj0LsbDOxodOEMI5tifo02L7r4Lzj++EbqtKv8IUq2kzYoQ2xjhKfFiGjeYOn008\n9PGnhNblEHZgCHguGR6GsfN8bfA2XNl9B5Ysl5ybX1kAVs/TuLZR4oDMZ/pW2S75\nhuyNnSgcgq7vl2gLGefuPs9lxkg5Fj3GZr7XPZk4pt/x1oiH7yXxV4UzrUwg2CC1\nfHrBlNbQ4uJNY8TFcwof52Z0cagM5Qb/ZSLglHbqEDGA68HPqbwf5z2hQyU2/U4\u003d\n\u003dZtUX\n-----END PGP SIGNATURE-----",
+ "tagger": {
+ "name": "David Pursehouse",
+ "email": "david.pursehouse@sonymobile.com",
+ "date": "2014-10-06 09:02:16.000000000",
+ "tz": 540
+ }
+ },
+ {
+ "ref": "refs/tags/v2.0",
+ "revision": "1624f5af8ae89148d1a3730df8c290413e3dcf30"
+ },
+ {
+ "ref": "refs/tags/v1.0",
+ "revision": "49ce77fdcfd3398dc0dedbe016d1a425fd52d666",
+ "object": "1624f5af8ae89148d1a3730df8c290413e3dcf30",
+ "message": "Annotated tag",
+ "tagger": {
+ "name": "David Pursehouse",
+ "email": "david.pursehouse@sonymobile.com",
+ "date": "2014-10-06 07:35:03.000000000",
+ "tz": 540
+ }
+ }
+ ]
+----
+
Substring(m)::
Limit the results to those tags that match the specified substring.
+
diff --git a/java/com/google/gerrit/acceptance/AccountIndexedCounter.java b/java/com/google/gerrit/acceptance/AccountIndexedCounter.java
index 88b97c7..17e0559 100644
--- a/java/com/google/gerrit/acceptance/AccountIndexedCounter.java
+++ b/java/com/google/gerrit/acceptance/AccountIndexedCounter.java
@@ -52,6 +52,11 @@
countsByAccount.remove(accountId.get());
}
+ public void assertReindexAtLeastOnceOf(Account.Id accountId) {
+ assertThat(countsByAccount.asMap().getOrDefault(accountId.get(), 0L)).isAtLeast(1);
+ countsByAccount.remove(accountId.get());
+ }
+
public void assertNoReindex() {
assertThat(countsByAccount.asMap()).isEmpty();
}
diff --git a/java/com/google/gerrit/acceptance/PushOneCommit.java b/java/com/google/gerrit/acceptance/PushOneCommit.java
index ba1dbbc..2d53533 100644
--- a/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -176,7 +176,7 @@
private final TestRepository<?>.CommitBuilder commitBuilder;
@AssistedInject
- PushOneCommit(
+ public PushOneCommit(
Result.Factory pushResultFactory,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo)
@@ -185,7 +185,7 @@
}
@AssistedInject
- PushOneCommit(
+ public PushOneCommit(
Result.Factory pushResultFactory,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -202,7 +202,7 @@
}
@AssistedInject
- PushOneCommit(
+ public PushOneCommit(
Result.Factory pushResultFactory,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -212,7 +212,7 @@
}
@AssistedInject
- PushOneCommit(
+ public PushOneCommit(
Result.Factory pushResultFactory,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -224,7 +224,7 @@
}
@AssistedInject
- PushOneCommit(
+ public PushOneCommit(
Result.Factory pushResultFactory,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -236,7 +236,7 @@
}
@AssistedInject
- PushOneCommit(
+ public PushOneCommit(
Result.Factory pushResultFactory,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -256,7 +256,7 @@
}
@AssistedInject
- PushOneCommit(
+ public PushOneCommit(
Result.Factory pushResultFactory,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -275,7 +275,7 @@
}
@AssistedInject
- PushOneCommit(
+ public PushOneCommit(
Result.Factory pushResultFactory,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -303,6 +303,11 @@
commitBuilder.message(subject).author(i).committer(new PersonIdent(i, testRepo.getDate()));
}
+ @UsedAt(Project.GOOGLE)
+ protected TestRepository<?> testRepository() {
+ return testRepo;
+ }
+
@CanIgnoreReturnValue
public PushOneCommit setParents(List<RevCommit> parents) throws Exception {
commitBuilder.noParents();
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
index 577cda8..5b5895f 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
@@ -71,7 +71,7 @@
this::createAccount, externalIdFactory.arePasswordsAllowed());
}
- private Account.Id createAccount(TestAccountCreation testAccountCreation) throws Exception {
+ protected Account.Id createAccount(TestAccountCreation testAccountCreation) throws Exception {
Account.Id accountId = Account.id(seq.nextAccountId());
Consumer<AccountDelta.Builder> accountCreation =
deltaBuilder -> initAccountDelta(deltaBuilder, testAccountCreation, accountId);
diff --git a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
index 1169364..fa94e0d 100644
--- a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
+++ b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
@@ -72,6 +72,7 @@
abstract class ListRefsRequest<T extends RefInfo> {
protected int limit;
protected int start;
+ protected boolean descendingOrder;
protected String substring;
protected String regex;
protected String nextPageToken;
@@ -88,6 +89,11 @@
return this;
}
+ public ListRefsRequest<T> withDescendingOrder(boolean descendingOrder) {
+ this.descendingOrder = descendingOrder;
+ return this;
+ }
+
public ListRefsRequest<T> withNextPageToken(String token) {
this.nextPageToken = token;
return this;
@@ -111,6 +117,10 @@
return start;
}
+ public boolean getDescendingOrder() {
+ return descendingOrder;
+ }
+
public String getNextPageToken() {
return nextPageToken;
}
diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java
index 29f5a7c..d08aeb7 100644
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -365,7 +365,8 @@
+ e.getDuplicateKey().get()
+ "\" to account "
+ newId
- + "; external ID already in use.");
+ + "; external ID already in use.",
+ e);
} finally {
// If adding the account failed, it may be that it actually was the
// first account. So we reset the 'check for first account'-guard, as
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 74f8ccc..3b07829 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -107,6 +107,9 @@
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gerrit.server.logging.Metadata;
+import com.google.gerrit.server.logging.TraceContext;
+import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -477,22 +480,27 @@
}
}
- private void ensureLoaded(Iterable<ChangeData> all) {
+ private void ensureLoaded(Collection<ChangeData> all) {
if (lazyLoad) {
- for (ChangeData cd : all) {
- // Mark all ChangeDatas as coming from the index, but allow backfilling data from NoteDb
- cd.setStorageConstraint(ChangeData.StorageConstraint.INDEX_PRIMARY_NOTEDB_SECONDARY);
+ try (TraceTimer timer =
+ TraceContext.newTimer(
+ "Load change data for lazyLoad options",
+ Metadata.builder().resourceCount(all.size()).build())) {
+ for (ChangeData cd : all) {
+ // Mark all ChangeDatas as coming from the index, but allow backfilling data from NoteDb
+ cd.setStorageConstraint(ChangeData.StorageConstraint.INDEX_PRIMARY_NOTEDB_SECONDARY);
+ }
+ ChangeData.ensureChangeLoaded(all);
+ if (has(ALL_REVISIONS)) {
+ ChangeData.ensureAllPatchSetsLoaded(all);
+ } else if (has(CURRENT_REVISION) || has(MESSAGES)) {
+ ChangeData.ensureCurrentPatchSetLoaded(all);
+ }
+ if (has(REVIEWED) && userProvider.get().isIdentifiedUser()) {
+ ChangeData.ensureReviewedByLoadedForOpenChanges(all);
+ }
+ ChangeData.ensureCurrentApprovalsLoaded(all);
}
- ChangeData.ensureChangeLoaded(all);
- if (has(ALL_REVISIONS)) {
- ChangeData.ensureAllPatchSetsLoaded(all);
- } else if (has(CURRENT_REVISION) || has(MESSAGES)) {
- ChangeData.ensureCurrentPatchSetLoaded(all);
- }
- if (has(REVIEWED) && userProvider.get().isIdentifiedUser()) {
- ChangeData.ensureReviewedByLoadedForOpenChanges(all);
- }
- ChangeData.ensureCurrentApprovalsLoaded(all);
} else {
for (ChangeData cd : all) {
// Mark all ChangeDatas as coming from the index. Disallow using NoteDb
@@ -618,10 +626,12 @@
if (has(CHECK)) {
out.problems = checkerProvider.get().check(cd.notes(), fix).problems();
// If any problems were fixed, the ChangeData needs to be reloaded.
- for (ProblemInfo p : out.problems) {
- if (p.status == ProblemInfo.Status.FIXED) {
+ if (out.problems.stream().anyMatch(p -> p.status == ProblemInfo.Status.FIXED)) {
+ try (TraceTimer timer =
+ TraceContext.newTimer(
+ "Reload change data after fixing a problem",
+ Metadata.builder().changeId(cd.change().getChangeId()).build())) {
cd = changeDataFactory.create(cd.project(), cd.getId());
- break;
}
}
}
diff --git a/java/com/google/gerrit/server/config/GerritInstanceNameProvider.java b/java/com/google/gerrit/server/config/GerritInstanceNameProvider.java
index 740bb01..b2e80d7 100644
--- a/java/com/google/gerrit/server/config/GerritInstanceNameProvider.java
+++ b/java/com/google/gerrit/server/config/GerritInstanceNameProvider.java
@@ -30,9 +30,8 @@
@Inject
public GerritInstanceNameProvider(
- @GerritServerConfig Config config,
- @CanonicalWebUrl @Nullable Provider<String> canonicalUrlProvider) {
- this.instanceName = getInstanceName(config, canonicalUrlProvider);
+ @GerritServerConfig Config config, @CanonicalWebUrl @Nullable String canonicalUrl) {
+ this.instanceName = getInstanceName(config, canonicalUrl);
}
@Override
@@ -40,14 +39,13 @@
return instanceName;
}
- private static String getInstanceName(
- Config config, @Nullable Provider<String> canonicalUrlProvider) {
+ private static String getInstanceName(Config config, String canonicalUrl) {
String instanceName = config.getString("gerrit", null, "instanceName");
- if (instanceName != null || canonicalUrlProvider == null) {
+ if (instanceName != null) {
return instanceName;
}
- return extractInstanceName(canonicalUrlProvider.get());
+ return extractInstanceName(canonicalUrl);
}
private static String extractInstanceName(String canonicalUrl) {
diff --git a/java/com/google/gerrit/server/config/ProjectConfigEntry.java b/java/com/google/gerrit/server/config/ProjectConfigEntry.java
index e11d6aa..1ff0a8b 100644
--- a/java/com/google/gerrit/server/config/ProjectConfigEntry.java
+++ b/java/com/google/gerrit/server/config/ProjectConfigEntry.java
@@ -44,7 +44,7 @@
private final String displayName;
private final String description;
private final boolean inheritable;
- private final String defaultValue;
+ @Nullable private final String defaultValue;
private final ProjectConfigEntryType type;
private final List<String> permittedValues;
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 69dae04..7caa988 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -369,6 +369,7 @@
private final BatchUpdate.Factory batchUpdateFactory;
private final CancellationMetrics cancellationMetrics;
private final ChangeEditUtil editUtil;
+ private final ChangeData.Factory changeDataFactory;
private final ChangeIndexer indexer;
private final ChangeInserter.Factory changeInserterFactory;
private final ChangeNotes.Factory notesFactory;
@@ -459,6 +460,7 @@
ProjectConfig.Factory projectConfigFactory,
@GerritServerConfig Config config,
ChangeEditUtil editUtil,
+ ChangeData.Factory changeDataFactory,
ChangeIndexer indexer,
ChangeInserter.Factory changeInserterFactory,
ChangeNotes.Factory notesFactory,
@@ -525,6 +527,7 @@
this.deadlineCheckerFactory = deadlineCheckerFactory;
this.diffOperationsForCommitValidationFactory = diffOperationsForCommitValidationFactory;
this.editUtil = editUtil;
+ this.changeDataFactory = changeDataFactory;
this.hashtagsFactory = hashtagsFactory;
this.setTopicFactory = setTopicFactory;
this.indexer = indexer;
@@ -884,7 +887,7 @@
logger.atFine().log("Added %d additional ref updates", added);
SubmissionExecutor submissionExecutor =
- new SubmissionExecutor(false, superprojectUpdateSubmissionListeners);
+ new SubmissionExecutor(changeDataFactory, false, superprojectUpdateSubmissionListeners);
submissionExecutor.execute(ImmutableList.of(bu));
diff --git a/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java b/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
index 5a439f8..59d1b9b 100644
--- a/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
+++ b/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
@@ -142,7 +142,7 @@
@Override
public boolean canEmail(String address) {
if (!isEnabled()) {
- logger.atWarning().log("Not emailing %s (email is disabled)", address);
+ logger.atFine().log("Not emailing %s (email is disabled)", address);
return false;
}
@@ -163,7 +163,7 @@
if (denyrcpt.contains(address)
|| denyrcpt.contains(domain)
|| denyrcpt.contains("@" + domain)) {
- logger.atInfo().log("Not emailing %s (prohibited by sendemail.denyrcpt)", address);
+ logger.atFine().log("Not emailing %s (prohibited by sendemail.denyrcpt)", address);
return true;
}
@@ -182,7 +182,7 @@
return true;
}
- logger.atWarning().log("Not emailing %s (prohibited by sendemail.allowrcpt)", address);
+ logger.atFine().log("Not emailing %s (prohibited by sendemail.allowrcpt)", address);
return false;
}
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftCommentsUtil.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftCommentsUtil.java
index a49952c..6fe464d 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteDraftCommentsUtil.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftCommentsUtil.java
@@ -122,7 +122,7 @@
// were,
// all updates from this operation only happen in All-Users and thus are fully atomic, so
// allowing partial failure would have little value.
- BatchUpdate.execute(updates.values(), ImmutableList.of(), false);
+ BatchUpdate.execute(changeDataFactory, updates.values(), ImmutableList.of(), false);
}
return ops.stream().map(Op::getResult).filter(Objects::nonNull).collect(toImmutableList());
}
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index f54df5b..709d6a7 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -146,7 +146,6 @@
private final BatchUpdate.Factory updateFactory;
private final PostReviewOp.Factory postReviewOpFactory;
private final ChangeResource.Factory changeResourceFactory;
- private final ChangeData.Factory changeDataFactory;
private final AccountCache accountCache;
private final ApprovalsUtil approvalsUtil;
private final DraftCommentsReader draftCommentsReader;
@@ -171,7 +170,6 @@
BatchUpdate.Factory updateFactory,
PostReviewOp.Factory postReviewOpFactory,
ChangeResource.Factory changeResourceFactory,
- ChangeData.Factory changeDataFactory,
AccountCache accountCache,
ApprovalsUtil approvalsUtil,
DraftCommentsReader draftCommentsReader,
@@ -191,7 +189,6 @@
this.updateFactory = updateFactory;
this.postReviewOpFactory = postReviewOpFactory;
this.changeResourceFactory = changeResourceFactory;
- this.changeDataFactory = changeDataFactory;
this.accountCache = accountCache;
this.draftCommentsReader = draftCommentsReader;
this.approvalsUtil = approvalsUtil;
@@ -293,6 +290,8 @@
}
output.labels = input.labels;
+ BatchUpdate.Result batchUpdateResult;
+
// Notify based on ReviewInput, ignoring the notify settings from any ReviewerInputs.
NotifyResolver.Result notify = notifyResolver.resolve(input.notify, input.notifyDetails);
try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
@@ -380,12 +379,13 @@
// Adjust the attention set based on the input
replyAttentionSetUpdates.updateAttentionSetOnPostReview(
bu, postReviewOp, revision.getNotes(), input, revision.getUser());
- bu.execute();
+
+ batchUpdateResult = bu.execute();
}
}
- // Re-read change to take into account results of the update.
- ChangeData cd = changeDataFactory.create(revision.getProject(), revision.getChange().getId());
+ ChangeData cd =
+ batchUpdateResult.getChangeData(revision.getProject(), revision.getChange().getId());
for (ReviewerModification reviewerResult : reviewerResults) {
reviewerResult.gatherResults(cd);
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
index 338ff0d..3e8002b 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
@@ -14,17 +14,11 @@
package com.google.gerrit.server.restapi.project;
-import static com.google.gerrit.server.project.ProjectCache.illegalState;
-import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
-
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.AccessSection;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.InvalidNameException;
import com.google.gerrit.extensions.api.access.ProjectAccessInput;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -33,89 +27,35 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.server.Sequences;
-import com.google.gerrit.server.approval.ApprovalsUtil;
-import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeJson;
-import com.google.gerrit.server.git.meta.MetaDataUpdate;
-import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.ProjectPermission;
-import com.google.gerrit.server.permissions.RefPermission;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
-import com.google.gerrit.server.update.context.RefUpdateContext;
-import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.ObjectReader;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
@Singleton
public class CreateAccessChange implements RestModifyView<ProjectResource, ProjectAccessInput> {
- private final PermissionBackend permissionBackend;
- private final Sequences seq;
- private final ChangeInserter.Factory changeInserterFactory;
- private final BatchUpdate.Factory updateFactory;
- private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
private final SetAccessUtil setAccess;
private final ChangeJson.Factory jsonFactory;
- private final ProjectCache projectCache;
- private final ProjectConfig.Factory projectConfigFactory;
+ private final RepoMetaDataUpdater repoMetaDataUpdater;
@Inject
CreateAccessChange(
- PermissionBackend permissionBackend,
- ChangeInserter.Factory changeInserterFactory,
- BatchUpdate.Factory updateFactory,
- Sequences seq,
- Provider<MetaDataUpdate.User> metaDataUpdateFactory,
SetAccessUtil accessUtil,
ChangeJson.Factory jsonFactory,
- ProjectCache projectCache,
- ProjectConfig.Factory projectConfigFactory) {
- this.permissionBackend = permissionBackend;
- this.seq = seq;
- this.changeInserterFactory = changeInserterFactory;
- this.updateFactory = updateFactory;
- this.metaDataUpdateFactory = metaDataUpdateFactory;
+ RepoMetaDataUpdater repoMetaDataUpdater) {
this.setAccess = accessUtil;
this.jsonFactory = jsonFactory;
- this.projectCache = projectCache;
- this.projectConfigFactory = projectConfigFactory;
+ this.repoMetaDataUpdater = repoMetaDataUpdater;
}
@Override
public Response<ChangeInfo> apply(ProjectResource rsrc, ProjectAccessInput input)
- throws PermissionBackendException, AuthException, IOException, ConfigInvalidException,
- InvalidNameException, UpdateException, RestApiException {
- PermissionBackend.ForProject forProject =
- permissionBackend.user(rsrc.getUser()).project(rsrc.getNameKey());
- if (!check(forProject, ProjectPermission.READ_CONFIG)) {
- throw new AuthException(RefNames.REFS_CONFIG + " not visible");
- }
- if (!check(forProject, ProjectPermission.WRITE_CONFIG)) {
- try {
- forProject.ref(RefNames.REFS_CONFIG).check(RefPermission.CREATE_CHANGE);
- } catch (AuthException denied) {
- throw new AuthException("cannot create change for " + RefNames.REFS_CONFIG, denied);
- }
- }
- projectCache
- .get(rsrc.getNameKey())
- .orElseThrow(illegalState(rsrc.getNameKey()))
- .checkStatePermitsWrite();
-
- MetaDataUpdate.User metaDataUpdateUser = metaDataUpdateFactory.get();
+ throws PermissionBackendException, IOException, ConfigInvalidException, InvalidNameException,
+ UpdateException, RestApiException {
ImmutableList<AccessSection> removals =
setAccess.getAccessSections(input.remove, /* rejectNonResolvableGroups= */ false);
ImmutableList<AccessSection> additions =
@@ -123,81 +63,30 @@
Project.NameKey newParentProjectName =
input.parent == null ? null : Project.nameKey(input.parent);
-
- try (MetaDataUpdate md = metaDataUpdateUser.create(rsrc.getNameKey())) {
- ProjectConfig config = projectConfigFactory.read(md);
- ObjectId oldCommit = config.getRevision();
- String oldCommitSha1 = oldCommit == null ? null : oldCommit.getName();
-
- setAccess.validateChanges(config, removals, additions);
- setAccess.applyChanges(config, removals, additions);
- try {
- setAccess.setParentName(
- rsrc.getUser().asIdentifiedUser(),
- config,
- rsrc.getNameKey(),
- newParentProjectName,
- false);
- } catch (AuthException e) {
- throw new IllegalStateException(e);
- }
-
- if (!Strings.isNullOrEmpty(input.message)) {
- if (!input.message.endsWith("\n")) {
- input.message += "\n";
- }
- md.setMessage(input.message);
- } else {
- md.setMessage("Review access change\n");
- }
-
- md.setInsertChangeId(true);
- Change.Id changeId = Change.id(seq.nextChangeId());
- try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
- RevCommit commit =
- config.commitToNewRef(
- md, PatchSet.id(changeId, Change.INITIAL_PATCH_SET_ID).toRefName());
-
- if (commit.name().equals(oldCommitSha1)) {
- throw new BadRequestException("no change");
- }
-
- try (ObjectInserter objInserter = md.getRepository().newObjectInserter();
- ObjectReader objReader = objInserter.newReader();
- RevWalk rw = new RevWalk(objReader);
- BatchUpdate bu =
- updateFactory.create(rsrc.getNameKey(), rsrc.getUser(), TimeUtil.now())) {
- bu.setRepository(md.getRepository(), rw, objInserter);
- ChangeInserter ins = newInserter(changeId, commit);
- bu.insertChange(ins);
- bu.execute();
- return Response.created(jsonFactory.noOptions().format(ins.getChange()));
- }
- }
+ String message = !Strings.isNullOrEmpty(input.message) ? input.message : "Review access change";
+ try {
+ Change change =
+ repoMetaDataUpdater.updateAndCreateChangeForReview(
+ rsrc.getNameKey(),
+ rsrc.getUser(),
+ message,
+ config -> {
+ setAccess.validateChanges(config, removals, additions);
+ setAccess.applyChanges(config, removals, additions);
+ try {
+ setAccess.setParentName(
+ rsrc.getUser().asIdentifiedUser(),
+ config,
+ rsrc.getNameKey(),
+ newParentProjectName,
+ false);
+ } catch (AuthException e) {
+ throw new IllegalStateException(e);
+ }
+ });
+ return Response.created(jsonFactory.noOptions().format(change));
} catch (InvalidNameException e) {
throw new BadRequestException(e.toString());
}
}
-
- // ProjectConfig doesn't currently support fusing into a BatchUpdate.
- @SuppressWarnings("deprecation")
- private ChangeInserter newInserter(Change.Id changeId, RevCommit commit) {
- return changeInserterFactory
- .create(changeId, commit, RefNames.REFS_CONFIG)
- .setMessage(
- // Same message as in ReceiveCommits.CreateRequest.
- ApprovalsUtil.renderMessageWithApprovals(1, ImmutableMap.of(), ImmutableMap.of()))
- .setValidate(false)
- .setUpdateRef(false);
- }
-
- private boolean check(PermissionBackend.ForProject perm, ProjectPermission p)
- throws PermissionBackendException {
- try {
- perm.check(p);
- return true;
- } catch (AuthException denied) {
- return false;
- }
- }
}
diff --git a/java/com/google/gerrit/server/restapi/project/ListTags.java b/java/com/google/gerrit/server/restapi/project/ListTags.java
index 83d29de..5eca1fc 100644
--- a/java/com/google/gerrit/server/restapi/project/ListTags.java
+++ b/java/com/google/gerrit/server/restapi/project/ListTags.java
@@ -42,6 +42,7 @@
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
@@ -78,6 +79,14 @@
}
@Option(
+ name = "--descending",
+ aliases = {"-d"},
+ usage = "sort the returned tags in descending order")
+ public void setDescendingOrder(boolean descendingOrder) {
+ this.descendingOrder = descendingOrder;
+ }
+
+ @Option(
name = "--match",
aliases = {"-m"},
metaVar = "MATCH",
@@ -97,6 +106,7 @@
private int limit;
private int start;
+ private boolean descendingOrder;
private String matchSubstring;
private String matchRegex;
@@ -111,6 +121,7 @@
public ListTags request(ListRefsRequest<TagInfo> request) {
this.setLimit(request.getLimit());
this.setStart(request.getStart());
+ this.setDescendingOrder(request.getDescendingOrder());
this.setMatchSubstring(request.getSubstring());
this.setMatchRegex(request.getRegex());
return this;
@@ -137,6 +148,9 @@
}
tags.sort(comparing(t -> t.ref));
+ if (descendingOrder) {
+ Collections.reverse(tags);
+ }
return Response.ok(
new RefFilter<>(Constants.R_TAGS, (TagInfo tag) -> tag.ref)
diff --git a/java/com/google/gerrit/server/restapi/project/PutConfig.java b/java/com/google/gerrit/server/restapi/project/PutConfig.java
index 7bca46d..d5f61ce 100644
--- a/java/com/google/gerrit/server/restapi/project/PutConfig.java
+++ b/java/com/google/gerrit/server/restapi/project/PutConfig.java
@@ -222,7 +222,8 @@
value = Joiner.on("\n").join(v.getValue().values);
}
- if (projectConfigEntry.getDefaultValue().equals(value)) {
+ String defaultValue = projectConfigEntry.getDefaultValue();
+ if (defaultValue != null && defaultValue.equals(value)) {
// If the value is equal to the default, unset in case it existed.
if (oldValue != null) {
validateProjectConfigEntryIsEditable(
diff --git a/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java b/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java
new file mode 100644
index 0000000..c45a009
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/project/RepoMetaDataUpdater.java
@@ -0,0 +1,215 @@
+// Copyright (C) 2024 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.restapi.project;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.gerrit.server.project.ProjectCache.illegalState;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.InvalidNameException;
+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.RestApiException;
+import com.google.gerrit.server.CreateGroupPermissionSyncer;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.Sequences;
+import com.google.gerrit.server.approval.ApprovalsUtil;
+import com.google.gerrit.server.change.ChangeInserter;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.git.meta.MetaDataUpdate.User;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.UpdateException;
+import com.google.gerrit.server.update.context.RefUpdateContext;
+import com.google.gerrit.server.util.time.TimeUtil;
+import java.io.IOException;
+import javax.inject.Inject;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/** Updates repo refs/meta/config content. */
+@Singleton
+public class RepoMetaDataUpdater {
+ private final CreateGroupPermissionSyncer createGroupPermissionSyncer;
+ private final Provider<User> metaDataUpdateFactory;
+ private final ProjectConfig.Factory projectConfigFactory;
+ private final ProjectCache projectCache;
+ private final ChangeInserter.Factory changeInserterFactory;
+ private final Sequences seq;
+
+ private final BatchUpdate.Factory updateFactory;
+
+ private final PermissionBackend permissionBackend;
+
+ @Inject
+ RepoMetaDataUpdater(
+ CreateGroupPermissionSyncer createGroupPermissionSyncer,
+ Provider<User> metaDataUpdateFactory,
+ ProjectConfig.Factory projectConfigFactory,
+ ProjectCache projectCache,
+ ChangeInserter.Factory changeInserterFactory,
+ Sequences seq,
+ BatchUpdate.Factory updateFactory,
+ PermissionBackend permissionBackend) {
+ this.createGroupPermissionSyncer = createGroupPermissionSyncer;
+ this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.projectConfigFactory = projectConfigFactory;
+ this.projectCache = projectCache;
+ this.changeInserterFactory = changeInserterFactory;
+ this.seq = seq;
+ this.updateFactory = updateFactory;
+ this.permissionBackend = permissionBackend;
+ }
+
+ public Change updateAndCreateChangeForReview(
+ Project.NameKey projectName,
+ CurrentUser user,
+ String message,
+ ProjectConfigUpdater projectConfigUpdater)
+ throws ConfigInvalidException, IOException, RestApiException, UpdateException,
+ InvalidNameException, PermissionBackendException {
+ checkArgument(!message.isBlank(), "The message must not be empty");
+ message = validateMessage(message);
+
+ PermissionBackend.ForProject forProject = permissionBackend.user(user).project(projectName);
+ if (!check(forProject, ProjectPermission.READ_CONFIG)) {
+ throw new AuthException(RefNames.REFS_CONFIG + " not visible");
+ }
+ if (!check(forProject, ProjectPermission.WRITE_CONFIG)) {
+ try {
+ forProject.ref(RefNames.REFS_CONFIG).check(RefPermission.CREATE_CHANGE);
+ } catch (AuthException denied) {
+ throw new AuthException("cannot create change for " + RefNames.REFS_CONFIG, denied);
+ }
+ }
+ projectCache.get(projectName).orElseThrow(illegalState(projectName)).checkStatePermitsWrite();
+
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(projectName)) {
+ ProjectConfig config = projectConfigFactory.read(md);
+ ObjectId oldCommit = config.getRevision();
+ String oldCommitSha1 = oldCommit == null ? null : oldCommit.getName();
+
+ projectConfigUpdater.update(config);
+ md.setMessage(message);
+ md.setInsertChangeId(true);
+
+ Change.Id changeId = Change.id(seq.nextChangeId());
+ try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ RevCommit commit =
+ config.commitToNewRef(
+ md, PatchSet.id(changeId, Change.INITIAL_PATCH_SET_ID).toRefName());
+
+ if (commit.name().equals(oldCommitSha1)) {
+ throw new BadRequestException("no change");
+ }
+
+ try (ObjectInserter objInserter = md.getRepository().newObjectInserter();
+ ObjectReader objReader = objInserter.newReader();
+ RevWalk rw = new RevWalk(objReader);
+ BatchUpdate bu = updateFactory.create(projectName, user, TimeUtil.now())) {
+ bu.setRepository(md.getRepository(), rw, objInserter);
+ ChangeInserter ins = newInserter(changeId, commit);
+ bu.insertChange(ins);
+ bu.execute();
+ return ins.getChange();
+ }
+ }
+ }
+ }
+
+ public void updateWithoutReview(
+ Project.NameKey projectName, String message, ProjectConfigUpdater projectConfigUpdater)
+ throws ConfigInvalidException, IOException, PermissionBackendException, AuthException,
+ ResourceConflictException, InvalidNameException, BadRequestException {
+ updateWithoutReview(
+ projectName, message, /*skipPermissionsCheck=*/ false, projectConfigUpdater);
+ }
+
+ public void updateWithoutReview(
+ Project.NameKey projectName,
+ String message,
+ boolean skipPermissionsCheck,
+ ProjectConfigUpdater projectConfigUpdater)
+ throws ConfigInvalidException, IOException, PermissionBackendException, AuthException,
+ ResourceConflictException, InvalidNameException, BadRequestException {
+ message = validateMessage(message);
+ if (!skipPermissionsCheck) {
+ permissionBackend.currentUser().project(projectName).check(ProjectPermission.WRITE_CONFIG);
+ }
+
+ try (MetaDataUpdate md = metaDataUpdateFactory.get().create(projectName)) {
+ ProjectConfig config = projectConfigFactory.read(md);
+
+ projectConfigUpdater.update(config);
+ md.setMessage(message);
+ config.commit(md);
+ projectCache.evictAndReindex(config.getProject());
+ createGroupPermissionSyncer.syncIfNeeded();
+ }
+ }
+
+ private String validateMessage(String message) {
+ if (!message.endsWith("\n")) {
+ return message + "\n";
+ }
+ return message;
+ }
+
+ // ProjectConfig doesn't currently support fusing into a BatchUpdate.
+ @SuppressWarnings("deprecation")
+ private ChangeInserter newInserter(Change.Id changeId, RevCommit commit) {
+ return changeInserterFactory
+ .create(changeId, commit, RefNames.REFS_CONFIG)
+ .setMessage(
+ // Same message as in ReceiveCommits.CreateRequest.
+ ApprovalsUtil.renderMessageWithApprovals(1, ImmutableMap.of(), ImmutableMap.of()))
+ .setValidate(false)
+ .setUpdateRef(false);
+ }
+
+ private boolean check(PermissionBackend.ForProject perm, ProjectPermission p)
+ throws PermissionBackendException {
+ try {
+ perm.check(p);
+ return true;
+ } catch (AuthException denied) {
+ return false;
+ }
+ }
+
+ @FunctionalInterface
+ public interface ProjectConfigUpdater {
+ void update(ProjectConfig config)
+ throws BadRequestException, InvalidNameException, PermissionBackendException,
+ ResourceConflictException, AuthException;
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/project/SetAccess.java b/java/com/google/gerrit/server/restapi/project/SetAccess.java
index e4e4373..75fe280 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccess.java
@@ -29,15 +29,11 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.server.CreateGroupPermissionSyncer;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupBackend;
-import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.RefPermission;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -49,92 +45,72 @@
public class SetAccess implements RestModifyView<ProjectResource, ProjectAccessInput> {
protected final GroupBackend groupBackend;
private final PermissionBackend permissionBackend;
- private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
private final GetAccess getAccess;
- private final ProjectCache projectCache;
private final Provider<IdentifiedUser> identifiedUser;
private final SetAccessUtil accessUtil;
- private final CreateGroupPermissionSyncer createGroupPermissionSyncer;
- private final ProjectConfig.Factory projectConfigFactory;
+ private final RepoMetaDataUpdater repoMetaDataUpdater;
@Inject
private SetAccess(
GroupBackend groupBackend,
PermissionBackend permissionBackend,
- Provider<MetaDataUpdate.User> metaDataUpdateFactory,
- ProjectCache projectCache,
GetAccess getAccess,
Provider<IdentifiedUser> identifiedUser,
SetAccessUtil accessUtil,
- CreateGroupPermissionSyncer createGroupPermissionSyncer,
- ProjectConfig.Factory projectConfigFactory) {
+ RepoMetaDataUpdater repoMetaDataUpdater) {
this.groupBackend = groupBackend;
this.permissionBackend = permissionBackend;
- this.metaDataUpdateFactory = metaDataUpdateFactory;
this.getAccess = getAccess;
- this.projectCache = projectCache;
this.identifiedUser = identifiedUser;
this.accessUtil = accessUtil;
- this.createGroupPermissionSyncer = createGroupPermissionSyncer;
- this.projectConfigFactory = projectConfigFactory;
+ this.repoMetaDataUpdater = repoMetaDataUpdater;
}
@Override
public Response<ProjectAccessInfo> apply(ProjectResource rsrc, ProjectAccessInput input)
throws Exception {
- MetaDataUpdate.User metaDataUpdateUser = metaDataUpdateFactory.get();
-
validateInput(input);
- ProjectConfig config;
-
ImmutableList<AccessSection> removals =
accessUtil.getAccessSections(input.remove, /* rejectNonResolvableGroups= */ false);
ImmutableList<AccessSection> additions =
accessUtil.getAccessSections(input.add, /* rejectNonResolvableGroups= */ true);
- try (MetaDataUpdate md = metaDataUpdateUser.create(rsrc.getNameKey())) {
- config = projectConfigFactory.read(md);
-
- // Check that the user has the right permissions.
- boolean checkedAdmin = false;
- for (AccessSection section : Iterables.concat(additions, removals)) {
- boolean isGlobalCapabilities = AccessSection.GLOBAL_CAPABILITIES.equals(section.getName());
- if (isGlobalCapabilities) {
- if (!checkedAdmin) {
- permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
- checkedAdmin = true;
- }
- } else {
- permissionBackend
- .currentUser()
- .project(rsrc.getNameKey())
- .ref(section.getName())
- .check(RefPermission.WRITE_CONFIG);
- }
- }
-
- accessUtil.validateChanges(config, removals, additions);
- accessUtil.applyChanges(config, removals, additions);
-
- accessUtil.setParentName(
- identifiedUser.get(),
- config,
+ String message = !Strings.isNullOrEmpty(input.message) ? input.message : "Modify access rules";
+ try {
+ this.repoMetaDataUpdater.updateWithoutReview(
rsrc.getNameKey(),
- input.parent == null ? null : Project.nameKey(input.parent),
- !checkedAdmin);
+ message,
+ /*skipPermissionsCheck=*/ true,
+ config -> {
+ // Check that the user has the right permissions.
+ boolean checkedAdmin = false;
+ for (AccessSection section : Iterables.concat(additions, removals)) {
+ boolean isGlobalCapabilities =
+ AccessSection.GLOBAL_CAPABILITIES.equals(section.getName());
+ if (isGlobalCapabilities) {
+ if (!checkedAdmin) {
+ permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
+ checkedAdmin = true;
+ }
+ } else {
+ permissionBackend
+ .currentUser()
+ .project(rsrc.getNameKey())
+ .ref(section.getName())
+ .check(RefPermission.WRITE_CONFIG);
+ }
+ }
- if (!Strings.isNullOrEmpty(input.message)) {
- if (!input.message.endsWith("\n")) {
- input.message += "\n";
- }
- md.setMessage(input.message);
- } else {
- md.setMessage("Modify access rules\n");
- }
+ accessUtil.validateChanges(config, removals, additions);
+ accessUtil.applyChanges(config, removals, additions);
- config.commit(md);
- projectCache.evictAndReindex(config.getProject());
- createGroupPermissionSyncer.syncIfNeeded();
+ accessUtil.setParentName(
+ identifiedUser.get(),
+ config,
+ rsrc.getNameKey(),
+ input.parent == null ? null : Project.nameKey(input.parent),
+ !checkedAdmin);
+ });
} catch (InvalidNameException e) {
throw new BadRequestException(e.toString());
} catch (ConfigInvalidException e) {
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index bc14f34..bb1f25f 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -558,7 +558,8 @@
}
SubmissionExecutor submissionExecutor =
- new SubmissionExecutor(dryrun, superprojectUpdateSubmissionListeners);
+ new SubmissionExecutor(
+ changeDataFactory, dryrun, superprojectUpdateSubmissionListeners);
RetryTracker retryTracker = new RetryTracker();
@SuppressWarnings("unused")
var unused =
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index cebb5e3..02afedb 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -24,6 +24,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.submit.MergeOpRepoManager.OpenRepo;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
@@ -39,13 +40,16 @@
@Singleton
public static class Factory {
+ private final ChangeData.Factory changeDataFactory;
private final SubscriptionGraph.Factory subscriptionGraphFactory;
private final SubmoduleCommits.Factory submoduleCommitsFactory;
@Inject
Factory(
+ ChangeData.Factory changeDataFactory,
SubscriptionGraph.Factory subscriptionGraphFactory,
SubmoduleCommits.Factory submoduleCommitsFactory) {
+ this.changeDataFactory = changeDataFactory;
this.subscriptionGraphFactory = subscriptionGraphFactory;
this.submoduleCommitsFactory = submoduleCommitsFactory;
}
@@ -54,6 +58,7 @@
Map<BranchNameKey, ReceiveCommand> updatedBranches, MergeOpRepoManager orm)
throws SubmoduleConflictException {
return new SubmoduleOp(
+ changeDataFactory,
updatedBranches,
orm,
subscriptionGraphFactory.compute(updatedBranches.keySet(), orm),
@@ -61,6 +66,7 @@
}
}
+ private final ChangeData.Factory changeDataFactory;
private final Map<BranchNameKey, ReceiveCommand> updatedBranches;
private final MergeOpRepoManager orm;
private final SubscriptionGraph subscriptionGraph;
@@ -68,10 +74,12 @@
private final UpdateOrderCalculator updateOrderCalculator;
private SubmoduleOp(
+ ChangeData.Factory changeDataFactory,
Map<BranchNameKey, ReceiveCommand> updatedBranches,
MergeOpRepoManager orm,
SubscriptionGraph subscriptionGraph,
SubmoduleCommits submoduleCommits) {
+ this.changeDataFactory = changeDataFactory;
this.updatedBranches = updatedBranches;
this.orm = orm;
this.subscriptionGraph = subscriptionGraph;
@@ -108,6 +116,7 @@
}
try (RefUpdateContext ctx = RefUpdateContext.open(UPDATE_SUPERPROJECT)) {
BatchUpdate.execute(
+ changeDataFactory,
orm.batchUpdates(superProjects, /* refLogMessage= */ "merged"),
ImmutableList.of(),
dryrun);
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index b28ab27..0e29a4e 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -103,7 +103,6 @@
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.PushCertificate;
import org.eclipse.jgit.transport.ReceiveCommand;
-import org.eclipse.jgit.transport.ReceiveCommand.Result;
/**
* Helper for a set of change updates that should be applied to the NoteDb database.
@@ -142,12 +141,42 @@
BatchUpdate create(Project.NameKey project, CurrentUser user, Instant when);
}
- public static void execute(
- Collection<BatchUpdate> updates, ImmutableList<BatchUpdateListener> listeners, boolean dryrun)
+ public static class Result {
+ private final ChangeData.Factory changeDataFactory;
+ private final Map<Change.Id, ChangeData> changeDatas;
+
+ private Result(ChangeData.Factory changeDataFactory) {
+ this(changeDataFactory, new HashMap<>());
+ }
+
+ private Result(ChangeData.Factory changeDataFactory, Map<Change.Id, ChangeData> changeDatas) {
+ this.changeDataFactory = changeDataFactory;
+ this.changeDatas = changeDatas;
+ }
+
+ /**
+ * Returns the updated {@link ChangeData} for the given project and change ID.
+ *
+ * <p>If the requested {@link ChangeData} was already loaded after the {@link BatchUpdate} has
+ * been executed the cached {@link ChangeData} instance is returned, otherwise the requested
+ * {@link ChangeData} is loaded and put into the cache.
+ */
+ public ChangeData getChangeData(Project.NameKey projectName, Change.Id changeId) {
+ return changeDatas.computeIfAbsent(
+ changeId, id -> changeDataFactory.create(projectName, changeId));
+ }
+ }
+
+ @CanIgnoreReturnValue
+ public static Result execute(
+ ChangeData.Factory changeDataFactory,
+ Collection<BatchUpdate> updates,
+ ImmutableList<BatchUpdateListener> listeners,
+ boolean dryrun)
throws UpdateException, RestApiException {
requireNonNull(listeners);
if (updates.isEmpty()) {
- return;
+ return new Result(changeDataFactory);
}
checkDifferentProject(updates);
@@ -193,8 +222,11 @@
u.executePostOps(changeDatas);
}
}
+
+ return new Result(changeDataFactory, changeDatas);
} catch (Exception e) {
wrapAndThrowException(e);
+ return new Result(changeDataFactory);
}
}
@@ -484,12 +516,14 @@
}
}
- public void execute(BatchUpdateListener listener) throws UpdateException, RestApiException {
- execute(ImmutableList.of(this), ImmutableList.of(listener), false);
+ @CanIgnoreReturnValue
+ public Result execute(BatchUpdateListener listener) throws UpdateException, RestApiException {
+ return execute(changeDataFactory, ImmutableList.of(this), ImmutableList.of(listener), false);
}
- public void execute() throws UpdateException, RestApiException {
- execute(ImmutableList.of(this), ImmutableList.of(), false);
+ @CanIgnoreReturnValue
+ public Result execute() throws UpdateException, RestApiException {
+ return execute(changeDataFactory, ImmutableList.of(this), ImmutableList.of(), false);
}
public boolean isExecuted() {
@@ -584,7 +618,7 @@
*/
public Map<BranchNameKey, ReceiveCommand> getSuccessfullyUpdatedBranches(boolean dryrun) {
return getRefUpdates().entrySet().stream()
- .filter(entry -> dryrun || entry.getValue().getResult() == Result.OK)
+ .filter(entry -> dryrun || entry.getValue().getResult() == ReceiveCommand.Result.OK)
.collect(
toMap(entry -> BranchNameKey.create(project, entry.getKey()), Map.Entry::getValue));
}
diff --git a/java/com/google/gerrit/server/update/SubmissionExecutor.java b/java/com/google/gerrit/server/update/SubmissionExecutor.java
index 39eda58..bab6ddd 100644
--- a/java/com/google/gerrit/server/update/SubmissionExecutor.java
+++ b/java/com/google/gerrit/server/update/SubmissionExecutor.java
@@ -16,18 +16,23 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.submit.MergeOpRepoManager;
import java.util.Collection;
import java.util.Optional;
import java.util.stream.Collectors;
public class SubmissionExecutor {
-
+ private final ChangeData.Factory changeDataFactory;
private final ImmutableList<SubmissionListener> submissionListeners;
private final boolean dryrun;
private ImmutableList<BatchUpdateListener> additionalListeners = ImmutableList.of();
- public SubmissionExecutor(boolean dryrun, ImmutableList<SubmissionListener> submissionListeners) {
+ public SubmissionExecutor(
+ ChangeData.Factory changeDataFactory,
+ boolean dryrun,
+ ImmutableList<SubmissionListener> submissionListeners) {
+ this.changeDataFactory = changeDataFactory;
this.dryrun = dryrun;
this.submissionListeners = submissionListeners;
if (dryrun) {
@@ -58,7 +63,7 @@
.map(Optional::get)
.collect(Collectors.toList()))
.build();
- BatchUpdate.execute(updates, listeners, dryrun);
+ BatchUpdate.execute(changeDataFactory, updates, listeners, dryrun);
}
/**
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 01c5710..65954e4 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -144,7 +144,9 @@
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException;
import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdFactory;
import com.google.gerrit.server.account.externalids.ExternalIdKeyFactory;
+import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdFactoryNoteDbImpl;
import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdNotes;
import com.google.gerrit.server.account.externalids.storage.notedb.ExternalIdsNoteDbImpl;
@@ -244,7 +246,7 @@
@Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
@Inject private AccountIndexer accountIndexer;
@Inject private ExternalIdNotes.Factory extIdNotesFactory;
- @Inject private ExternalIdsNoteDbImpl externalIds;
+ @Inject private ExternalIdsNoteDbImpl externalIdsNoteDbImpl;
@Inject private GitReferenceUpdated gitReferenceUpdated;
@Inject private Provider<InternalAccountQuery> accountQueryProvider;
@Inject private Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
@@ -255,7 +257,7 @@
@Inject private VersionedAuthorizedKeys.Accessor authorizedKeys;
@Inject private PluginSetContext<ExceptionHook> exceptionHooks;
@Inject private ExternalIdKeyFactory externalIdKeyFactory;
- @Inject private ExternalIdFactoryNoteDbImpl externalIdFactory;
+ @Inject private ExternalIdFactoryNoteDbImpl externalIdFactoryNoteDbImpl;
@Inject private AuthConfig authConfig;
@Inject private AccountControl.Factory accountControlFactory;
@Inject private AccountOperations accountOperations;
@@ -406,10 +408,10 @@
Account.Id accountId = Account.id(accountInfo._accountId);
accountIndexedCounter.assertReindexOf(accountId, 1);
- assertThat(externalIds.byAccount(accountId))
+ assertThat(getExternalIdsReader().byAccount(accountId))
.containsExactly(
- externalIdFactory.createUsername(input.username, accountId, null),
- externalIdFactory.createEmail(accountId, input.email));
+ getExternalIdFactory().createUsername(input.username, accountId, null),
+ getExternalIdFactory().createEmail(accountId, input.email));
}
}
@@ -462,7 +464,7 @@
public void createAtomically() throws Exception {
Account.Id accountId = Account.id(seq.nextAccountId());
String fullName = "Foo";
- ExternalId extId = externalIdFactory.createEmail(accountId, "foo@example.com");
+ ExternalId extId = getExternalIdFactory().createEmail(accountId, "foo@example.com");
AccountState accountState =
accountsUpdateProvider
.get()
@@ -630,7 +632,9 @@
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
Account.Id activatableAccountId =
accountOperations.newAccount().inactive().preferredEmail("foo@activatable.com").create();
- accountIndexedCounter.assertReindexOf(activatableAccountId, 1);
+ // Implementation of the accountOperations can create an account in several steps,
+ // with more than one reindexing.
+ accountIndexedCounter.assertReindexAtLeastOnceOf(activatableAccountId);
}
@SuppressWarnings("unused")
@@ -644,7 +648,9 @@
extensionRegistry.newRegistration().add(accountIndexedCounter)) {
Account.Id activatableAccountId =
accountOperations.newAccount().inactive().username("foo").create();
- accountIndexedCounter.assertReindexOf(activatableAccountId, 1);
+ // Implementation of the accountOperations can create an account in several steps,
+ // with more than one reindexing.
+ accountIndexedCounter.assertReindexAtLeastOnceOf(activatableAccountId);
}
@SuppressWarnings("unused")
@@ -1274,11 +1280,13 @@
admin.id(),
u ->
u.addExternalId(
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse(extId1), admin.id(), email))
+ getExternalIdFactory()
+ .createWithEmail(
+ externalIdKeyFactory.parse(extId1), admin.id(), email))
.addExternalId(
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse(extId2), admin.id(), email)));
+ getExternalIdFactory()
+ .createWithEmail(
+ externalIdKeyFactory.parse(extId2), admin.id(), email)));
accountIndexedCounter.assertReindexOf(admin);
assertThat(
gApi.accounts().self().getExternalIds().stream()
@@ -1314,8 +1322,9 @@
admin.id(),
u ->
u.addExternalId(
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse(ldapExternalId), admin.id(), ldapEmail)));
+ getExternalIdFactory()
+ .createWithEmail(
+ externalIdKeyFactory.parse(ldapExternalId), admin.id(), ldapEmail)));
assertThat(
gApi.accounts().self().getExternalIds().stream()
.map(e -> e.identity)
@@ -1353,13 +1362,17 @@
admin.id(),
u ->
u.addExternalId(
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse(nonLdapExternalId),
- admin.id(),
- nonLdapEMail))
+ getExternalIdFactory()
+ .createWithEmail(
+ externalIdKeyFactory.parse(nonLdapExternalId),
+ admin.id(),
+ nonLdapEMail))
.addExternalId(
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse(ldapExternalId), admin.id(), ldapEmail)));
+ getExternalIdFactory()
+ .createWithEmail(
+ externalIdKeyFactory.parse(ldapExternalId),
+ admin.id(),
+ ldapEmail)));
assertThat(
gApi.accounts().self().getExternalIds().stream().map(e -> e.identity).collect(toSet()))
.containsAtLeast(ldapExternalId, nonLdapExternalId);
@@ -1422,8 +1435,9 @@
admin.id(),
u ->
u.addExternalId(
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse("foo:bar"), admin.id(), email)));
+ getExternalIdFactory()
+ .createWithEmail(
+ externalIdKeyFactory.parse("foo:bar"), admin.id(), email)));
assertEmail(emails.getAccountFor(email), admin);
// wrong case doesn't match
@@ -1739,7 +1753,7 @@
.update(
"Add External ID",
user.id(),
- u -> u.addExternalId(externalIdFactory.create("foo", "myId", user.id())));
+ u -> u.addExternalId(getExternalIdFactory().create("foo", "myId", user.id())));
accountIndexedCounter.assertReindexOf(user);
TestKey key = validKeyWithSecondUserId();
@@ -2042,7 +2056,7 @@
.update(
"Delete External ID",
account.id(),
- u -> u.deleteExternalId(externalIdFactory.createEmail(account.id(), email)));
+ u -> u.deleteExternalId(getExternalIdFactory().createEmail(account.id(), email)));
expectedProblems.add(
new ConsistencyProblemInfo(
ConsistencyProblemInfo.Status.ERROR,
@@ -2199,7 +2213,7 @@
String fullName = "Foo";
AtomicBoolean doneBgUpdate = new AtomicBoolean(false);
AccountsUpdate update =
- getAccountsUpdate(
+ getAccountsUpdateWithRunnables(
() -> {
if (!doneBgUpdate.getAndSet(true)) {
try {
@@ -2237,7 +2251,7 @@
String fullName = "Foo";
AtomicInteger bgCounter = new AtomicInteger(0);
AccountsUpdate update =
- getAccountsUpdate(
+ getAccountsUpdateWithRunnables(
() -> {
try {
accountsUpdateProvider
@@ -2282,15 +2296,19 @@
}
@Test
- public void atomicReadMofifyWrite() throws Exception {
+ public void atomicReadModifyWrite() throws Exception {
gApi.accounts().id(admin.id().get()).setStatus("A-1");
- AtomicInteger bgCounterA1 = new AtomicInteger(0);
- AtomicInteger bgCounterA2 = new AtomicInteger(0);
+ AtomicBoolean bgIndicatorA1ToA2 = new AtomicBoolean(false);
AccountsUpdate update =
- getAccountsUpdate(
+ getAccountsUpdateWithRunnables(
Runnables.doNothing(),
() -> {
+ if (bgIndicatorA1ToA2.get()) {
+ // In the Google architecture, this runnable might be called multiple times. Only
+ // do the replacement once.
+ return;
+ }
try {
accountsUpdateProvider
.get()
@@ -2298,29 +2316,31 @@
} catch (IOException | ConfigInvalidException | StorageException e) {
// Ignore, the expected exception is asserted later
}
+ bgIndicatorA1ToA2.set(true);
});
- assertThat(bgCounterA1.get()).isEqualTo(0);
- assertThat(bgCounterA2.get()).isEqualTo(0);
+
assertThat(gApi.accounts().id(admin.id().get()).get().status).isEqualTo("A-1");
+ AtomicBoolean bgIndicatorA1ToB1 = new AtomicBoolean(false);
+ AtomicBoolean bgIndicatorA2ToB2 = new AtomicBoolean(false);
Optional<AccountState> updatedAccountState =
update.update(
"Set Status",
admin.id(),
(a, u) -> {
if ("A-1".equals(a.account().status())) {
- bgCounterA1.getAndIncrement();
+ bgIndicatorA1ToB1.set(true);
u.setStatus("B-1");
}
if ("A-2".equals(a.account().status())) {
- bgCounterA2.getAndIncrement();
+ bgIndicatorA2ToB2.set(true);
u.setStatus("B-2");
}
});
- assertThat(bgCounterA1.get()).isEqualTo(1);
- assertThat(bgCounterA2.get()).isEqualTo(1);
+ assertThat(bgIndicatorA1ToB1.get()).isTrue();
+ assertThat(bgIndicatorA2ToB2.get()).isTrue();
assertThat(updatedAccountState).isPresent();
assertThat(updatedAccountState.get().account().status()).isEqualTo("B-2");
@@ -2329,64 +2349,70 @@
}
@Test
- public void atomicReadMofifyWriteExternalIds() throws Exception {
+ public void atomicReadModifyWriteExternalIds() throws Exception {
projectOperations
.allProjectsForUpdate()
.add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
.update();
Account.Id accountId = Account.id(seq.nextAccountId());
- ExternalId extIdA1 = externalIdFactory.create("foo", "A-1", accountId);
+ ExternalId extIdA1 = getExternalIdFactory().create("foo", "A-1", accountId);
accountsUpdateProvider
.get()
.insert("Create Test Account", accountId, u -> u.addExternalId(extIdA1));
- AtomicInteger bgCounterA1 = new AtomicInteger(0);
- AtomicInteger bgCounterA2 = new AtomicInteger(0);
- ExternalId extIdA2 = externalIdFactory.create("foo", "A-2", accountId);
+ ExternalId extIdA2 = getExternalIdFactory().create("foo", "A-2", accountId);
+ AtomicBoolean bgIndicatorA1ToA2 = new AtomicBoolean(false);
AccountsUpdate update =
- getAccountsUpdate(
+ getAccountsUpdateWithRunnables(
Runnables.doNothing(),
() -> {
+ if (bgIndicatorA1ToA2.get()) {
+ // In the Google architecture, this runnable might be called multiple times. Only
+ // do the replacement once.
+ return;
+ }
try {
accountsUpdateProvider
.get()
.update(
- "Update External ID",
+ "Update External ID A1->A2",
accountId,
u -> u.replaceExternalId(extIdA1, extIdA2));
} catch (IOException | ConfigInvalidException | StorageException e) {
// Ignore, the expected exception is asserted later
}
+ bgIndicatorA1ToA2.set(true);
});
- assertThat(bgCounterA1.get()).isEqualTo(0);
- assertThat(bgCounterA2.get()).isEqualTo(0);
+
assertThat(
gApi.accounts().id(accountId.get()).getExternalIds().stream()
.map(i -> i.identity)
.collect(toSet()))
.containsExactly(extIdA1.key().get());
- ExternalId extIdB1 = externalIdFactory.create("foo", "B-1", accountId);
- ExternalId extIdB2 = externalIdFactory.create("foo", "B-2", accountId);
+ ExternalId extIdB1 = getExternalIdFactory().create("foo", "B-1", accountId);
+ ExternalId extIdB2 = getExternalIdFactory().create("foo", "B-2", accountId);
+ AtomicBoolean bgIndicatorA1ToB1 = new AtomicBoolean(false);
+ AtomicBoolean bgIndicatorA2ToB2 = new AtomicBoolean(false);
Optional<AccountState> updatedAccount =
update.update(
- "Update External ID",
+ "Conditionally update External IDs: A1->B1, A2->B2",
accountId,
(a, u) -> {
if (a.externalIds().contains(extIdA1)) {
- bgCounterA1.getAndIncrement();
+ bgIndicatorA1ToB1.set(true);
u.replaceExternalId(extIdA1, extIdB1);
}
if (a.externalIds().contains(extIdA2)) {
- bgCounterA2.getAndIncrement();
+ bgIndicatorA2ToB2.set(true);
u.replaceExternalId(extIdA2, extIdB2);
}
});
- assertThat(bgCounterA1.get()).isEqualTo(1);
- assertThat(bgCounterA2.get()).isEqualTo(1);
+ assertThat(bgIndicatorA1ToB1.get()).isTrue();
+ assertThat(bgIndicatorA2ToB2.get()).isTrue();
assertThat(updatedAccount).isPresent();
assertThat(updatedAccount.get().externalIds()).containsExactly(extIdB2);
@@ -2436,38 +2462,24 @@
try (Repository repo = repoManager.openRepository(allUsers)) {
testRefAction(
() -> {
- ExternalIdNotes extIdNotes =
- ExternalIdNotes.load(
- allUsers,
- repo,
- externalIdFactory,
- authConfig.isUserNameCaseInsensitiveMigrationMode());
+ ExternalIdNotes extIdNotes = getExternalIdNotes(repo);
ExternalId.Key key = externalIdKeyFactory.create("foo", "foo");
- extIdNotes.insert(externalIdFactory.create(key, accountId));
+ extIdNotes.insert(getExternalIdFactory().create(key, accountId));
try (MetaDataUpdate update = metaDataUpdateFactory.create(allUsers)) {
extIdNotes.commit(update);
}
assertStaleAccountAndReindex(accountId);
- extIdNotes =
- ExternalIdNotes.load(
- allUsers,
- repo,
- externalIdFactory,
- authConfig.isUserNameCaseInsensitiveMigrationMode());
- extIdNotes.upsert(externalIdFactory.createWithEmail(key, accountId, "foo@example.com"));
+ extIdNotes = getExternalIdNotes(repo);
+ extIdNotes.upsert(
+ getExternalIdFactory().createWithEmail(key, accountId, "foo@example.com"));
try (MetaDataUpdate update = metaDataUpdateFactory.create(allUsers)) {
extIdNotes.commit(update);
}
assertStaleAccountAndReindex(accountId);
- extIdNotes =
- ExternalIdNotes.load(
- allUsers,
- repo,
- externalIdFactory,
- authConfig.isUserNameCaseInsensitiveMigrationMode());
+ extIdNotes = getExternalIdNotes(repo);
extIdNotes.delete(accountId, key);
try (MetaDataUpdate update = metaDataUpdateFactory.create(allUsers)) {
extIdNotes.commit(update);
@@ -2732,11 +2744,11 @@
String extId1String = "foo:bar";
String extId2String = "foo:baz";
ExternalId extId1 =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse(extId1String), admin.id(), "1@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse(extId1String), admin.id(), "1@foo.com");
ExternalId extId2 =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse(extId2String), user.id(), "2@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse(extId2String), user.id(), "2@foo.com");
ObjectId revBefore;
try (Repository repo = repoManager.openRepository(allUsers)) {
@@ -2776,11 +2788,11 @@
@Test
public void externalIdBatchUpdates_fail_sameAccount() {
ExternalId extId1 =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse("foo:bar"), admin.id(), "1@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse("foo:bar"), admin.id(), "1@foo.com");
ExternalId extId2 =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse("foo:baz"), user.id(), "2@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse("foo:baz"), user.id(), "2@foo.com");
AccountsUpdate.UpdateArguments ua1 =
new AccountsUpdate.UpdateArguments(
@@ -2799,11 +2811,11 @@
@Test
public void externalIdBatchUpdates_fail_duplicateKey() {
ExternalId extIdAdmin =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse("foo:bar"), admin.id(), "1@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse("foo:bar"), admin.id(), "1@foo.com");
ExternalId extIdUser =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse("foo:bar"), user.id(), "2@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse("foo:bar"), user.id(), "2@foo.com");
AccountsUpdate.UpdateArguments ua1 =
new AccountsUpdate.UpdateArguments(
@@ -2821,11 +2833,11 @@
@Test
public void externalIdBatchUpdates_commitMsg_multipleAccounts() throws Exception {
ExternalId extId1 =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse("foo:bar"), admin.id(), "1@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse("foo:bar"), admin.id(), "1@foo.com");
ExternalId extId2 =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse("foo:baz"), user.id(), "2@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse("foo:baz"), user.id(), "2@foo.com");
AccountsUpdate.UpdateArguments ua1 =
new AccountsUpdate.UpdateArguments(
@@ -2847,8 +2859,8 @@
@Test
public void externalIdBatchUpdates_commitMsg_singleAccount() throws Exception {
ExternalId extId =
- externalIdFactory.createWithEmail(
- externalIdKeyFactory.parse("foo:bar"), admin.id(), "1@foo.com");
+ getExternalIdFactory()
+ .createWithEmail(externalIdKeyFactory.parse("foo:bar"), admin.id(), "1@foo.com");
accountsUpdateProvider.get().update("foobar", admin.id(), (a, u) -> u.addExternalId(extId));
@@ -2967,7 +2979,7 @@
AccountState preUpdateState = accountCache.get(admin.id()).get();
requestScopeOperations.setApiUser(admin.id());
- ExternalId externalId = externalIdFactory.create("custom", "value", admin.id());
+ ExternalId externalId = getExternalIdFactory().create("custom", "value", admin.id());
accountsUpdateProvider
.get()
.update("Add External ID", admin.id(), (a, u) -> u.addExternalId(externalId));
@@ -2999,8 +3011,9 @@
requestScopeOperations.setApiUser(admin.id());
ExternalId externalId =
- externalIdFactory.createWithEmail(
- SCHEME_MAILTO, "secondary@non.google", admin.id(), "secondary@non.google");
+ getExternalIdFactory()
+ .createWithEmail(
+ SCHEME_MAILTO, "secondary@non.google", admin.id(), "secondary@non.google");
accountsUpdateProvider
.get()
.update("Update External ID", admin.id(), (a, u) -> u.updateExternalId(externalId));
@@ -3019,17 +3032,27 @@
requestScopeOperations.setApiUser(admin.id());
ExternalId externalId =
- externalIdFactory.createWithEmail(
- SCHEME_MAILTO, "secondary@non.google", admin.id(), "secondary@non.google");
+ getExternalIdFactory()
+ .createWithEmail(
+ SCHEME_MAILTO, "secondary@non.google", admin.id(), "secondary@non.google");
+ AtomicBoolean bgIndicator = new AtomicBoolean(false);
accountsUpdateProvider
.get()
.update(
"Replace External ID",
admin.id(),
- (a, u) ->
- u.replaceExternalId(
- externalIds.get(createEmailExternalId(admin.id(), admin.email()).key()).get(),
- externalId));
+ (a, u) -> {
+ if (bgIndicator.getAndSet(true)) {
+ // In the Google architecture, this runnable might be called multiple times. Only
+ // do the replacement once.
+ return;
+ }
+ u.replaceExternalId(
+ getExternalIdsReader()
+ .get(createEmailExternalId(admin.id(), admin.email()).key())
+ .get(),
+ externalId);
+ });
assertExternalIds(admin.id(), ImmutableSet.of("mailto:secondary@non.google", "username:admin"));
AccountState updatedState = accountCache.get(admin.id()).get();
@@ -3044,10 +3067,11 @@
requestScopeOperations.setApiUser(admin.id());
ExternalId extId1 =
- externalIdFactory.createWithEmail("custom", "admin-id", admin.id(), "admin-id@test.com");
+ getExternalIdFactory()
+ .createWithEmail("custom", "admin-id", admin.id(), "admin-id@test.com");
ExternalId extId2 =
- externalIdFactory.createWithEmail("custom", "user-id", user.id(), "user-id@test.com");
+ getExternalIdFactory().createWithEmail("custom", "user-id", user.id(), "user-id@test.com");
AccountsUpdate.UpdateArguments ua1 =
new AccountsUpdate.UpdateArguments(
@@ -3115,7 +3139,7 @@
}
protected ExternalId createEmailExternalId(Account.Id accountId, String email) {
- return externalIdFactory.createWithEmail(SCHEME_MAILTO, email, accountId, email);
+ return getExternalIdFactory().createWithEmail(SCHEME_MAILTO, email, accountId, email);
}
@Test
@@ -3240,7 +3264,8 @@
gApi.accounts().self().delete();
requestScopeOperations.setApiUser(admin.id());
- assertThat(externalIds.byEmails("deleted@internal.com", "deleted@external.com")).isEmpty();
+ assertThat(getExternalIdsReader().byEmails("deleted@internal.com", "deleted@external.com"))
+ .isEmpty();
// Clean up the test framework
accountCreator.evict(deleted.id());
@@ -3556,7 +3581,7 @@
Iterable<String> expectedFps =
expected.transform(k -> BaseEncoding.base16().encode(k.getPublicKey().getFingerprint()));
Set<String> actualFps =
- externalIds.byAccount(currAccountId, SCHEME_GPGKEY).stream()
+ getExternalIdsReader().byAccount(currAccountId, SCHEME_GPGKEY).stream()
.map(e -> e.key().id())
.collect(toSet());
assertWithMessage("external IDs in database")
@@ -3598,7 +3623,8 @@
account.id(),
u ->
u.addExternalId(
- externalIdFactory.createWithEmail(name("test"), email, account.id(), email)));
+ getExternalIdFactory()
+ .createWithEmail(name("test"), email, account.id(), email)));
accountIndexedCounter.assertReindexOf(account);
requestScopeOperations.setApiUser(account.id());
}
@@ -3677,8 +3703,9 @@
"login?account_id=" + accountId, HttpServletResponse.SC_MOVED_TEMPORARILY);
}
- private AccountsUpdate getAccountsUpdate(Runnable afterReadRevision, Runnable beforeCommit) {
- return getAccountsUpdate(
+ private AccountsUpdate getAccountsUpdateWithRunnables(
+ Runnable afterReadRevision, Runnable beforeCommit) {
+ return getAccountsUpdateWithRunnables(
afterReadRevision,
beforeCommit,
new RetryHelper(
@@ -3692,14 +3719,40 @@
r -> r.withBlockStrategy(noSleepBlockStrategy)));
}
- private AccountsUpdate getAccountsUpdate(
+ private ExternalIdNotes getExternalIdNotes(Repository allUsersRepo)
+ throws ConfigInvalidException, IOException {
+ return ExternalIdNotes.load(
+ allUsers,
+ allUsersRepo,
+ externalIdFactoryNoteDbImpl,
+ authConfig.isUserNameCaseInsensitiveMigrationMode());
+ }
+
+ @UsedAt(UsedAt.Project.GOOGLE)
+ protected ExternalIdFactory getExternalIdFactory() {
+ return externalIdFactoryNoteDbImpl;
+ }
+
+ @UsedAt(UsedAt.Project.GOOGLE)
+ protected ExternalIds getExternalIdsReader() {
+ return externalIdsNoteDbImpl;
+ }
+
+ @UsedAt(UsedAt.Project.GOOGLE)
+ protected AccountsUpdate getAccountsUpdateWithRunnables(
+ Runnable afterReadRevision, Runnable beforeCommit, RetryHelper retryHelper) {
+ return getAccountsUpdateNoteDbImplWithRunnables(afterReadRevision, beforeCommit, retryHelper);
+ }
+
+ @UsedAt(UsedAt.Project.GOOGLE)
+ protected final AccountsUpdateNoteDbImpl getAccountsUpdateNoteDbImplWithRunnables(
Runnable afterReadRevision, Runnable beforeCommit, RetryHelper retryHelper) {
return new AccountsUpdateNoteDbImpl(
repoManager,
gitReferenceUpdated,
Optional.empty(),
allUsers,
- externalIds,
+ externalIdsNoteDbImpl,
extIdNotesFactory,
metaDataUpdateInternalFactory,
retryHelper,
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
index 7449a5c..07ce95b 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
@@ -40,6 +40,7 @@
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.AuthResult;
import com.google.gerrit.server.account.SetInactiveFlag;
+import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdFactory;
import com.google.gerrit.server.account.externalids.ExternalIdKeyFactory;
@@ -593,7 +594,7 @@
AccountException thrown =
assertThrows(AccountException.class, () -> accountManager.authenticate(whoOAuth));
- assertThat(thrown).hasMessageThat().contains("Cannot assign external ID \"username:foo\" to");
+ assertThat(thrown).hasCauseThat().isInstanceOf(DuplicateExternalIdKeyException.class);
}
@Test
@@ -647,9 +648,7 @@
AccountException thrown =
assertThrows(AccountException.class, () -> accountManager.authenticate(whoOAuth));
- assertThat(thrown)
- .hasMessageThat()
- .contains("Cannot assign external ID \"username:foo\" to account");
+ assertThat(thrown).hasCauseThat().isInstanceOf(DuplicateExternalIdKeyException.class);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 12d3ced..8d30d45 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1152,11 +1152,13 @@
.modifyFile(FILE_NAME, RawInputUtil.create("foo".getBytes(UTF_8)));
requestScopeOperations.setApiUser(admin.id());
+ String expected = RefNames.refsUsers(user.id()) + "/edit-" + result.getChange().getId() + "/1";
try (Repository repo = repoManager.openRepository(project)) {
- String expected =
- RefNames.refsUsers(user.id()) + "/edit-" + result.getChange().getId() + "/1";
assertThat(repo.getRefDatabase().getRefsByPrefix(expected)).isNotEmpty();
gApi.changes().id(changeId).delete();
+ }
+ // On google infra, repo should be reopened for getting updated refs.
+ try (Repository repo = repoManager.openRepository(project)) {
assertThat(repo.getRefDatabase().getRefsByPrefix(expected)).isEmpty();
}
}
@@ -1457,7 +1459,9 @@
assertThat(r.reviewers).hasSize(1);
ReviewerInfo reviewer = r.reviewers.get(0);
assertThat(reviewer._accountId).isEqualTo(id.get());
- assertThat(reviewer.username).isEqualTo(username);
+ if (server.isUsernameSupported()) {
+ assertThat(reviewer.username).isEqualTo(username);
+ }
}
@Test
@@ -1476,7 +1480,9 @@
assertThat(r.reviewers).hasSize(1);
ReviewerInfo reviewer = r.reviewers.get(0);
assertThat(reviewer._accountId).isEqualTo(id.get());
- assertThat(reviewer.username).isEqualTo(username);
+ if (server.isUsernameSupported()) {
+ assertThat(reviewer.username).isEqualTo(username);
+ }
}
@Test
@@ -1485,20 +1491,20 @@
conf.enableReviewerByEmail = InheritableBoolean.TRUE;
gApi.projects().name(project.get()).config(conf);
PushOneCommit.Result result = createChange();
- String username = "user@domain.com";
- Account.Id id = accountOperations.newAccount().username(username).inactive().create();
+ String email = "user@domain.com";
+ Account.Id id = accountOperations.newAccount().preferredEmail(email).inactive().create();
ReviewerInput in = new ReviewerInput();
- in.reviewer = username;
+ in.reviewer = email;
in.state = ReviewerState.CC;
ReviewerResult r = gApi.changes().id(result.getChangeId()).addReviewer(in);
- assertThat(r.input).isEqualTo(username);
+ assertThat(r.input).isEqualTo(email);
assertThat(r.error).isNull();
assertThat(r.ccs).hasSize(1);
AccountInfo reviewer = r.ccs.get(0);
assertThat(reviewer._accountId).isEqualTo(id.get());
- assertThat(reviewer.username).isEqualTo(username);
+ assertThat(reviewer.email).isEqualTo(email);
}
@Test
@@ -1587,17 +1593,22 @@
String username1 = name("user1");
String email1 = username1 + "@example.com";
- accountOperations
- .newAccount()
- .username(username1)
- .preferredEmail(email1)
- .fullname("User1")
- .create();
+ Account.Id user1Id =
+ accountOperations
+ .newAccount()
+ .username(username1)
+ .preferredEmail(email1)
+ .fullname("User1")
+ .create();
in.reviewer = email1;
in.state = ReviewerState.CC;
gApi.changes().id(r.getChangeId()).addReviewer(in);
- assertThat(gApi.changes().id(r.getChangeId()).reviewers().stream().map(a -> a.username))
- .containsExactly(user.username(), username1);
+ if (server.isUsernameSupported()) {
+ assertThat(gApi.changes().id(r.getChangeId()).reviewers().stream().map(a -> a.username))
+ .containsExactly(user.username(), username1);
+ }
+ assertThat(gApi.changes().id(r.getChangeId()).reviewers().stream().map(a -> a._accountId))
+ .containsExactly(user.id().get(), user1Id.get());
}
@Test
@@ -1821,7 +1832,9 @@
@Test
public void implicitlyCcOnNonVotingReviewForUserWithoutUserNamePgStyle() throws Exception {
com.google.gerrit.acceptance.TestAccount accountWithoutUsername = accountCreator.create();
- assertThat(accountWithoutUsername.username()).isNull();
+ if (server.isUsernameSupported()) {
+ assertThat(accountWithoutUsername.username()).isNull();
+ }
testImplicitlyCcOnNonVotingReviewPgStyle(accountWithoutUsername);
}
@@ -3470,7 +3483,9 @@
// permittedVotingRange is not served if DETAILED_LABELS is not requested.
assertThat(codeReviewApproval.permittedVotingRange).isNull();
assertThat(codeReviewApproval.value).isEqualTo(1);
- assertThat(codeReviewApproval.username).isEqualTo(admin.username());
+ if (server.isUsernameSupported()) {
+ assertThat(codeReviewApproval.username).isEqualTo(admin.username());
+ }
// Add another +1 vote as user
requestScopeOperations.setApiUser(user.id());
@@ -3486,8 +3501,12 @@
.containsExactly(null, null);
assertThat(codeReviewApprovals.stream().map(a -> a.value).collect(toList()))
.containsExactly(1, 1);
- assertThat(codeReviewApprovals.stream().map(a -> a.username).collect(toList()))
- .containsExactly(admin.username(), user.username());
+ if (server.isUsernameSupported()) {
+ assertThat(codeReviewApprovals.stream().map(a -> a.username).collect(toList()))
+ .containsExactly(admin.username(), user.username());
+ }
+ assertThat(codeReviewApprovals.stream().map(a -> a._accountId).collect(toList()))
+ .containsExactly(admin.id().get(), user.id().get());
}
@Test
@@ -3602,12 +3621,16 @@
assertThat(codeReviewApprovals).hasSize(1);
assertThat(codeReviewApprovals.get(0).value).isEqualTo(2);
- assertThat(codeReviewApprovals.get(0).username).isEqualTo(admin.username());
+ if (server.isUsernameSupported()) {
+ assertThat(codeReviewApprovals.get(0).username).isEqualTo(admin.username());
+ }
assertThat(codeReviewApprovals.get(0).permittedVotingRange).isNull();
assertThat(verifiedApprovals).hasSize(1);
assertThat(verifiedApprovals.get(0).value).isEqualTo(1);
- assertThat(verifiedApprovals.get(0).username).isEqualTo(admin.username());
+ if (server.isUsernameSupported()) {
+ assertThat(verifiedApprovals.get(0).username).isEqualTo(admin.username());
+ }
assertThat(codeReviewApprovals.get(0).permittedVotingRange).isNull();
}
@@ -4154,6 +4177,8 @@
PushOneCommit push = pushFactory.create(user.newIdent(), userTestRepo);
PushOneCommit.Result r = push.to("refs/for/master");
r.assertOkStatus();
+ assertThat(gApi.changes().id(r.getChangeId()).info().owner._accountId)
+ .isEqualTo(user.id().get());
// Try to change the commit message
AuthException thrown =
assertThrows(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
index 3771bb9..314dc44 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
@@ -93,6 +93,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -771,6 +772,20 @@
}
@Test
+ public void submitRulesAreInvokedOnlyOnce_allOptionsSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ TestSubmitRule testSubmitRule = new TestSubmitRule();
+ try (Registration registration = extensionRegistry.newRegistration().add(testSubmitRule)) {
+ ReviewInput input = new ReviewInput().label(LabelId.CODE_REVIEW, 1);
+ input.responseFormatOptions = ImmutableList.copyOf(EnumSet.allOf(ListChangesOption.class));
+ gApi.changes().id(r.getChangeId()).current().review(input);
+ }
+
+ assertThat(testSubmitRule.count).isEqualTo(1);
+ }
+
+ @Test
public void addingReviewers() throws Exception {
PushOneCommit.Result r = createChange();
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
index cd687f3..b69ca1c 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
@@ -23,6 +23,7 @@
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -155,6 +156,10 @@
// With conflicting options
assertBadRequest(getTags().withSubstring("ag-B").withRegex("^tag-[c|d]$"));
+
+ // with descending order
+ result = getTags().withDescendingOrder(true).get();
+ assertTagList(FluentIterable.from(Lists.reverse(testTags)), result);
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ApprovalCopierIT.java b/javatests/com/google/gerrit/acceptance/server/change/ApprovalCopierIT.java
index 35082ec..6bfd988 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ApprovalCopierIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ApprovalCopierIT.java
@@ -33,7 +33,7 @@
import com.google.common.truth.StandardSubjectBuilder;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
-import com.google.common.truth.Truth8;
+import com.google.common.truth.Truth;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -489,7 +489,7 @@
approvalData.patchSetApproval().label().equals(labelId)
&& approvalData.patchSetApproval().accountId().equals(accountId))
.findAny();
- Truth8.assertThat(approvalDataForLabelAndAccount).isPresent();
+ Truth.assertThat(approvalDataForLabelAndAccount).isPresent();
return assertAbout(approvalDatas()).that(approvalDataForLabelAndAccount.get());
}
diff --git a/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java b/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
index d0610b3..24767cb 100644
--- a/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
@@ -22,7 +22,6 @@
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
-import com.google.common.truth.Truth8;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.config.FactoryModule;
@@ -124,7 +123,7 @@
OptionalLong tokens =
quotaBackend.user(identifiedAdmin).availableTokens("testGroup").availableTokens();
- Truth8.assertThat(tokens).isPresent();
+ assertThat(tokens).isPresent();
assertThat(tokens.getAsLong()).isEqualTo(10L);
verify(quotaEnforcerA).availableTokens("testGroup", ctx);
@@ -139,7 +138,7 @@
OptionalLong tokens =
quotaBackend.user(identifiedAdmin).availableTokens("testGroup").availableTokens();
- Truth8.assertThat(tokens).isPresent();
+ assertThat(tokens).isPresent();
assertThat(tokens.getAsLong()).isEqualTo(20L);
verify(quotaEnforcerA).availableTokens("testGroup", ctx);
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index 44fc4bb..6204115 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -22,6 +22,7 @@
import static com.google.gerrit.extensions.restapi.testing.BinaryResultSubject.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.MapSubject.assertThatMap;
+import static com.google.gerrit.truth.OptionalSubject.assertThat;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 1adfc98..fd8ddd2 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -67,6 +67,7 @@
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/server/schema/testing",
+ "//java/com/google/gerrit/server/util/git",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/sshd",
"//java/com/google/gerrit/testing:assertable-executor",
diff --git a/javatests/com/google/gerrit/server/config/GerritInstanceNameProviderTest.java b/javatests/com/google/gerrit/server/config/GerritInstanceNameProviderTest.java
new file mode 100644
index 0000000..edf9b8b
--- /dev/null
+++ b/javatests/com/google/gerrit/server/config/GerritInstanceNameProviderTest.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2024 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.config;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.server.util.git.DelegateSystemReader;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.util.SystemReader;
+import org.junit.Test;
+
+public class GerritInstanceNameProviderTest {
+
+ @Test
+ public void instanceNameSet_canonicalWebUrlUnset_useInstanceName() {
+ Config cfg = new Config();
+ cfg.setString("gerrit", null, "instanceName", "myName");
+
+ GerritInstanceNameProvider provider = new GerritInstanceNameProvider(cfg, null);
+ assertThat(provider.get()).isEqualTo("myName");
+ }
+
+ @Test
+ public void instanceNameSet_canonicalWebUrlSet_useInstanceName() {
+ Config cfg = new Config();
+ cfg.setString("gerrit", null, "instanceName", "myName");
+
+ GerritInstanceNameProvider provider = new GerritInstanceNameProvider(cfg, "http://myhost");
+ assertThat(provider.get()).isEqualTo("myName");
+ }
+
+ @Test
+ public void instanceNameNotSet_canonicalWebUrlSet_useHostFromCanonicalWebUrl() {
+ Config cfg = new Config();
+ GerritInstanceNameProvider provider = new GerritInstanceNameProvider(cfg, "http://myhost");
+ assertThat(provider.get()).isEqualTo("myhost");
+ }
+
+ @Test
+ public void instanceNameNotSet_canonicalWebUrlNotSet_useSystemHostName() {
+ SystemReader oldSystemReader = SystemReader.getInstance();
+ SystemReader.setInstance(
+ new DelegateSystemReader(oldSystemReader) {
+ @Override
+ public String getHostname() {
+ return "systemHostName";
+ }
+ });
+ Config cfg = new Config();
+ GerritInstanceNameProvider provider = new GerritInstanceNameProvider(cfg, null);
+ assertThat(provider.get()).isEqualTo("systemHostName");
+ }
+}
diff --git a/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java b/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java
index e40ccfc..8d93f66 100644
--- a/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java
+++ b/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java
@@ -17,7 +17,6 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableList;
-import com.google.common.truth.Truth8;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.client.SubmitType;
import java.nio.file.Path;
@@ -148,7 +147,7 @@
@Test
public void basePathWhenNotConfigured() {
- Truth8.assertThat(repoCfg.getBasePath(Project.nameKey("someProject"))).isNull();
+ assertThat(repoCfg.getBasePath(Project.nameKey("someProject"))).isNull();
}
@Test
@@ -162,7 +161,7 @@
public void basePathForSpecificFilter() {
String basePath = "/someAbsolutePath/someDirectory";
configureBasePath("someProject", basePath);
- Truth8.assertThat(repoCfg.getBasePath(Project.nameKey("someOtherProject"))).isNull();
+ assertThat(repoCfg.getBasePath(Project.nameKey("someOtherProject"))).isNull();
assertThat(repoCfg.getBasePath(Project.nameKey("someProject")).toString()).isEqualTo(basePath);
}
diff --git a/javatests/com/google/gerrit/server/config/SitePathsTest.java b/javatests/com/google/gerrit/server/config/SitePathsTest.java
index 6ecf549..1de6c30 100644
--- a/javatests/com/google/gerrit/server/config/SitePathsTest.java
+++ b/javatests/com/google/gerrit/server/config/SitePathsTest.java
@@ -17,7 +17,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-import com.google.common.truth.Truth8;
import com.google.gerrit.server.ioutil.HostPlatform;
import java.io.IOException;
import java.nio.file.Files;
@@ -31,8 +30,8 @@
final Path root = random();
final SitePaths site = new SitePaths(root);
assertThat(site.isNew).isTrue();
- Truth8.assertThat(site.site_path).isEqualTo(root);
- Truth8.assertThat(site.etc_dir).isEqualTo(root.resolve("etc"));
+ assertThat(site.site_path).isEqualTo(root);
+ assertThat(site.etc_dir).isEqualTo(root.resolve("etc"));
}
@Test
@@ -43,7 +42,7 @@
final SitePaths site = new SitePaths(root);
assertThat(site.isNew).isTrue();
- Truth8.assertThat(site.site_path).isEqualTo(root);
+ assertThat(site.site_path).isEqualTo(root);
} finally {
Files.delete(root);
}
@@ -59,7 +58,7 @@
final SitePaths site = new SitePaths(root);
assertThat(site.isNew).isFalse();
- Truth8.assertThat(site.site_path).isEqualTo(root);
+ assertThat(site.site_path).isEqualTo(root);
} finally {
Files.delete(txt);
Files.delete(root);
@@ -83,15 +82,15 @@
final Path root = random();
final SitePaths site = new SitePaths(root);
- Truth8.assertThat(site.resolve(null)).isNull();
- Truth8.assertThat(site.resolve("")).isNull();
+ assertThat(site.resolve(null)).isNull();
+ assertThat(site.resolve("")).isNull();
- Truth8.assertThat(site.resolve("a")).isNotNull();
- Truth8.assertThat(site.resolve("a")).isEqualTo(root.resolve("a").toAbsolutePath().normalize());
+ assertThat(site.resolve("a")).isNotNull();
+ assertThat(site.resolve("a")).isEqualTo(root.resolve("a").toAbsolutePath().normalize());
final String pfx = HostPlatform.isWin32() ? "C:/" : "/";
- Truth8.assertThat(site.resolve(pfx + "a")).isNotNull();
- Truth8.assertThat(site.resolve(pfx + "a")).isEqualTo(Path.of(pfx + "a"));
+ assertThat(site.resolve(pfx + "a")).isNotNull();
+ assertThat(site.resolve(pfx + "a")).isEqualTo(Path.of(pfx + "a"));
}
private static Path random() throws IOException {
diff --git a/javatests/com/google/gerrit/server/notedb/IntBlobTest.java b/javatests/com/google/gerrit/server/notedb/IntBlobTest.java
index f335201..7ed2391 100644
--- a/javatests/com/google/gerrit/server/notedb/IntBlobTest.java
+++ b/javatests/com/google/gerrit/server/notedb/IntBlobTest.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static com.google.gerrit.truth.OptionalSubject.assertThat;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 6919982..f406511 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -153,6 +153,7 @@
import {modalStyles} from '../../../styles/gr-modal-styles';
import {relatedChangesModelToken} from '../../../models/change/related-changes-model';
import {KnownExperimentId} from '../../../services/flags/flags';
+import {assign} from '../../../utils/location-util';
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
@@ -1810,7 +1811,10 @@
if (this.loggedIn) {
this.openReplyDialog(FocusTarget.ANY);
} else {
- this.getNavigation().setUrl(this.loginUrl);
+ // We are not using `this.getNavigation().setUrl()`, because the login
+ // page is served directly from the backend and is not part of the web
+ // app.
+ assign(window.location, this.loginUrl);
}
}
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index 94241ea..3d21e07 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -10,7 +10,12 @@
export interface NavigationService {
/**
* This is similar to letting the browser navigate to this URL when the user
- * clicks it, or to just setting `window.location.href` directly.
+ * clicks it, or to just calling `window.location.assign()` directly.
+ *
+ * CAUTION: You should actually use `window.location.assign()` directly for
+ * URLs that are not handled by gr-router. Otherwise we will call
+ * `pushState()` and then `window.location.reload()` from the router, which
+ * will break the browser's back button.
*
* This adds a new entry to the browser location history. Consier using
* `replaceUrl()`, if you want to avoid that.
@@ -23,6 +28,11 @@
* Navigate to this URL, but replace the current URL in the history instead of
* adding a new one (which is what `setUrl()` would do).
*
+ * CAUTION: You should actually use `window.location.replace()` directly for
+ * URLs that are not handled by gr-router. Otherwise we will call
+ * `replaceState()` and then `window.location.reload()` from the router, which
+ * will break the browser's back button.
+ *
* page.redirect() eventually just calls `window.history.replaceState()`.
*/
replaceUrl(url: string): void;
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-page.ts b/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
index 5e7cc3b..cb96d77 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
@@ -53,6 +53,11 @@
path?: string;
}
+export const UNHANDLED_URL_PATTERNS = [
+ /^\/log(in|out)(\/(.+))?$/,
+ /^\/plugins\/(.+)$/,
+];
+
const clickEvent = document.ontouchstart ? 'touchstart' : 'click';
export class Page {
@@ -239,6 +244,18 @@
if (this.base && orig === path && window.location.protocol !== 'file:') {
return;
}
+
+ // See issue 40015337: We have to make sure that we only use
+ // show()/pushState() for URLs that gr-router will actually handle.
+ // Calling pushState() tells the browser that both the previous and the
+ // next URL are handled by the same single page application with a
+ // popstate event handler. But if we call pushState() and then
+ // later `window.location.reload()` from the router and a separate page
+ // and document are loaded, then the BACK button will stop working.
+ if (UNHANDLED_URL_PATTERNS.find(pattern => pattern.test(path))) {
+ return;
+ }
+
e.preventDefault();
this.show(orig);
};
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-page_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-page_test.ts
index d194bf55..729a15b 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-page_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-page_test.ts
@@ -20,14 +20,49 @@
page.stop();
});
- test('click handler', async () => {
- const spy = sinon.spy();
- page.registerRoute(/\/settings/, spy);
- const link = await fixture<HTMLAnchorElement>(
- html`<a href="/settings"></a>`
- );
- link.click();
- assert.isTrue(spy.calledOnce);
+ suite('click handler', () => {
+ const clickListener = (e: Event) => e.preventDefault();
+ let spy: sinon.SinonSpy;
+ let link: HTMLAnchorElement;
+
+ setup(async () => {
+ spy = sinon.spy();
+ link = await fixture<HTMLAnchorElement>(html`<a href="/settings"></a>`);
+
+ document.addEventListener('click', clickListener);
+ });
+
+ teardown(() => {
+ document.removeEventListener('click', clickListener);
+ });
+
+ test('click handled by specific route', async () => {
+ page.registerRoute(/\/settings/, spy);
+ link.href = '/settings';
+ link.click();
+ assert.isTrue(spy.calledOnce);
+ });
+
+ test('click handled by default route', async () => {
+ page.registerRoute(/.*/, spy);
+ link.href = '/something';
+ link.click();
+ assert.isTrue(spy.called);
+ });
+
+ test('click not handled for /plugins/... links', async () => {
+ page.registerRoute(/.*/, spy);
+ link.href = '/plugins/gitiles';
+ link.click();
+ assert.isFalse(spy.called);
+ });
+
+ test('click not handled for /login/... links', async () => {
+ page.registerRoute(/.*/, spy);
+ link.href = '/login';
+ link.click();
+ assert.isFalse(spy.called);
+ });
});
test('register route and exit', () => {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index a3cb0ee..ad3e15c 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -106,6 +106,7 @@
timeoutPromise,
} from '../../../utils/async-util';
import {Finalizable} from '../../../types/types';
+import {assign} from '../../../utils/location-util';
// TODO: Move all patterns to view model files and use the `Route` interface,
// which will enforce using `RegExp` in its `urlPattern` property.
@@ -120,12 +121,6 @@
NEW_AGREEMENTS: /^\/settings\/new-agreement\/?/,
REGISTER: /^\/register(\/.*)?$/,
- // Pattern for login and logout URLs intended to be passed-through. May
- // include a return URL.
- // TODO: Maybe this pattern and its handler can just be removed, because
- // passing through is what the default router would eventually do anyway.
- LOG_IN_OR_OUT: /^\/log(in|out)(\/(.+))?$/,
-
// Pattern for a catchall route when no other pattern is matched.
DEFAULT: /.*/,
@@ -171,8 +166,6 @@
// Matches /admin/repos/<repos>,access.
REPO_DASHBOARDS: /^\/admin\/repos\/(.+),dashboards$/,
- PLUGINS: /^\/plugins\/(.+)$/,
-
// Matches /admin/plugins with optional filter and offset.
PLUGIN_LIST: /^\/admin\/plugins\/?(?:\/q\/filter:(.*?))?(?:,(\d+))?$/,
// Matches /admin/groups with optional filter and offset.
@@ -460,7 +453,11 @@
*/
redirectToLogin(returnUrl: string) {
const basePath = getBaseUrl() || '';
- this.setUrl(
+ // We are not using `this.getNavigation().setUrl()`, because the login
+ // page is served directly from the backend and is not part of the web
+ // app.
+ assign(
+ window.location,
'/login/' + encodeURIComponent(returnUrl.substring(basePath.length))
);
}
@@ -583,6 +580,8 @@
* page.show() eventually just calls `window.history.pushState()`.
*/
setUrl(url: string) {
+ // TODO: Use window.location.assign() instead of page.show(), if the URL is
+ // external, i.e. not handled by the router.
this.page.show(url);
}
@@ -593,6 +592,8 @@
* this.page.redirect() eventually just calls `window.history.replaceState()`.
*/
replaceUrl(url: string) {
+ // TODO: Use window.location.replace() instead of page.redirect(), if the
+ // URL is external, i.e. not handled by the router.
this.redirect(url);
}
@@ -809,10 +810,6 @@
this.handleRepoRoute(ctx)
);
- this.mapRoute(RoutePattern.PLUGINS, 'handlePassThroughRoute', () =>
- this.handlePassThroughRoute()
- );
-
this.mapRoute(
RoutePattern.PLUGIN_LIST,
'handlePluginListFilterRoute',
@@ -921,10 +918,6 @@
this.handleRegisterRoute(ctx)
);
- this.mapRoute(RoutePattern.LOG_IN_OR_OUT, 'handlePassThroughRoute', () =>
- this.handlePassThroughRoute()
- );
-
this.mapRoute(
RoutePattern.IMPROPERLY_ENCODED_PLUS,
'handleImproperlyEncodedPlusRoute',
@@ -1589,14 +1582,6 @@
}
/**
- * Handler for routes that should pass through the router and not be caught
- * by the catchall _handleDefaultRoute handler.
- */
- handlePassThroughRoute() {
- windowLocationReload();
- }
-
- /**
* URL may sometimes have /+/ encoded to / /.
* Context: Issue 6888, Issue 7100
*/
@@ -1651,10 +1636,15 @@
this.show404();
} else {
// Route can be recognized by server, so we pass it to server.
- this.handlePassThroughRoute();
+ this.windowReload();
}
}
+ // Allows stubbing in tests.
+ windowReload() {
+ windowLocationReload();
+ }
+
private show404() {
// Note: the app's 404 display is tightly-coupled with catching 404
// network responses, so we simulate a 404 response status to display it.
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index fbc0338..6f4e528 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -181,7 +181,6 @@
'handleDocumentationSearchRedirectRoute',
'handleLegacyLinenum',
'handleImproperlyEncodedPlusRoute',
- 'handlePassThroughRoute',
'handleProjectDashboardRoute',
'handleLegacyProjectDashboardRoute',
'handleProjectsOldRoute',
@@ -334,7 +333,7 @@
suite('route handlers', () => {
let redirectStub: sinon.SinonStub;
let setStateStub: sinon.SinonStub;
- let handlePassThroughRoute: sinon.SinonStub;
+ let windowReloadStub: sinon.SinonStub;
let redirectToLoginStub: sinon.SinonStub;
async function checkUrlToState<T extends ViewState>(
@@ -367,18 +366,12 @@
assert.equal(redirectToLoginStub.lastCall.firstArg, toUrl);
}
- async function checkUrlNotMatched(url: string) {
- handlePassThroughRoute.reset();
- router.page.show(url);
- await waitUntilCalled(handlePassThroughRoute, 'handlePassThroughRoute');
- }
-
setup(() => {
stubRestApi('addRepoNameToCache');
redirectStub = sinon.stub(router, 'redirect');
redirectToLoginStub = sinon.stub(router, 'redirectToLogin');
setStateStub = sinon.stub(router, 'setState');
- handlePassThroughRoute = sinon.stub(router, 'handlePassThroughRoute');
+ windowReloadStub = sinon.stub(router, 'windowReload');
router._testOnly_startRouter();
});
@@ -445,7 +438,7 @@
onExit!('', () => {}); // we left page;
router.handleDefaultRoute();
- assert.isTrue(handlePassThroughRoute.calledOnce);
+ assert.isTrue(windowReloadStub.calledOnce);
});
test('IMPROPERLY_ENCODED_PLUS', async () => {
@@ -1039,12 +1032,6 @@
});
});
- test('LOG_IN_OR_OUT pass through', async () => {
- // LOG_IN_OR_OUT: /^\/log(in|out)(\/(.+))?$/,
- await checkUrlNotMatched('/login/asdf');
- await checkUrlNotMatched('/logout/asdf');
- });
-
test('PLUGIN_SCREEN', async () => {
// PLUGIN_SCREEN: /^\/x\/([\w-]+)\/([\w-]+)\/?/,
await checkUrlToState('/x/foo/bar', {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index bc9bd12..1640b7a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -15,6 +15,7 @@
import '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
import '../gr-account-label/gr-account-label';
import '../gr-suggestion-diff-preview/gr-suggestion-diff-preview';
+import '../gr-fix-suggestions/gr-fix-suggestions';
import {getAppContext} from '../../../services/app-context';
import {css, html, LitElement, nothing, PropertyValues} from 'lit';
import {customElement, property, query, state} from 'lit/decorators.js';
@@ -91,7 +92,7 @@
export const AUTO_SAVE_DEBOUNCE_DELAY_MS = 2000;
export const GENERATE_SUGGESTION_DEBOUNCE_DELAY_MS = 1500;
export const ENABLE_GENERATE_SUGGESTION_STORAGE_KEY =
- 'enableGenerateSuggestionStorageKey';
+ 'enableGenerateSuggestionStorageKeyForCommentWithId-';
declare global {
interface HTMLElementEventMap {
@@ -313,8 +314,16 @@
for (const modifier of [Modifier.CTRL_KEY, Modifier.META_KEY]) {
this.shortcuts.addLocal(
{key: Key.ENTER, modifiers: [modifier]},
- () => {
+ e => {
this.save();
+ // We don't stop propagation for patchset comment
+ // (this.permanentEditingMode = true), but we stop it for normal
+ // comments. This prevents accidentally sending a reply when
+ // editing/saving them in the reply dialog.
+ if (!this.permanentEditingMode) {
+ e.preventDefault();
+ e.stopPropagation();
+ }
},
{preventDefault: false}
);
@@ -416,12 +425,14 @@
this.suggestionsProvider = suggestionsPlugins?.[0]?.provider;
});
- const generateSuggestionStoredContent =
- this.getStorage().getEditableContentItem(
- ENABLE_GENERATE_SUGGESTION_STORAGE_KEY
- );
- if (generateSuggestionStoredContent?.message === 'false') {
- this.generateSuggestion = false;
+ if (this.comment?.id) {
+ const generateSuggestionStoredContent =
+ this.getStorage().getEditableContentItem(
+ ENABLE_GENERATE_SUGGESTION_STORAGE_KEY + this.comment.id
+ );
+ if (generateSuggestionStoredContent?.message === 'false') {
+ this.generateSuggestion = false;
+ }
}
}
@@ -661,8 +672,8 @@
<gr-endpoint-slot name="above-actions"></gr-endpoint-slot>
${this.renderHumanActions()} ${this.renderRobotActions()}
</div>
- ${this.renderGeneratedSuggestionPreview()}
- ${this.renderFixSuggestionPreview()}
+ ${/* if this.editing */ this.renderGeneratedSuggestionPreview()}
+ ${/* if !this.editing */ this.renderFixSuggestionPreview()}
</div>
</gr-endpoint-decorator>
${this.renderConfirmDialog()}
@@ -999,10 +1010,11 @@
}
private renderFixSuggestionPreview() {
- if (!this.comment?.fix_suggestions || this.editing) return nothing;
- return html`<gr-suggestion-diff-preview
- .fixReplacementInfos=${this.comment?.fix_suggestions?.[0].replacements}
- ></gr-suggestion-diff-preview>`;
+ if (!this.comment?.fix_suggestions || this.editing || isRobot(this.comment))
+ return nothing;
+ return html`<gr-fix-suggestions
+ .comment=${this.comment}
+ ></gr-fix-suggestions>`;
}
// private but used in test
@@ -1029,13 +1041,17 @@
}
private renderGeneratedSuggestionPreview() {
- if (!this.showGeneratedSuggestion() || !this.generateSuggestion)
+ if (
+ !this.editing ||
+ !this.showGeneratedSuggestion() ||
+ !this.generateSuggestion
+ )
return nothing;
if (!isDraft(this.comment)) return nothing;
if (this.generatedFixSuggestion) {
return html`<gr-suggestion-diff-preview
- .fixReplacementInfos=${this.generatedFixSuggestion.replacements}
+ .fixSuggestionInfo=${this.generatedFixSuggestion}
></gr-suggestion-diff-preview>`;
} else if (this.generatedSuggestion) {
return html`<gr-suggestion-diff-preview
@@ -1063,10 +1079,12 @@
?checked=${this.generateSuggestion}
@change=${() => {
this.generateSuggestion = !this.generateSuggestion;
- this.getStorage().setEditableContentItem(
- ENABLE_GENERATE_SUGGESTION_STORAGE_KEY,
- this.generateSuggestion.toString()
- );
+ if (this.comment?.id) {
+ this.getStorage().setEditableContentItem(
+ ENABLE_GENERATE_SUGGESTION_STORAGE_KEY + this.comment.id,
+ this.generateSuggestion.toString()
+ );
+ }
if (this.generateSuggestion) {
this.generateSuggestionTrigger$.next();
} else {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 4c32da7..098b79a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -39,7 +39,7 @@
import {ReplyToCommentEvent} from '../../../types/events';
import {GrConfirmDeleteCommentDialog} from '../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog';
import {assertIsDefined} from '../../../utils/common-util';
-import {Modifier} from '../../../utils/dom-util';
+import {Key, Modifier} from '../../../utils/dom-util';
import {SinonStubbedMember} from 'sinon';
import {fixture, html, assert} from '@open-wc/testing';
import {GrButton} from '../gr-button/gr-button';
@@ -241,7 +241,6 @@
</gr-button>
</div>
</div>
- <gr-suggestion-diff-preview></gr-suggestion-diff-preview>
</div>
</gr-endpoint-decorator>
<dialog id="confirmDeleteModal" tabindex="-1">
@@ -597,6 +596,46 @@
assert.isTrue(spy.called);
});
+ suite('ctrl+ENTER ', () => {
+ test('saves comment', async () => {
+ const spy = sinon.stub(element, 'save');
+ element.messageText = 'is that the horse from horsing around??';
+ element.editing = true;
+ await element.updateComplete;
+ pressKey(
+ element.textarea!.textarea!.textarea,
+ Key.ENTER,
+ Modifier.CTRL_KEY
+ );
+ assert.isTrue(spy.called);
+ });
+ test('propagates on patchset comment', async () => {
+ const event = new KeyboardEvent('keydown', {
+ key: Key.ENTER,
+ ctrlKey: true,
+ });
+ const stopPropagationStub = sinon.stub(event, 'stopPropagation');
+ element.permanentEditingMode = true;
+ element.messageText = 'is that the horse from horsing around??';
+ element.editing = true;
+ await element.updateComplete;
+ element.dispatchEvent(event);
+ assert.isFalse(stopPropagationStub.called);
+ });
+ test('does not propagate on normal comment', async () => {
+ const event = new KeyboardEvent('keydown', {
+ key: Key.ENTER,
+ ctrlKey: true,
+ });
+ const stopPropagationStub = sinon.stub(event, 'stopPropagation');
+ element.messageText = 'is that the horse from horsing around??';
+ element.editing = true;
+ await element.updateComplete;
+ element.dispatchEvent(event);
+ assert.isTrue(stopPropagationStub.called);
+ });
+ });
+
test('save', async () => {
const savePromise = mockPromise<DraftInfo>();
const stub = sinon.stub(commentsModel, 'saveDraft').returns(savePromise);
@@ -956,8 +995,8 @@
element.editing = false;
await element.updateComplete;
assert.dom.equal(
- queryAndAssert(element, 'gr-suggestion-diff-preview'),
- /* HTML */ '<gr-suggestion-diff-preview> </gr-suggestion-diff-preview>'
+ queryAndAssert(element, 'gr-fix-suggestions'),
+ /* HTML */ '<gr-fix-suggestions> </gr-fix-suggestions>'
);
});
@@ -985,8 +1024,8 @@
element.editing = false;
await element.updateComplete;
assert.dom.equal(
- queryAndAssert(element, 'gr-suggestion-diff-preview'),
- /* HTML */ '<gr-suggestion-diff-preview> </gr-suggestion-diff-preview>'
+ queryAndAssert(element, 'gr-fix-suggestions'),
+ /* HTML */ '<gr-fix-suggestions> </gr-fix-suggestions>'
);
});
@@ -1035,7 +1074,7 @@
.initiallyCollapsed=${false}
></gr-comment>`
);
- element.editing = false;
+ element.editing = true;
sinon.stub(element, 'showGeneratedSuggestion').returns(true);
element.generateSuggestion = true;
element.generatedFixSuggestion = generatedFixSuggestion;
diff --git a/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts b/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts
new file mode 100644
index 0000000..02d64ff
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-fix-suggestions/gr-fix-suggestions.ts
@@ -0,0 +1,162 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../shared/gr-button/gr-button';
+import '../../shared/gr-icon/gr-icon';
+import '../../shared/gr-copy-clipboard/gr-copy-clipboard';
+import '../gr-suggestion-diff-preview/gr-suggestion-diff-preview';
+import {css, html, LitElement} from 'lit';
+import {customElement, state, query, property} from 'lit/decorators.js';
+import {fire} from '../../../utils/event-util';
+import {getDocUrl} from '../../../utils/url-util';
+import {subscribe} from '../../lit/subscription-controller';
+import {resolve} from '../../../models/dependency';
+import {configModelToken} from '../../../models/config/config-model';
+import {GrSuggestionDiffPreview} from '../gr-suggestion-diff-preview/gr-suggestion-diff-preview';
+import {changeModelToken} from '../../../models/change/change-model';
+import {Comment, isDraft, PatchSetNumber} from '../../../types/common';
+import {OpenFixPreviewEventDetail} from '../../../types/events';
+
+/**
+ * gr-fix-suggestions is UI for comment.fix_suggestions.
+ * gr-fix-suggestions is wrapper for gr-suggestion-diff-preview with buttons
+ * to preview and apply fix and for giving a context about suggestion.
+ */
+@customElement('gr-fix-suggestions')
+export class GrFixSuggestions extends LitElement {
+ @query('gr-suggestion-diff-preview')
+ suggestionDiffPreview?: GrSuggestionDiffPreview;
+
+ @property({type: Object})
+ comment?: Comment;
+
+ @state() private docsBaseUrl = '';
+
+ @state() private applyingFix = false;
+
+ @state() latestPatchNum?: PatchSetNumber;
+
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getConfigModel().docsBaseUrl$,
+ docsBaseUrl => (this.docsBaseUrl = docsBaseUrl)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().latestPatchNum$,
+ x => (this.latestPatchNum = x)
+ );
+ }
+
+ static override get styles() {
+ return [
+ css`
+ .header {
+ background-color: var(--background-color-primary);
+ border: 1px solid var(--border-color);
+ padding: var(--spacing-xs) var(--spacing-xl);
+ display: flex;
+ align-items: center;
+ border-top-left-radius: var(--border-radius);
+ border-top-right-radius: var(--border-radius);
+ }
+ .header .title {
+ flex: 1;
+ }
+ .copyButton {
+ margin-right: var(--spacing-l);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`<div class="header">
+ <div class="title">
+ <span>Suggested edit</span>
+ <a
+ href=${getDocUrl(this.docsBaseUrl, 'user-suggest-edits.html')}
+ target="_blank"
+ rel="noopener noreferrer"
+ ><gr-icon icon="help" title="read documentation"></gr-icon
+ ></a>
+ </div>
+ <div>
+ <gr-button
+ secondary
+ flatten
+ class="action show-fix"
+ @click=${this.handleShowFix}
+ >
+ Show edit
+ </gr-button>
+ <gr-button
+ secondary
+ flatten
+ .loading=${this.applyingFix}
+ .disabled=${this.isApplyEditDisabled()}
+ class="action show-fix"
+ @click=${this.handleApplyFix}
+ .title=${this.computeApplyEditTooltip()}
+ >
+ Apply edit
+ </gr-button>
+ </div>
+ </div>
+ <gr-suggestion-diff-preview
+ .fixSuggestionInfo=${this.comment?.fix_suggestions?.[0]}
+ ></gr-suggestion-diff-preview>`;
+ }
+
+ handleShowFix() {
+ if (!this.comment?.fix_suggestions || !this.comment?.patch_set) return;
+ const eventDetail: OpenFixPreviewEventDetail = {
+ fixSuggestions: this.comment.fix_suggestions.map(s => {
+ return {
+ ...s,
+ description: 'Suggested Edit from comment',
+ };
+ }),
+ patchNum: this.comment.patch_set,
+ onCloseFixPreviewCallbacks: [],
+ };
+ fire(this, 'open-fix-preview', eventDetail);
+ }
+
+ async handleApplyFix() {
+ if (!this.comment?.fix_suggestions) return;
+ this.applyingFix = true;
+ try {
+ await this.suggestionDiffPreview?.applyFixSuggestion();
+ } finally {
+ this.applyingFix = false;
+ }
+ }
+
+ private isApplyEditDisabled() {
+ if (this.comment?.patch_set === undefined) return true;
+ if (isDraft(this.comment)) return true;
+ return this.comment.patch_set !== this.latestPatchNum;
+ }
+
+ private computeApplyEditTooltip() {
+ if (this.comment?.patch_set === undefined) return '';
+ return this.comment.patch_set !== this.latestPatchNum
+ ? 'You cannot apply this fix because it is from a previous patchset'
+ : '';
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-fix-suggestions': GrFixSuggestions;
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index 01f137a..b57e4ff 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -305,8 +305,7 @@
private convertCodeToSuggestions() {
const marks = this.renderRoot.querySelectorAll('mark');
- if (marks.length > 0) {
- const userSuggestionMark = marks[0];
+ for (const userSuggestionMark of marks) {
const userSuggestion = document.createElement('gr-user-suggestion-fix');
// Temporary workaround for bug - tabs replacement
if (this.content.includes('\t')) {
diff --git a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
index 497ed2b..923a00e 100644
--- a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
+++ b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
@@ -19,7 +19,7 @@
import {GrSyntaxLayerWorker} from '../../../embed/diff/gr-syntax-layer/gr-syntax-layer-worker';
import {resolve} from '../../../models/dependency';
import {highlightServiceToken} from '../../../services/highlight/highlight-service';
-import {FixReplacementInfo, NumericChangeId} from '../../../api/rest-api';
+import {FixSuggestionInfo, NumericChangeId} from '../../../api/rest-api';
import {changeModelToken} from '../../../models/change/change-model';
import {subscribe} from '../../lit/subscription-controller';
import {FilePreview} from '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog';
@@ -45,9 +45,11 @@
/**
* Diff preview for
- * 1. suggestion vs commented Text
- * or 2. fixReplacementInfos
- * that are attached to a comment.
+ * 1. code block suggestion vs commented Text
+ * or 2. fixSuggestionInfo that are attached to a comment.
+ *
+ * It shouldn't be created with both 1. and 2. but if it is
+ * it shows just for 1. (code block suggestion)
*/
@customElement('gr-suggestion-diff-preview')
export class GrSuggestionDiffPreview extends LitElement {
@@ -55,7 +57,7 @@
suggestion?: string;
@property({type: Object})
- fixReplacementInfos?: FixReplacementInfo[];
+ fixSuggestionInfo?: FixSuggestionInfo;
@property({type: Boolean})
showAddSuggestionButton = false;
@@ -73,7 +75,7 @@
layers: DiffLayer[] = [];
@state()
- previewLoadedFor?: string | FixReplacementInfo[];
+ previewLoadedFor?: string | FixSuggestionInfo;
@state() repo?: RepoName;
@@ -178,14 +180,14 @@
}
if (changed.has('changeNum') || changed.has('comment')) {
- if (this.previewLoadedFor !== this.fixReplacementInfos) {
- this.fetchFixReplacementInfosPreview();
+ if (this.previewLoadedFor !== this.fixSuggestionInfo) {
+ this.fetchfixSuggestionInfoPreview();
}
}
}
override render() {
- if (!this.suggestion && !this.fixReplacementInfos) return nothing;
+ if (!this.suggestion && !this.fixSuggestionInfo) return nothing;
const code = this.suggestion;
return html`
${when(
@@ -259,29 +261,20 @@
return res;
}
- private async fetchFixReplacementInfosPreview() {
+ private async fetchfixSuggestionInfoPreview() {
if (
this.suggestion ||
!this.changeNum ||
!this.comment?.patch_set ||
- !this.fixReplacementInfos
+ !this.fixSuggestionInfo
)
return;
- // TODO (milutin): This is a temporary fix for the broken path issue.
- // Our experimental plugin currently returns only the file extension.
- const replacements = this.fixReplacementInfos.map(fixInfo => {
- return {
- ...fixInfo,
- path: this.comment?.path ?? fixInfo.path,
- };
- });
-
this.reporting.time(Timing.PREVIEW_FIX_LOAD);
const res = await this.restApiService.getFixPreview(
this.changeNum,
this.comment?.patch_set,
- replacements
+ this.fixSuggestionInfo.replacements
);
if (!res) return;
@@ -293,43 +286,58 @@
});
if (currentPreviews.length > 0) {
this.preview = currentPreviews[0];
- this.previewLoadedFor = this.fixReplacementInfos;
+ this.previewLoadedFor = this.fixSuggestionInfo;
}
return res;
}
/**
- * Applies a fix previewed in `suggestion-diff-preview`,
- * navigating to the new change URL with the EDIT patchset.
+ * Applies a fix (fix_suggestion in comment) previewed in
+ * `suggestion-diff-preview`, navigating to the new change URL with the EDIT
+ * patchset.
+ *
+ * Similar code flow is in gr-apply-fix-dialog.handleApplyFix
+ * Used in gr-fix-suggestions
+ */
+ public applyFixSuggestion() {
+ if (this.suggestion || !this.fixSuggestionInfo) return;
+ this.applyFix(this.fixSuggestionInfo);
+ }
+
+ /**
+ * Applies a fix (codeblock in comment message) previewed in
+ * `suggestion-diff-preview`, navigating to the new change URL with the EDIT
+ * patchset.
*
* Similar code flow is in gr-apply-fix-dialog.handleApplyFix
* Used in gr-user-suggestion-fix
*/
- public async applyFix() {
- if (
- !this.changeNum ||
- !this.comment?.patch_set ||
- !this.suggestion ||
- !this.commentedText
- )
- return;
- const changeNum = this.changeNum;
- const basePatchNum = this.comment?.patch_set as BasePatchSetNum;
+ public applyUserSuggestedFix() {
+ if (!this.comment || !this.suggestion || !this.commentedText) return;
+
const fixSuggestions = createUserFixSuggestion(
this.comment,
this.commentedText,
this.suggestion
);
+ this.applyFix(fixSuggestions[0]);
+ }
+
+ private async applyFix(fixSuggestion: FixSuggestionInfo) {
+ const changeNum = this.changeNum;
+ const basePatchNum = this.comment?.patch_set as BasePatchSetNum;
+ if (!changeNum || !basePatchNum || !fixSuggestion) return;
+
this.reporting.time(Timing.APPLY_FIX_LOAD);
const res = await this.restApiService.applyFixSuggestion(
- this.changeNum,
- this.comment?.patch_set,
- fixSuggestions[0].replacements
+ changeNum,
+ basePatchNum,
+ fixSuggestion.replacements
);
this.reporting.timeEnd(Timing.APPLY_FIX_LOAD, {
method: '1-click',
- description: fixSuggestions?.[0].description,
+ description: fixSuggestion.description,
});
if (res?.ok) {
this.getNavigation().setUrl(
diff --git a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
index be73b87..c8de209 100644
--- a/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
+++ b/polygerrit-ui/app/elements/shared/gr-user-suggestion-fix/gr-user-suggestion-fix.ts
@@ -147,7 +147,7 @@
async handleApplyFix() {
if (!this.textContent) return;
this.applyingFix = true;
- await this.suggestionDiffPreview?.applyFix();
+ await this.suggestionDiffPreview?.applyUserSuggestedFix();
this.applyingFix = false;
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
index 19a6457..8b333e6 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
@@ -582,7 +582,8 @@
gr-diff-row tr.diff-row,
gr-diff-row td.content,
gr-diff-section tbody.contextControl,
- gr-diff-row td.blame {
+ gr-diff-row td.blame,
+ #diffHeader {
-webkit-user-select: none;
-moz-user-select: none;
-ms-user-select: none;
diff --git a/polygerrit-ui/app/utils/location-util.ts b/polygerrit-ui/app/utils/location-util.ts
new file mode 100644
index 0000000..d0eac74
--- /dev/null
+++ b/polygerrit-ui/app/utils/location-util.ts
@@ -0,0 +1,22 @@
+/**
+ * @license
+ * Copyright 2024 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+// This file adds some simple checks to match internal Google rules.
+// Internally at Google it has different a implementation.
+
+import {safeLocation} from 'safevalues/dom';
+
+export function setHref(loc: Location, url: string) {
+ safeLocation.setHref(loc, url);
+}
+
+export function replace(loc: Location, url: string) {
+ safeLocation.replace(loc, url);
+}
+
+export function assign(loc: Location, url: string) {
+ safeLocation.assign(loc, url);
+}
diff --git a/proto/entities.proto b/proto/entities.proto
index 335db62..9d4b394f 100644
--- a/proto/entities.proto
+++ b/proto/entities.proto
@@ -62,6 +62,31 @@
reserved 101; // note_db_state
}
+// Serialized form of com.google.gerrit.extensions.common.ChangeInput.
+// Next ID: 19
+message ChangeInput {
+ optional string project = 1;
+ optional string branch = 2;
+ optional string subject = 3;
+ optional string topic = 4;
+ optional ChangeStatus status = 5;
+ optional bool is_private = 6;
+ optional bool work_in_progress = 7;
+ optional string base_change = 8;
+ optional string base_commit = 9;
+ optional bool new_branch = 10;
+ map<string, string> validation_options = 11;
+ map<string, string> custom_keyed_values = 12;
+ optional MergeInput merge = 13;
+ optional ApplyPatchInput patch = 14;
+ optional AccountInput author = 15;
+ repeated ListChangesOption response_format_options = 16;
+ optional NotifyHandling notify = 17 [default = ALL];
+ // The key is the string representation of the RecipientType enum.
+ // We use a string here because proto does not allow enum keys in maps.
+ map<string, NotifyInfo> notify_details = 18;
+}
+
// Serialized form of com.google.gerrit.enities.ChangeMessage.
// Next ID: 3
message ChangeMessage_Key {
@@ -81,6 +106,97 @@
optional Account_Id real_author = 7;
}
+// Serialized form of com.google.gerrit.extensions.client.ChangeStatus.
+// Next ID: 3
+enum ChangeStatus {
+ NEW = 0;
+ MERGED = 1;
+ ABANDONED = 2;
+}
+
+// Serialized form of com.google.gerrit.extensions.common.MergeInput.
+// Next ID: 5
+message MergeInput {
+ optional string source = 1;
+ optional string source_branch = 2;
+ optional string strategy = 3;
+ optional bool allow_conflicts = 4;
+}
+
+// Serialized form of com.google.gerrit.extensions.api.changes.ApplyPatchInput.
+// Next ID: 2
+message ApplyPatchInput {
+ optional string patch = 1;
+}
+
+// Serialized form of com.google.gerrit.extensions.api.accounts.AccountInput.
+// Next ID: 8
+message AccountInput {
+ optional string username = 1;
+ optional string name = 2;
+ optional string display_name = 3;
+ optional string email = 4;
+ optional string ssh_key = 5;
+ optional string http_password = 6;
+ repeated string groups = 7;
+}
+
+// Serialized form of com.google.gerrit.extensions.client.ListChangesOption.
+// Next ID: 28
+enum ListChangesOption {
+ LABELS = 0;
+ CURRENT_REVISION = 1;
+ ALL_REVISIONS = 2;
+ CURRENT_COMMIT = 3;
+ ALL_COMMITS = 4;
+ CURRENT_FILES = 5;
+ ALL_FILES = 6;
+ DETAILED_ACCOUNTS = 7;
+ DETAILED_LABELS = 8;
+ MESSAGES = 9;
+ CURRENT_ACTIONS = 10;
+ REVIEWED = 11;
+ DRAFT_COMMENTS = 12;
+ DOWNLOAD_COMMANDS = 13;
+ WEB_LINKS = 14;
+ CHECK = 15;
+ CHANGE_ACTIONS = 16;
+ COMMIT_FOOTERS = 17;
+ PUSH_CERTIFICATES = 18;
+ REVIEWER_UPDATES = 19;
+ SUBMITTABLE = 20;
+ TRACKING_IDS = 21;
+ SKIP_MERGEABLE = 22;
+ SKIP_DIFFSTAT = 23;
+ SUBMIT_REQUIREMENTS = 24;
+ CUSTOM_KEYED_VALUES = 25;
+ STAR = 26;
+ PARENTS = 27;
+}
+
+// Serialized form of com.google.gerrit.extensions.api.changes.NotifyHandling.
+// Next ID: 4
+enum NotifyHandling {
+ NONE = 0;
+ OWNER = 1;
+ OWNER_REVIEWERS = 2;
+ ALL = 3;
+}
+
+// Serialized form of com.google.gerrit.extensions.api.changes.RecipientType.
+// Next ID: 3
+enum RecipientType {
+ TO = 0;
+ CC = 1;
+ BCC = 2;
+}
+
+// Serialized form of com.google.gerrit.extensions.api.changes.NotifyInfo.
+// Next ID: 2
+message NotifyInfo {
+ repeated string accounts = 1;
+}
+
// Serialized form of com.google.gerrit.entities.PatchSet.Id.
// Next ID: 3
message PatchSet_Id {
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index f5fe24c..5e7ddb9 100644
--- a/tools/maven/gerrit-acceptance-framework_pom.xml
+++ b/tools/maven/gerrit-acceptance-framework_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-acceptance-framework</artifactId>
- <version>3.9.2-SNAPSHOT</version>
+ <version>3.9.3-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Acceptance Test Framework</name>
<description>Framework for Gerrit's acceptance tests</description>
diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml
index abe1793..95a911a 100644
--- a/tools/maven/gerrit-extension-api_pom.xml
+++ b/tools/maven/gerrit-extension-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-extension-api</artifactId>
- <version>3.9.2-SNAPSHOT</version>
+ <version>3.9.3-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Extension API</name>
<description>API for Gerrit Extensions</description>
diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml
index 3240b14..1823f72 100644
--- a/tools/maven/gerrit-plugin-api_pom.xml
+++ b/tools/maven/gerrit-plugin-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-plugin-api</artifactId>
- <version>3.9.2-SNAPSHOT</version>
+ <version>3.9.3-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Plugin API</name>
<description>API for Gerrit Plugins</description>
diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml
index 52e98a04..499ad32 100644
--- a/tools/maven/gerrit-war_pom.xml
+++ b/tools/maven/gerrit-war_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-war</artifactId>
- <version>3.9.2-SNAPSHOT</version>
+ <version>3.9.3-SNAPSHOT</version>
<packaging>war</packaging>
<name>Gerrit Code Review - WAR</name>
<description>Gerrit WAR</description>
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index deee0d4..ac3f668 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -271,30 +271,30 @@
sha1 = "48462eb319817c90c27d377341684b6b81372e08",
)
- TRUTH_VERS = "1.4.0"
+ TRUTH_VERS = "1.4.2"
maven_jar(
name = "truth",
artifact = "com.google.truth:truth:" + TRUTH_VERS,
- sha1 = "2a9475ed8cf2081b859fda8f9c860d5c449cd9ed",
+ sha1 = "2322d861290bd84f84cbb178e43539725a4588fd",
)
maven_jar(
name = "truth-java8-extension",
artifact = "com.google.truth.extensions:truth-java8-extension:" + TRUTH_VERS,
- sha1 = "807a8226b465e5df74cc0591762f729195c3f144",
+ sha1 = "bfa44a01e1bb5a1df50bc9c678d6588b4d9eb73a",
)
maven_jar(
name = "truth-liteproto-extension",
artifact = "com.google.truth.extensions:truth-liteproto-extension:" + TRUTH_VERS,
- sha1 = "849821a1b377119104df95f87cb3e5b9d674c8a8",
+ sha1 = "062a2716b3b0ba9d8e72c913dad43a8139b12202",
)
maven_jar(
name = "truth-proto-extension",
artifact = "com.google.truth.extensions:truth-proto-extension:" + TRUTH_VERS,
- sha1 = "20a6fbb4276320e4df5fb42fe327bc9fb0f56dd1",
+ sha1 = "53cfc94dfa435c5dcd6f8b6844b82b423ea0a5af",
)
LUCENE_VERS = "9.8.0"
diff --git a/version.bzl b/version.bzl
index d55ddba..c5fca8b 100644
--- a/version.bzl
+++ b/version.bzl
@@ -2,4 +2,4 @@
# Used by :api_install and :api_deploy targets
# when talking to the destination repository.
#
-GERRIT_VERSION = "3.9.2-SNAPSHOT"
+GERRIT_VERSION = "3.9.3-SNAPSHOT"
