Merge "Add dryRun option to CommentJsonMigrator"
diff --git a/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java b/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
index 5007ec9..b250a34 100644
--- a/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
+++ b/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
@@ -14,9 +14,11 @@
package com.google.gerrit.server.notedb;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.server.notedb.RevisionNote.MAX_NOTE_SZ;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
@@ -29,6 +31,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.internal.storage.file.PackInserter;
@@ -54,7 +57,7 @@
public static class ProjectMigrationResult {
public int skipped;
public boolean ok;
- public int refsUpdated;
+ public List<String> refsUpdated;
}
private final LegacyChangeNoteRead legacyChangeNoteRead;
@@ -77,9 +80,12 @@
this.allUsers = allUsers;
}
- public ProjectMigrationResult migrateProject(Project.NameKey project, Repository repo) {
+ public ProjectMigrationResult migrateProject(
+ Project.NameKey project, Repository repo, boolean dryRun) {
ProjectMigrationResult progress = new ProjectMigrationResult();
progress.ok = true;
+ progress.skipped = 0;
+ progress.refsUpdated = ImmutableList.of();
try (RevWalk rw = new RevWalk(repo);
ObjectInserter ins = newPackInserter(repo)) {
BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
@@ -89,17 +95,20 @@
progress.ok &= migrateDrafts(allUsers, repo, rw, ins, bru);
}
- progress.refsUpdated += bru.getCommands().size();
+ progress.refsUpdated =
+ bru.getCommands().stream().map(c -> c.getRefName()).collect(toImmutableList());
if (!bru.getCommands().isEmpty()) {
- ins.flush();
- RefUpdateUtil.executeChecked(bru, rw);
-
+ if (!dryRun) {
+ ins.flush();
+ RefUpdateUtil.executeChecked(bru, rw);
+ }
} else {
progress.skipped++;
}
} catch (IOException e) {
progress.ok = false;
}
+
return progress;
}
diff --git a/java/com/google/gerrit/server/schema/Schema_169.java b/java/com/google/gerrit/server/schema/Schema_169.java
index 75dd459..2779d47 100644
--- a/java/com/google/gerrit/server/schema/Schema_169.java
+++ b/java/com/google/gerrit/server/schema/Schema_169.java
@@ -72,7 +72,7 @@
int skipped = 0;
for (Project.NameKey project : projects) {
try (Repository repo = repoManager.openRepository(project)) {
- ProjectMigrationResult progress = migrator.migrateProject(project, repo);
+ ProjectMigrationResult progress = migrator.migrateProject(project, repo, false);
skipped += progress.skipped;
} catch (IOException e) {
ok = false;
diff --git a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
index 718e287..2e19d20 100644
--- a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
@@ -38,11 +38,10 @@
import com.google.gerrit.testing.TestChanges;
import com.google.inject.Inject;
import java.io.ByteArrayOutputStream;
+import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jgit.junit.TestRepository;
-import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -92,7 +91,7 @@
getRevId(notes, 2), ps2Comment.toString());
ChangeNotes oldNotes = notes;
- migrate(project, 0);
+ checkMigrate(project, ImmutableList.of());
assertNoDifferences(notes, oldNotes);
assertThat(notes.getMetaId()).isEqualTo(oldNotes.getMetaId());
}
@@ -165,7 +164,7 @@
.containsExactly(ps1Comment1.key, true, ps1Comment2.key, true, ps2Comment1.key, true);
ChangeNotes oldNotes = notes;
- migrate(project, 1);
+ checkMigrate(project, ImmutableList.of(RefNames.changeMetaRef(c.getId())));
// Comment content is the same.
notes = newNotes(c);
@@ -267,7 +266,11 @@
.containsExactly(otherCommentPs1.key, true);
ChangeNotes oldNotes = notes;
- migrate(allUsers, 2);
+ checkMigrate(
+ allUsers,
+ ImmutableList.of(
+ RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()),
+ RefNames.refsDraftComments(c.getId(), otherUser.getAccountId())));
assertNoDifferences(notes, oldNotes);
// Migration doesn't touch change ref.
@@ -368,7 +371,7 @@
.containsExactly(ps1Comment.key, true, ps2Comment.key, false, ps3Comment.key, true);
ChangeNotes oldNotes = notes;
- migrate(project, 1);
+ checkMigrate(project, ImmutableList.of(RefNames.changeMetaRef(c.getId())));
assertNoDifferences(notes, oldNotes);
// Comment content is the same.
@@ -393,23 +396,12 @@
.containsExactly(ps1Comment.key, false, ps2Comment.key, false, ps3Comment.key, false);
}
- @FunctionalInterface
- interface MigrateFunction {
- boolean call(
- Project.NameKey project,
- Repository repo,
- RevWalk rw,
- ObjectInserter ins,
- BatchRefUpdate bru)
- throws Exception;
- }
-
- private void migrate(Project.NameKey project, int expectedCommands) throws Exception {
+ private void checkMigrate(Project.NameKey project, List<String> expectedRefs) throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
- ProjectMigrationResult progress = migrator.migrateProject(project, repo);
+ ProjectMigrationResult progress = migrator.migrateProject(project, repo, false);
assertThat(progress.ok).isTrue();
- assertThat(progress.refsUpdated).isEqualTo(expectedCommands);
+ assertThat(progress.refsUpdated).isEqualTo(expectedRefs);
}
}
