diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index 4d75d823ec..667e3f0bb9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -78,21 +78,8 @@ public class ChangeNotes extends VersionedMetaData { this.repoManager = repoManager; } - // TODO(dborowitz): Wrap fewer exceptions if/when we kill gwtorm. - public ChangeNotes load(Change change) throws OrmException { - Repository repo; - try { - repo = repoManager.openRepository(change.getProject()); - } catch (IOException e) { - throw new OrmException(e); - } - try { - return new ChangeNotes(repo, change); - } catch (ConfigInvalidException | IOException e) { - throw new OrmException(e); - } finally { - repo.close(); - } + public ChangeNotes create(Change change) { + return new ChangeNotes(repoManager, change); } } @@ -241,15 +228,41 @@ public class ChangeNotes extends VersionedMetaData { } } + private final GitRepositoryManager repoManager; private final Change change; + private boolean loaded; private ImmutableListMultimap approvals; private ImmutableSetMultimap reviewers; @VisibleForTesting - ChangeNotes(Repository repo, Change change) - throws ConfigInvalidException, IOException { + ChangeNotes(GitRepositoryManager repoManager, Change change) { + this.repoManager = repoManager; this.change = change; - load(repo); + } + + // TODO(dborowitz): Wrap fewer exceptions if/when we kill gwtorm. + public ChangeNotes load() throws OrmException { + if (!loaded) { + Repository repo; + try { + repo = repoManager.openRepository(change.getProject()); + } catch (IOException e) { + throw new OrmException(e); + } + try { + load(repo); + loaded = true; + } catch (ConfigInvalidException | IOException e) { + throw new OrmException(e); + } finally { + repo.close(); + } + } + return this; + } + + public Change getChange() { + return change; } public ImmutableListMultimap getApprovals() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java index f73a2f70ee..4c2a30bca3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java @@ -31,6 +31,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -171,6 +172,7 @@ public class ChangeControl { interface AssistedFactory { ChangeControl create(RefControl refControl, Change change); + ChangeControl create(RefControl refControl, ChangeNotes notes); } /** @@ -189,23 +191,34 @@ public class ChangeControl { private final ApprovalsUtil approvalsUtil; private final ChangeData.Factory changeDataFactory; private final RefControl refControl; - private final Change change; + private final ChangeNotes notes; + + @AssistedInject + ChangeControl( + ApprovalsUtil approvalsUtil, + ChangeData.Factory changeDataFactory, + ChangeNotes.Factory notesFactory, + @Assisted RefControl refControl, + @Assisted Change change) { + this(approvalsUtil, changeDataFactory, refControl, + notesFactory.create(change)); + } @AssistedInject ChangeControl( ApprovalsUtil approvalsUtil, ChangeData.Factory changeDataFactory, @Assisted RefControl refControl, - @Assisted Change change) { + @Assisted ChangeNotes notes) { this.approvalsUtil = approvalsUtil; this.changeDataFactory = changeDataFactory; this.refControl = refControl; - this.change = change; + this.notes = notes; } public ChangeControl forUser(final CurrentUser who) { return new ChangeControl(approvalsUtil, changeDataFactory, - getRefControl().forUser(who), change); + getRefControl().forUser(who), notes); } public RefControl getRefControl() { @@ -225,12 +238,17 @@ public class ChangeControl { } public Change getChange() { - return change; + return notes.getChange(); + } + + public ChangeNotes getNotes() { + return notes; } /** Can this user see this change? */ public boolean isVisible(ReviewDb db) throws OrmException { - if (change.getStatus() == Change.Status.DRAFT && !isDraftVisible(db, null)) { + if (getChange().getStatus() == Change.Status.DRAFT + && !isDraftVisible(db, null)) { return false; } return isRefVisible(); @@ -326,7 +344,7 @@ public class ChangeControl { public boolean isOwner() { if (getCurrentUser().isIdentifiedUser()) { final IdentifiedUser i = (IdentifiedUser) getCurrentUser(); - return i.getAccountId().equals(change.getOwner()); + return i.getAccountId().equals(getChange().getOwner()); } return false; } @@ -384,7 +402,7 @@ public class ChangeControl { /** Can this user edit the topic name? */ public boolean canEditTopicName() { - if (change.getStatus().isOpen()) { + if (getChange().getStatus().isOpen()) { return isOwner() // owner (aka creator) of the change can edit topic || getRefControl().isOwner() // branch owner can edit topic || getProjectControl().isOwner() // project owner can edit topic @@ -411,18 +429,18 @@ public class ChangeControl { public List canSubmit(ReviewDb db, PatchSet patchSet, @Nullable ChangeData cd, boolean fastEvalLabels, boolean allowClosed, boolean allowDraft) { - if (!allowClosed && change.getStatus().isClosed()) { + if (!allowClosed && getChange().getStatus().isClosed()) { SubmitRecord rec = new SubmitRecord(); rec.status = SubmitRecord.Status.CLOSED; return Collections.singletonList(rec); } - if (!patchSet.getId().equals(change.currentPatchSetId())) { + if (!patchSet.getId().equals(getChange().currentPatchSetId())) { return ruleError("Patch set " + patchSet.getPatchSetId() + " is not current"); } cd = changeData(db, cd); - if ((change.getStatus() == Change.Status.DRAFT || patchSet.isDraft()) + if ((getChange().getStatus() == Change.Status.DRAFT || patchSet.isDraft()) && !allowDraft) { return cannotSubmitDraft(db, patchSet, cd); } @@ -432,7 +450,7 @@ public class ChangeControl { try { evaluator = new SubmitRuleEvaluator(db, patchSet, getProjectControl(), - this, change, cd, + this, getChange(), cd, fastEvalLabels, "locate_submit_rule", "can_submit", "locate_submit_filter", "filter_submit_results"); @@ -447,7 +465,7 @@ public class ChangeControl { // required for this change to be submittable. Each label will indicate // whether or not that is actually possible given the permissions. log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change " - + change.getId() + " of " + getProject().getName() + + getChange().getId() + " of " + getProject().getName() + " has no solution."); return ruleError("Project submit rule has no solution"); } @@ -569,7 +587,8 @@ public class ChangeControl { @Nullable ChangeData cd) { cd = changeData(db, cd); try { - if (change.getStatus() == Change.Status.DRAFT && !isDraftVisible(db, cd)) { + if (getChange().getStatus() == Change.Status.DRAFT + && !isDraftVisible(db, cd)) { return typeRuleError("Patch set " + patchSet.getPatchSetId() + " not found"); } @@ -586,7 +605,7 @@ public class ChangeControl { SubmitRuleEvaluator evaluator; try { evaluator = new SubmitRuleEvaluator(db, patchSet, - getProjectControl(), this, change, cd, + getProjectControl(), this, getChange(), cd, false, "locate_submit_type", "get_submit_type", "locate_submit_type_filter", "filter_submit_type_results"); @@ -598,7 +617,7 @@ public class ChangeControl { if (results.isEmpty()) { // Should never occur for a well written rule log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change " - + change.getId() + " of " + getProject().getName() + + getChange().getId() + " of " + getProject().getName() + " has no solution."); return typeRuleError("Project submit rule has no solution"); } @@ -606,7 +625,7 @@ public class ChangeControl { Term typeTerm = results.get(0); if (!typeTerm.isSymbol()) { log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change " - + change.getId() + " of " + getProject().getName() + + getChange().getId() + " of " + getProject().getName() + " did not return a symbol."); return typeRuleError("Project submit rule has invalid solution"); } @@ -621,7 +640,7 @@ public class ChangeControl { } private List logInvalidResult(Term rule, Term record, String reason) { - return logRuleError("Submit rule " + rule + " for change " + change.getId() + return logRuleError("Submit rule " + rule + " for change " + getChange().getId() + " of " + getProject().getName() + " output invalid result: " + record + (reason == null ? "" : ". Reason: " + reason)); } @@ -649,7 +668,7 @@ public class ChangeControl { private SubmitTypeRecord logInvalidType(Term rule, String record) { return logTypeRuleError("Submit type rule " + rule + " for change " - + change.getId() + " of " + getProject().getName() + + getChange().getId() + " of " + getProject().getName() + " output invalid result: " + record); } @@ -671,7 +690,7 @@ public class ChangeControl { } private ChangeData changeData(ReviewDb db, @Nullable ChangeData cd) { - return cd != null ? cd : changeDataFactory.create(db, change); + return cd != null ? cd : changeDataFactory.create(db, getChange()); } private void appliedBy(SubmitRecord.Label label, Term status) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index 6d72456b73..01e01bf4e2 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -40,6 +40,7 @@ import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.util.TimeUtil; import com.google.gerrit.testutil.FakeAccountCache; import com.google.gerrit.testutil.InMemoryRepositoryManager; +import com.google.gwtorm.server.OrmException; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; @@ -377,9 +378,8 @@ public class ChangeNotesTest { TimeUtil.nowTs(), TZ); } - private ChangeNotes newNotes(Change c) - throws ConfigInvalidException, IOException { - return new ChangeNotes(repo, c); + private ChangeNotes newNotes(Change c) throws OrmException { + return new ChangeNotes(repoManager, c).load(); } private static void incrementPatchSet(Change change) { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java index 6e59b7cf12..f09f2d8982 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java @@ -244,7 +244,7 @@ public class Util { return new ProjectControl(Collections. emptySet(), Collections. emptySet(), projectCache, - sectionSorter, null, changeControlFactory, canonicalWebUrl, + sectionSorter, repoManager, changeControlFactory, canonicalWebUrl, new MockUser(name, memberOf), newProjectState(local)); }