Move more logic of JSON migration into CommentJsonMigrator
This simplifies reuse in other code.
Change-Id: I20e48a22fb2d95aa95829d31dbe8414b4501c0a0
diff --git a/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java b/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
index 9a9a211..5007ec9 100644
--- a/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
+++ b/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
@@ -23,11 +23,15 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerIdProvider;
+import com.google.gerrit.server.update.RefUpdateUtil;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
+import org.eclipse.jgit.internal.storage.file.PackInserter;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
@@ -47,22 +51,59 @@
public class CommentJsonMigrator {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ public static class ProjectMigrationResult {
+ public int skipped;
+ public boolean ok;
+ public int refsUpdated;
+ }
+
private final LegacyChangeNoteRead legacyChangeNoteRead;
private final ChangeNoteJson changeNoteJson;
+ private final AllUsersName allUsers;
@Inject
CommentJsonMigrator(
- ChangeNoteJson changeNoteJson, GerritServerIdProvider gerritServerIdProvider) {
+ ChangeNoteJson changeNoteJson,
+ GerritServerIdProvider gerritServerIdProvider,
+ AllUsersName allUsers) {
this.changeNoteJson = changeNoteJson;
+ this.allUsers = allUsers;
this.legacyChangeNoteRead = new LegacyChangeNoteRead(gerritServerIdProvider.get());
}
- CommentJsonMigrator(ChangeNoteJson changeNoteJson, String serverId) {
+ CommentJsonMigrator(ChangeNoteJson changeNoteJson, String serverId, AllUsersName allUsers) {
this.changeNoteJson = changeNoteJson;
this.legacyChangeNoteRead = new LegacyChangeNoteRead(serverId);
+ this.allUsers = allUsers;
}
- public boolean migrateChanges(
+ public ProjectMigrationResult migrateProject(Project.NameKey project, Repository repo) {
+ ProjectMigrationResult progress = new ProjectMigrationResult();
+ progress.ok = true;
+ try (RevWalk rw = new RevWalk(repo);
+ ObjectInserter ins = newPackInserter(repo)) {
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ bru.setAllowNonFastForwards(true);
+ progress.ok &= migrateChanges(project, repo, rw, ins, bru);
+ if (project.equals(allUsers)) {
+ progress.ok &= migrateDrafts(allUsers, repo, rw, ins, bru);
+ }
+
+ progress.refsUpdated += bru.getCommands().size();
+ if (!bru.getCommands().isEmpty()) {
+ ins.flush();
+ RefUpdateUtil.executeChecked(bru, rw);
+
+ } else {
+ progress.skipped++;
+ }
+ } catch (IOException e) {
+ progress.ok = false;
+ }
+ return progress;
+ }
+
+ private boolean migrateChanges(
Project.NameKey project, Repository repo, RevWalk rw, ObjectInserter ins, BatchRefUpdate bru)
throws IOException {
boolean ok = true;
@@ -76,7 +117,7 @@
return ok;
}
- public boolean migrateDrafts(
+ private boolean migrateDrafts(
Project.NameKey allUsers,
Repository allUsersRepo,
RevWalk rw,
@@ -187,4 +228,13 @@
rw.sort(RevSort.REVERSE);
rw.markStart(rw.parseCommit(id));
}
+
+ private static ObjectInserter newPackInserter(Repository repo) {
+ if (!(repo instanceof FileRepository)) {
+ return repo.newObjectInserter();
+ }
+ PackInserter ins = ((FileRepository) repo).getObjectDatabase().newPackInserter();
+ ins.checkExisting(false);
+ return ins;
+ }
}
diff --git a/java/com/google/gerrit/server/schema/Schema_169.java b/java/com/google/gerrit/server/schema/Schema_169.java
index 11aebdd..75dd459 100644
--- a/java/com/google/gerrit/server/schema/Schema_169.java
+++ b/java/com/google/gerrit/server/schema/Schema_169.java
@@ -18,32 +18,25 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.CommentJsonMigrator;
+import com.google.gerrit.server.notedb.CommentJsonMigrator.ProjectMigrationResult;
import com.google.gerrit.server.notedb.MutableNotesMigration;
import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.server.update.RefUpdateUtil;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
import java.util.SortedSet;
-import org.eclipse.jgit.internal.storage.file.FileRepository;
-import org.eclipse.jgit.internal.storage.file.PackInserter;
-import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.TextProgressMonitor;
-import org.eclipse.jgit.revwalk.RevWalk;
/** Migrate NoteDb inline comments to JSON format. */
public class Schema_169 extends SchemaVersion {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final AllUsersName allUsers;
private final CommentJsonMigrator migrator;
private final GitRepositoryManager repoManager;
private final NotesMigration notesMigration;
@@ -51,12 +44,10 @@
@Inject
Schema_169(
Provider<Schema_168> prior,
- AllUsersName allUsers,
CommentJsonMigrator migrator,
GitRepositoryManager repoManager,
@GerritServerConfig Config config) {
super(prior);
- this.allUsers = allUsers;
this.migrator = migrator;
this.repoManager = repoManager;
this.notesMigration = MutableNotesMigration.fromConfig(config);
@@ -67,15 +58,6 @@
migrateData(ui);
}
- private static ObjectInserter newPackInserter(Repository repo) {
- if (!(repo instanceof FileRepository)) {
- return repo.newObjectInserter();
- }
- PackInserter ins = ((FileRepository) repo).getObjectDatabase().newPackInserter();
- ins.checkExisting(false);
- return ins;
- }
-
@VisibleForTesting
protected void migrateData(UpdateUI ui) throws OrmException {
// If the migration hasn't started, no need to look for non-JSON
@@ -89,28 +71,16 @@
pm.beginTask("Migrating projects", projects.size());
int skipped = 0;
for (Project.NameKey project : projects) {
- try (Repository repo = repoManager.openRepository(project);
- RevWalk rw = new RevWalk(repo);
- ObjectInserter ins = newPackInserter(repo)) {
- BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
- bru.setAllowNonFastForwards(true);
- ok &= migrator.migrateChanges(project, repo, rw, ins, bru);
- if (project.equals(allUsers)) {
- ok &= migrator.migrateDrafts(allUsers, repo, rw, ins, bru);
- }
-
- if (!bru.getCommands().isEmpty()) {
- ins.flush();
- RefUpdateUtil.executeChecked(bru, rw);
- } else {
- skipped++;
- }
+ try (Repository repo = repoManager.openRepository(project)) {
+ ProjectMigrationResult progress = migrator.migrateProject(project, repo);
+ skipped += progress.skipped;
} catch (IOException e) {
- logger.atWarning().log("Error migrating project " + project, e);
ok = false;
+ logger.atWarning().log("Error migrating project " + project, e);
}
pm.update(1);
}
+
pm.endTask();
ui.message(
"Skipped " + skipped + " project" + (skipped == 1 ? "" : "s") + " with no legacy comments");
diff --git a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
index a5c4e49..718e287 100644
--- a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
@@ -33,7 +33,8 @@
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.update.RefUpdateUtil;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.notedb.CommentJsonMigrator.ProjectMigrationResult;
import com.google.gerrit.testing.TestChanges;
import com.google.inject.Inject;
import java.io.ByteArrayOutputStream;
@@ -57,13 +58,14 @@
@Inject private ChangeNoteUtil noteUtil;
@Inject private CommentsUtil commentsUtil;
@Inject private LegacyChangeNoteWrite legacyChangeNoteWrite;
+ @Inject private AllUsersName allUsersName;
private AtomicInteger uuidCounter;
@Before
public void setUpCounter() {
uuidCounter = new AtomicInteger();
- migrator = new CommentJsonMigrator(new ChangeNoteJson(), "gerrit");
+ migrator = new CommentJsonMigrator(new ChangeNoteJson(), "gerrit", allUsersName);
}
@Test
@@ -90,7 +92,7 @@
getRevId(notes, 2), ps2Comment.toString());
ChangeNotes oldNotes = notes;
- migrate(project, migrator::migrateChanges, 0);
+ migrate(project, 0);
assertNoDifferences(notes, oldNotes);
assertThat(notes.getMetaId()).isEqualTo(oldNotes.getMetaId());
}
@@ -163,7 +165,7 @@
.containsExactly(ps1Comment1.key, true, ps1Comment2.key, true, ps2Comment1.key, true);
ChangeNotes oldNotes = notes;
- migrate(project, migrator::migrateChanges, 1);
+ migrate(project, 1);
// Comment content is the same.
notes = newNotes(c);
@@ -265,7 +267,7 @@
.containsExactly(otherCommentPs1.key, true);
ChangeNotes oldNotes = notes;
- migrate(allUsers, migrator::migrateDrafts, 2);
+ migrate(allUsers, 2);
assertNoDifferences(notes, oldNotes);
// Migration doesn't touch change ref.
@@ -366,7 +368,7 @@
.containsExactly(ps1Comment.key, true, ps2Comment.key, false, ps3Comment.key, true);
ChangeNotes oldNotes = notes;
- migrate(project, migrator::migrateChanges, 1);
+ migrate(project, 1);
assertNoDifferences(notes, oldNotes);
// Comment content is the same.
@@ -402,19 +404,12 @@
throws Exception;
}
- private void migrate(Project.NameKey project, MigrateFunction func, int expectedCommands)
- throws Exception {
- try (Repository repo = repoManager.openRepository(project);
- RevWalk rw = new RevWalk(repo);
- ObjectInserter ins = repo.newObjectInserter()) {
- BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
- bru.setAllowNonFastForwards(true);
- assertThat(func.call(project, repo, rw, ins, bru)).isTrue();
- assertThat(bru.getCommands()).hasSize(expectedCommands);
- if (!bru.getCommands().isEmpty()) {
- ins.flush();
- RefUpdateUtil.executeChecked(bru, rw);
- }
+ private void migrate(Project.NameKey project, int expectedCommands) throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ ProjectMigrationResult progress = migrator.migrateProject(project, repo);
+
+ assertThat(progress.ok).isTrue();
+ assertThat(progress.refsUpdated).isEqualTo(expectedCommands);
}
}
