Merge branch 'stable-2.15' into stable-2.16
* stable-2.15:
Add revert procedure if rename was not successful
RenameProject: Make each renaming step correspond to a specific stage
Change-Id: Iff2938dfefcccf08a883141c4af21af6eaa689b9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/Module.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/Module.java
index db344d5..6962811 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/Module.java
@@ -44,5 +44,6 @@
bind(FilesystemRenameHandler.class);
bind(RenamePreconditions.class);
bind(IndexUpdateHandler.class);
+ bind(RevertRenameProject.class);
}
}
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 2a49cf0..d1f1ba5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RenameProject.java
@@ -17,6 +17,7 @@
import static com.googlesource.gerrit.plugins.renameproject.RenameOwnProjectCapability.RENAME_OWN_PROJECT;
import static com.googlesource.gerrit.plugins.renameproject.RenameProjectCapability.RENAME_PROJECT;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.cache.Cache;
import com.google.gerrit.extensions.annotations.PluginName;
@@ -46,6 +47,7 @@
import com.googlesource.gerrit.plugins.renameproject.fs.FilesystemRenameHandler;
import com.googlesource.gerrit.plugins.renameproject.monitor.ProgressMonitor;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
@@ -74,6 +76,9 @@
private final RenameLog renameLog;
private final PermissionBackend permissionBackend;
private final Cache<Change.Id, String> changeIdProjectCache;
+ private final RevertRenameProject revertRenameProject;
+
+ private List<Step> stepsPerformed;
@Inject
RenameProject(
@@ -88,7 +93,8 @@
@PluginName String pluginName,
RenameLog renameLog,
PermissionBackend permissionBackend,
- @Named(CACHE_NAME) Cache<Change.Id, String> changeIdProjectCache) {
+ @Named(CACHE_NAME) Cache<Change.Id, String> changeIdProjectCache,
+ RevertRenameProject revertRenameProject) {
this.dbHandler = dbHandler;
this.fsHandler = fsHandler;
this.cacheHandler = cacheHandler;
@@ -101,6 +107,8 @@
this.renameLog = renameLog;
this.permissionBackend = permissionBackend;
this.changeIdProjectCache = changeIdProjectCache;
+ this.revertRenameProject = revertRenameProject;
+ this.stepsPerformed = new ArrayList<>();
}
private void assertNewNameNotNull(Input input) throws BadRequestException {
@@ -157,19 +165,18 @@
Project.NameKey oldProjectKey = rsrc.getNameKey();
Project.NameKey newProjectKey = new Project.NameKey(input.name);
Exception ex = null;
+ stepsPerformed.clear();
try {
- fsHandler.rename(oldProjectKey, newProjectKey, pm);
- log.debug("Renamed the git repo to {} successfully.", newProjectKey.get());
- cacheHandler.update(rsrc.getProjectState().getProject(), newProjectKey);
+ fsRenameStep(oldProjectKey, newProjectKey, pm);
- List<Change.Id> updatedChangeIds =
- dbHandler.rename(changeIds, oldProjectKey, newProjectKey, pm);
- log.debug("Updated the changes in DB successfully for project {}.", oldProjectKey.get());
+ cacheRenameStep(rsrc.getNameKey(), newProjectKey);
+
+ List<Change.Id> updatedChangeIds = dbRenameStep(changeIds, oldProjectKey, newProjectKey, pm);
// if the DB update is successful, update the secondary index
- indexHandler.updateIndex(updatedChangeIds, newProjectKey, pm);
- log.debug("Updated the secondary index successfully for project {}.", oldProjectKey.get());
+ indexRenameStep(updatedChangeIds, oldProjectKey, newProjectKey, pm);
+ // no need to revert this since newProjectKey will be removed from project cache before
lockUnlockProject.unlock(newProjectKey);
log.debug("Unlocked the repo {} after rename operation.", newProjectKey.get());
@@ -178,6 +185,25 @@
pluginEvent.fire(pluginName, pluginName, oldProjectKey.get() + ":" + newProjectKey.get());
} catch (Exception e) {
+ if (stepsPerformed.isEmpty()) {
+ log.error("Renaming procedure failed. Exception caught: {}", e.toString());
+ } else {
+ log.error(
+ "Renaming procedure failed, last successful step {}. Exception caught: {}",
+ stepsPerformed.get(stepsPerformed.size() - 1).toString(),
+ e.toString());
+ }
+ try {
+ revertRenameProject.performRevert(
+ stepsPerformed, changeIds, oldProjectKey, newProjectKey, pm);
+ } catch (Exception revertEx) {
+ log.error(
+ "Failed to revert renaming procedure for {}. Exception caught: {}",
+ oldProjectKey.get(),
+ revertEx.toString());
+ ex = revertEx;
+ throw new RenameRevertException(revertEx, e);
+ }
ex = e;
throw e;
} finally {
@@ -185,6 +211,71 @@
}
}
+ void fsRenameStep(
+ Project.NameKey oldProjectKey, Project.NameKey newProjectKey, ProgressMonitor pm)
+ throws IOException {
+ fsHandler.rename(oldProjectKey, newProjectKey, pm);
+ logPerformedStep(Step.FILESYSTEM, newProjectKey, oldProjectKey);
+ }
+
+ void cacheRenameStep(Project.NameKey oldProjectKey, Project.NameKey newProjectKey)
+ throws IOException {
+ cacheHandler.update(oldProjectKey, newProjectKey);
+ logPerformedStep(Step.CACHE, newProjectKey, oldProjectKey);
+ }
+
+ List<Change.Id> dbRenameStep(
+ List<Change.Id> changeIds,
+ Project.NameKey oldProjectKey,
+ Project.NameKey newProjectKey,
+ ProgressMonitor pm)
+ throws OrmException {
+ List<Change.Id> updatedChangeIds =
+ dbHandler.rename(changeIds, oldProjectKey, newProjectKey, pm);
+ logPerformedStep(Step.DATABASE, newProjectKey, oldProjectKey);
+ return updatedChangeIds;
+ }
+
+ void indexRenameStep(
+ List<Change.Id> updatedChangeIds,
+ Project.NameKey oldProjectKey,
+ Project.NameKey newProjectKey,
+ ProgressMonitor pm)
+ throws InterruptedException {
+ indexHandler.updateIndex(updatedChangeIds, newProjectKey, pm);
+ logPerformedStep(Step.INDEX, newProjectKey, oldProjectKey);
+ }
+
+ enum Step {
+ FILESYSTEM,
+ CACHE,
+ DATABASE,
+ INDEX
+ }
+
+ private void logPerformedStep(
+ Step step, Project.NameKey newProjectKey, Project.NameKey oldProjectKey) {
+ stepsPerformed.add(step);
+ switch (step) {
+ case FILESYSTEM:
+ log.debug("Renamed the git repo to {} successfully.", newProjectKey.get());
+ break;
+ case CACHE:
+ log.debug("Successfully updated project cache for project {}.", newProjectKey.get());
+ break;
+ case DATABASE:
+ log.debug("Updated the changes in DB successfully for project {}.", oldProjectKey.get());
+ break;
+ case INDEX:
+ log.debug("Updated the secondary index successfully for project {}.", oldProjectKey.get());
+ }
+ }
+
+ @VisibleForTesting
+ List<Step> getStepsPerformed() {
+ return stepsPerformed;
+ }
+
List<Change.Id> getChanges(ProjectResource rsrc, ProgressMonitor pm)
throws OrmException, IOException {
pm.beginTask("Retrieving the list of changes from DB");
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProject.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProject.java
new file mode 100644
index 0000000..5de66f2
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProject.java
@@ -0,0 +1,107 @@
+// Copyright (C) 2019 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.googlesource.gerrit.plugins.renameproject;
+
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Change.Id;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.renameproject.RenameProject.Step;
+import com.googlesource.gerrit.plugins.renameproject.cache.CacheRenameHandler;
+import com.googlesource.gerrit.plugins.renameproject.database.DatabaseRenameHandler;
+import com.googlesource.gerrit.plugins.renameproject.database.IndexUpdateHandler;
+import com.googlesource.gerrit.plugins.renameproject.fs.FilesystemRenameHandler;
+import com.googlesource.gerrit.plugins.renameproject.monitor.ProgressMonitor;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RevertRenameProject {
+ private static final Logger log = LoggerFactory.getLogger(RevertRenameProject.class);
+
+ private final DatabaseRenameHandler dbHandler;
+ private final FilesystemRenameHandler fsHandler;
+ private final CacheRenameHandler cacheHandler;
+ private final IndexUpdateHandler indexHandler;
+
+ @Inject
+ RevertRenameProject(
+ DatabaseRenameHandler dbHandler,
+ FilesystemRenameHandler fsHandler,
+ CacheRenameHandler cacheHandler,
+ IndexUpdateHandler indexHandler) {
+ this.dbHandler = dbHandler;
+ this.fsHandler = fsHandler;
+ this.cacheHandler = cacheHandler;
+ this.indexHandler = indexHandler;
+ }
+
+ void performRevert(
+ List<Step> stepsPerformed,
+ List<Id> changeIds,
+ Project.NameKey oldProjectKey,
+ Project.NameKey newProjectKey,
+ ProgressMonitor pm)
+ throws IOException, OrmException {
+ pm.beginTask("Reverting the rename procedure.");
+ List<Change.Id> updatedChangeIds = Collections.emptyList();
+ if (stepsPerformed.contains(Step.FILESYSTEM)) {
+ try {
+ fsHandler.rename(newProjectKey, oldProjectKey, pm);
+ log.debug("Reverted the git repo name to {} successfully.", oldProjectKey.get());
+ } catch (IOException e) {
+ log.error(
+ "Failed to revert git repo name. Aborting revert. Exception caught: {}", e.toString());
+ throw e;
+ }
+ }
+ if (stepsPerformed.contains(Step.CACHE)) {
+ cacheHandler.update(newProjectKey, oldProjectKey);
+ log.debug("Successfully removed project {} from project cache.", newProjectKey.get());
+ }
+ if (stepsPerformed.contains(Step.DATABASE)) {
+ try {
+ updatedChangeIds = dbHandler.rename(changeIds, newProjectKey, oldProjectKey, pm);
+ log.debug(
+ "Reverted the changes in DB successfully from project {} to project {}.",
+ newProjectKey.get(),
+ oldProjectKey.get());
+ } catch (OrmException e) {
+ log.error(
+ "Failed to revert changes in DB for project {}. Secondary indexes not reverted. Exception caught: {}",
+ oldProjectKey.get(),
+ e.toString());
+ throw e;
+ }
+ }
+ if (stepsPerformed.contains(Step.INDEX)) {
+ try {
+ indexHandler.updateIndex(updatedChangeIds, oldProjectKey, pm);
+ log.debug(
+ "Reverted the secondary index successfully from project {} to project {}.",
+ newProjectKey.get(),
+ oldProjectKey.get());
+ } catch (InterruptedException e) {
+ log.error(
+ "Secondary index revert failed for {}. Exception caught: {}",
+ oldProjectKey.get(),
+ e.toString());
+ }
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/renameproject/cache/CacheRenameHandler.java b/src/main/java/com/googlesource/gerrit/plugins/renameproject/cache/CacheRenameHandler.java
index 0ca033e..4526489 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/renameproject/cache/CacheRenameHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/renameproject/cache/CacheRenameHandler.java
@@ -30,8 +30,9 @@
this.projectCache = projectCache;
}
- public void update(Project oldProject, Project.NameKey newProjectKey) throws IOException {
- projectCache.remove(oldProject);
+ public void update(Project.NameKey oldProjectKey, Project.NameKey newProjectKey)
+ throws IOException {
+ projectCache.remove(oldProjectKey);
projectCache.onCreateProject(newProjectKey);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProjectTest.java b/src/test/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProjectTest.java
new file mode 100644
index 0000000..d060414
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/renameproject/RevertRenameProjectTest.java
@@ -0,0 +1,155 @@
+// Copyright (C) 2019 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.googlesource.gerrit.plugins.renameproject;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit.Result;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.project.ProjectState;
+import com.googlesource.gerrit.plugins.renameproject.RenameProject.Step;
+import com.googlesource.gerrit.plugins.renameproject.monitor.ProgressMonitor;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+@TestPlugin(
+ name = "rename-project",
+ sysModule = "com.googlesource.gerrit.plugins.renameproject.Module",
+ sshModule = "com.googlesource.gerrit.plugins.renameproject.SshModule")
+public class RevertRenameProjectTest extends LightweightPluginDaemonTest {
+ private static final String NEW_PROJECT_NAME = "newProject";
+
+ private RenameProject renameProject;
+ private RevertRenameProject revertRenameProject;
+ private Project.NameKey oldProjectKey;
+ private Project.NameKey newProjectKey;
+ private ProgressMonitor pm;
+ private ProjectResource oldRsrc;
+
+ @Before
+ public void init() {
+ renameProject = plugin.getSysInjector().getInstance(RenameProject.class);
+ revertRenameProject = plugin.getSysInjector().getInstance(RevertRenameProject.class);
+
+ oldProjectKey = project;
+ newProjectKey = new Project.NameKey(NEW_PROJECT_NAME);
+
+ pm = Mockito.mock(ProgressMonitor.class);
+
+ oldRsrc = Mockito.mock(ProjectResource.class);
+ when(oldRsrc.getNameKey()).thenReturn(oldProjectKey);
+ }
+
+ @Test
+ @UseLocalDisk
+ public void testRevertFromFsHandler() throws Exception {
+ Result result = createChange();
+ List<Change.Id> changeIds = renameProject.getChanges(oldRsrc, pm);
+
+ renameProject.fsRenameStep(oldProjectKey, newProjectKey, pm);
+ assertRenamed(result);
+
+ revertRenameProject.performRevert(
+ renameProject.getStepsPerformed(), changeIds, oldProjectKey, newProjectKey, pm);
+ assertReverted();
+ }
+
+ @Test
+ @UseLocalDisk
+ public void testRevertFromCacheHandler() throws Exception {
+ Result result = createChange();
+ List<Change.Id> changeIds = renameProject.getChanges(oldRsrc, pm);
+
+ renameProject.fsRenameStep(oldProjectKey, newProjectKey, pm);
+ renameProject.cacheRenameStep(oldProjectKey, newProjectKey);
+ assertRenamed(result);
+
+ revertRenameProject.performRevert(
+ renameProject.getStepsPerformed(), changeIds, oldProjectKey, newProjectKey, pm);
+ assertReverted();
+ }
+
+ @Test
+ @UseLocalDisk
+ public void testRevertFromDbHandler() throws Exception {
+ Result result = createChange();
+ List<Change.Id> changeIds = renameProject.getChanges(oldRsrc, pm);
+
+ renameProject.fsRenameStep(oldProjectKey, newProjectKey, pm);
+ renameProject.cacheRenameStep(oldProjectKey, newProjectKey);
+ renameProject.dbRenameStep(changeIds, oldProjectKey, newProjectKey, pm);
+ assertRenamed(result);
+
+ revertRenameProject.performRevert(
+ renameProject.getStepsPerformed(), changeIds, oldProjectKey, newProjectKey, pm);
+ assertReverted();
+ }
+
+ @Test
+ @UseLocalDisk
+ public void testRevertFromIndexHandler() throws Exception {
+ Result result = createChange();
+ List<Change.Id> changeIds = renameProject.getChanges(oldRsrc, pm);
+
+ renameProject.fsRenameStep(oldProjectKey, newProjectKey, pm);
+ renameProject.cacheRenameStep(oldProjectKey, newProjectKey);
+ renameProject.dbRenameStep(changeIds, oldProjectKey, newProjectKey, pm);
+ renameProject.indexRenameStep(changeIds, oldProjectKey, newProjectKey, pm);
+ assertRenamed(result);
+
+ revertRenameProject.performRevert(
+ renameProject.getStepsPerformed(), changeIds, oldProjectKey, newProjectKey, pm);
+ assertReverted();
+ }
+
+ private void assertReverted() throws Exception {
+ ProjectState oldProjectState = projectCache.get(oldProjectKey);
+ assertThat(oldProjectState).isNotNull();
+
+ ProjectState newProjectState = projectCache.get(newProjectKey);
+ assertThat(newProjectState).isNull();
+
+ assertThat(queryProvider.get().byProject(oldProjectKey)).isNotEmpty();
+ assertThat(queryProvider.get().byProject(newProjectKey)).isEmpty();
+ }
+
+ private void assertRenamed(Result result) throws Exception {
+ ProjectState oldProjectState = projectCache.get(oldProjectKey);
+ assertThat(oldProjectState).isNull();
+
+ ProjectState newProjectState = projectCache.get(newProjectKey);
+ assertThat(newProjectState).isNotNull();
+
+ if (renameProject.getStepsPerformed().contains(Step.DATABASE)) {
+ ChangeApi changeApi = gApi.changes().id(NEW_PROJECT_NAME, result.getChange().getId().get());
+ ChangeInfo changeInfo = changeApi.info();
+ assertThat(changeInfo.changeId).isEqualTo(result.getChangeId());
+ }
+
+ if (renameProject.getStepsPerformed().contains(Step.INDEX)) {
+ assertThat(queryProvider.get().byProject(oldProjectKey)).isEmpty();
+ }
+ }
+}
