Allow for an online migration of checker ref
This commit is a follow-up to the checker ref migration. It allows
for an online migration, that is, a migration while the system is
running. To do so, we check if the legacy ref exists and use it,
if it does.
This way, we can roll out the new code without having to migrate
the refs in the same go. It also allows for rollbacks more easily.
The code will use the new ref, if the legacy ref does not exist.
We add a test to assert the behavior and apply some finishing
touches to the existing test code was well as move the migration
class from the 'api' to the 'db' package.
Change-Id: I86dea2bd1617cf0ec0db35b6cee59630e9e136a4
diff --git a/java/com/google/gerrit/plugins/checks/Init.java b/java/com/google/gerrit/plugins/checks/Init.java
index 45b311d..eddfad2 100644
--- a/java/com/google/gerrit/plugins/checks/Init.java
+++ b/java/com/google/gerrit/plugins/checks/Init.java
@@ -18,7 +18,7 @@
import static com.google.gerrit.pgm.init.api.InitUtil.extract;
import com.google.gerrit.pgm.init.api.InitStep;
-import com.google.gerrit.plugins.checks.api.CheckerRefMigration;
+import com.google.gerrit.plugins.checks.db.CheckerRefMigration;
import com.google.gerrit.plugins.checks.email.ChecksEmailModule;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckerRefMigration.java b/java/com/google/gerrit/plugins/checks/db/CheckerRefMigration.java
similarity index 94%
rename from java/com/google/gerrit/plugins/checks/api/CheckerRefMigration.java
rename to java/com/google/gerrit/plugins/checks/db/CheckerRefMigration.java
index 2897c00..09e5485 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckerRefMigration.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckerRefMigration.java
@@ -11,9 +11,8 @@
// 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.plugins.checks.api;
+package com.google.gerrit.plugins.checks.db;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -29,7 +28,6 @@
@Singleton
public class CheckerRefMigration {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final String TMP_REF = "refs/tmp/checker-migration";
private static final String LEGACY_REFS_META_CHECKERS = "refs/meta/checkers/";
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotes.java b/java/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotes.java
index 3d99cbb..5019e9e 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotes.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckersByRepositoryNotes.java
@@ -31,6 +31,7 @@
import com.google.common.hash.Hashing;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.server.config.AllProjectsName;
@@ -111,7 +112,18 @@
@Override
protected String getRefName() {
- return CheckerRef.REFS_META_CHECKERS;
+ // To allow for an online migration of the old checker ref (refs/meta/checkers/) to the new ref
+ // (refs/meta/checkers) we need to check which state we are in here. If we omit the legacy ref
+ // exists, we operate on that instead. The migration will move to the new ref eventually and
+ // delete the old ref. At that point, we'll start using the new ref here.
+ // TODO(paiking): Remove when migration on googlesource.com is done.
+ try {
+ return repo.exactRef("refs/meta/checkers/") != null
+ ? "refs/meta/checkers/"
+ : CheckerRef.REFS_META_CHECKERS;
+ } catch (IOException e) {
+ throw new StorageException(e);
+ }
}
/**
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerRefMigrationIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerRefMigrationIT.java
deleted file mode 100644
index 78fac62..0000000
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerRefMigrationIT.java
+++ /dev/null
@@ -1,61 +0,0 @@
-// Copyright (C) 2020 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.plugins.checks.acceptance.testsuite;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.gerrit.plugins.checks.CheckerUuid;
-import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
-import com.google.gerrit.plugins.checks.api.CheckerRefMigration;
-import com.google.gerrit.plugins.checks.db.CheckersByRepositoryNotes;
-import com.google.inject.Inject;
-import org.eclipse.jgit.junit.TestRepository;
-import org.eclipse.jgit.lib.Repository;
-import org.junit.Before;
-import org.junit.Test;
-
-public class CheckerRefMigrationIT extends AbstractCheckersTest {
- private CheckerOperationsImpl checkerOperations;
-
- @Before
- public void setUp() {
- checkerOperations = plugin.getSysInjector().getInstance(CheckerOperationsImpl.class);
- }
-
- @Inject private CheckerRefMigration checkerRefMigration;
-
- @Test
- public void migrateLegacyMetaRef() throws Exception {
- CheckerUuid checkerWithBadRef = CheckerUuid.parse("test:my-checker1");
-
- try (Repository repo = repoManager.openRepository(allProjects);
- TestRepository<Repository> testRepo = new TestRepository<>(repo)) {
- testRepo
- .branch("refs/meta/checkers/")
- .commit()
- .add(
- CheckersByRepositoryNotes.computeRepositorySha1(allProjects).getName(),
- checkerWithBadRef.toString())
- .create();
- }
-
- assertThat(repo().exactRef("refs/meta/checkers/")).isNotNull();
- assertThat(repo().exactRef("refs/meta/checkers")).isNull();
- checkerRefMigration.migrate();
-
- assertThat(repo().exactRef("refs/meta/checkers")).isNotNull();
- assertThat(repo().exactRef("refs/meta/checkers/")).isNull();
- }
-}
diff --git a/javatests/com/google/gerrit/plugins/checks/db/CheckerRefMigrationTest.java b/javatests/com/google/gerrit/plugins/checks/db/CheckerRefMigrationTest.java
new file mode 100644
index 0000000..ba80ba9
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/checks/db/CheckerRefMigrationTest.java
@@ -0,0 +1,98 @@
+// Copyright (C) 2020 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.plugins.checks.db;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.plugins.checks.CheckerUuid;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.testing.InMemoryRepositoryManager;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+public class CheckerRefMigrationTest {
+ private InMemoryRepositoryManager inMemoryRepositoryManager;
+ private AllProjectsName allProjectsName;
+ private Repository allProjectsRepo;
+
+ @Before
+ public void setUp() throws Exception {
+ inMemoryRepositoryManager = new InMemoryRepositoryManager();
+ allProjectsName = new AllProjectsName("Test Repository");
+ allProjectsRepo = inMemoryRepositoryManager.createRepository(allProjectsName);
+ }
+
+ @Test
+ public void migrateLegacyMetaRef() throws Exception {
+ CheckerUuid checkerWithBadRef = CheckerUuid.parse("test:my-checker1");
+ try (TestRepository<Repository> testRepo = new TestRepository<>(allProjectsRepo)) {
+ testRepo
+ .branch("refs/meta/checkers/")
+ .commit()
+ .add(
+ CheckersByRepositoryNotes.computeRepositorySha1(allProjectsName).getName(),
+ checkerWithBadRef.toString())
+ .create();
+ }
+
+ assertThat(allProjectsRepo.exactRef("refs/meta/checkers/")).isNotNull();
+ assertThat(allProjectsRepo.exactRef("refs/meta/checkers")).isNull();
+ CheckerRefMigration checkerRefMigration =
+ new CheckerRefMigration(inMemoryRepositoryManager, allProjectsName);
+ checkerRefMigration.migrate();
+ assertThat(allProjectsRepo.exactRef("refs/meta/checkers")).isNotNull();
+ assertThat(allProjectsRepo.exactRef("refs/meta/checkers/")).isNull();
+ }
+
+ @Test
+ public void readFromLegacyRefUntilMigrated() throws Exception {
+ CheckerUuid checkerUuid = CheckerUuid.parse("test:my-checker1");
+ Project.NameKey projectName = Project.nameKey("foo");
+ // Create a checker map on the legacy ref
+ try (TestRepository<Repository> repo = new TestRepository<>(allProjectsRepo)) {
+ repo.branch("refs/meta/checkers/")
+ .commit()
+ .add(
+ CheckersByRepositoryNotes.computeRepositorySha1(projectName).getName(),
+ checkerUuid.toString())
+ .create();
+ }
+ // Assert that the map gets loaded even though it's on the legacy ref.
+ // This is important to allow for an online migration.
+ assertThat(CheckersByRepositoryNotes.load(allProjectsName, allProjectsRepo).get(projectName))
+ .containsExactly(checkerUuid);
+ }
+
+ @Test
+ public void readFromNewRefWhenMigrated() throws Exception {
+ CheckerUuid checkerUuid = CheckerUuid.parse("test:my-checker1");
+ Project.NameKey projectName = Project.nameKey("foo");
+ // Create a checker map on the new ref
+ try (TestRepository<Repository> repo = new TestRepository<>(allProjectsRepo)) {
+ repo.branch("refs/meta/checkers")
+ .commit()
+ .add(
+ CheckersByRepositoryNotes.computeRepositorySha1(projectName).getName(),
+ checkerUuid.toString())
+ .create();
+ }
+ // Assert that the map gets loaded
+ assertThat(CheckersByRepositoryNotes.load(allProjectsName, allProjectsRepo).get(projectName))
+ .containsExactly(checkerUuid);
+ }
+}
