opendev/gerrit
Code Issues Proposed changes

Merge "Move more logic of JSON migration into CommentJsonMigrator"

Browse Source
This commit is contained in:
Han-Wen Nienhuys 2018-07-11 15:05:55 +00:00 committed by Gerrit Code Review
commit 3fe0ac38d0
3 changed files with 74 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
View File

@ -23,11 +23,15 @@ import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
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 @@ import org.eclipse.jgit.util.MutableInteger;
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 @@ public class CommentJsonMigrator {
return ok;
}
public boolean migrateDrafts(
private boolean migrateDrafts(
Project.NameKey allUsers,
Repository allUsersRepo,
RevWalk rw,
@ -187,4 +228,13 @@ public class CommentJsonMigrator {
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;
}
}

42
java/com/google/gerrit/server/schema/Schema_169.java
View File

@ -18,32 +18,25 @@ import com.google.common.annotations.VisibleForTesting;
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 @@ public class Schema_169 extends SchemaVersion {
@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 @@ public class Schema_169 extends SchemaVersion {
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 @@ public class Schema_169 extends SchemaVersion {
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");

33
javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
View File

@ -33,7 +33,8 @@ import com.google.gerrit.reviewdb.client.RefNames;
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 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
@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 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
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 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
.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 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
.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 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
.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 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
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);
}
}