Merge changes from topic "import-changes" into stable-3.7
* changes:
Resolve accounts by imported externalId
Introduce imported external ids for accounts from other servers
Allow the rendering of changes imported from other GerritServerIds
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 234389e..be6e140 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2543,6 +2543,26 @@
by the target version of the upgrade. Refer to the release notes and check whether
the rolling upgrade is possible or not and the associated constraints.
+[[gerrit.importedServerId]]gerrit.importedServerId::
++
+ServerId of the repositories imported from other Gerrit servers. Changes coming
+associated with the imported serverIds are indexed and displayed in the UI.
++
+Specify multiple `gerrit.importedServerId` for allowing the import from multiple
+Gerrit servers with different serverIds.
++
+[NOTE]
+The account-ids referenced in the imported changes are used for looking up the
+associated account-id locally, using the `imported:` external-id.
+Example: the account-id 1000 from the imported server-id 59a4964e-6376-4ed9-beef
+will be looked up in the local accounts using the `imported:1000@59a4964e-6376-4ed9-beef`
+external-id.
++
+If this value is not set, all changes imported from other Gerrit servers will be
+ignored.
++
+By default empty.
+
[[gerrit.serverId]]gerrit.serverId::
+
Used by NoteDb to, amongst other things, identify author identities from
diff --git a/java/com/google/gerrit/entities/Account.java b/java/com/google/gerrit/entities/Account.java
index 303e79f..85dbdeb 100644
--- a/java/com/google/gerrit/entities/Account.java
+++ b/java/com/google/gerrit/entities/Account.java
@@ -46,6 +46,10 @@
*/
@AutoValue
public abstract class Account {
+
+ /** Placeholder for indicating an account-id that does not correspond to any local account */
+ public static final Id UNKNOWN_ACCOUNT_ID = id(0);
+
public static Id id(int id) {
return new AutoValue_Account_Id(id);
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalId.java b/java/com/google/gerrit/server/account/externalids/ExternalId.java
index 0a51171..1616198 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalId.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalId.java
@@ -135,6 +135,9 @@
/** Scheme used for GPG public keys. */
public static final String SCHEME_GPGKEY = "gpgkey";
+ /** Scheme for imported accounts from other servers with different GerritServerId */
+ public static final String SCHEME_IMPORTED = "imported";
+
/** Scheme for external auth used during authentication, e.g. OAuth Token */
public static final String SCHEME_EXTERNAL = "external";
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
index 9f766d0..fe8feac 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
@@ -30,7 +30,7 @@
*
* <p>All returned collections are unmodifiable.
*/
-interface ExternalIdCache {
+public interface ExternalIdCache {
Optional<ExternalId> byKey(ExternalId.Key key) throws IOException;
ImmutableSet<ExternalId> byAccount(Account.Id accountId) throws IOException;
diff --git a/java/com/google/gerrit/server/config/GerritImportedServerIds.java b/java/com/google/gerrit/server/config/GerritImportedServerIds.java
new file mode 100644
index 0000000..c47d3be
--- /dev/null
+++ b/java/com/google/gerrit/server/config/GerritImportedServerIds.java
@@ -0,0 +1,29 @@
+// Copyright (C) 2022 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 java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+import java.lang.annotation.Retention;
+
+/**
+ * List of ServerIds of the Gerrit data imported from other servers.
+ *
+ * <p>This values correspond to the {@code GerritServerId} of other servers.
+ */
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface GerritImportedServerIds {}
diff --git a/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java b/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java
new file mode 100644
index 0000000..f3f7645
--- /dev/null
+++ b/java/com/google/gerrit/server/config/GerritImportedServerIdsProvider.java
@@ -0,0 +1,37 @@
+// Copyright (C) 2022 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 com.google.common.collect.ImmutableSet;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import org.eclipse.jgit.lib.Config;
+
+public class GerritImportedServerIdsProvider implements Provider<ImmutableSet<String>> {
+ public static final String SECTION = "gerrit";
+ public static final String KEY = "importedServerId";
+
+ private final ImmutableSet<String> importedIds;
+
+ @Inject
+ public GerritImportedServerIdsProvider(@GerritServerConfig Config cfg) {
+ importedIds = ImmutableSet.copyOf(cfg.getStringList(SECTION, null, KEY));
+ }
+
+ @Override
+ public ImmutableSet<String> get() {
+ return importedIds;
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
index 158972f..3757244 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
@@ -17,6 +17,7 @@
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Change;
@@ -24,6 +25,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritImportedServerIds;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
@@ -51,6 +53,7 @@
public final AllUsersName allUsers;
public final NoteDbMetrics metrics;
public final String serverId;
+ public final ImmutableSet<String> importedServerIds;
// Providers required to avoid dependency cycles.
@@ -64,7 +67,8 @@
ChangeNoteJson changeNoteJson,
NoteDbMetrics metrics,
Provider<ChangeNotesCache> cache,
- @GerritServerId String serverId) {
+ @GerritServerId String serverId,
+ @GerritImportedServerIds ImmutableSet<String> importedServerIds) {
this.failOnLoadForTest = new AtomicBoolean();
this.repoManager = repoManager;
this.allUsers = allUsers;
@@ -72,6 +76,7 @@
this.metrics = metrics;
this.cache = cache;
this.serverId = serverId;
+ this.importedServerIds = importedServerIds;
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 26d5933..31934d5 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -406,6 +406,10 @@
return state.metaId();
}
+ public String getServerId() {
+ return state.serverId();
+ }
+
public ImmutableSortedMap<PatchSet.Id, PatchSet> getPatchSets() {
if (patchSets == null) {
ImmutableSortedMap.Builder<PatchSet.Id, PatchSet> b =
@@ -655,7 +659,9 @@
* be to bump the cache version, but that would invalidate all persistent cache entries, what we
* rather try to avoid.
*/
- if (!Strings.isNullOrEmpty(stateServerId) && !args.serverId.equals(stateServerId)) {
+ if (!Strings.isNullOrEmpty(stateServerId)
+ && !args.serverId.equals(stateServerId)
+ && !args.importedServerIds.contains(stateServerId)) {
throw new InvalidServerIdException(args.serverId, stateServerId);
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 19044e6..0f2c877 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -26,6 +26,7 @@
import com.google.gerrit.proto.Protos;
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.cache.proto.Cache.ChangeNotesKeyProto;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
@@ -366,7 +367,13 @@
"Load change notes for change %s of project %s", key.changeId(), key.project());
ChangeNotesParser parser =
new ChangeNotesParser(
- key.changeId(), key.id(), walkSupplier.get(), args.changeNoteJson, args.metrics);
+ key.changeId(),
+ key.id(),
+ walkSupplier.get(),
+ args.changeNoteJson,
+ args.metrics,
+ args.serverId,
+ externalIdCache);
ChangeNotesState result = parser.parseAll();
// This assignment only happens if call() was actually called, which only
// happens when Cache#get(K, Callable<V>) incurs a cache miss.
@@ -377,11 +384,16 @@
private final Cache<Key, ChangeNotesState> cache;
private final Args args;
+ private final ExternalIdCache externalIdCache;
@Inject
- ChangeNotesCache(@Named(CACHE_NAME) Cache<Key, ChangeNotesState> cache, Args args) {
+ ChangeNotesCache(
+ @Named(CACHE_NAME) Cache<Key, ChangeNotesState> cache,
+ Args args,
+ ExternalIdCache externalIdCache) {
this.cache = cache;
this.args = args;
+ this.externalIdCache = externalIdCache;
}
Value get(
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 6e5e19c..f2a659d 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -79,6 +79,7 @@
import com.google.gerrit.server.ReviewerByEmailSet;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.gerrit.server.notedb.ChangeNoteUtil.ParsedPatchSetApproval;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
import com.google.gerrit.server.util.LabelVote;
@@ -199,18 +200,24 @@
// the latest record unsets the field).
private Optional<PatchSet.Id> cherryPickOf;
private Instant mergedOn;
+ private final ExternalIdCache externalIdCache;
+ private final String gerritServerId;
ChangeNotesParser(
Change.Id changeId,
ObjectId tip,
ChangeNotesRevWalk walk,
ChangeNoteJson changeNoteJson,
- NoteDbMetrics metrics) {
+ NoteDbMetrics metrics,
+ String gerritServerId,
+ ExternalIdCache externalIdCache) {
this.id = changeId;
this.tip = tip;
this.walk = walk;
this.changeNoteJson = changeNoteJson;
this.metrics = metrics;
+ this.externalIdCache = externalIdCache;
+ this.gerritServerId = gerritServerId;
approvals = new LinkedHashMap<>();
bufferedApprovals = new ArrayList<>();
reviewers = HashBasedTable.create();
@@ -1406,7 +1413,7 @@
}
private Account.Id parseIdent(PersonIdent ident) throws ConfigInvalidException {
- return NoteDbUtil.parseIdent(ident)
+ return NoteDbUtil.parseIdent(ident, gerritServerId, externalIdCache)
.orElseThrow(
() -> parseException("cannot retrieve account id: %s", ident.getEmailAddress()));
}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUtil.java b/java/com/google/gerrit/server/notedb/NoteDbUtil.java
index 396e29b..64bf430 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUtil.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUtil.java
@@ -20,8 +20,12 @@
import com.google.common.primitives.Ints;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
+import java.io.IOException;
import java.sql.Timestamp;
import java.util.Optional;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.util.GitDateFormatter;
import org.eclipse.jgit.util.GitDateFormatter.Format;
@@ -48,6 +52,45 @@
return Optional.empty();
}
+ /**
+ * Returns an AccountId for the given email address and the current serverId. Reverse lookup the
+ * AccountId using the ExternalIdCache if the account has a foreign serverId.
+ *
+ * @param ident the accountId@serverId identity
+ * @param serverId the Gerrit's serverId
+ * @param externalIdCache reference to the cache for looking up the external ids
+ * @return a defined accountId if the account was found, {@link Account#UNKNOWN_ACCOUNT_ID} if the
+ * lookup via external-id did not return any account, or an empty value if the identity was
+ * malformed.
+ * @throws ConfigInvalidException when the lookup of the external-id failed
+ */
+ public static Optional<Account.Id> parseIdent(
+ PersonIdent ident, String serverId, ExternalIdCache externalIdCache)
+ throws ConfigInvalidException {
+ String email = ident.getEmailAddress();
+ int at = email.indexOf('@');
+ if (at >= 0) {
+ Integer id = Ints.tryParse(email.substring(0, at));
+ String accountServerId = email.substring(at + 1);
+ if (id != null) {
+ if (accountServerId.equals(serverId)) {
+ return Optional.of(Account.id(id));
+ }
+
+ ExternalId.Key extIdKey = ExternalId.Key.create(ExternalId.SCHEME_IMPORTED, email, false);
+ try {
+ return externalIdCache
+ .byKey(extIdKey)
+ .map(ExternalId::accountId)
+ .or(() -> Optional.of(Account.UNKNOWN_ACCOUNT_ID));
+ } catch (IOException e) {
+ throw new ConfigInvalidException("Unable to lookup external id from cache", e);
+ }
+ }
+ }
+ return Optional.empty();
+ }
+
public static String extractHostPartFromPersonIdent(PersonIdent ident) {
String email = ident.getEmailAddress();
int at = email.indexOf('@');
diff --git a/java/com/google/gerrit/server/schema/SchemaModule.java b/java/com/google/gerrit/server/schema/SchemaModule.java
index ff2073d..e0e64a3 100644
--- a/java/com/google/gerrit/server/schema/SchemaModule.java
+++ b/java/com/google/gerrit/server/schema/SchemaModule.java
@@ -16,6 +16,7 @@
import static com.google.inject.Scopes.SINGLETON;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.GerritPersonIdentProvider;
@@ -25,9 +26,12 @@
import com.google.gerrit.server.config.AllUsersNameProvider;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
+import com.google.gerrit.server.config.GerritImportedServerIds;
+import com.google.gerrit.server.config.GerritImportedServerIdsProvider;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.config.GerritServerIdProvider;
import com.google.gerrit.server.index.group.GroupIndexCollection;
+import com.google.inject.TypeLiteral;
import org.eclipse.jgit.lib.PersonIdent;
/** Bindings for low-level Gerrit schema data. */
@@ -51,6 +55,11 @@
.toProvider(GerritServerIdProvider.class)
.in(SINGLETON);
+ bind(new TypeLiteral<ImmutableSet<String>>() {})
+ .annotatedWith(GerritImportedServerIds.class)
+ .toProvider(GerritImportedServerIdsProvider.class)
+ .in(SINGLETON);
+
// It feels wrong to have this binding in a seemingly unrelated module, but it's a dependency of
// SchemaCreatorImpl, so it's needed.
// TODO(dborowitz): Is there any way to untangle this?
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index b00cadb..b1ab6b1 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -19,6 +19,7 @@
import static com.google.inject.Scopes.SINGLETON;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperationsImpl;
@@ -61,6 +62,8 @@
import com.google.gerrit.server.config.FileBasedAllProjectsConfigProvider;
import com.google.gerrit.server.config.FileBasedGlobalPluginConfigProvider;
import com.google.gerrit.server.config.GerritGlobalModule;
+import com.google.gerrit.server.config.GerritImportedServerIds;
+import com.google.gerrit.server.config.GerritImportedServerIdsProvider;
import com.google.gerrit.server.config.GerritInstanceIdModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
import com.google.gerrit.server.config.GerritOptions;
@@ -311,6 +314,17 @@
@Provides
@Singleton
+ @GerritImportedServerIds
+ public ImmutableSet<String> createImportedServerIds() {
+ ImmutableSet<String> serverIds =
+ ImmutableSet.copyOf(
+ cfg.getStringList(
+ GerritServerIdProvider.SECTION, null, GerritImportedServerIdsProvider.KEY));
+ return serverIds;
+ }
+
+ @Provides
+ @Singleton
@SendEmailExecutor
public ExecutorService createSendEmailExecutor() {
return newDirectExecutorService();
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index d7c779e..1b286d1 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -18,6 +18,7 @@
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
@@ -39,6 +40,8 @@
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.Realm;
import com.google.gerrit.server.account.ServiceUserClassifier;
+import com.google.gerrit.server.account.externalids.DisabledExternalIdCache;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.gerrit.server.approval.PatchSetApprovalUuidGenerator;
import com.google.gerrit.server.approval.testing.TestPatchSetApprovalUuidGenerator;
import com.google.gerrit.server.config.AllUsersName;
@@ -48,11 +51,13 @@
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.DefaultUrlFormatter.DefaultUrlFormatterModule;
import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
+import com.google.gerrit.server.config.GerritImportedServerIds;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitModule;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.RepositoryCaseMismatchException;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.NullProjectCache;
import com.google.gerrit.server.project.ProjectCache;
@@ -67,9 +72,12 @@
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
+import com.google.inject.Module;
+import com.google.inject.TypeLiteral;
import java.time.Instant;
import java.time.ZoneId;
import java.util.concurrent.ExecutorService;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
@@ -84,10 +92,13 @@
@Ignore
@RunWith(ConfigSuite.class)
public abstract class AbstractChangeNotesTest {
+ protected static final String LOCAL_SERVER_ID = "gerrit";
+
private static final ZoneId ZONE_ID = ZoneId.of("America/Los_Angeles");
@ConfigSuite.Parameter public Config testConfig;
+ protected Account.Id changeOwnerId;
protected Account.Id otherUserId;
protected FakeAccountCache accountCache;
protected IdentifiedUser changeOwner;
@@ -111,11 +122,21 @@
@Inject @GerritServerId protected String serverId;
+ @Inject protected ExternalIdCache externalIdCache;
+
protected Injector injector;
private String systemTimeZone;
@Before
public void setUpTestEnvironment() throws Exception {
+ setupTestPrerequisites();
+
+ injector = createTestInjector(LOCAL_SERVER_ID);
+ createAllUsers(injector);
+ injector.injectMembers(this);
+ }
+
+ protected void setupTestPrerequisites() throws Exception {
setTimeForTesting();
serverIdent = new PersonIdent("Gerrit Server", "noreply@gerrit.com", TimeUtil.now(), ZONE_ID);
@@ -134,60 +155,77 @@
ou.setPreferredEmail("other@account.com");
accountCache.put(ou.build());
assertableFanOutExecutor = new AssertableExecutorService();
-
- injector =
- Guice.createInjector(
- new FactoryModule() {
- @Override
- public void configure() {
- install(new GitModule());
-
- install(new DefaultUrlFormatterModule());
- install(NoteDbModule.forTest());
- bind(AllUsersName.class).toProvider(AllUsersNameProvider.class);
- bind(String.class).annotatedWith(GerritServerId.class).toInstance("gerrit");
- bind(GitRepositoryManager.class).toInstance(repoManager);
- bind(ProjectCache.class).to(NullProjectCache.class);
- bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(testConfig);
- bind(String.class)
- .annotatedWith(AnonymousCowardName.class)
- .toProvider(AnonymousCowardNameProvider.class);
- bind(String.class)
- .annotatedWith(CanonicalWebUrl.class)
- .toInstance("http://localhost:8080/");
- bind(Boolean.class)
- .annotatedWith(EnablePeerIPInReflogRecord.class)
- .toInstance(Boolean.FALSE);
- bind(Realm.class).to(FakeRealm.class);
- bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
- bind(AccountCache.class).toInstance(accountCache);
- bind(PersonIdent.class)
- .annotatedWith(GerritPersonIdent.class)
- .toInstance(serverIdent);
- bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
- bind(MetricMaker.class).to(DisabledMetricMaker.class);
- bind(ExecutorService.class)
- .annotatedWith(FanOutExecutor.class)
- .toInstance(assertableFanOutExecutor);
- bind(ServiceUserClassifier.class).to(ServiceUserClassifier.NoOp.class);
- bind(InternalChangeQuery.class)
- .toProvider(
- () -> {
- throw new UnsupportedOperationException();
- });
- bind(PatchSetApprovalUuidGenerator.class)
- .to(TestPatchSetApprovalUuidGenerator.class);
- }
- });
-
- injector.injectMembers(this);
- repoManager.createRepository(allUsers);
- changeOwner = userFactory.create(co.id());
- otherUser = userFactory.create(ou.id());
- otherUserId = otherUser.getAccountId();
+ changeOwnerId = co.id();
+ otherUserId = ou.id();
internalUser = new InternalUser();
}
+ protected Injector createTestInjector(String serverId, String... importedServerIds)
+ throws Exception {
+ return createTestInjector(DisabledExternalIdCache.module(), serverId, importedServerIds);
+ }
+
+ protected Injector createTestInjector(
+ Module extraGuiceModule, String serverId, String... importedServerIds) throws Exception {
+
+ return Guice.createInjector(
+ new FactoryModule() {
+ @Override
+ public void configure() {
+ install(extraGuiceModule);
+ install(new GitModule());
+
+ install(new DefaultUrlFormatterModule());
+ install(NoteDbModule.forTest());
+ bind(AllUsersName.class).toProvider(AllUsersNameProvider.class);
+ bind(String.class).annotatedWith(GerritServerId.class).toInstance(serverId);
+ bind(new TypeLiteral<ImmutableSet<String>>() {})
+ .annotatedWith(GerritImportedServerIds.class)
+ .toInstance(new ImmutableSet.Builder<String>().add(importedServerIds).build());
+ bind(GitRepositoryManager.class).toInstance(repoManager);
+ bind(ProjectCache.class).to(NullProjectCache.class);
+ bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(testConfig);
+ bind(String.class)
+ .annotatedWith(AnonymousCowardName.class)
+ .toProvider(AnonymousCowardNameProvider.class);
+ bind(String.class)
+ .annotatedWith(CanonicalWebUrl.class)
+ .toInstance("http://localhost:8080/");
+ bind(Boolean.class)
+ .annotatedWith(EnablePeerIPInReflogRecord.class)
+ .toInstance(Boolean.FALSE);
+ bind(Realm.class).to(FakeRealm.class);
+ bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
+ bind(AccountCache.class).toInstance(accountCache);
+ bind(PersonIdent.class).annotatedWith(GerritPersonIdent.class).toInstance(serverIdent);
+ bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
+ bind(MetricMaker.class).to(DisabledMetricMaker.class);
+ bind(ExecutorService.class)
+ .annotatedWith(FanOutExecutor.class)
+ .toInstance(assertableFanOutExecutor);
+ bind(ServiceUserClassifier.class).to(ServiceUserClassifier.NoOp.class);
+ bind(InternalChangeQuery.class)
+ .toProvider(
+ () -> {
+ throw new UnsupportedOperationException();
+ });
+ bind(PatchSetApprovalUuidGenerator.class).to(TestPatchSetApprovalUuidGenerator.class);
+ }
+ });
+ }
+
+ protected void createAllUsers(Injector injector)
+ throws RepositoryCaseMismatchException, RepositoryNotFoundException {
+ AllUsersName allUsersName = injector.getInstance(AllUsersName.class);
+
+ repoManager.createRepository(allUsersName);
+
+ IdentifiedUser.GenericFactory identifiedUserFactory =
+ injector.getInstance(IdentifiedUser.GenericFactory.class);
+ changeOwner = identifiedUserFactory.create(changeOwnerId);
+ otherUser = identifiedUserFactory.create(otherUserId);
+ }
+
private void setTimeForTesting() {
systemTimeZone = System.setProperty("user.timezone", "US/Eastern");
TestTimeUtil.resetWithClockStep(1, SECONDS);
@@ -200,8 +238,12 @@
}
protected Change newChange(boolean workInProgress) throws Exception {
+ return newChange(injector, workInProgress);
+ }
+
+ protected Change newChange(Injector injector, boolean workInProgress) throws Exception {
Change c = TestChanges.newChange(project, changeOwner.getAccountId());
- ChangeUpdate u = newUpdateForNewChange(c, changeOwner);
+ ChangeUpdate u = newUpdate(injector, c, changeOwner, false);
u.setChangeId(c.getKey().get());
u.setBranch(c.getDest().branch());
u.setWorkInProgress(workInProgress);
@@ -218,15 +260,20 @@
}
protected ChangeUpdate newUpdateForNewChange(Change c, CurrentUser user) throws Exception {
- return newUpdate(c, user, false);
+ return newUpdate(injector, c, user, false);
}
protected ChangeUpdate newUpdate(Change c, CurrentUser user) throws Exception {
- return newUpdate(c, user, true);
+ return newUpdate(injector, c, user, true);
}
protected ChangeUpdate newUpdate(Change c, CurrentUser user, boolean shouldExist)
throws Exception {
+ return newUpdate(injector, c, user, shouldExist);
+ }
+
+ protected ChangeUpdate newUpdate(
+ Injector injector, Change c, CurrentUser user, boolean shouldExist) throws Exception {
ChangeUpdate update = TestChanges.newUpdate(injector, c, user, shouldExist);
update.setPatchSetId(c.currentPatchSetId());
update.setAllowWriteToNewRef(true);
@@ -237,6 +284,10 @@
return new ChangeNotes(args, c, true, null).load();
}
+ protected ChangeNotes newNotes(AbstractChangeNotes.Args cArgs, Change c) {
+ return new ChangeNotes(cArgs, c, true, null).load();
+ }
+
protected static SubmitRecord submitRecord(
String status, String errorMessage, SubmitRecord.Label... labels) {
SubmitRecord rec = new SubmitRecord();
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java
index 666b8fc..9445f4a 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesCommitTest.java
@@ -98,7 +98,8 @@
private ChangeNotesParser newParser(ObjectId tip) throws Exception {
walk.reset();
ChangeNoteJson changeNoteJson = injector.getInstance(ChangeNoteJson.class);
- return new ChangeNotesParser(newChange().getId(), tip, walk, changeNoteJson, args.metrics);
+ return new ChangeNotesParser(
+ newChange().getId(), tip, walk, changeNoteJson, args.metrics, serverId, externalIdCache);
}
private RevCommit writeCommit(String body) throws Exception {
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index 276f093..4543b50 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -201,11 +201,11 @@
+ "Branch: refs/heads/master\n"
+ "Change-id: I577fb248e474018276351785930358ec0450e9f7\n"
+ "Patch-set: 1\n"
- + "Copied-Label: Label1=+1 Account <1@gerrit>,Other Account <2@Gerrit>\n"
+ + "Copied-Label: Label1=+1 Account <1@gerrit>,Other Account <2@gerrit>\n"
+ "Copied-Label: Label2=+1 Account <1@gerrit>\n"
- + "Copied-Label: Label3=+1 Account <1@gerrit>,Other Account <2@Gerrit> :\"tag\"\n"
- + "Copied-Label: Label4=+1 Account <1@Gerrit> :\"tag with characters %^#@^( *::!\"\n"
- + "Copied-Label: -Label1 Account <1@gerrit>,Other Account <2@Gerrit>\\n"
+ + "Copied-Label: Label3=+1 Account <1@gerrit>,Other Account <2@gerrit> :\"tag\"\n"
+ + "Copied-Label: Label4=+1 Account <1@gerrit> :\"tag with characters %^#@^( *::!\"\n"
+ + "Copied-Label: -Label1 Account <1@gerrit>,Other Account <2@gerrit>\\n"
+ "Copied-Label: -Label1 Account <1@gerrit>\n"
+ "Subject: This is a test change\n");
@@ -782,6 +782,7 @@
private ChangeNotesParser newParser(ObjectId tip) throws Exception {
walk.reset();
ChangeNoteJson changeNoteJson = injector.getInstance(ChangeNoteJson.class);
- return new ChangeNotesParser(newChange().getId(), tip, walk, changeNoteJson, args.metrics);
+ return new ChangeNotesParser(
+ newChange().getId(), tip, walk, changeNoteJson, args.metrics, serverId, externalIdCache);
}
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 8def660..4edfa8b4 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -2084,7 +2084,13 @@
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithComments =
new ChangeNotesParser(
- c.getId(), commitWithComments.copy(), rw, changeNoteJson, args.metrics);
+ c.getId(),
+ commitWithComments.copy(),
+ rw,
+ changeNoteJson,
+ args.metrics,
+ serverId,
+ externalIdCache);
ChangeNotesState state = notesWithComments.parseAll();
assertThat(state.approvals()).isEmpty();
assertThat(state.publishedComments()).hasSize(1);
@@ -2093,7 +2099,13 @@
try (ChangeNotesRevWalk rw = ChangeNotesCommit.newRevWalk(repo)) {
ChangeNotesParser notesWithApprovals =
new ChangeNotesParser(
- c.getId(), commitWithApprovals.copy(), rw, changeNoteJson, args.metrics);
+ c.getId(),
+ commitWithApprovals.copy(),
+ rw,
+ changeNoteJson,
+ args.metrics,
+ serverId,
+ externalIdCache);
ChangeNotesState state = notesWithApprovals.parseAll();
assertThat(state.approvals()).hasSize(1);
diff --git a/javatests/com/google/gerrit/server/notedb/ImportedChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ImportedChangeNotesTest.java
new file mode 100644
index 0000000..bb49a6d
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/ImportedChangeNotesTest.java
@@ -0,0 +1,114 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdCache;
+import com.google.gerrit.server.git.RepositoryCaseMismatchException;
+import com.google.inject.AbstractModule;
+import java.util.Optional;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ImportedChangeNotesTest extends AbstractChangeNotesTest {
+
+ private static final String FOREIGN_SERVER_ID = "foreign-server-id";
+ private static final String IMPORTED_SERVER_ID = "gerrit-imported-1";
+
+ private ExternalIdCache externalIdCacheMock;
+
+ @Before
+ @Override
+ public void setUpTestEnvironment() throws Exception {
+ setupTestPrerequisites();
+ }
+
+ private void initServerIds(String serverId, String... importedServerIds)
+ throws Exception, RepositoryCaseMismatchException, RepositoryNotFoundException {
+ externalIdCacheMock = mock(ExternalIdCache.class);
+ injector =
+ createTestInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(ExternalIdCache.class).toInstance(externalIdCacheMock);
+ }
+ },
+ serverId,
+ importedServerIds);
+ injector.injectMembers(this);
+ createAllUsers(injector);
+ }
+
+ @Test
+ public void allowChangeFromImportedServerId() throws Exception {
+ initServerIds(LOCAL_SERVER_ID, IMPORTED_SERVER_ID);
+
+ ExternalId.Key importedAccountIdKey =
+ ExternalId.Key.create(
+ ExternalId.SCHEME_IMPORTED,
+ changeOwner.getAccountId() + "@" + IMPORTED_SERVER_ID,
+ false);
+ ExternalId importedAccountId =
+ ExternalId.create(importedAccountIdKey, changeOwner.getAccountId(), null, null, null);
+
+ when(externalIdCacheMock.byKey(eq(importedAccountIdKey)))
+ .thenReturn(Optional.of(importedAccountId));
+
+ Change importedChange = newChange(createTestInjector(IMPORTED_SERVER_ID), false);
+ Change localChange = newChange();
+
+ assertThat(newNotes(importedChange).getServerId()).isEqualTo(IMPORTED_SERVER_ID);
+ assertThat(newNotes(localChange).getServerId()).isEqualTo(LOCAL_SERVER_ID);
+ }
+
+ @Test
+ public void rejectChangeWithForeignServerId() throws Exception {
+ initServerIds(LOCAL_SERVER_ID);
+ when(externalIdCacheMock.byKey(any())).thenReturn(Optional.empty());
+
+ Change foreignChange = newChange(createTestInjector(FOREIGN_SERVER_ID), false);
+
+ InvalidServerIdException invalidServerIdEx =
+ assertThrows(InvalidServerIdException.class, () -> newNotes(foreignChange));
+
+ String invalidServerIdMessage = invalidServerIdEx.getMessage();
+ assertThat(invalidServerIdMessage).contains("expected " + LOCAL_SERVER_ID);
+ assertThat(invalidServerIdMessage).contains("actual: " + FOREIGN_SERVER_ID);
+ }
+
+ @Test
+ public void changeFromImportedServerIdWithUnknownAccountId() throws Exception {
+ initServerIds(LOCAL_SERVER_ID, IMPORTED_SERVER_ID);
+
+ when(externalIdCacheMock.byKey(any())).thenReturn(Optional.empty());
+
+ Change importedChange = newChange(createTestInjector(IMPORTED_SERVER_ID), false);
+ assertThat(newNotes(importedChange).getServerId()).isEqualTo(IMPORTED_SERVER_ID);
+
+ assertThat(newNotes(importedChange).getChange().getOwner())
+ .isEqualTo(Account.UNKNOWN_ACCOUNT_ID);
+ }
+}
