Merge branch 'stable-3.11' into stable-3.12 * stable-3.11: Revert "Invoke changeServerId() function when calculating virtual Id" Fix ref existance check in CREATE ReceiveCommits Find changes by change number only if imported server IDs are configured AbtractFakeIndex: use changenumber instead of legacy_is_str Update git submodules Revert "Lookup imported change by change number in ChangeFinder::find" Update git submodules Set version to 3.11.3-SNAPSHOT Set version to 3.11.2 Set version to 3.10.6-SNAPSHOT Set version to 3.10.5 Set version to 3.9.11-SNAPSHOT Set version to 3.9.10 Release-Notes: skip Change-Id: I60dc871e7ad72057f5f35b289995842820c5953a
diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java index b7b3835..143bbcd 100644 --- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java +++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -15,6 +15,7 @@ package com.google.gerrit.index.testing; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.gerrit.server.index.change.ChangeField.CHANGENUM_SPEC; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; @@ -270,7 +271,7 @@ @Override protected Change.Id keyFor(ChangeData value) { - return value.getId(); + return value.virtualId(); } @Override @@ -300,11 +301,16 @@ @Override protected ChangeData valueFor(Map<String, Object> doc) { + int legacyId = Integer.parseInt((String) doc.get(ChangeField.NUMERIC_ID_STR_SPEC.getName())); ChangeData cd = changeDataFactory.create( Project.nameKey((String) doc.get(ChangeField.PROJECT_SPEC.getName())), Change.id( - Integer.valueOf((String) doc.get(ChangeField.NUMERIC_ID_STR_SPEC.getName())))); + doc.get(CHANGENUM_SPEC.getName()) != null + ? (Integer) doc.get(CHANGENUM_SPEC.getName()) + : legacyId)); + cd.setVirtualId(legacyId); + for (SchemaField<ChangeData, ?> field : getSchema().getSchemaFields().values()) { boolean isProtoField = SchemaFieldDefs.isProtoField(field);
diff --git a/java/com/google/gerrit/server/change/ChangeFinder.java b/java/com/google/gerrit/server/change/ChangeFinder.java index 5350a64..d158649 100644 --- a/java/com/google/gerrit/server/change/ChangeFinder.java +++ b/java/com/google/gerrit/server/change/ChangeFinder.java
@@ -31,11 +31,11 @@ import com.google.gerrit.metrics.Field; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.server.cache.CacheModule; +import com.google.gerrit.server.config.GerritImportedServerIds; import com.google.gerrit.server.logging.Metadata; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.change.ChangeData; -import com.google.gerrit.server.query.change.ChangeNumberVirtualIdAlgorithm; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.inject.Inject; import com.google.inject.Module; @@ -74,21 +74,22 @@ } private final IndexConfig indexConfig; + private final boolean hasImportedChanges; private final Cache<Change.Id, String> changeIdProjectCache; private final Provider<InternalChangeQuery> queryProvider; private final ChangeNotes.Factory changeNotesFactory; private final Counter1<ChangeIdType> changeIdCounter; - private final ChangeNumberVirtualIdAlgorithm virtualIdAlgorithm; @Inject ChangeFinder( IndexConfig indexConfig, + @GerritImportedServerIds ImmutableList<String> importedServerIds, @Named(CACHE_NAME) Cache<Change.Id, String> changeIdProjectCache, Provider<InternalChangeQuery> queryProvider, ChangeNotes.Factory changeNotesFactory, - MetricMaker metricMaker, - ChangeNumberVirtualIdAlgorithm virtualIdAlgorithm) { + MetricMaker metricMaker) { this.indexConfig = indexConfig; + this.hasImportedChanges = !importedServerIds.isEmpty(); this.changeIdProjectCache = changeIdProjectCache; this.queryProvider = queryProvider; this.changeNotesFactory = changeNotesFactory; @@ -101,7 +102,6 @@ Field.ofEnum(ChangeIdType.class, "change_id_type", Metadata.Builder::changeIdType) .description("The type of the change identifier.") .build()); - this.virtualIdAlgorithm = virtualIdAlgorithm; } public Optional<ChangeNotes> findOne(String id) { @@ -227,10 +227,8 @@ // Use the index to search for changes, but don't return any stored fields, // to force rereading in case the index is stale. InternalChangeQuery query = queryProvider.get().noFields(); - List<ChangeData> r = - virtualIdAlgorithm.isVirtualChangeId(id) - ? query.byChangeNumber(id) - : query.byLegacyChangeId(id); + List<ChangeData> r = hasImportedChanges ? query.byChangeNumber(id) : query.byLegacyChangeId(id); + if (r.size() == 1) { changeIdProjectCache.put(id, Url.encode(r.get(0).project().get())); }
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java index bd9218e..a89b356 100644 --- a/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -681,11 +681,16 @@ public Change.Id virtualId() { if (virtualId == null) { - return virtualIdFunc == null ? legacyId : virtualIdFunc.apply(changeServerId(), legacyId); + return virtualIdFunc == null ? legacyId : virtualIdFunc.apply(changeServerId, legacyId); } return virtualId; } + @VisibleForTesting + public void setVirtualId(int virtualId) { + this.virtualId = Change.id(virtualId); + } + public Project.NameKey project() { return project; }
diff --git a/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java b/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java index 6483df7..ecd1fe2 100644 --- a/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java +++ b/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java
@@ -43,7 +43,6 @@ Integer.BYTES * 8 - CHANGE_NUM_BIT_LEN; // Allows up to 64 ServerIds private final ImmutableMap<String, Integer> serverIdCodes; - private final boolean hasServerIdCodes; private final String localServerId; @Inject @@ -62,7 +61,6 @@ } serverIdCodes = serverIdCodesBuilder.build(); - this.hasServerIdCodes = !serverIdCodes.isEmpty(); this.localServerId = localServerId; } @@ -93,9 +91,4 @@ return Change.id(virtualId); } } - - @Override - public boolean isVirtualChangeId(Change.Id id) { - return hasServerIdCodes && ((id.get() & ~LEGACY_ID_BIT_MASK) != 0); - } }
diff --git a/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java b/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java index 4ab4ad1..6daf16f 100644 --- a/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java +++ b/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java
@@ -33,12 +33,4 @@ * @return virtual id which combines serverId and legacyChangeNum together */ Change.Id apply(String serverId, Change.Id legacyChangeNum); - - /** - * Check if a given change id is a virtual one - * - * @param id to be checked - * @return `true` when it is - */ - boolean isVirtualChangeId(Change.Id id); }
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index 9b85582..dac0e97 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -24,6 +24,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Enums; import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -74,6 +75,7 @@ import com.google.gerrit.server.change.MergeabilityComputationBehavior; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.config.GerritImportedServerIds; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.HasOperandAliasConfig; import com.google.gerrit.server.config.OperatorAliasConfig; @@ -124,6 +126,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuilder> { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private boolean hasImportedChanges; + public interface ChangeOperatorFactory extends OperatorFactory<ChangeData, ChangeQueryBuilder> {} /** @@ -521,14 +525,27 @@ protected static final Splitter LABEL_SPLITTER = Splitter.on(","); @Inject + protected ChangeQueryBuilder( + Arguments args, @GerritImportedServerIds ImmutableList<String> importedServerIds) { + this(mydef, args, importedServerIds); + } + protected ChangeQueryBuilder(Arguments args) { - this(mydef, args); + this(args, ImmutableList.of()); + } + + protected ChangeQueryBuilder(Definition<ChangeData, ChangeQueryBuilder> def, Arguments args) { + this(def, args, ImmutableList.of()); } @VisibleForTesting - protected ChangeQueryBuilder(Definition<ChangeData, ChangeQueryBuilder> def, Arguments args) { + protected ChangeQueryBuilder( + Definition<ChangeData, ChangeQueryBuilder> def, + Arguments args, + @GerritImportedServerIds ImmutableList<String> importedServerIds) { super(def, args.opFactories); this.args = args; + this.hasImportedChanges = !importedServerIds.isEmpty(); setupAliases(); } @@ -593,10 +610,14 @@ return getPredicateChangeData(query, changeId -> ChangePredicates.changeNumber(changeId, args)); } - // Keep using the index legacy document-id (legacy_id_str) for URLs queries like: "/q/123456", - // "/q/Iasdw2312321", "/q/project~123456" that are expecting to always find a single element. private Predicate<ChangeData> defaultSearch(String query) throws QueryParseException { - return getPredicateChangeData(query, ChangePredicates::idStr); + + return getPredicateChangeData( + query, + changeId -> + hasImportedChanges + ? ChangePredicates.changeNumber(changeId, args) + : ChangePredicates.idStr(changeId)); } private Predicate<ChangeData> getPredicateChangeData(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java index 25075b3..83f36f9 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -122,6 +122,7 @@ import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.NotifyInfo; import com.google.gerrit.extensions.api.changes.RecipientType; +import com.google.gerrit.extensions.api.changes.RelatedChangesInfo; import com.google.gerrit.extensions.api.changes.RevertInput; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling; @@ -150,6 +151,7 @@ import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.common.CommitInfo; import com.google.gerrit.extensions.common.CommitMessageInfo; +import com.google.gerrit.extensions.common.FileInfo; import com.google.gerrit.extensions.common.LabelInfo; import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.extensions.common.TrackingIdInfo; @@ -215,9 +217,13 @@ import java.util.stream.Stream; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.RefUpdate.Result; import org.eclipse.jgit.lib.Repository; @@ -4898,6 +4904,120 @@ assertThat(result.changeInfo.currentRevision).isNotNull(); } + @Test + @GerritConfig(name = "gerrit.importedServerId", value = "imported-server-id") + public void searchByChangeNumberOnlyReturnsImportedChanges() throws Exception { + PushOneCommit.Result change = createImportedChange("foo.txt"); + + Change.Id id = change.getChange().getId(); + String idAsString = id.toString(); + List<String> r = query(idAsString).stream().map(ci -> ci.changeId).toList(); + assertThat(r).containsExactly(change.getChangeId()); + } + + @Test + @GerritConfig(name = "gerrit.importedServerId", value = "imported-server-id") + public void searchByChangeNumberPredicateReturnsImportedChanges() throws Exception { + PushOneCommit.Result change = createImportedChange("foo.txt"); + + Change.Id id = change.getChange().getId(); + String idAsString = id.toString(); + List<String> r = query("change:" + idAsString).stream().map(ci -> ci.changeId).toList(); + assertThat(r).containsExactly(change.getChangeId()); + } + + @Test + @GerritConfig(name = "gerrit.importedServerId", value = "imported-server-id") + public void changesCollectionReturnsImportedChanges() throws Exception { + PushOneCommit.Result change = createImportedChange("foo.txt"); + + String idAsString = change.getChange().getId().toString(); + assertThat(gApi.changes().id(idAsString)).isNotNull(); + } + + @Test + @GerritConfig(name = "gerrit.importedServerId", value = "imported-server-id") + public void changesCollectionFilesReturnsImportedChanges() throws Exception { + String fileName = "foo.txt"; + PushOneCommit.Result change = createImportedChange(fileName); + + String idAsString = change.getChange().getId().toString(); + + Map<String, FileInfo> files = + gApi.changes().id(idAsString).revision(change.getPatchSet().number()).files(); + assertThat(files.keySet()).containsExactly(fileName, "/COMMIT_MSG"); + } + + @Test + @GerritConfig(name = "gerrit.importedServerId", value = "imported-server-id") + public void changesCollectionRelatedReturnsImportedChanges() throws Exception { + String fileName = "foo.txt"; + PushOneCommit.Result change = createImportedChange(fileName); + + String idAsString = change.getChange().getId().toString(); + + RelatedChangesInfo related = + gApi.changes().id(idAsString).revision(change.getPatchSet().number()).related(); + assertThat(related).isNotNull(); + assertThat(related.changes.size()).isEqualTo(0); + } + + private PushOneCommit.Result createImportedChange(String fileName) throws Exception { + PushOneCommit.Result change = createChange("subject", fileName, "test content"); + Change.Id changeId = change.getChange().getId(); + String metaRef = changeMetaRef(changeId); + + indexer.delete(changeId); + + try (Repository repo = repoManager.openRepository(project); + ObjectInserter inserter = repo.newObjectInserter(); + ObjectReader reader = repo.newObjectReader(); + RevWalk revWalk = new RevWalk(reader); + RefUpdateContext ctx = + RefUpdateContext.openDirectPush(Optional.of("Create imported change"))) { + + Ref ref = repo.getRefDatabase().exactRef(metaRef); + RevCommit tip = revWalk.parseCommit(ref.getObjectId()); + + CommitBuilder commit = new CommitBuilder(); + commit.setTreeId(tip.getTree()); + commit.setAuthor( + new PersonIdent("Gerrit User " + admin.id(), admin.id() + "@imported-server-id")); + commit.setCommitter(new PersonIdent("Gerrit Code Review", admin.email())); + commit.setMessage(buildChangeMessage(change)); + + ObjectId commitId = inserter.insert(commit); + inserter.flush(); + + RefUpdate refUpdate = repo.updateRef(metaRef); + refUpdate.setNewObjectId(commitId); + refUpdate.forceUpdate(); + + indexer.index(project, changeId); + } + + return change; + } + + private String buildChangeMessage(PushOneCommit.Result change) { + return String.join( + "\n", + "Create change", + "", + "Uploaded patch set 1.", + "", + "Patch-set: 1", + "Change-id: " + change.getChangeId(), + "Subject: Test subject", + "Branch: refs/heads/master", + "Status: new", + "Topic:", + "Commit: " + change.getCommit().name(), + "Tag: autogenerated:gerrit:newWipPatchSet", + "Private: false", + "Work-in-progress: true"); + } + private void testEmailSubjectContainsChangeSizeBucket( int numberOfLines, String expectedSizeBucket) throws Exception { String change;
diff --git a/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java b/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java index 9bfba3a..f954a57 100644 --- a/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java +++ b/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
@@ -63,18 +63,7 @@ Change.id(1), 1, ObjectId.zeroId(), - new ChangeNumberVirtualIdAlgorithm() { - - @Override - public boolean isVirtualChangeId(Change.Id arg0) { - return true; - } - - @Override - public Change.Id apply(String s, Change.Id l) { - return encodedChangeNum; - } - }, + (s, c) -> encodedChangeNum, changeNotesMock); assertThat(cd.virtualId().get()).isEqualTo(encodedChangeNum.get());