Adapt whole plugin codebase to 3.0 release API use
Adapt to the sole NoteDb, thus remove ReviewDb incl. OrmException usage.
DatabaseRenameHandler:
- Make oldProjectKey a consistently reused instance field, for
getChangeIds to logically initialize -as before this change.
- Introduce a changeNotes instance field, built from that oldProjectKey.
- Preserve the classic iterator use in getChangeIds; refactor follow-up.
- Adapt rollback to NoteDb, which path being still not covered by tests.
- Should a rollback itself fail, keep trying to roll other updates back.
RenameLogLayout: use TODO to denote the already inaccurate doc comments.
Bug: Issue 10525
Change-Id: I95f151cdcd83e2b1244359e2cd85e4ad9e082141
diff --git a/WORKSPACE b/WORKSPACE
index 6a7b3f9..dd03f37 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -3,7 +3,7 @@
load("//:bazlets.bzl", "load_bazlets")
load_bazlets(
- commit = "5514e42e08d7852d226849e391857d7f139f7ed9",
+ commit = "3c0a1bd325152af99640655831aef109e460bbe7",
#local_path = "/home/<user>/projects/bazlets",
)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/LockUnlockProject.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/LockUnlockProject.java
index 04426bc..170e0d6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/LockUnlockProject.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/LockUnlockProject.java
@@ -28,17 +28,22 @@
public class LockUnlockProject {
private final MetaDataUpdate.Server metaDataUpdateFactory;
private final ProjectCache projectCache;
+ private final ProjectConfig.Factory projectConfigFactory;
@Inject
- LockUnlockProject(MetaDataUpdate.Server metaDataUpdateFactory, ProjectCache projectCache) {
+ LockUnlockProject(
+ MetaDataUpdate.Server metaDataUpdateFactory,
+ ProjectCache projectCache,
+ ProjectConfig.Factory projectConfigFactory) {
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.projectCache = projectCache;
+ this.projectConfigFactory = projectConfigFactory;
}
public void lock(Project.NameKey key) throws IOException, ConfigInvalidException {
MetaDataUpdate md = metaDataUpdateFactory.create(key);
- ProjectConfig projectConfig = ProjectConfig.read(md);
+ ProjectConfig projectConfig = projectConfigFactory.read(md);
Project p = projectConfig.getProject();
p.setState(ProjectState.READ_ONLY);
@@ -50,7 +55,7 @@
public void unlock(Project.NameKey key) throws IOException, ConfigInvalidException {
MetaDataUpdate md = metaDataUpdateFactory.create(key);
- ProjectConfig projectConfig = ProjectConfig.read(md);
+ ProjectConfig projectConfig = projectConfigFactory.read(md);
Project p = projectConfig.getProject();
p.setState(ProjectState.ACTIVE);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameCommand.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameCommand.java
index 14b5898..db1e862 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameCommand.java
@@ -24,7 +24,6 @@
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.googlesource.gerrit.plugins.renameproject.monitor.CommandProgressMonitor;
@@ -80,7 +79,7 @@
stdout.flush();
}
}
- } catch (RestApiException | OrmException | IOException e) {
+ } catch (RestApiException | IOException e) {
throw die(e);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameLog.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameLog.java
index 88c0e3b..6449cba 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameLog.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameLog.java
@@ -17,10 +17,10 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
import com.google.gerrit.extensions.systemstatus.ServerInformation;
+import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.AuditEvent;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.OutputFormat;
import com.google.gerrit.server.audit.AuditService;
import com.google.gerrit.server.util.PluginLogFile;
import com.google.gerrit.server.util.SystemLog;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameLogLayout.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameLogLayout.java
index 5a11a17..053e8b9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameLogLayout.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameLogLayout.java
@@ -33,8 +33,8 @@
* +0100] INFO 1000000 admin OK \ myProject {"name": newName}
*
* <p>The log entry for a failed project rename will look like this: [2015-03-05 12:14:30,180
- * +0100] ERROR 1000000 admin FAIL \ myProject {"name": newName}
- * com.google.gwtorm.server.OrmException: \ Failed to access the database
+ * +0100] ERROR 1000000 admin FAIL \ myProject {"name": newName} TODO(mmiller): \ Failed to access
+ * the database
*/
@Override
public String format(LoggingEvent event) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java
index c42b2ab..4d4f671 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java
@@ -33,7 +33,6 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectResource;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -147,7 +146,7 @@
}
void doRename(List<Change.Id> changeIds, ProjectResource rsrc, Input input, ProgressMonitor pm)
- throws InterruptedException, OrmException, ConfigInvalidException, IOException {
+ throws InterruptedException, ConfigInvalidException, IOException {
Project.NameKey oldProjectKey = rsrc.getNameKey();
Project.NameKey newProjectKey = new Project.NameKey(input.name);
Exception ex = null;
@@ -157,7 +156,7 @@
cacheHandler.update(rsrc.getProjectState().getProject(), newProjectKey);
List<Change.Id> updatedChangeIds =
- dbHandler.rename(changeIds, oldProjectKey, newProjectKey, pm);
+ dbHandler.rename(changeIds, newProjectKey, pm);
log.debug("Updated the changes in DB successfully for project {}.", oldProjectKey.get());
// if the DB update is successful, update the secondary index
@@ -176,7 +175,7 @@
}
}
- List<Change.Id> getChanges(ProjectResource rsrc, ProgressMonitor pm) throws OrmException {
+ List<Change.Id> getChanges(ProjectResource rsrc, ProgressMonitor pm) throws IOException {
pm.beginTask("Retrieving the list of changes from DB");
Project.NameKey oldProjectKey = rsrc.getNameKey();
return dbHandler.getChangeIds(oldProjectKey);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/conditions/RenamePreconditions.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/conditions/RenamePreconditions.java
index 9efa51d..3e30f6e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/conditions/RenamePreconditions.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/conditions/RenamePreconditions.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.submit.MergeOpRepoManager;
import com.google.gerrit.server.submit.SubmoduleException;
import com.google.gerrit.server.submit.SubmoduleOp;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -102,7 +101,7 @@
log.error(message);
throw new CannotRenameProjectException(message);
}
- } catch (PermissionBackendException | OrmException | RestApiException e) {
+ } catch (PermissionBackendException | RestApiException e) {
throw new CannotRenameProjectException(e);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/DatabaseRenameHandler.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/DatabaseRenameHandler.java
index d8fc1e7..62e517e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/DatabaseRenameHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/DatabaseRenameHandler.java
@@ -19,30 +19,31 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
import com.google.gerrit.server.account.ProjectWatches.ProjectWatchKey;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.query.account.InternalAccountQuery;
-import com.google.gwtorm.jdbc.JdbcSchema;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.renameproject.monitor.ProgressMonitor;
import java.io.IOException;
-import java.sql.Connection;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Statement;
+import java.time.Instant;
import java.util.ArrayList;
+import java.util.Date;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -51,88 +52,111 @@
public class DatabaseRenameHandler {
private static final Logger log = LoggerFactory.getLogger(DatabaseRenameHandler.class);
- private final SchemaFactory<ReviewDb> schemaFactory;
+ private final ChangeNotes.Factory schemaFactory;
+ private final ChangeUpdate.Factory updateFactory;
+ private final GitRepositoryManager repoManager;
+ private final Provider<CurrentUser> userProvider;
private final Provider<InternalAccountQuery> accountQueryProvider;
private final Provider<AccountsUpdate> accountsUpdateProvider;
+ private final Map<Change.Id, ChangeNotes> changeNotes = new HashMap<>();
+
+ private Project.NameKey oldProjectKey;
@Inject
public DatabaseRenameHandler(
- SchemaFactory<ReviewDb> schemaFactory,
+ ChangeNotes.Factory schemaFactory,
+ ChangeUpdate.Factory updateFactory,
+ GitRepositoryManager repoManager,
+ Provider<CurrentUser> userProvider,
Provider<InternalAccountQuery> accountQueryProvider,
@ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider) {
this.accountQueryProvider = accountQueryProvider;
this.schemaFactory = schemaFactory;
+ this.updateFactory = updateFactory;
+ this.repoManager = repoManager;
+ this.userProvider = userProvider;
this.accountsUpdateProvider = accountsUpdateProvider;
}
- public List<Change.Id> getChangeIds(Project.NameKey oldProjectKey) throws OrmException {
+ public List<Change.Id> getChangeIds(Project.NameKey oldProjectKey) throws IOException {
log.debug("Starting to retrieve changes from the DB for project {}", oldProjectKey.get());
+ this.oldProjectKey = oldProjectKey;
List<Change.Id> changeIds = new ArrayList<>();
- Connection conn = ((JdbcSchema) schemaFactory.open()).getConnection();
- String query =
- "select change_id from changes where dest_project_name ='" + oldProjectKey.get() + "';";
- try (Statement stmt = conn.createStatement();
- ResultSet changes = stmt.executeQuery(query)) {
- while (changes != null && changes.next()) {
- Change.Id changeId = new Change.Id(changes.getInt(1));
- changeIds.add(changeId);
- }
- log.debug(
- "Number of changes related to the project {} are {}",
- oldProjectKey.get(),
- changeIds.size());
- return changeIds;
- } catch (SQLException e) {
- throw new OrmException(e);
+ Stream<ChangeNotesResult> changes =
+ schemaFactory.scan(repoManager.openRepository(oldProjectKey), oldProjectKey);
+ Iterator<ChangeNotesResult> iterator = changes.iterator();
+ while (iterator.hasNext()) {
+ ChangeNotesResult change = iterator.next();
+ Change.Id changeId = change.id();
+ changeIds.add(changeId);
+ changeNotes.put(changeId, change.notes());
}
+ log.debug(
+ "Number of changes related to the project {} are {}",
+ oldProjectKey.get(),
+ changeIds.size());
+ return changeIds;
}
public List<Change.Id> rename(
- List<Change.Id> changes,
- Project.NameKey oldProjectKey,
- Project.NameKey newProjectKey,
- ProgressMonitor pm)
- throws OrmException {
+ List<Change.Id> changes, Project.NameKey newProjectKey, ProgressMonitor pm)
+ throws IOException {
pm.beginTask("Updating changes in the database");
- Connection conn = ((JdbcSchema) schemaFactory.open()).getConnection();
- try (Statement stmt = conn.createStatement()) {
- conn.setAutoCommit(false);
- try {
- log.debug("Updating the changes in the DB related to project {}", oldProjectKey.get());
- for (Change.Id cd : changes) {
- stmt.addBatch(
- "update changes set dest_project_name='"
- + newProjectKey.get()
- + "' where change_id ="
- + cd.id
- + ";");
- }
- stmt.executeBatch();
- updateWatchEntries(oldProjectKey, newProjectKey);
- conn.commit();
- log.debug(
- "Successfully updated the changes in the DB related to project {}",
- oldProjectKey.get());
- return changes;
- } finally {
- conn.setAutoCommit(true);
+ log.debug("Updating the changes in the DB related to project {}", oldProjectKey.get());
+ List<ChangeUpdate> updates = getChangeUpdates(changes, newProjectKey);
+ updateWatchEntries(newProjectKey);
+ List<Change> updated = new ArrayList<>();
+ try {
+ for (ChangeUpdate update : updates) {
+ update.commit();
+ updated.add(update.getChange());
}
- } catch (SQLException e) {
- try {
- log.error(
- "Failed to update changes in the DB for the project {}, rolling back the operation.",
- oldProjectKey.get());
- conn.rollback();
- } catch (SQLException ex) {
- throw new OrmException(ex);
- }
- throw new OrmException(e);
+ } catch (IOException e) {
+ // TODO(mmiller): Consider covering this path with tests, albeit exceptional.
+ log.error(
+ "Failed to update changes in the DB for the project {}, rolling back the operation.",
+ oldProjectKey.get());
+ rollback(updated, newProjectKey);
+ throw e;
}
+ log.debug(
+ "Successfully updated the changes in the DB related to project {}", oldProjectKey.get());
+ return changes;
}
- private void updateWatchEntries(Project.NameKey oldProjectKey, Project.NameKey newProjectKey)
- throws OrmException {
+ private List<ChangeUpdate> getChangeUpdates(List<Change.Id> changeIds, Project.NameKey nameKey) {
+ List<ChangeUpdate> updates = new ArrayList<>();
+ Date from = Date.from(Instant.now());
+ changeIds.forEach(
+ changeId -> {
+ ChangeNotes notes = schemaFactory.create(nameKey, changeId);
+ ChangeUpdate update = updateFactory.create(notes, userProvider.get(), from);
+ updates.add(update);
+ });
+ return updates;
+ }
+
+ private void rollback(List<Change> changes, Project.NameKey newProjectKey) {
+ Date from = Date.from(Instant.now());
+ changes.forEach(
+ change -> {
+ ChangeNotes notes = changeNotes.get(change.getId());
+ ChangeUpdate revert = updateFactory.create(notes, userProvider.get(), from);
+ revert.setRevertOf(change.getChangeId());
+ try {
+ revert.commit();
+ } catch (IOException ex) {
+ log.error(
+ "Failed to rollback change {} in DB from project {} to {}; rolling back others still.",
+ change.getChangeId(),
+ newProjectKey.get(),
+ oldProjectKey.get());
+ }
+ });
+ }
+
+ private void updateWatchEntries(Project.NameKey newProjectKey) {
for (AccountState a : accountQueryProvider.get().byWatchedProject(newProjectKey)) {
Account.Id accountId = a.getAccount().getId();
ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>> projectWatches =
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/IndexUpdateHandler.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/IndexUpdateHandler.java
index ed12dc3..daef534 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/IndexUpdateHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/database/IndexUpdateHandler.java
@@ -17,15 +17,11 @@
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.index.change.ChangeIndexer;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.renameproject.Configuration;
import com.googlesource.gerrit.plugins.renameproject.monitor.ProgressMonitor;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
@@ -40,14 +36,11 @@
public class IndexUpdateHandler {
private static final Logger log = LoggerFactory.getLogger(IndexUpdateHandler.class);
- private final SchemaFactory<ReviewDb> schemaFactory;
private final ChangeIndexer indexer;
private final Configuration config;
@Inject
- public IndexUpdateHandler(
- SchemaFactory<ReviewDb> schemaFactory, ChangeIndexer indexer, Configuration config) {
- this.schemaFactory = schemaFactory;
+ public IndexUpdateHandler(ChangeIndexer indexer, Configuration config) {
this.indexer = indexer;
this.config = config;
}
@@ -100,14 +93,9 @@
@Override
public Boolean call() throws Exception {
- try (ReviewDb db = schemaFactory.open()) {
- indexer.index(db, newProjectKey, changeId);
- monitor.update(1);
- return Boolean.TRUE;
- } catch (OrmException | IOException e) {
- log.error("Failed to reindex change {} from index, Cause: {}", changeId, e);
- throw e;
- }
+ indexer.index(newProjectKey, changeId);
+ monitor.update(1);
+ return Boolean.TRUE;
}
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/renameproject/RenameIT.java b/src/test/java/com/googlesource/gerrit/plugins/renameproject/RenameIT.java
index 879e660..efb8886 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/renameproject/RenameIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/renameproject/RenameIT.java
@@ -83,12 +83,13 @@
@Test
@UseLocalDisk
public void testRenameSubscribedFail() throws Exception {
- NameKey superProject = createProject("super-project");
+ NameKey superProject = createProjectOverAPI("super-project", null, true, null);
TestRepository<?> superRepo = cloneProject(superProject);
- NameKey subProject = createProject("subscribed-to-project");
+ NameKey subProject = createProjectOverAPI("subscribed-to-project", null, true, null);
SubmoduleUtil.allowSubmoduleSubscription(
metaDataUpdateFactory,
projectCache,
+ projectConfigFactory,
subProject,
"refs/heads/master",
superProject,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/renameproject/SubmoduleUtil.java b/src/test/java/com/googlesource/gerrit/plugins/renameproject/SubmoduleUtil.java
index e6fcdd1..68515c4 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/renameproject/SubmoduleUtil.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/renameproject/SubmoduleUtil.java
@@ -45,6 +45,7 @@
static void allowSubmoduleSubscription(
Server metaDataUpdateFactory,
ProjectCache projectCache,
+ ProjectConfig.Factory projectConfigFactory,
NameKey sub,
String subBranch,
NameKey superName,
@@ -54,7 +55,7 @@
try (MetaDataUpdate md = metaDataUpdateFactory.create(sub)) {
md.setMessage("Added superproject subscription");
SubscribeSection s;
- ProjectConfig pc = ProjectConfig.read(md);
+ ProjectConfig pc = projectConfigFactory.read(md);
if (pc.getSubscribeSections().containsKey(superName)) {
s = pc.getSubscribeSections().get(superName);
} else {
