Merge "Refactor gr-router_test"
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
index 0966bbe..62ad7c4 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -139,9 +140,10 @@
RevWalk revWalk = new RevWalk(objectInserter.newReader())) {
Instant now = TimeUtil.now();
IdentifiedUser changeOwner = getChangeOwner(changeCreation);
- PersonIdent authorAndCommitter = changeOwner.newCommitterIdent(now, serverIdent.getZoneId());
+ PersonIdent author = getAuthorIdent(now, changeCreation);
+ PersonIdent committer = getCommitterIdent(now, changeCreation);
ObjectId commitId =
- createCommit(repository, revWalk, objectInserter, changeCreation, authorAndCommitter);
+ createCommit(repository, revWalk, objectInserter, changeCreation, author, committer);
String refName = RefNames.fullName(changeCreation.branch());
ChangeInserter inserter = getChangeInserter(changeId, refName, commitId);
@@ -189,6 +191,30 @@
return getArbitraryUser();
}
+ private PersonIdent getAuthorIdent(Instant when, TestChangeCreation changeCreation)
+ throws IOException, ConfigInvalidException {
+ if (changeCreation.authorIdent().isPresent()) {
+ return new PersonIdent(changeCreation.authorIdent().get(), when);
+ }
+
+ return (changeCreation.author().isPresent()
+ ? userFactory.create(changeCreation.author().get())
+ : getChangeOwner(changeCreation))
+ .newCommitterIdent(when, serverIdent.getZoneId());
+ }
+
+ private PersonIdent getCommitterIdent(Instant when, TestChangeCreation changeCreation)
+ throws IOException, ConfigInvalidException {
+ if (changeCreation.committerIdent().isPresent()) {
+ return new PersonIdent(changeCreation.committerIdent().get(), when);
+ }
+
+ return (changeCreation.committer().isPresent()
+ ? userFactory.create(changeCreation.committer().get())
+ : getChangeOwner(changeCreation))
+ .newCommitterIdent(when, serverIdent.getZoneId());
+ }
+
private IdentifiedUser getArbitraryUser() throws ConfigInvalidException, IOException {
ImmutableSet<Account.Id> foundAccounts = resolver.resolveIgnoreVisibility("").asIdSet();
checkState(
@@ -202,15 +228,15 @@
RevWalk revWalk,
ObjectInserter objectInserter,
TestChangeCreation changeCreation,
- PersonIdent authorAndCommitter)
+ PersonIdent author,
+ PersonIdent committer)
throws IOException, BadRequestException {
ImmutableList<ObjectId> parentCommits = getParentCommits(repository, revWalk, changeCreation);
TreeCreator treeCreator =
getTreeCreator(objectInserter, parentCommits, changeCreation.mergeStrategy());
ObjectId tree = createNewTree(repository, treeCreator, changeCreation.treeModifications());
String commitMessage = correctCommitMessage(changeCreation.commitMessage());
- return createCommit(
- objectInserter, tree, parentCommits, authorAndCommitter, authorAndCommitter, commitMessage);
+ return createCommit(objectInserter, tree, parentCommits, author, committer, commitMessage);
}
private ImmutableList<ObjectId> getParentCommits(
@@ -432,9 +458,18 @@
ObjectInserter objectInserter = repository.newObjectInserter();
RevWalk revWalk = new RevWalk(objectInserter.newReader())) {
Instant now = TimeUtil.now();
+ PersonIdent authorIdent = getAuthorIdent(now, patchsetCreation);
+ PersonIdent committerIdent = getCommitterIdent(now, patchsetCreation);
ObjectId newPatchsetCommit =
createPatchsetCommit(
- repository, revWalk, objectInserter, changeNotes, patchsetCreation, now);
+ repository,
+ revWalk,
+ objectInserter,
+ changeNotes,
+ patchsetCreation,
+ authorIdent,
+ committerIdent,
+ now);
PatchSet.Id patchsetId =
ChangeUtil.nextPatchSetId(repository, changeNotes.getCurrentPatchSet().id());
@@ -453,12 +488,44 @@
}
}
+ @Nullable
+ private PersonIdent getAuthorIdent(Instant when, TestPatchsetCreation patchsetCreation) {
+ if (patchsetCreation.authorIdent().isPresent()) {
+ return new PersonIdent(patchsetCreation.authorIdent().get(), when);
+ }
+
+ if (patchsetCreation.author().isPresent()) {
+ return userFactory
+ .create(patchsetCreation.author().get())
+ .newCommitterIdent(when, serverIdent.getZoneId());
+ }
+
+ return null;
+ }
+
+ @Nullable
+ private PersonIdent getCommitterIdent(Instant when, TestPatchsetCreation patchsetCreation) {
+ if (patchsetCreation.committerIdent().isPresent()) {
+ return new PersonIdent(patchsetCreation.committerIdent().get(), when);
+ }
+
+ if (patchsetCreation.committer().isPresent()) {
+ return userFactory
+ .create(patchsetCreation.committer().get())
+ .newCommitterIdent(when, serverIdent.getZoneId());
+ }
+
+ return null;
+ }
+
private ObjectId createPatchsetCommit(
Repository repository,
RevWalk revWalk,
ObjectInserter objectInserter,
ChangeNotes changeNotes,
TestPatchsetCreation patchsetCreation,
+ @Nullable PersonIdent author,
+ @Nullable PersonIdent committer,
Instant now)
throws IOException, BadRequestException {
ObjectId oldPatchsetCommitId = changeNotes.getCurrentPatchSet().commitId();
@@ -474,9 +541,13 @@
changeNotes.getChange().getKey().get(),
patchsetCreation.commitMessage().orElseGet(oldPatchsetCommit::getFullMessage));
- PersonIdent author = getAuthor(oldPatchsetCommit);
- PersonIdent committer = getCommitter(oldPatchsetCommit, now);
- return createCommit(objectInserter, tree, parentCommitIds, author, committer, commitMessage);
+ return createCommit(
+ objectInserter,
+ tree,
+ parentCommitIds,
+ Optional.ofNullable(author).orElse(getAuthor(oldPatchsetCommit)),
+ Optional.ofNullable(committer).orElse(getCommitter(oldPatchsetCommit, now)),
+ commitMessage);
}
private String correctCommitMessage(String oldChangeId, String desiredCommitMessage)
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java
index f01a138..a0746e2 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestChangeCreation.java
@@ -14,6 +14,8 @@
package com.google.gerrit.acceptance.testsuite.change;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -24,6 +26,7 @@
import com.google.gerrit.server.edit.tree.TreeModification;
import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.merge.MergeStrategy;
/** Initial attributes of the change. If not provided, arbitrary values will be used. */
@@ -35,6 +38,14 @@
public abstract Optional<Account.Id> owner();
+ public abstract Optional<Account.Id> author();
+
+ public abstract Optional<PersonIdent> authorIdent();
+
+ public abstract Optional<Account.Id> committer();
+
+ public abstract Optional<PersonIdent> committerIdent();
+
public abstract Optional<String> topic();
public abstract ImmutableMap<String, Short> approvals();
@@ -69,9 +80,65 @@
*/
public abstract Builder branch(String branch);
- /** The change owner. Must be an existing user account. */
+ /**
+ * The change owner.
+ *
+ * <p>Must be an existing user account.
+ */
public abstract Builder owner(Account.Id owner);
+ /**
+ * The author of the commit for which the change is created.
+ *
+ * <p>Must be an existing user account.
+ *
+ * <p>Cannot be set together with {@link #authorIdent()} is set.
+ *
+ * <p>If neither {@link #author()} nor {@link #authorIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the author.
+ */
+ public abstract Builder author(Account.Id author);
+
+ /**
+ * The author ident of the commit for which the change is created.
+ *
+ * <p>Cannot be set together with {@link #author()} is set.
+ *
+ * <p>If neither {@link #author()} nor {@link #authorIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the author.
+ */
+ public abstract Builder authorIdent(PersonIdent authorIdent);
+
+ public abstract Optional<Account.Id> author();
+
+ public abstract Optional<PersonIdent> authorIdent();
+
+ /**
+ * The committer of the commit for which the change is created.
+ *
+ * <p>Must be an existing user account.
+ *
+ * <p>Cannot be set together with {@link #committerIdent()} is set.
+ *
+ * <p>If neither {@link #committer()} nor {@link #committerIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the committer.
+ */
+ public abstract Builder committer(Account.Id committer);
+
+ /**
+ * The committer ident of the commit for which the change is created.
+ *
+ * <p>Cannot be set together with {@link #committer()} is set.
+ *
+ * <p>If neither {@link #committer()} nor {@link #committerIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the committer.
+ */
+ public abstract Builder committerIdent(PersonIdent committerIdent);
+
+ public abstract Optional<Account.Id> committer();
+
+ public abstract Optional<PersonIdent> committerIdent();
+
/** The topic to add this change to. */
public abstract Builder topic(String topic);
@@ -156,13 +223,23 @@
abstract TestChangeCreation autoBuild();
+ public TestChangeCreation build() {
+ checkState(
+ author().isEmpty() || authorIdent().isEmpty(),
+ "author and authorIdent cannot be set together");
+ checkState(
+ committer().isEmpty() || committerIdent().isEmpty(),
+ "committer and committerIdent cannot be set together");
+ return autoBuild();
+ }
+
/**
* Creates the change.
*
* @return the {@code Change.Id} of the created change
*/
public Change.Id create() {
- TestChangeCreation changeUpdate = autoBuild();
+ TestChangeCreation changeUpdate = build();
return changeUpdate.changeCreator().applyAndThrowSilently(changeUpdate);
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
index 32731c1..f8ca977 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
@@ -14,6 +14,8 @@
package com.google.gerrit.acceptance.testsuite.change;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
@@ -21,6 +23,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.server.edit.tree.TreeModification;
import java.util.Optional;
+import org.eclipse.jgit.lib.PersonIdent;
/** Initial attributes of the patchset. If not provided, arbitrary values will be used. */
@AutoValue
@@ -28,6 +31,14 @@
public abstract Optional<Account.Id> uploader();
+ public abstract Optional<Account.Id> author();
+
+ public abstract Optional<PersonIdent> authorIdent();
+
+ public abstract Optional<Account.Id> committer();
+
+ public abstract Optional<PersonIdent> committerIdent();
+
public abstract Optional<String> commitMessage();
public abstract ImmutableList<TreeModification> treeModifications();
@@ -44,11 +55,66 @@
@AutoValue.Builder
public abstract static class Builder {
/**
- * The uploader for the new patch set. If not set the new patch set is uploaded by the change
- * owner.
+ * The uploader for the new patch set.
+ *
+ * <p>Must be an existing user account.
+ *
+ * <p>If not set the new patch set is uploaded by the change owner.
*/
public abstract Builder uploader(Account.Id uploader);
+ /**
+ * The author of the commit for which the change is created.
+ *
+ * <p>Must be an existing user account.
+ *
+ * <p>Cannot be set together with {@link #authorIdent()} is set.
+ *
+ * <p>If neither {@link #author()} nor {@link #authorIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the author.
+ */
+ public abstract Builder author(Account.Id author);
+
+ /**
+ * The author ident of the commit for which the change is created.
+ *
+ * <p>Cannot be set together with {@link #author()} is set.
+ *
+ * <p>If neither {@link #author()} nor {@link #authorIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the author.
+ */
+ public abstract Builder authorIdent(PersonIdent authorIdent);
+
+ public abstract Optional<Account.Id> author();
+
+ public abstract Optional<PersonIdent> authorIdent();
+
+ /**
+ * The committer of the commit for which the change is created.
+ *
+ * <p>Must be an existing user account.
+ *
+ * <p>Cannot be set together with {@link #committerIdent()} is set.
+ *
+ * <p>If neither {@link #committer()} nor {@link #committerIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the committer.
+ */
+ public abstract Builder committer(Account.Id committer);
+
+ /**
+ * The committer ident of the commit for which the change is created.
+ *
+ * <p>Cannot be set together with {@link #committer()} is set.
+ *
+ * <p>If neither {@link #committer()} nor {@link #committerIdent()} is set the {@link
+ * TestChangeCreation#owner()} is used as the committer.
+ */
+ public abstract Builder committerIdent(PersonIdent committerIdent);
+
+ public abstract Optional<Account.Id> committer();
+
+ public abstract Optional<PersonIdent> committerIdent();
+
public abstract Builder commitMessage(String commitMessage);
/** Modified file of the patchset. The file content is specified via the returned builder. */
@@ -100,13 +166,23 @@
abstract TestPatchsetCreation autoBuild();
+ public TestPatchsetCreation build() {
+ checkState(
+ author().isEmpty() || authorIdent().isEmpty(),
+ "author and authorIdent cannot be set together");
+ checkState(
+ committer().isEmpty() || committerIdent().isEmpty(),
+ "committer and committerIdent cannot be set together");
+ return autoBuild();
+ }
+
/**
* Creates the patchset.
*
* @return the {@code PatchSet.Id} of the created patchset
*/
public PatchSet.Id create() {
- TestPatchsetCreation patchsetCreation = autoBuild();
+ TestPatchsetCreation patchsetCreation = build();
return patchsetCreation.patchsetCreator().applyAndThrowSilently(patchsetCreation);
}
}
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index df2c5cb..7293f35 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -46,6 +46,7 @@
import com.google.gerrit.lucene.LuceneIndexModule;
import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker;
import com.google.gerrit.pgm.util.LogFileCompressor.LogFileCompressorModule;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.LibModuleLoader;
import com.google.gerrit.server.LibModuleType;
import com.google.gerrit.server.ModuleOverloader;
@@ -310,6 +311,7 @@
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new GerritApiModule());
modules.add(new ProjectQueryBuilderModule());
+ modules.add(new DefaultRefLogIdentityProvider.Module());
modules.add(new PluginApiModule());
modules.add(new SearchingChangeCacheImplModule());
modules.add(new InternalAccountDirectoryModule());
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 0342fe5..845cc9a 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -55,6 +55,7 @@
import com.google.gerrit.pgm.util.LogFileCompressor.LogFileCompressorModule;
import com.google.gerrit.pgm.util.RuntimeShutdown;
import com.google.gerrit.pgm.util.SiteProgram;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.LibModuleLoader;
import com.google.gerrit.server.LibModuleType;
import com.google.gerrit.server.ModuleOverloader;
@@ -448,6 +449,7 @@
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new GerritApiModule());
modules.add(new ProjectQueryBuilderModule());
+ modules.add(new DefaultRefLogIdentityProvider.Module());
modules.add(new PluginApiModule());
modules.add(new SearchingChangeCacheImplModule(replica));
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 5bffce7..cae7ca6 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -28,6 +28,7 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.LibModuleLoader;
@@ -83,19 +84,19 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRequirementsEvaluatorImpl;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
-import com.google.gerrit.server.query.FileEditsPredicate;
import com.google.gerrit.server.query.approval.ApprovalModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ConflictsCacheImpl;
-import com.google.gerrit.server.query.change.DistinctVotersPredicate;
-import com.google.gerrit.server.query.change.HasSubmoduleUpdatePredicate;
import com.google.gerrit.server.restapi.group.GroupModule;
import com.google.gerrit.server.rules.DefaultSubmitRule.DefaultSubmitRuleModule;
import com.google.gerrit.server.rules.IgnoreSelfApprovalRule.IgnoreSelfApprovalRuleModule;
import com.google.gerrit.server.rules.PrologModule;
import com.google.gerrit.server.rules.SubmitRule;
+import com.google.gerrit.server.submitrequirement.predicate.DistinctVotersPredicate;
+import com.google.gerrit.server.submitrequirement.predicate.FileEditsPredicate;
+import com.google.gerrit.server.submitrequirement.predicate.HasSubmoduleUpdatePredicate;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.inject.Injector;
import com.google.inject.Key;
@@ -127,6 +128,7 @@
modules.add(PatchListCacheImpl.module());
modules.add(new DefaultUrlFormatterModule());
modules.add(DiffOperationsImpl.module());
+ modules.add(new DefaultRefLogIdentityProvider.Module());
// There is the concept of LifecycleModule, in Gerrit's own extension to Guice, which has these:
// listener().to(SomeClassImplementingLifecycleListener.class);
diff --git a/java/com/google/gerrit/server/DefaultRefLogIdentityProvider.java b/java/com/google/gerrit/server/DefaultRefLogIdentityProvider.java
new file mode 100644
index 0000000..bef276a
--- /dev/null
+++ b/java/com/google/gerrit/server/DefaultRefLogIdentityProvider.java
@@ -0,0 +1,97 @@
+// Copyright (C) 2023 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;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.config.AnonymousCowardName;
+import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+import java.time.Instant;
+import java.time.ZoneId;
+import org.eclipse.jgit.lib.PersonIdent;
+
+@Singleton
+public class DefaultRefLogIdentityProvider implements RefLogIdentityProvider {
+ public static class Module extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(RefLogIdentityProvider.class).to(DefaultRefLogIdentityProvider.class);
+ }
+ }
+
+ private final String anonymousCowardName;
+ private final Boolean enablePeerIPInReflogRecord;
+
+ @Inject
+ DefaultRefLogIdentityProvider(
+ @AnonymousCowardName String anonymousCowardName,
+ @EnablePeerIPInReflogRecord Boolean enablePeerIPInReflogRecord) {
+ this.anonymousCowardName = anonymousCowardName;
+ this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
+ }
+
+ @Override
+ public PersonIdent newRefLogIdent(IdentifiedUser user, Instant when, ZoneId zoneId) {
+ Account account = user.getAccount();
+
+ String name = account.fullName();
+ if (name == null || name.isEmpty()) {
+ name = account.preferredEmail();
+ }
+ if (name == null || name.isEmpty()) {
+ name = anonymousCowardName;
+ }
+
+ String email;
+ if (enablePeerIPInReflogRecord) {
+ email = constructMailAddress(user, guessHost(user));
+ } else {
+ email =
+ Strings.isNullOrEmpty(account.preferredEmail())
+ ? constructMailAddress(user, "unknown")
+ : account.preferredEmail();
+ }
+
+ return new PersonIdent(name, email, when, zoneId);
+ }
+
+ private String constructMailAddress(IdentifiedUser user, String host) {
+ return user.getUserName().orElse("")
+ + "|account-"
+ + user.getAccountId().toString()
+ + "@"
+ + host;
+ }
+
+ private String guessHost(IdentifiedUser user) {
+ String host = null;
+ SocketAddress remotePeer = user.getRemotePeer();
+ if (remotePeer instanceof InetSocketAddress) {
+ InetSocketAddress sa = (InetSocketAddress) remotePeer;
+ InetAddress in = sa.getAddress();
+ host = in != null ? in.getHostAddress() : sa.getHostName();
+ }
+ if (Strings.isNullOrEmpty(host)) {
+ return "unknown";
+ }
+ return host;
+ }
+}
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java
index eda6e09..36d7888 100644
--- a/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -19,7 +19,6 @@
import static com.google.common.flogger.LazyArgs.lazy;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
@@ -44,8 +43,6 @@
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
import com.google.inject.util.Providers;
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
import java.net.MalformedURLException;
import java.net.SocketAddress;
import java.net.URL;
@@ -66,6 +63,7 @@
private final AuthConfig authConfig;
private final Realm realm;
private final String anonymousCowardName;
+ private final RefLogIdentityProvider refLogIdentityProvider;
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
@@ -76,6 +74,7 @@
AuthConfig authConfig,
Realm realm,
@AnonymousCowardName String anonymousCowardName,
+ RefLogIdentityProvider refLogIdentityProvider,
@CanonicalWebUrl Provider<String> canonicalUrl,
@EnablePeerIPInReflogRecord Boolean enablePeerIPInReflogRecord,
AccountCache accountCache,
@@ -83,6 +82,7 @@
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
+ this.refLogIdentityProvider = refLogIdentityProvider;
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
@@ -94,6 +94,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -131,6 +132,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -153,6 +155,7 @@
private final AuthConfig authConfig;
private final Realm realm;
private final String anonymousCowardName;
+ private final RefLogIdentityProvider refLogIdentityProvider;
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
@@ -164,6 +167,7 @@
AuthConfig authConfig,
Realm realm,
@AnonymousCowardName String anonymousCowardName,
+ RefLogIdentityProvider refLogIdentityProvider,
@CanonicalWebUrl Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
@@ -172,6 +176,7 @@
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
+ this.refLogIdentityProvider = refLogIdentityProvider;
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
@@ -188,6 +193,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -203,6 +209,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -224,6 +231,7 @@
private final Realm realm;
private final GroupBackend groupBackend;
private final String anonymousCowardName;
+ private final RefLogIdentityProvider refLogIdentityProvider;
private final Boolean enablePeerIPInReflogRecord;
private final Set<String> validEmails = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
private final CurrentUser realUser; // Must be final since cached properties depend on it.
@@ -235,11 +243,13 @@
private boolean loadedAllEmails;
private Set<String> invalidEmails;
private GroupMembership effectiveGroups;
+ private PersonIdent refLogIdent;
private IdentifiedUser(
AuthConfig authConfig,
Realm realm,
String anonymousCowardName,
+ RefLogIdentityProvider refLogIdentityProvider,
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
@@ -251,6 +261,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -266,6 +277,7 @@
AuthConfig authConfig,
Realm realm,
String anonymousCowardName,
+ RefLogIdentityProvider refLogIdentityProvider,
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
@@ -281,6 +293,7 @@
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
+ this.refLogIdentityProvider = refLogIdentityProvider;
this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
this.remotePeerProvider = remotePeerProvider;
this.accountId = id;
@@ -426,36 +439,27 @@
return getAccountId();
}
+ @Nullable
+ public SocketAddress getRemotePeer() {
+ try {
+ return remotePeerProvider.get();
+ } catch (OutOfScopeException | ProvisionException e) {
+ return null;
+ }
+ }
+
public PersonIdent newRefLogIdent() {
- return newRefLogIdent(Instant.now(), ZoneId.systemDefault());
+ return refLogIdentityProvider.newRefLogIdent(this);
}
public PersonIdent newRefLogIdent(Instant when, ZoneId zoneId) {
- final Account ua = getAccount();
-
- String name = ua.fullName();
- if (name == null || name.isEmpty()) {
- name = ua.preferredEmail();
+ if (refLogIdent != null) {
+ refLogIdent =
+ new PersonIdent(refLogIdent.getName(), refLogIdent.getEmailAddress(), when, zoneId);
+ return refLogIdent;
}
- if (name == null || name.isEmpty()) {
- name = anonymousCowardName;
- }
-
- String user;
- if (enablePeerIPInReflogRecord) {
- user = constructMailAddress(ua, guessHost());
- } else {
- user =
- Strings.isNullOrEmpty(ua.preferredEmail())
- ? constructMailAddress(ua, "unknown")
- : ua.preferredEmail();
- }
-
- return new PersonIdent(name, user, when, zoneId);
- }
-
- private String constructMailAddress(Account ua, String host) {
- return getUserName().orElse("") + "|account-" + ua.id().toString() + "@" + host;
+ refLogIdent = refLogIdentityProvider.newRefLogIdent(this, when, zoneId);
+ return refLogIdent;
}
public PersonIdent newCommitterIdent(PersonIdent ident) {
@@ -533,6 +537,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
Providers.of(canonicalUrl.get()),
accountCache,
groupBackend,
@@ -546,23 +551,4 @@
public boolean hasSameAccountId(CurrentUser other) {
return getAccountId().get() == other.getAccountId().get();
}
-
- private String guessHost() {
- String host = null;
- SocketAddress remotePeer = null;
- try {
- remotePeer = remotePeerProvider.get();
- } catch (OutOfScopeException | ProvisionException e) {
- // Leave null.
- }
- if (remotePeer instanceof InetSocketAddress) {
- InetSocketAddress sa = (InetSocketAddress) remotePeer;
- InetAddress in = sa.getAddress();
- host = in != null ? in.getHostAddress() : sa.getHostName();
- }
- if (Strings.isNullOrEmpty(host)) {
- return "unknown";
- }
- return host;
- }
}
diff --git a/java/com/google/gerrit/server/RefLogIdentityProvider.java b/java/com/google/gerrit/server/RefLogIdentityProvider.java
new file mode 100644
index 0000000..2a5d2b0
--- /dev/null
+++ b/java/com/google/gerrit/server/RefLogIdentityProvider.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2023 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;
+
+import java.time.Instant;
+import java.time.ZoneId;
+import org.eclipse.jgit.lib.PersonIdent;
+
+/**
+ * Extension point that allows to control which identity should be recorded in the reflog for ref
+ * updates done by a user or done on behalf of a user.
+ */
+public interface RefLogIdentityProvider {
+ /**
+ * Creates a {@link PersonIdent} for the given user that should be used as the user identity in
+ * the reflog for ref updates done by this user or done on behalf of this user.
+ *
+ * <p>The returned {@link PersonIdent} is created with the current timestamp and the system
+ * default timezone.
+ *
+ * @param user the user for which a reflog identity should be created
+ */
+ default PersonIdent newRefLogIdent(IdentifiedUser user) {
+ return newRefLogIdent(user, Instant.now(), ZoneId.systemDefault());
+ }
+
+ /**
+ * Creates a {@link PersonIdent} for the given user that should be used as the user identity in
+ * the reflog for ref updates done by this user or done on behalf of this user.
+ *
+ * @param user the user for which a reflog identity should be created
+ * @param when the timestamp that should be used to create the {@link PersonIdent}
+ * @param zoneId the zone ID identifying the timezone that should be used to create the {@link
+ * PersonIdent}
+ */
+ PersonIdent newRefLogIdent(IdentifiedUser user, Instant when, ZoneId zoneId);
+}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index f442500..f194434 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -194,14 +194,11 @@
import com.google.gerrit.server.project.SubmitRequirementConfigValidator;
import com.google.gerrit.server.project.SubmitRequirementsEvaluatorImpl;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
-import com.google.gerrit.server.query.FileEditsPredicate;
import com.google.gerrit.server.query.approval.ApprovalModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ConflictsCacheImpl;
-import com.google.gerrit.server.query.change.DistinctVotersPredicate;
-import com.google.gerrit.server.query.change.HasSubmoduleUpdatePredicate;
import com.google.gerrit.server.quota.QuotaEnforcer;
import com.google.gerrit.server.restapi.change.OnPostReview;
import com.google.gerrit.server.restapi.change.SuggestReviewers;
@@ -217,6 +214,9 @@
import com.google.gerrit.server.submit.MergeSuperSetComputation;
import com.google.gerrit.server.submit.SubmitStrategy;
import com.google.gerrit.server.submit.SubscriptionGraph;
+import com.google.gerrit.server.submitrequirement.predicate.DistinctVotersPredicate;
+import com.google.gerrit.server.submitrequirement.predicate.FileEditsPredicate;
+import com.google.gerrit.server.submitrequirement.predicate.HasSubmoduleUpdatePredicate;
import com.google.gerrit.server.tools.ToolsCatalog;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.util.IdGenerator;
diff --git a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
index 3f4c158..698628e 100644
--- a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
@@ -19,8 +19,12 @@
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryBuilder;
import com.google.gerrit.index.query.QueryParseException;
-import com.google.gerrit.server.query.FileEditsPredicate;
-import com.google.gerrit.server.query.FileEditsPredicate.FileEditsArgs;
+import com.google.gerrit.server.submitrequirement.predicate.ConstantPredicate;
+import com.google.gerrit.server.submitrequirement.predicate.DistinctVotersPredicate;
+import com.google.gerrit.server.submitrequirement.predicate.FileEditsPredicate;
+import com.google.gerrit.server.submitrequirement.predicate.FileEditsPredicate.FileEditsArgs;
+import com.google.gerrit.server.submitrequirement.predicate.HasSubmoduleUpdatePredicate;
+import com.google.gerrit.server.submitrequirement.predicate.RegexAuthorEmailPredicate;
import com.google.inject.Inject;
import java.util.List;
import java.util.Locale;
diff --git a/java/com/google/gerrit/server/query/change/ConstantPredicate.java b/java/com/google/gerrit/server/submitrequirement/predicate/ConstantPredicate.java
similarity index 85%
rename from java/com/google/gerrit/server/query/change/ConstantPredicate.java
rename to java/com/google/gerrit/server/submitrequirement/predicate/ConstantPredicate.java
index f0a85fe..c493fa4 100644
--- a/java/com/google/gerrit/server/query/change/ConstantPredicate.java
+++ b/java/com/google/gerrit/server/submitrequirement/predicate/ConstantPredicate.java
@@ -12,8 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.query.change;
+package com.google.gerrit.server.submitrequirement.predicate;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.SubmitRequirementPredicate;
import com.google.inject.Singleton;
/**
diff --git a/java/com/google/gerrit/server/query/change/DistinctVotersPredicate.java b/java/com/google/gerrit/server/submitrequirement/predicate/DistinctVotersPredicate.java
similarity index 95%
rename from java/com/google/gerrit/server/query/change/DistinctVotersPredicate.java
rename to java/com/google/gerrit/server/submitrequirement/predicate/DistinctVotersPredicate.java
index 5a51f5d..e392989 100644
--- a/java/com/google/gerrit/server/query/change/DistinctVotersPredicate.java
+++ b/java/com/google/gerrit/server/submitrequirement/predicate/DistinctVotersPredicate.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.query.change;
+package com.google.gerrit.server.submitrequirement.predicate;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
@@ -22,6 +22,8 @@
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.SubmitRequirementPredicate;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.Optional;
diff --git a/java/com/google/gerrit/server/query/FileEditsPredicate.java b/java/com/google/gerrit/server/submitrequirement/predicate/FileEditsPredicate.java
similarity index 98%
rename from java/com/google/gerrit/server/query/FileEditsPredicate.java
rename to java/com/google/gerrit/server/submitrequirement/predicate/FileEditsPredicate.java
index 7058765..515dc4a 100644
--- a/java/com/google/gerrit/server/query/FileEditsPredicate.java
+++ b/java/com/google/gerrit/server/submitrequirement/predicate/FileEditsPredicate.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.query;
+package com.google.gerrit.server.submitrequirement.predicate;
import com.google.auto.value.AutoValue;
import com.google.common.collect.Iterables;
diff --git a/java/com/google/gerrit/server/query/change/HasSubmoduleUpdatePredicate.java b/java/com/google/gerrit/server/submitrequirement/predicate/HasSubmoduleUpdatePredicate.java
similarity index 95%
rename from java/com/google/gerrit/server/query/change/HasSubmoduleUpdatePredicate.java
rename to java/com/google/gerrit/server/submitrequirement/predicate/HasSubmoduleUpdatePredicate.java
index 4ff40a4..1774628 100644
--- a/java/com/google/gerrit/server/query/change/HasSubmoduleUpdatePredicate.java
+++ b/java/com/google/gerrit/server/submitrequirement/predicate/HasSubmoduleUpdatePredicate.java
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.query.change;
+package com.google.gerrit.server.submitrequirement.predicate;
import static com.google.gerrit.server.query.change.SubmitRequirementChangeQueryBuilder.SUBMODULE_UPDATE_HAS_ARG;
@@ -23,6 +23,8 @@
import com.google.gerrit.server.patch.DiffOperations;
import com.google.gerrit.server.patch.DiffOptions;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.SubmitRequirementPredicate;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
diff --git a/java/com/google/gerrit/server/query/change/RegexAuthorEmailPredicate.java b/java/com/google/gerrit/server/submitrequirement/predicate/RegexAuthorEmailPredicate.java
similarity index 89%
rename from java/com/google/gerrit/server/query/change/RegexAuthorEmailPredicate.java
rename to java/com/google/gerrit/server/submitrequirement/predicate/RegexAuthorEmailPredicate.java
index 22891bc..eb7f666 100644
--- a/java/com/google/gerrit/server/query/change/RegexAuthorEmailPredicate.java
+++ b/java/com/google/gerrit/server/submitrequirement/predicate/RegexAuthorEmailPredicate.java
@@ -12,9 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.query.change;
+package com.google.gerrit.server.submitrequirement.predicate;
import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.SubmitRequirementPredicate;
import dk.brics.automaton.RegExp;
import dk.brics.automaton.RunAutomaton;
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index b828037..936b448 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -36,6 +36,7 @@
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.CacheRefreshExecutor;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.GerritPersonIdentProvider;
@@ -193,6 +194,7 @@
install(new AuthModule(authConfig));
install(new GerritApiModule());
install(new ProjectQueryBuilderModule());
+ install(new DefaultRefLogIdentityProvider.Module());
factory(PluginUser.Factory.class);
install(new PluginApiModule());
install(new DefaultPermissionBackendModule());
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
index d055875..62ef118 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
@@ -33,6 +33,7 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.RefLogIdentityProvider;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.DefaultRealm;
@@ -57,6 +58,7 @@
public class EmailIT extends AbstractDaemonTest {
@Inject private @AnonymousCowardName String anonymousCowardName;
+ @Inject private RefLogIdentityProvider refLogIdentityProvider;
@Inject private @CanonicalWebUrl Provider<String> canonicalUrl;
@Inject private @EnablePeerIPInReflogRecord boolean enablePeerIPInReflogRecord;
@Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
@@ -283,6 +285,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
enablePeerIPInReflogRecord,
accountCache,
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index fd5f6fc..3b97372 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -29,6 +29,7 @@
import com.google.common.truth.Correspondence;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.account.TestAccount;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.Account;
@@ -49,6 +50,7 @@
import com.google.inject.Inject;
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.junit.Test;
public class ChangeOperationsImplTest extends AbstractDaemonTest {
@@ -611,6 +613,155 @@
}
@Test
+ public void createdChangeHasOwnerAsAuthor() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ TestAccount changeOwner = accountOperations.account(Account.id(change.owner._accountId)).get();
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(changeOwner.fullname().get());
+ assertThat(revision.commit.author.email).isEqualTo(changeOwner.preferredEmail().get());
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedOwnerAsAuthor() throws Exception {
+ String changeOwnerName = "Change Owner";
+ String changeOwnerEmail = "change-owner@example.com";
+ Account.Id changeOwner =
+ accountOperations
+ .newAccount()
+ .fullname(changeOwnerName)
+ .preferredEmail(changeOwnerEmail)
+ .create();
+ Change.Id changeId = changeOperations.newChange().owner(changeOwner).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isEqualTo(changeOwner.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(changeOwnerName);
+ assertThat(revision.commit.author.email).isEqualTo(changeOwnerEmail);
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedAuthor() throws Exception {
+ String authorName = "Author";
+ String authorEmail = "author@example.com";
+ Account.Id author =
+ accountOperations.newAccount().fullname(authorName).preferredEmail(authorEmail).create();
+ Change.Id changeId = changeOperations.newChange().author(author).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isNotEqualTo(author.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorName);
+ assertThat(revision.commit.author.email).isEqualTo(authorEmail);
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedAuthorIdent() throws Exception {
+ PersonIdent authorIdent = new PersonIdent("Author", "author@example.com");
+ Change.Id changeId = changeOperations.newChange().authorIdent(authorIdent).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorIdent.getName());
+ assertThat(revision.commit.author.email).isEqualTo(authorIdent.getEmailAddress());
+ }
+
+ @Test
+ public void changeCannotBeCreatedWithAuthorAndAuthorIdent() throws Exception {
+ Account.Id author = accountOperations.newAccount().create();
+ PersonIdent authorIdent = new PersonIdent("Author", "author@example.com");
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () -> changeOperations.newChange().author(author).authorIdent(authorIdent).create());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("author and authorIdent cannot be set together");
+ }
+
+ @Test
+ public void createdChangeHasOwnerAsCommitter() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ TestAccount changeOwner = accountOperations.account(Account.id(change.owner._accountId)).get();
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(changeOwner.fullname().get());
+ assertThat(revision.commit.committer.email).isEqualTo(changeOwner.preferredEmail().get());
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedOwnerAsCommitter() throws Exception {
+ String changeOwnerName = "Change Owner";
+ String changeOwnerEmail = "change-owner@example.com";
+ Account.Id changeOwner =
+ accountOperations
+ .newAccount()
+ .fullname(changeOwnerName)
+ .preferredEmail(changeOwnerEmail)
+ .create();
+ Change.Id changeId = changeOperations.newChange().owner(changeOwner).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isEqualTo(changeOwner.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(changeOwnerName);
+ assertThat(revision.commit.committer.email).isEqualTo(changeOwnerEmail);
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedCommitter() throws Exception {
+ String committerName = "Committer";
+ String committerEmail = "committer@example.com";
+ Account.Id committer =
+ accountOperations
+ .newAccount()
+ .fullname(committerName)
+ .preferredEmail(committerEmail)
+ .create();
+ Change.Id changeId = changeOperations.newChange().committer(committer).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isNotEqualTo(committer.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerName);
+ assertThat(revision.commit.committer.email).isEqualTo(committerEmail);
+ }
+
+ @Test
+ public void createdChangeHasSpecifiedCommitterIdent() throws Exception {
+ PersonIdent committerIdent = new PersonIdent("Committer", "committer@example.com");
+ Change.Id changeId = changeOperations.newChange().committerIdent(committerIdent).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerIdent.getName());
+ assertThat(revision.commit.committer.email).isEqualTo(committerIdent.getEmailAddress());
+ }
+
+ @Test
+ public void changeCannotBeCreatedWithCommitterAndCommitterIdent() throws Exception {
+ Account.Id committer = accountOperations.newAccount().create();
+ PersonIdent committerIdent = new PersonIdent("Committer", "committer@example.com");
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .newChange()
+ .committer(committer)
+ .committerIdent(committerIdent)
+ .create());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("committer and committerIdent cannot be set together");
+ }
+
+ @Test
public void createdChangeHasSpecifiedTopic() throws Exception {
Change.Id changeId = changeOperations.newChange().topic("test-topic").create();
@@ -812,6 +963,156 @@
}
@Test
+ public void createdPatchsetPreviousAuthorAsAuthor() throws Exception {
+ String authorName = "Author";
+ String authorEmail = "author@example.com";
+ Account.Id author =
+ accountOperations.newAccount().fullname(authorName).preferredEmail(authorEmail).create();
+ Change.Id changeId = changeOperations.newChange().author(author).create();
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorName);
+ assertThat(revision.commit.author.email).isEqualTo(authorEmail);
+
+ changeOperations.change(changeId).newPatchset().create();
+ change = getChangeFromServer(changeId);
+ revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorName);
+ assertThat(revision.commit.author.email).isEqualTo(authorEmail);
+ }
+
+ @Test
+ public void createdPatchsetHasSpecifiedAuthor() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String authorName = "Author";
+ String authorEmail = "author@example.com";
+ Account.Id author =
+ accountOperations.newAccount().fullname(authorName).preferredEmail(authorEmail).create();
+ changeOperations.change(changeId).newPatchset().author(author).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isNotEqualTo(author.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorName);
+ assertThat(revision.commit.author.email).isEqualTo(authorEmail);
+ }
+
+ @Test
+ public void createdPatchsetHasSpecifiedAuthorIdent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ PersonIdent authorIdent = new PersonIdent("Author", "author@example.com");
+ changeOperations.change(changeId).newPatchset().authorIdent(authorIdent).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.author.name).isEqualTo(authorIdent.getName());
+ assertThat(revision.commit.author.email).isEqualTo(authorIdent.getEmailAddress());
+ }
+
+ @Test
+ public void patchsetCannotBeCreatedWithAuthorAndAuthorIdent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ Account.Id author = accountOperations.newAccount().create();
+ PersonIdent authorIdent = new PersonIdent("Author", "author@example.com");
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .author(author)
+ .authorIdent(authorIdent)
+ .create());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("author and authorIdent cannot be set together");
+ }
+
+ @Test
+ public void createdPatchsetPreviousCommitterAsCommitter() throws Exception {
+ String committerName = "Committer";
+ String committerEmail = "committer@example.com";
+ Account.Id committer =
+ accountOperations
+ .newAccount()
+ .fullname(committerName)
+ .preferredEmail(committerEmail)
+ .create();
+ Change.Id changeId = changeOperations.newChange().committer(committer).create();
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerName);
+ assertThat(revision.commit.committer.email).isEqualTo(committerEmail);
+
+ changeOperations.change(changeId).newPatchset().create();
+ change = getChangeFromServer(changeId);
+ revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerName);
+ assertThat(revision.commit.committer.email).isEqualTo(committerEmail);
+ }
+
+ @Test
+ public void createdPatchsetHasSpecifiedCommitter() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String committerName = "Committer";
+ String committerEmail = "committer@example.com";
+ Account.Id committer =
+ accountOperations
+ .newAccount()
+ .fullname(committerName)
+ .preferredEmail(committerEmail)
+ .create();
+ changeOperations.change(changeId).newPatchset().committer(committer).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ assertThat(change.owner._accountId).isNotEqualTo(committer.get());
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerName);
+ assertThat(revision.commit.committer.email).isEqualTo(committerEmail);
+ }
+
+ @Test
+ public void createdPatchsetHasSpecifiedCommitterIdent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ PersonIdent committerIdent = new PersonIdent("Committer", "committer@example.com");
+ changeOperations.change(changeId).newPatchset().committerIdent(committerIdent).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo revision = change.revisions.get(change.currentRevision);
+ assertThat(revision.commit.committer.name).isEqualTo(committerIdent.getName());
+ assertThat(revision.commit.committer.email).isEqualTo(committerIdent.getEmailAddress());
+ }
+
+ @Test
+ public void patchsetCannotBeCreatedWithCommitterAndCommitterIdent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ Account.Id committer = accountOperations.newAccount().create();
+ PersonIdent committerIdent = new PersonIdent("Committer", "committer@example.com");
+
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .committer(committer)
+ .committerIdent(committerIdent)
+ .create());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("committer and committerIdent cannot be set together");
+ }
+
+ @Test
public void newPatchsetCanHaveUpdatedCommitMessage() throws Exception {
Change.Id changeId = changeOperations.newChange().commitMessage("Old message").create();
diff --git a/javatests/com/google/gerrit/server/IdentifiedUserTest.java b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
index 855a0bc..30ae4aa 100644
--- a/javatests/com/google/gerrit/server/IdentifiedUserTest.java
+++ b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
@@ -92,6 +92,7 @@
bind(AccountCache.class).toInstance(accountCache);
bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
bind(Realm.class).toInstance(mockRealm);
+ install(new DefaultRefLogIdentityProvider.Module());
}
};
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index be8f1f9..1e6ba3a 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -31,6 +31,7 @@
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -180,6 +181,7 @@
install(new DefaultUrlFormatterModule());
install(NoteDbModule.forTest());
+ install(new DefaultRefLogIdentityProvider.Module());
bind(AllUsersName.class).toProvider(AllUsersNameProvider.class);
bind(String.class).annotatedWith(GerritServerId.class).toInstance(serverId);
bind(new TypeLiteral<ImmutableSet<String>>() {})
