Store ChangeEdit in RevisionResource

This may be looked up as soon as the revision is parsed, so later
code may want to access it without doing the lookup again.

Change-Id: Ic5a065e59df127ea9aa11223ce9b30a262bf6297
This commit is contained in:
Dave Borowitz
2014-09-11 15:51:39 +02:00
committed by David Ostrovsky
parent ceef9ffcda
commit 59039f507d
2 changed files with 49 additions and 14 deletions

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import com.google.common.base.Optional;
import com.google.gerrit.extensions.restapi.RestResource; import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestResource.HasETag; import com.google.gerrit.extensions.restapi.RestResource.HasETag;
import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.RestView;
@@ -21,6 +22,7 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.inject.TypeLiteral; import com.google.inject.TypeLiteral;
@@ -31,11 +33,18 @@ public class RevisionResource implements RestResource, HasETag {
private final ChangeResource change; private final ChangeResource change;
private final PatchSet ps; private final PatchSet ps;
private final Optional<ChangeEdit> edit;
private boolean cacheable = true; private boolean cacheable = true;
public RevisionResource(ChangeResource change, PatchSet ps) { public RevisionResource(ChangeResource change, PatchSet ps) {
this(change, ps, Optional.<ChangeEdit> absent());
}
public RevisionResource(ChangeResource change, PatchSet ps,
Optional<ChangeEdit> edit) {
this.change = change; this.change = change;
this.ps = ps; this.ps = ps;
this.edit = edit;
} }
public boolean isCacheable() { public boolean isCacheable() {
@@ -82,4 +91,8 @@ public class RevisionResource implements RestResource, HasETag {
cacheable = false; cacheable = false;
return this; return this;
} }
Optional<ChangeEdit> getEdit() {
return edit;
}
} }

View File

@@ -14,7 +14,9 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import com.google.common.base.Function;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
@@ -74,17 +76,18 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
} }
throw new ResourceNotFoundException(id); throw new ResourceNotFoundException(id);
} }
List<PatchSet> match = Lists.newArrayListWithExpectedSize(2); List<RevisionResource> match = Lists.newArrayListWithExpectedSize(2);
for (PatchSet ps : find(change, id.get())) { for (RevisionResource rsrc : find(change, id.get())) {
Change.Id changeId = ps.getId().getParentKey(); Change.Id changeId = rsrc.getChange().getId();
if (changeId.equals(change.getChange().getId()) && visible(change, ps)) { if (changeId.equals(change.getChange().getId())
match.add(ps); && visible(change, rsrc.getPatchSet())) {
match.add(rsrc);
} }
} }
if (match.size() != 1) { if (match.size() != 1) {
throw new ResourceNotFoundException(id); throw new ResourceNotFoundException(id);
} }
return new RevisionResource(change, match.get(0)); return match.get(0);
} }
private boolean visible(ChangeResource change, PatchSet ps) private boolean visible(ChangeResource change, PatchSet ps)
@@ -92,7 +95,7 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
return change.getControl().isPatchVisible(ps, dbProvider.get()); return change.getControl().isPatchVisible(ps, dbProvider.get());
} }
private List<PatchSet> find(ChangeResource change, String id) private List<RevisionResource> find(ChangeResource change, String id)
throws OrmException { throws OrmException {
ReviewDb db = dbProvider.get(); ReviewDb db = dbProvider.get();
@@ -104,7 +107,7 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
change.getChange().getId(), change.getChange().getId(),
Integer.parseInt(id))); Integer.parseInt(id)));
if (ps != null) { if (ps != null) {
return Collections.singletonList(ps); return toResources(change, ps);
} }
return Collections.emptyList(); return Collections.emptyList();
} else if (id.length() < 4 || id.length() > RevId.LEN) { } else if (id.length() < 4 || id.length() > RevId.LEN) {
@@ -118,27 +121,29 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
// for all patch sets in the change. // for all patch sets in the change.
RevId revid = new RevId(id); RevId revid = new RevId(id);
if (revid.isComplete()) { if (revid.isComplete()) {
List<PatchSet> list = db.patchSets().byRevision(revid).toList(); List<RevisionResource> list =
toResources(change, db.patchSets().byRevision(revid));
if (list.isEmpty()) { if (list.isEmpty()) {
return loadEdit(change, revid); return loadEdit(change, revid);
} }
return list; return list;
} else { } else {
return db.patchSets().byRevisionRange(revid, revid.max()).toList(); return toResources(
change, db.patchSets().byRevisionRange(revid, revid.max()));
} }
} else { } else {
// Chance of collision rises; look at all patch sets on the change. // Chance of collision rises; look at all patch sets on the change.
List<PatchSet> out = Lists.newArrayList(); List<RevisionResource> out = Lists.newArrayList();
for (PatchSet ps : db.patchSets().byChange(change.getChange().getId())) { for (PatchSet ps : db.patchSets().byChange(change.getChange().getId())) {
if (ps.getRevision() != null && ps.getRevision().get().startsWith(id)) { if (ps.getRevision() != null && ps.getRevision().get().startsWith(id)) {
out.add(ps); out.add(new RevisionResource(change, ps));
} }
} }
return out; return out;
} }
} }
private List<PatchSet> loadEdit(ChangeResource change, RevId revid) private List<RevisionResource> loadEdit(ChangeResource change, RevId revid)
throws OrmException { throws OrmException {
try { try {
Optional<ChangeEdit> edit = editUtil.byChange(change.getChange()); Optional<ChangeEdit> edit = editUtil.byChange(change.getChange());
@@ -147,7 +152,8 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
change.getChange().getId(), 0)); change.getChange().getId(), 0));
ps.setRevision(edit.get().getRevision()); ps.setRevision(edit.get().getRevision());
if (revid == null || edit.get().getRevision().equals(revid)) { if (revid == null || edit.get().getRevision().equals(revid)) {
return Collections.singletonList(ps); return Collections.singletonList(
new RevisionResource(change, ps, edit));
} }
} }
} catch (AuthException | IOException | InvalidChangeOperationException e) { } catch (AuthException | IOException | InvalidChangeOperationException e) {
@@ -155,4 +161,20 @@ public class Revisions implements ChildCollection<ChangeResource, RevisionResour
} }
return Collections.emptyList(); return Collections.emptyList();
} }
private static List<RevisionResource> toResources(final ChangeResource change,
Iterable<PatchSet> patchSets) {
return FluentIterable.from(patchSets)
.transform(new Function<PatchSet, RevisionResource>() {
@Override
public RevisionResource apply(PatchSet in) {
return new RevisionResource(change, in);
}
}).toList();
}
private static List<RevisionResource> toResources(ChangeResource change,
PatchSet ps) {
return Collections.singletonList(new RevisionResource(change, ps));
}
} }