Merge "Add note to use init when upgrading"
diff --git a/java/com/google/gerrit/plugins/checks/CheckerRef.java b/java/com/google/gerrit/plugins/checks/CheckerRef.java
index 2e7e263..6e2e246 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerRef.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerRef.java
@@ -24,7 +24,7 @@
public static final String REFS_CHECKERS = "refs/checkers/";
/** Ref that stores the repository to checkers map. */
- public static final String REFS_META_CHECKERS = "refs/meta/checkers/";
+ public static final String REFS_META_CHECKERS = "refs/meta/checkers";
/** Suffix for check refs. */
public static final String CHECKS_SUFFIX = "/checks";
diff --git a/java/com/google/gerrit/plugins/checks/Init.java b/java/com/google/gerrit/plugins/checks/Init.java
index 5e7407b..eddfad2 100644
--- a/java/com/google/gerrit/plugins/checks/Init.java
+++ b/java/com/google/gerrit/plugins/checks/Init.java
@@ -18,6 +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.db.CheckerRefMigration;
import com.google.gerrit.plugins.checks.email.ChecksEmailModule;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
@@ -25,16 +26,19 @@
public class Init implements InitStep {
private final SitePaths site;
+ private final CheckerRefMigration checkerRefMigration;
@Inject
- Init(SitePaths site) {
+ Init(SitePaths site, CheckerRefMigration checkerRefMigration) {
this.site = site;
+ this.checkerRefMigration = checkerRefMigration;
}
@Override
public void run() throws Exception {
extractMailExample("CombinedCheckStateUpdated.soy");
extractMailExample("CombinedCheckStateUpdatedHtml.soy");
+ checkerRefMigration.migrate();
}
private void extractMailExample(String orig) throws Exception {
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckerRefMigration.java b/java/com/google/gerrit/plugins/checks/db/CheckerRefMigration.java
new file mode 100644
index 0000000..09e5485
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/db/CheckerRefMigration.java
@@ -0,0 +1,86 @@
+// 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 com.google.gerrit.plugins.checks.CheckerRef;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefRename;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.RefUpdate.Result;
+import org.eclipse.jgit.lib.Repository;
+
+@Singleton
+public class CheckerRefMigration {
+ private static final String TMP_REF = "refs/tmp/checker-migration";
+ private static final String LEGACY_REFS_META_CHECKERS = "refs/meta/checkers/";
+
+ private final GitRepositoryManager repoManager;
+ private final AllProjectsName allProjectsName;
+
+ @Inject
+ CheckerRefMigration(GitRepositoryManager repoManager, AllProjectsName allProjectsName) {
+ this.repoManager = repoManager;
+ this.allProjectsName = allProjectsName;
+ }
+
+ public void migrate() throws Exception {
+ try (Repository repo = repoManager.openRepository(allProjectsName)) {
+
+ // This part is specifically for cases where the rename failed half-way last time.
+ Ref ref = repo.exactRef(TMP_REF);
+ if (ref != null) {
+ renameRef(TMP_REF, CheckerRef.REFS_META_CHECKERS, repo);
+ }
+
+ ref = repo.exactRef(LEGACY_REFS_META_CHECKERS);
+ if (ref == null) {
+ return;
+ }
+ renameRef(LEGACY_REFS_META_CHECKERS, TMP_REF, repo);
+ renameRef(TMP_REF, CheckerRef.REFS_META_CHECKERS, repo);
+ } catch (Exception ex) {
+ throw new IllegalStateException(
+ ex.getMessage()
+ + " Ensure that refs/tmp/checker-migration doesn't exist. Also,"
+ + "ensure that refs/meta/checkers/ (with trailing '/') has been migrated to refs/meta/checkers (without "
+ + "trailing '/'). Consider documentation for updating refs manually: https://git-scm.com/docs/git-update-ref");
+ }
+ }
+
+ private void renameRef(String currentName, String targetName, Repository repo)
+ throws IOException {
+ RefRename refRename = repo.renameRef(currentName, targetName);
+ Result result = refRename.rename();
+ if (result != Result.RENAMED) {
+ throw new IllegalStateException(
+ String.format(
+ "Rename of %s to %s failed with the failure: %s.",
+ currentName, targetName, result.name()));
+ }
+ // After the rename, we need to delete. For same reason, due to a bug in JGit, the rename
+ // doesn't delete the previous
+ // name.
+ RefUpdate update = repo.updateRef(currentName);
+ update.setExpectedOldObjectId(repo.exactRef(currentName).getObjectId());
+ update.setNewObjectId(ObjectId.zeroId());
+ update.setForceUpdate(true);
+ update.delete();
+ }
+}
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/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);
+ }
+}
