Extract deletion preconditions to its own class
Instead of spreading the assertions done before deleting a change in
several classes, extract them in a dedicated class, DeletePreconditions.
This eases the unit testing of the now simplified classes and better
reflects the fact that some of these assertions do not belong anymore to
the class where they were hosted. One example: the check for submodules
was in the DatabaseDeleteHandler class because this information used to
be in the database, but this is no longer the case.
Additional test coverage for this class will be provided in an upcoming
change adding IT tests.
Change-Id: Ie1f513f5981035aba217200f608c6ca6e6357cb1
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteAction.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteAction.java
index 1ce0ee3..459d819 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteAction.java
@@ -14,7 +14,6 @@
package com.googlesource.gerrit.plugins.deleteproject;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.project.ProjectResource;
@@ -23,7 +22,6 @@
import com.googlesource.gerrit.plugins.deleteproject.cache.CacheDeleteHandler;
import com.googlesource.gerrit.plugins.deleteproject.database.DatabaseDeleteHandler;
import com.googlesource.gerrit.plugins.deleteproject.fs.FilesystemDeleteHandler;
-import com.googlesource.gerrit.plugins.deleteproject.projectconfig.ProjectConfigDeleteHandler;
public class DeleteAction extends DeleteProject implements UiAction<ProjectResource> {
private final ProtectedProjects protectedProjects;
@@ -34,20 +32,18 @@
DatabaseDeleteHandler dbHandler,
FilesystemDeleteHandler fsHandler,
CacheDeleteHandler cacheHandler,
- ProjectConfigDeleteHandler pcHandler,
Provider<CurrentUser> userProvider,
- @PluginName String pluginName,
DeleteLog deleteLog,
+ DeletePreconditions preConditions,
Configuration cfg,
HideProject hideProject) {
super(
dbHandler,
fsHandler,
cacheHandler,
- pcHandler,
userProvider,
- pluginName,
deleteLog,
+ preConditions,
cfg,
hideProject);
this.protectedProjects = protectedProjects;
@@ -62,6 +58,6 @@
? String.format("Not allowed to delete %s", rsrc.getName())
: String.format("Delete project %s", rsrc.getName()))
.setEnabled(!protectedProjects.isProtected(rsrc))
- .setVisible(canDelete(rsrc));
+ .setVisible(preConditions.canDelete(rsrc));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteCommand.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteCommand.java
index f7124e4..b833d7d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteCommand.java
@@ -24,12 +24,19 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.IOException;
-import java.util.Collection;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.Option;
@CommandMetaData(name = "delete", description = "Delete specific project")
public final class DeleteCommand extends SshCommand {
+ private static final String FORCE_DELETE =
+ "%s - To really delete '%s', re-run with the --force flag.";
+ private static final String REALLY_DELETE =
+ "Really delete '%s'?\n"
+ + "This is an operation which permanently deletes data. This cannot be undone!\n"
+ + "If you are sure you wish to delete this project, re-run with the "
+ + "--yes-really-delete flag.\n";
+
@Argument(index = 0, required = true, metaVar = "NAME", usage = "project to delete")
private ProjectControl projectControl;
@@ -43,10 +50,12 @@
private boolean preserveGitRepository = false;
private final DeleteProject deleteProject;
+ private final DeletePreconditions preConditions;
@Inject
- protected DeleteCommand(DeleteProject deleteProject) {
+ protected DeleteCommand(DeleteProject deleteProject, DeletePreconditions preConditions) {
this.deleteProject = deleteProject;
+ this.preConditions = preConditions;
}
@Override
@@ -57,41 +66,21 @@
input.preserve = preserveGitRepository;
ProjectResource rsrc = new ProjectResource(projectControl);
- deleteProject.assertDeletePermission(rsrc);
- deleteProject.assertCanDelete(rsrc, input);
+ preConditions.assertDeletePermission(rsrc);
if (!yesReallyDelete) {
- StringBuilder msgBuilder = new StringBuilder();
- msgBuilder.append("Really delete ");
- msgBuilder.append(rsrc.getName());
- msgBuilder.append("?\n");
- msgBuilder.append("This is an operation which permanently deletes ");
- msgBuilder.append("data. This cannot be undone!\n");
- msgBuilder.append("If you are sure you wish to delete this project, ");
- msgBuilder.append("re-run\n");
- msgBuilder.append("with the --yes-really-delete flag.\n");
- throw new UnloggedFailure(msgBuilder.toString());
+ throw new UnloggedFailure(String.format(REALLY_DELETE, rsrc.getName()));
}
if (!force) {
- Collection<String> warnings = deleteProject.getWarnings(rsrc);
- if (warnings != null && !warnings.isEmpty()) {
- StringBuilder msgBuilder = new StringBuilder();
- msgBuilder.append("There are warnings against deleting ");
- msgBuilder.append(rsrc.getName());
- msgBuilder.append(":\n");
- for (String warning : warnings) {
- msgBuilder.append(" * ");
- msgBuilder.append(warning);
- msgBuilder.append("\n");
- }
- msgBuilder.append("To really delete ");
- msgBuilder.append(rsrc.getName());
- msgBuilder.append(", re-run with the --force flag.");
- throw new UnloggedFailure(msgBuilder.toString());
+ try {
+ preConditions.assertHasOpenChanges(rsrc.getNameKey(), false);
+ } catch (CannotDeleteProjectException e) {
+ throw new UnloggedFailure(String.format(FORCE_DELETE, e.getMessage(), rsrc.getName()));
}
}
+ preConditions.assertCanBeDeleted(rsrc, input);
deleteProject.doDelete(rsrc, input);
} catch (AuthException
| ResourceNotFoundException
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeletePreconditions.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeletePreconditions.java
new file mode 100644
index 0000000..47ef932
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeletePreconditions.java
@@ -0,0 +1,175 @@
+// Copyright (C) 2018 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.deleteproject;
+
+import static com.googlesource.gerrit.plugins.deleteproject.DeleteOwnProjectCapability.DELETE_OWN_PROJECT;
+import static com.googlesource.gerrit.plugins.deleteproject.DeleteProjectCapability.DELETE_PROJECT;
+import static java.util.stream.Collectors.joining;
+
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.common.ProjectInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.CapabilityControl;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MergeOpRepoManager;
+import com.google.gerrit.server.git.SubmoduleOp;
+import com.google.gerrit.server.project.ListChildProjects;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.deleteproject.DeleteProject.Input;
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+
+@Singleton
+class DeletePreconditions {
+ private final Configuration config;
+ private final Provider<ListChildProjects> listChildProjectsProvider;
+ private final Provider<MergeOpRepoManager> mergeOpProvider;
+ private final String pluginName;
+ private final Provider<InternalChangeQuery> queryProvider;
+ private final GitRepositoryManager repoManager;
+ private final SubmoduleOp.Factory subOpFactory;
+ private final Provider<CurrentUser> userProvider;
+ private final ProtectedProjects protectedProjects;
+
+ @Inject
+ public DeletePreconditions(
+ Configuration config,
+ Provider<ListChildProjects> listChildProjectsProvider,
+ Provider<MergeOpRepoManager> mergeOpProvider,
+ @PluginName String pluginName,
+ Provider<InternalChangeQuery> queryProvider,
+ GitRepositoryManager repoManager,
+ SubmoduleOp.Factory subOpFactory,
+ Provider<CurrentUser> userProvider,
+ ProtectedProjects protectedProjects) {
+ this.config = config;
+ this.listChildProjectsProvider = listChildProjectsProvider;
+ this.mergeOpProvider = mergeOpProvider;
+ this.pluginName = pluginName;
+ this.queryProvider = queryProvider;
+ this.repoManager = repoManager;
+ this.subOpFactory = subOpFactory;
+ this.userProvider = userProvider;
+ this.protectedProjects = protectedProjects;
+ }
+
+ void assertDeletePermission(ProjectResource rsrc) throws AuthException {
+ if (!canDelete(rsrc)) {
+ throw new AuthException("not allowed to delete project");
+ }
+ }
+
+ boolean canDelete(ProjectResource rsrc) {
+ CapabilityControl ctl = userProvider.get().getCapabilities();
+ return ctl.canAdministrateServer()
+ || ctl.canPerform(pluginName + "-" + DELETE_PROJECT)
+ || (ctl.canPerform(pluginName + "-" + DELETE_OWN_PROJECT) && rsrc.getControl().isOwner());
+ }
+
+ void assertCanBeDeleted(ProjectResource rsrc, Input input) throws ResourceConflictException {
+ try {
+ protectedProjects.assertIsNotProtected(rsrc);
+ assertHasNoChildProjects(rsrc);
+ Project.NameKey projectNameKey = rsrc.getNameKey();
+ assertIsNotSubmodule(projectNameKey);
+ assertDeleteWithTags(projectNameKey, input != null && input.preserve);
+ assertHasOpenChanges(projectNameKey, input != null && input.force);
+ } catch (CannotDeleteProjectException e) {
+ throw new ResourceConflictException(e.getMessage());
+ }
+ }
+
+ public void assertHasOpenChanges(Project.NameKey projectNameKey, boolean force)
+ throws CannotDeleteProjectException {
+ if (!force) {
+ try {
+ List<ChangeData> openChanges = queryProvider.get().byProjectOpen(projectNameKey);
+ if (!openChanges.isEmpty()) {
+ throw new CannotDeleteProjectException(
+ String.format("Project '%s' has open changes.", projectNameKey.get()));
+ }
+ } catch (OrmException e) {
+ throw new CannotDeleteProjectException(
+ String.format("Unable to verify if '%s' has open changes.", projectNameKey.get()));
+ }
+ }
+ }
+
+ private void assertHasNoChildProjects(ProjectResource rsrc) throws CannotDeleteProjectException {
+ List<ProjectInfo> children = listChildProjectsProvider.get().apply(rsrc);
+ if (!children.isEmpty()) {
+ throw new CannotDeleteProjectException(
+ "Cannot delete project because it has children: "
+ + children.stream().map(info -> info.name).collect(joining(",")));
+ }
+ }
+
+ private void assertIsNotSubmodule(Project.NameKey projectNameKey)
+ throws CannotDeleteProjectException {
+ try (Repository repo = repoManager.openRepository(projectNameKey);
+ MergeOpRepoManager mergeOp = mergeOpProvider.get()) {
+ Set<Branch.NameKey> branches = new HashSet<>();
+ for (Ref ref : repo.getRefDatabase().getRefs(RefNames.REFS_HEADS).values()) {
+ branches.add(new Branch.NameKey(projectNameKey, ref.getName()));
+ }
+ SubmoduleOp sub = subOpFactory.create(branches, mergeOp);
+ for (Branch.NameKey b : branches) {
+ if (!sub.superProjectSubscriptionsForSubmoduleBranch(b).isEmpty()) {
+ throw new CannotDeleteProjectException("Project is subscribed by other projects.");
+ }
+ }
+ } catch (RepositoryNotFoundException e) {
+ // we're trying to delete the repository,
+ // so this exception should not stop us
+ } catch (IOException e) {
+ throw new CannotDeleteProjectException("Project is subscribed by other projects.");
+ }
+ }
+
+ private void assertDeleteWithTags(Project.NameKey projectNameKey, boolean preserveGitRepository)
+ throws CannotDeleteProjectException {
+ if (!preserveGitRepository && !config.deletionWithTagsAllowed()) {
+ assertHasNoTags(projectNameKey);
+ }
+ }
+
+ private void assertHasNoTags(Project.NameKey projectNameKey) throws CannotDeleteProjectException {
+ try (Repository repo = repoManager.openRepository(projectNameKey)) {
+ if (!repo.getRefDatabase().getRefs(Constants.R_TAGS).isEmpty()) {
+ throw new CannotDeleteProjectException(
+ String.format("Project %s has tags", projectNameKey));
+ }
+ } catch (IOException e) {
+ throw new CannotDeleteProjectException(e);
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java
index 066da65..5d83c80 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/DeleteProject.java
@@ -14,10 +14,6 @@
package com.googlesource.gerrit.plugins.deleteproject;
-import static com.googlesource.gerrit.plugins.deleteproject.DeleteOwnProjectCapability.DELETE_OWN_PROJECT;
-import static com.googlesource.gerrit.plugins.deleteproject.DeleteProjectCapability.DELETE_PROJECT;
-
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -26,7 +22,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -36,9 +31,7 @@
import com.googlesource.gerrit.plugins.deleteproject.cache.CacheDeleteHandler;
import com.googlesource.gerrit.plugins.deleteproject.database.DatabaseDeleteHandler;
import com.googlesource.gerrit.plugins.deleteproject.fs.FilesystemDeleteHandler;
-import com.googlesource.gerrit.plugins.deleteproject.projectconfig.ProjectConfigDeleteHandler;
import java.io.IOException;
-import java.util.Collection;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@Singleton
@@ -48,12 +41,12 @@
boolean force;
}
+ protected final DeletePreconditions preConditions;
+
private final DatabaseDeleteHandler dbHandler;
private final FilesystemDeleteHandler fsHandler;
private final CacheDeleteHandler cacheHandler;
- private final ProjectConfigDeleteHandler pcHandler;
private final Provider<CurrentUser> userProvider;
- private final String pluginName;
private final DeleteLog deleteLog;
private final Configuration cfg;
private final HideProject hideProject;
@@ -63,19 +56,17 @@
DatabaseDeleteHandler dbHandler,
FilesystemDeleteHandler fsHandler,
CacheDeleteHandler cacheHandler,
- ProjectConfigDeleteHandler pcHandler,
Provider<CurrentUser> userProvider,
- @PluginName String pluginName,
DeleteLog deleteLog,
+ DeletePreconditions preConditions,
Configuration cfg,
HideProject hideProject) {
this.dbHandler = dbHandler;
this.fsHandler = fsHandler;
this.cacheHandler = cacheHandler;
- this.pcHandler = pcHandler;
this.userProvider = userProvider;
- this.pluginName = pluginName;
this.deleteLog = deleteLog;
+ this.preConditions = preConditions;
this.cfg = cfg;
this.hideProject = hideProject;
}
@@ -84,48 +75,13 @@
public Object apply(ProjectResource rsrc, Input input)
throws ResourceNotFoundException, ResourceConflictException, OrmException, IOException,
AuthException {
- assertDeletePermission(rsrc);
- assertCanDelete(rsrc, input);
-
- if (input == null || !input.force) {
- Collection<String> warnings = getWarnings(rsrc);
- if (!warnings.isEmpty()) {
- throw new ResourceConflictException(
- String.format("Project %s has open changes", rsrc.getName()));
- }
- }
+ preConditions.assertDeletePermission(rsrc);
+ preConditions.assertCanBeDeleted(rsrc, input);
doDelete(rsrc, input);
return Response.none();
}
- public void assertDeletePermission(ProjectResource rsrc) throws AuthException {
- if (!canDelete(rsrc)) {
- throw new AuthException("not allowed to delete project");
- }
- }
-
- protected boolean canDelete(ProjectResource rsrc) {
- CapabilityControl ctl = userProvider.get().getCapabilities();
- return ctl.canAdministrateServer()
- || ctl.canPerform(pluginName + "-" + DELETE_PROJECT)
- || (ctl.canPerform(pluginName + "-" + DELETE_OWN_PROJECT) && rsrc.getControl().isOwner());
- }
-
- public void assertCanDelete(ProjectResource rsrc, Input input) throws ResourceConflictException {
- try {
- pcHandler.assertCanDelete(rsrc);
- dbHandler.assertCanDelete(rsrc.getControl().getProject());
- fsHandler.assertCanDelete(rsrc, input == null ? false : input.preserve);
- } catch (CannotDeleteProjectException e) {
- throw new ResourceConflictException(e.getMessage());
- }
- }
-
- public Collection<String> getWarnings(ProjectResource rsrc) throws OrmException {
- return dbHandler.getWarnings(rsrc.getControl().getProject());
- }
-
public void doDelete(ProjectResource rsrc, Input input)
throws OrmException, IOException, ResourceNotFoundException, ResourceConflictException {
Project project = rsrc.getControl().getProject();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Module.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Module.java
index 857555e..8fc69d4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/Module.java
@@ -28,7 +28,6 @@
import com.googlesource.gerrit.plugins.deleteproject.database.DatabaseDeleteHandler;
import com.googlesource.gerrit.plugins.deleteproject.fs.DeleteTrashFolders;
import com.googlesource.gerrit.plugins.deleteproject.fs.FilesystemDeleteHandler;
-import com.googlesource.gerrit.plugins.deleteproject.projectconfig.ProjectConfigDeleteHandler;
public class Module extends AbstractModule {
@@ -47,7 +46,7 @@
.to(DeleteOwnProjectCapability.class);
bind(DatabaseDeleteHandler.class);
bind(FilesystemDeleteHandler.class);
- bind(ProjectConfigDeleteHandler.class);
+ bind(DeletePreconditions.class);
install(
new RestApiModule() {
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/database/DatabaseDeleteHandler.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/database/DatabaseDeleteHandler.java
index 9585cff..3dff386 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/database/DatabaseDeleteHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/database/DatabaseDeleteHandler.java
@@ -16,14 +16,11 @@
import static java.util.Collections.singleton;
-import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.StarredChangesUtil;
@@ -32,34 +29,23 @@
import com.google.gerrit.server.account.WatchConfig.Accessor;
import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
import com.google.gerrit.server.change.AccountPatchReviewStore;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.MergeOpRepoManager;
-import com.google.gerrit.server.git.SubmoduleOp;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.account.InternalAccountQuery;
-import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.jdbc.JdbcSchema;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import com.googlesource.gerrit.plugins.deleteproject.CannotDeleteProjectException;
import java.io.IOException;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
-import java.util.HashSet;
import java.util.List;
-import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -67,10 +53,6 @@
private static final Logger log = LoggerFactory.getLogger(DatabaseDeleteHandler.class);
private final Provider<ReviewDb> dbProvider;
- private final Provider<InternalChangeQuery> queryProvider;
- private final GitRepositoryManager repoManager;
- private final SubmoduleOp.Factory subOpFactory;
- private final Provider<MergeOpRepoManager> ormProvider;
private final StarredChangesUtil starredChangesUtil;
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
private final ChangeIndexer indexer;
@@ -81,9 +63,6 @@
public DatabaseDeleteHandler(
Provider<ReviewDb> dbProvider,
Provider<InternalChangeQuery> queryProvider,
- GitRepositoryManager repoManager,
- SubmoduleOp.Factory subOpFactory,
- Provider<MergeOpRepoManager> ormProvider,
StarredChangesUtil starredChangesUtil,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore,
ChangeIndexer indexer,
@@ -92,27 +71,11 @@
this.accountQueryProvider = accountQueryProvider;
this.watchConfig = watchConfig;
this.dbProvider = dbProvider;
- this.queryProvider = queryProvider;
- this.repoManager = repoManager;
- this.subOpFactory = subOpFactory;
- this.ormProvider = ormProvider;
this.starredChangesUtil = starredChangesUtil;
this.accountPatchReviewStore = accountPatchReviewStore;
this.indexer = indexer;
}
- public Collection<String> getWarnings(Project project) throws OrmException {
- Collection<String> ret = Lists.newArrayList();
-
- // Warn against open changes
- List<ChangeData> openChanges = queryProvider.get().byProjectOpen(project.getNameKey());
- if (openChanges.iterator().hasNext()) {
- ret.add(project.getName() + " has open changes");
- }
-
- return ret;
- }
-
public void delete(Project project) throws OrmException {
ReviewDb db = ReviewDbUtil.unwrapDb(dbProvider.get());
Connection conn = ((JdbcSchema) db).getConnection();
@@ -187,29 +150,6 @@
}
}
- public void assertCanDelete(Project project) throws CannotDeleteProjectException {
-
- Project.NameKey proj = project.getNameKey();
- try (Repository repo = repoManager.openRepository(proj);
- MergeOpRepoManager orm = ormProvider.get()) {
- Set<Branch.NameKey> branches = new HashSet<>();
- for (Ref ref : repo.getRefDatabase().getRefs(RefNames.REFS_HEADS).values()) {
- branches.add(new Branch.NameKey(proj, ref.getName()));
- }
- SubmoduleOp sub = subOpFactory.create(branches, orm);
- for (Branch.NameKey b : branches) {
- if (!sub.superProjectSubscriptionsForSubmoduleBranch(b).isEmpty()) {
- throw new CannotDeleteProjectException("Project is subscribed by other projects.");
- }
- }
- } catch (RepositoryNotFoundException e) {
- // we're trying to delete the repository,
- // so this exception should not stop us
- } catch (IOException e) {
- throw new CannotDeleteProjectException("Project is subscribed by other projects.");
- }
- }
-
public void atomicDelete(ReviewDb db, Project project, List<Change.Id> changeIds)
throws OrmException {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/FilesystemDeleteHandler.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/FilesystemDeleteHandler.java
index 31fafea..2989449 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/FilesystemDeleteHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/fs/FilesystemDeleteHandler.java
@@ -19,10 +19,7 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
-import com.googlesource.gerrit.plugins.deleteproject.CannotDeleteProjectException;
-import com.googlesource.gerrit.plugins.deleteproject.Configuration;
import com.googlesource.gerrit.plugins.deleteproject.TimeMachine;
import java.io.File;
import java.io.IOException;
@@ -34,7 +31,6 @@
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.BasicFileAttributes;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.RepositoryCache;
import org.slf4j.Logger;
@@ -45,16 +41,12 @@
private final GitRepositoryManager repoManager;
private final DynamicSet<ProjectDeletedListener> deletedListener;
- private final Configuration config;
@Inject
public FilesystemDeleteHandler(
- GitRepositoryManager repoManager,
- DynamicSet<ProjectDeletedListener> deletedListener,
- Configuration config) {
+ GitRepositoryManager repoManager, DynamicSet<ProjectDeletedListener> deletedListener) {
this.repoManager = repoManager;
this.deletedListener = deletedListener;
- this.config = config;
}
public void delete(Project project, boolean preserveGitRepository)
@@ -68,24 +60,6 @@
}
}
- public void assertCanDelete(ProjectResource rsrc, boolean preserveGitRepository)
- throws CannotDeleteProjectException {
- if (!preserveGitRepository && !config.deletionWithTagsAllowed()) {
- assertHasNoTags(rsrc);
- }
- }
-
- private void assertHasNoTags(ProjectResource rsrc) throws CannotDeleteProjectException {
- try (Repository repo = repoManager.openRepository(rsrc.getNameKey())) {
- if (!repo.getRefDatabase().getRefs(Constants.R_TAGS).isEmpty()) {
- throw new CannotDeleteProjectException(
- String.format("Project %s has tags", rsrc.getName()));
- }
- } catch (IOException e) {
- throw new CannotDeleteProjectException(e);
- }
- }
-
private void deleteGitRepository(final Project.NameKey project, final File repoFile)
throws IOException {
// Delete the repository from disk
diff --git a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/projectconfig/ProjectConfigDeleteHandler.java b/src/main/java/com/googlesource/gerrit/plugins/deleteproject/projectconfig/ProjectConfigDeleteHandler.java
deleted file mode 100644
index 6f6c3cf..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/deleteproject/projectconfig/ProjectConfigDeleteHandler.java
+++ /dev/null
@@ -1,53 +0,0 @@
-// Copyright (C) 2014 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.deleteproject.projectconfig;
-
-import static java.util.stream.Collectors.joining;
-
-import com.google.gerrit.extensions.common.ProjectInfo;
-import com.google.gerrit.server.project.ListChildProjects;
-import com.google.gerrit.server.project.ProjectResource;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.googlesource.gerrit.plugins.deleteproject.CannotDeleteProjectException;
-import com.googlesource.gerrit.plugins.deleteproject.ProtectedProjects;
-import java.util.List;
-
-public class ProjectConfigDeleteHandler {
-
- private final ProtectedProjects protectedProjects;
- private final Provider<ListChildProjects> listChildProjectsProvider;
-
- @Inject
- public ProjectConfigDeleteHandler(
- ProtectedProjects protectedProjects, Provider<ListChildProjects> listChildProjectsProvider) {
- this.protectedProjects = protectedProjects;
- this.listChildProjectsProvider = listChildProjectsProvider;
- }
-
- public void assertCanDelete(ProjectResource rsrc) throws CannotDeleteProjectException {
- protectedProjects.assertIsNotProtected(rsrc);
- assertHasNoChildProjects(rsrc);
- }
-
- private void assertHasNoChildProjects(ProjectResource rsrc) throws CannotDeleteProjectException {
- List<ProjectInfo> children = listChildProjectsProvider.get().apply(rsrc);
- if (!children.isEmpty()) {
- throw new CannotDeleteProjectException(
- "Cannot delete project because it has children: "
- + children.stream().map(info -> info.name).collect(joining(",")));
- }
- }
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/deleteproject/DeletePreconditionsTest.java b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/DeletePreconditionsTest.java
new file mode 100644
index 0000000..125041a
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/deleteproject/DeletePreconditionsTest.java
@@ -0,0 +1,173 @@
+// Copyright (C) 2018 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.deleteproject;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.deleteproject.DeleteOwnProjectCapability.DELETE_OWN_PROJECT;
+import static com.googlesource.gerrit.plugins.deleteproject.DeleteProjectCapability.DELETE_PROJECT;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.extensions.common.ProjectInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.CapabilityControl;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MergeOpRepoManager;
+import com.google.gerrit.server.git.SubmoduleOp;
+import com.google.gerrit.server.project.ListChildProjects;
+import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Provider;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class DeletePreconditionsTest {
+ private static final String PLUGIN_NAME = "delete-project";
+ private static final String DELETE_OWN_PROJECT_PERMISSION =
+ PLUGIN_NAME + "-" + DELETE_OWN_PROJECT;
+ private static final String DELETE_PROJECT_PERMISSION = PLUGIN_NAME + "-" + DELETE_PROJECT;
+ private static final Project.NameKey PROJECT_NAMEKEY = new Project.NameKey("test-project");
+
+ @Mock private Configuration config;
+ @Mock private Provider<ListChildProjects> listChildProjectsProvider;
+ @Mock private Provider<MergeOpRepoManager> mergeOpProvider;
+ @Mock private Provider<InternalChangeQuery> queryProvider;
+ @Mock private GitRepositoryManager repoManager;
+ @Mock private SubmoduleOp.Factory subOpFactory;
+ @Mock private Provider<CurrentUser> userProvider;
+ @Mock private ProtectedProjects protectedProjects;
+
+ @Mock private ProjectControl control;
+ @Mock private CapabilityControl ctl;
+ @Mock private CurrentUser user;
+
+ @Rule public ExpectedException expectedException = ExpectedException.none();
+
+ private ProjectResource rsrc;
+ private DeletePreconditions preConditions;
+
+ @Before
+ public void setUp() {
+ rsrc = new ProjectResource(control);
+ preConditions =
+ new DeletePreconditions(
+ config,
+ listChildProjectsProvider,
+ mergeOpProvider,
+ PLUGIN_NAME,
+ queryProvider,
+ repoManager,
+ subOpFactory,
+ userProvider,
+ protectedProjects);
+ }
+
+ @Test
+ public void testUserCanDeleteIfAdmin() {
+ when(ctl.canAdministrateServer()).thenReturn(true);
+ when(userProvider.get()).thenReturn(user);
+ when(user.getCapabilities()).thenReturn(ctl);
+ assertThat(preConditions.canDelete(rsrc)).isTrue();
+ }
+
+ @Test
+ public void testUserCanDeleteIfHasDeletePermission() {
+ when(ctl.canAdministrateServer()).thenReturn(false);
+ when(ctl.canPerform(DELETE_PROJECT_PERMISSION)).thenReturn(true);
+ when(userProvider.get()).thenReturn(user);
+ when(user.getCapabilities()).thenReturn(ctl);
+ assertThat(preConditions.canDelete(rsrc)).isTrue();
+ }
+
+ @Test
+ public void testUserCanDeleteIfIsOwnerAndHasDeleteOwnPermission() {
+ when(ctl.canAdministrateServer()).thenReturn(false);
+ when(ctl.canPerform(DELETE_PROJECT_PERMISSION)).thenReturn(false);
+ when(ctl.canPerform(DELETE_OWN_PROJECT_PERMISSION)).thenReturn(true);
+ when(userProvider.get()).thenReturn(user);
+ when(user.getCapabilities()).thenReturn(ctl);
+ when(control.isOwner()).thenReturn(true);
+ assertThat(preConditions.canDelete(rsrc)).isTrue();
+ }
+
+ @Test
+ public void testUserCannotDelete() throws Exception {
+ when(ctl.canAdministrateServer()).thenReturn(false);
+ when(ctl.canPerform(DELETE_PROJECT_PERMISSION)).thenReturn(false);
+ when(ctl.canPerform(DELETE_OWN_PROJECT_PERMISSION)).thenReturn(false);
+ when(userProvider.get()).thenReturn(user);
+ when(user.getCapabilities()).thenReturn(ctl);
+ expectedException.expect(AuthException.class);
+ expectedException.expectMessage("not allowed to delete project");
+ preConditions.assertDeletePermission(rsrc);
+ }
+
+ @Test
+ public void testIsProtectedSoCannotBeDeleted() throws Exception {
+ doThrow(CannotDeleteProjectException.class).when(protectedProjects).assertIsNotProtected(rsrc);
+ expectedException.expect(ResourceConflictException.class);
+ preConditions.assertCanBeDeleted(rsrc, new DeleteProject.Input());
+ }
+
+ @Test
+ public void testHasChildrenSoCannotBeDeleted() throws Exception {
+ doNothing().when(protectedProjects).assertIsNotProtected(rsrc);
+ ListChildProjects childProjects = mock(ListChildProjects.class);
+ when(listChildProjectsProvider.get()).thenReturn(childProjects);
+ when(childProjects.apply(rsrc)).thenReturn(ImmutableList.of(new ProjectInfo()));
+ expectedException.expect(ResourceConflictException.class);
+ expectedException.expectMessage("Cannot delete project because it has children:");
+ preConditions.assertCanBeDeleted(rsrc, new DeleteProject.Input());
+ }
+
+ @Test
+ public void testAssertHasOpenChangesNoForceSet() throws Exception {
+ InternalChangeQuery queryChange = mock(InternalChangeQuery.class);
+ ChangeData cd = mock(ChangeData.class);
+ when(queryChange.byProjectOpen(PROJECT_NAMEKEY)).thenReturn(ImmutableList.of(cd));
+ when(queryProvider.get()).thenReturn(queryChange);
+ String expectedMessage = String.format("Project '%s' has open changes.", PROJECT_NAMEKEY.get());
+ expectedException.expectMessage(expectedMessage);
+ expectedException.expect(CannotDeleteProjectException.class);
+ preConditions.assertHasOpenChanges(PROJECT_NAMEKEY, false);
+ }
+
+ @Test
+ public void testUnableToAssertOpenChanges() throws Exception {
+ InternalChangeQuery queryChange = mock(InternalChangeQuery.class);
+ doThrow(OrmException.class).when(queryChange).byProjectOpen(PROJECT_NAMEKEY);
+ when(queryProvider.get()).thenReturn(queryChange);
+ String expectedMessage =
+ String.format("Unable to verify if '%s' has open changes.", PROJECT_NAMEKEY.get());
+ expectedException.expectMessage(expectedMessage);
+ expectedException.expect(CannotDeleteProjectException.class);
+ preConditions.assertHasOpenChanges(PROJECT_NAMEKEY, false);
+ }
+}
