opendev/gerrit
Code Issues Proposed changes

Refactor MergeOpRepoManager by consolidating getRepo and openRepo

Browse Source 
There is no need for two methods with the same purpose. Assertion of
the repo's open status does not make any sense, since you can just
simply open the repo if it is not open, like openRepo does.

Change-Id: I55de272cbe430d2c7803f65621838cffc109c021
This commit is contained in:
Zhen Chen
2017-02-09 14:25:12 -08:00
committed by Dave Borowitz
parent 8eb7ed0215
commit d097e8bce2
6 changed files with 24 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
gerrit-server/src/main/java/com/google/gerrit/server/change/PreviewSubmit.java
View File

@@ -33,6 +33,7 @@ import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeOpRepoManager;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -162,6 +163,8 @@ public class PreviewSubmit implements RestReadView<RevisionResource> {
}
} catch (LimitExceededException e) {
throw new NotImplementedException("The bundle is too big to generate at the server");
} catch (NoSuchProjectException e) {
throw new IOException(e);
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/git/GitModules.java
View File

@@ -65,7 +65,7 @@ public class GitModules {
Project.NameKey project = branch.getParentKey();
logDebug("Loading .gitmodules of {} for project {}", branch, project);
try {
OpenRepo or = orm.openRepo(project);
OpenRepo or = orm.getRepo(project);
ObjectId id = or.repo.resolve(branch.get());
if (id == null) {
throw new IOException("Cannot open branch " + branch.get());

15
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@@ -37,6 +37,7 @@ import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
@@ -463,7 +464,9 @@ public class MergeOp implements AutoCloseable {
new SubmitStrategyListener(submitInput, strategies, commitStatus),
submissionId,
dryrun);
} catch (SubmoduleException e) {
} catch (NoSuchProjectException e) {
throw new ResourceNotFoundException(e.getMessage());
} catch (IOException | SubmoduleException e) {
throw new IntegrationException(e);
} catch (UpdateException e) {
// BatchUpdate may have inadvertently wrapped an IntegrationException
@@ -493,7 +496,7 @@ public class MergeOp implements AutoCloseable {
private List<SubmitStrategy> getSubmitStrategies(
Map<Branch.NameKey, BranchBatch> toSubmit, SubmoduleOp submoduleOp, boolean dryrun)
throws IntegrationException {
throws IntegrationException, NoSuchProjectException, IOException {
List<SubmitStrategy> strategies = new ArrayList<>();
Set<Branch.NameKey> allBranches = submoduleOp.getBranchesInOrder();
Set<CodeReviewCommit> allCommits =
@@ -731,10 +734,10 @@ public class MergeOp implements AutoCloseable {
private OpenRepo openRepo(Project.NameKey project) throws IntegrationException {
try {
return orm.openRepo(project);
} catch (NoSuchProjectException noProject) {
logWarn("Project " + noProject.project() + " no longer exists, abandoning open changes");
abandonAllOpenChangeForDeletedProject(noProject.project());
return orm.getRepo(project);
} catch (NoSuchProjectException e) {
logWarn("Project " + project + " no longer exists, abandoning open changes.");
abandonAllOpenChangeForDeletedProject(project);
} catch (IOException e) {
throw new IntegrationException("Error opening project " + project, e);
}

21
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOpRepoManager.java
View File

@@ -113,14 +113,6 @@ public class MergeOpRepoManager implements AutoCloseable {
return update;
}
/**
* Make sure the update has already executed before reset it. TODO:czhen Have a flag in
* BatchUpdate to mark if it has been executed
*/
void resetUpdate() {
update = null;
}
private void close() {
if (update != null) {
update.close();
@@ -191,13 +183,7 @@ public class MergeOpRepoManager implements AutoCloseable {
return submissionId;
}
public OpenRepo getRepo(Project.NameKey project) {
OpenRepo or = openRepos.get(project);
checkState(or != null, "repo not yet opened: %s", project);
return or;
}
public OpenRepo openRepo(Project.NameKey project) throws NoSuchProjectException, IOException {
public OpenRepo getRepo(Project.NameKey project) throws NoSuchProjectException, IOException {
if (openRepos.containsKey(project)) {
return openRepos.get(project);
}
@@ -211,11 +197,12 @@ public class MergeOpRepoManager implements AutoCloseable {
openRepos.put(project, or);
return or;
} catch (RepositoryNotFoundException e) {
throw new NoSuchProjectException(project);
throw new NoSuchProjectException(project, e);
}
}
public List<BatchUpdate> batchUpdates(Collection<Project.NameKey> projects) {
public List<BatchUpdate> batchUpdates(Collection<Project.NameKey> projects)
throws NoSuchProjectException, IOException {
List<BatchUpdate> updates = new ArrayList<>(projects.size());
for (Project.NameKey project : projects) {
updates.add(getRepo(project).getUpdate());

2
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
View File

@@ -288,7 +288,7 @@ public class MergeSuperSet {
closeOrm = true;
}
try {
OpenRepo or = orm.openRepo(project);
OpenRepo or = orm.getRepo(project);
checkState(or.rw.hasRevSort(RevSort.TOPO));
return or;
} catch (NoSuchProjectException e) {

12
gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java
View File

@@ -259,7 +259,7 @@ public class SubmoduleOp {
}
OpenRepo or;
try {
or = orm.openRepo(s.getProject());
or = orm.getRepo(s.getProject());
} catch (NoSuchProjectException e) {
// A project listed a non existent project to be allowed
// to subscribe to it. Allow this for now, i.e. no exception is
@@ -293,7 +293,7 @@ public class SubmoduleOp {
for (Branch.NameKey targetBranch : branches) {
Project.NameKey targetProject = targetBranch.getParentKey();
try {
OpenRepo or = orm.openRepo(targetProject);
OpenRepo or = orm.getRepo(targetProject);
ObjectId id = or.repo.resolve(targetBranch.get());
if (id == null) {
logDebug("The branch " + targetBranch + " doesn't exist.");
@@ -329,7 +329,7 @@ public class SubmoduleOp {
if (branchesByProject.containsKey(project)) {
superProjects.add(project);
// get a new BatchUpdate for the super project
OpenRepo or = orm.openRepo(project);
OpenRepo or = orm.getRepo(project);
for (Branch.NameKey branch : branchesByProject.get(project)) {
addOp(or.getUpdate(), branch);
}
@@ -347,7 +347,7 @@ public class SubmoduleOp {
throws IOException, SubmoduleException {
OpenRepo or;
try {
or = orm.openRepo(subscriber.getParentKey());
or = orm.getRepo(subscriber.getParentKey());
} catch (NoSuchProjectException | IOException e) {
throw new SubmoduleException("Cannot access superproject", e);
}
@@ -406,7 +406,7 @@ public class SubmoduleOp {
throws IOException, SubmoduleException {
OpenRepo or;
try {
or = orm.openRepo(subscriber.getParentKey());
or = orm.getRepo(subscriber.getParentKey());
} catch (NoSuchProjectException | IOException e) {
throw new SubmoduleException("Cannot access superproject", e);
}
@@ -447,7 +447,7 @@ public class SubmoduleOp {
throws SubmoduleException, IOException {
OpenRepo subOr;
try {
subOr = orm.openRepo(s.getSubmodule().getParentKey());
subOr = orm.getRepo(s.getSubmodule().getParentKey());
} catch (NoSuchProjectException | IOException e) {
throw new SubmoduleException("Cannot access submodule", e);
}