Make ChangeNumberBitmapMaskAlgorithm a NOOP for local serverId

When calling the virtualisation algorithm for change numbers on the
local serverIds, make the function a NOOP and return the same number.

This avoids code duplication of the logic in different parts of Gerrit
and eliminates the need of knowing how the algorithm works from outside
the calls, treating the virtualisation as a black-box function.

Release-Notes: skip
Change-Id: I385dcaf5e40e8429e9981040a73cde3a4214c4e4

5 files changed, 21 insertions, 23 deletions

diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 9a32a0344c..9fcce26bcd 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -75,7 +75,6 @@ import com.google.gerrit.server.change.CommentThreads;
 import com.google.gerrit.server.change.MergeabilityCache;
 import com.google.gerrit.server.change.PureRevert;
 import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.config.GerritServerId;
 import com.google.gerrit.server.config.SkipCurrentRulesEvaluationOnClosedChanges;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.git.GitRepositoryManager;
@@ -286,7 +285,7 @@ public class ChangeData {
    */
   public static ChangeData createForTest(
       Project.NameKey project, Change.Id id, int currentPatchSetId, ObjectId commitId) {
-    return createForTest(project, id, currentPatchSetId, commitId, null, null, null);
+    return createForTest(project, id, currentPatchSetId, commitId, null, null);
   }
 
   /**
@@ -299,7 +298,6 @@ public class ChangeData {
    * @param id change ID
    * @param currentPatchSetId current patchset number
    * @param commitId commit SHA1 of the current patchset
-   * @param serverId Gerrit server id
    * @param virtualIdAlgo algorithm for virtualising the Change number
    * @param changeNotes notes associated with the Change
    * @return instance for testing.
@@ -309,7 +307,6 @@ public class ChangeData {
       Change.Id id,
       int currentPatchSetId,
       ObjectId commitId,
-      @Nullable String serverId,
       @Nullable ChangeNumberVirtualIdAlgorithm virtualIdAlgo,
       @Nullable ChangeNotes changeNotes) {
     ChangeData cd =
@@ -331,7 +328,6 @@ public class ChangeData {
             null,
             null,
             null,
-            serverId,
             virtualIdAlgo,
             false,
             project,
@@ -434,7 +430,6 @@ public class ChangeData {
   private Optional<Instant> mergedOn;
   private ImmutableSetMultimap<NameKey, RefState> refStates;
   private ImmutableList<byte[]> refStatePatterns;
-  private String gerritServerId;
   private String changeServerId;
   private ChangeNumberVirtualIdAlgorithm virtualIdFunc;
 
@@ -457,7 +452,6 @@ public class ChangeData {
       SubmitRequirementsEvaluator submitRequirementsEvaluator,
       SubmitRequirementsUtil submitRequirementsUtil,
       SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
-      @GerritServerId String gerritServerId,
       ChangeNumberVirtualIdAlgorithm virtualIdFunc,
       @SkipCurrentRulesEvaluationOnClosedChanges Boolean skipCurrentRulesEvaluationOnClosedChange,
       @Assisted Project.NameKey project,
@@ -490,7 +484,6 @@ public class ChangeData {
     this.notes = notes;
 
     this.changeServerId = notes == null ? null : notes.getServerId();
-    this.gerritServerId = gerritServerId;
     this.virtualIdFunc = virtualIdFunc;
   }
 
@@ -613,11 +606,7 @@ public class ChangeData {
   }
 
   public Change.Id getVirtualId() {
-    if (virtualIdFunc == null || changeServerId == null || changeServerId.equals(gerritServerId)) {
-      return legacyId;
-    }
-
-    return Change.id(virtualIdFunc.apply(changeServerId, legacyId.get()));
+    return virtualIdFunc == null ? legacyId : virtualIdFunc.apply(changeServerId, legacyId);
   }
 
   public Project.NameKey project() {
diff --git a/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java b/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java
index 726a3767c4..95c287a21e 100644
--- a/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java
+++ b/java/com/google/gerrit/server/query/change/ChangeNumberBitmapMaskAlgorithm.java
@@ -16,7 +16,9 @@ package com.google.gerrit.server.query.change;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.entities.Change;
 import com.google.gerrit.server.config.GerritImportedServerIds;
+import com.google.gerrit.server.config.GerritServerId;
 import com.google.inject.Inject;
 import com.google.inject.ProvisionException;
 import com.google.inject.Singleton;
@@ -38,9 +40,11 @@ public class ChangeNumberBitmapMaskAlgorithm implements ChangeNumberVirtualIdAlg
       Integer.BYTES * 8 - CHANGE_NUM_BIT_LEN; // Allows up to 64 ServerIds
 
   private final ImmutableMap<String, Integer> serverIdCodes;
+  private final String localServerId;
 
   @Inject
   public ChangeNumberBitmapMaskAlgorithm(
+      @GerritServerId String localServerId,
       @GerritImportedServerIds ImmutableList<String> importedServerIds) {
     if (importedServerIds.size() >= 1 << SERVER_ID_BIT_LEN) {
       throw new ProvisionException(
@@ -54,10 +58,17 @@ public class ChangeNumberBitmapMaskAlgorithm implements ChangeNumberVirtualIdAlg
     }
 
     serverIdCodes = serverIdCodesBuilder.build();
+    this.localServerId = localServerId;
   }
 
   @Override
-  public int apply(String changeServerId, int changeNum) {
+  public Change.Id apply(String changeServerId, Change.Id changeNumId) {
+    if (changeServerId == null || localServerId.equals(changeServerId)) {
+      return changeNumId;
+    }
+
+    int changeNum = changeNumId.get();
+
     if ((changeNum & LEGACY_ID_BIT_MASK) != changeNum) {
       throw new IllegalArgumentException(
           String.format(
@@ -71,6 +82,6 @@ public class ChangeNumberBitmapMaskAlgorithm implements ChangeNumberVirtualIdAlg
     }
     int virtualId = (changeNum & LEGACY_ID_BIT_MASK) | (encodedServerId << CHANGE_NUM_BIT_LEN);
 
-    return virtualId;
+    return Change.id(virtualId);
   }
 }
diff --git a/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java b/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java
index ab217050b9..6daf16f266 100644
--- a/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java
+++ b/java/com/google/gerrit/server/query/change/ChangeNumberVirtualIdAlgorithm.java
@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.query.change;
 
+import com.google.gerrit.entities.Change;
 import com.google.inject.ImplementedBy;
 
 /**
@@ -31,5 +32,5 @@ public interface ChangeNumberVirtualIdAlgorithm {
    * @param legacyChangeNum legacy change number
    * @return virtual id which combines serverId and legacyChangeNum together
    */
-  int apply(String serverId, int legacyChangeNum);
+  Change.Id apply(String serverId, Change.Id legacyChangeNum);
 }
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
index e35941c2e2..72bba20a64 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
@@ -158,7 +158,7 @@ public class ChangeFieldTest {
   public void tolerateNullValuesForInsertion() {
     Project.NameKey project = Project.nameKey("project");
     ChangeData cd =
-        ChangeData.createForTest(project, Change.id(1), 1, ObjectId.zeroId(), null, null, null);
+        ChangeData.createForTest(project, Change.id(1), 1, ObjectId.zeroId(), null, null);
     assertThat(ChangeField.ADDED.setIfPossible(cd, new FakeStoredValue(null))).isTrue();
   }
 
@@ -166,7 +166,7 @@ public class ChangeFieldTest {
   public void tolerateNullValuesForDeletion() {
     Project.NameKey project = Project.nameKey("project");
     ChangeData cd =
-        ChangeData.createForTest(project, Change.id(1), 1, ObjectId.zeroId(), null, null, null);
+        ChangeData.createForTest(project, Change.id(1), 1, ObjectId.zeroId(), null, null);
     assertThat(ChangeField.DELETED.setIfPossible(cd, new FakeStoredValue(null))).isTrue();
   }
 
diff --git a/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java b/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
index 5124021af3..c8e709a2b3 100644
--- a/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
+++ b/javatests/com/google/gerrit/server/query/change/ChangeDataTest.java
@@ -34,8 +34,6 @@ import org.mockito.junit.MockitoJUnitRunner;
 
 @RunWith(MockitoJUnitRunner.class)
 public class ChangeDataTest {
-  private static final String GERRIT_SERVER_ID = UUID.randomUUID().toString();
-
   @Mock private ChangeNotes changeNotesMock;
 
   @Test
@@ -55,7 +53,7 @@ public class ChangeDataTest {
   @Test
   public void getChangeVirtualIdUsingAlgorithm() throws Exception {
     Project.NameKey project = Project.nameKey("project");
-    final int encodedChangeNum = 12345678;
+    final Change.Id encodedChangeNum = Change.id(12345678);
 
     when(changeNotesMock.getServerId()).thenReturn(UUID.randomUUID().toString());
 
@@ -65,11 +63,10 @@ public class ChangeDataTest {
             Change.id(1),
             1,
             ObjectId.zeroId(),
-            GERRIT_SERVER_ID,
             (s, c) -> encodedChangeNum,
             changeNotesMock);
 
-    assertThat(cd.getVirtualId().get()).isEqualTo(encodedChangeNum);
+    assertThat(cd.getVirtualId().get()).isEqualTo(encodedChangeNum.get());
   }
 
   private static PatchSet newPatchSet(Change.Id changeId, int num) {