Create ChangeResource with ChangeNotes and CurrentUser
After removing all callers of ChangeResource#getChangeControl()
ChangeResource can be constructed using ChangeNotes and CurrentUser
instead of ChangeControl.
Change-Id: I6707a29309065857d881666992344a4eaae641dc
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 0db732b..483e3d4 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -102,7 +102,6 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.MutableNotesMigration;
import com.google.gerrit.server.notedb.PatchSetState;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.Util;
import com.google.gerrit.server.query.change.ChangeData;
@@ -253,7 +252,6 @@
@Inject private InProcessProtocol inProcessProtocol;
@Inject private Provider<AnonymousUser> anonymousUser;
@Inject private SchemaFactory<ReviewDb> reviewDbProvider;
- @Inject private ChangeControl.GenericFactory changeControlFactory;
private List<Repository> toClose;
@@ -1119,8 +1117,7 @@
protected ChangeResource parseChangeResource(String changeId) throws Exception {
List<ChangeNotes> notes = changeFinder.find(changeId);
assertThat(notes).hasSize(1);
- return changeResourceFactory.create(
- changeControlFactory.controlFor(notes.get(0), atrScope.get().getUser()));
+ return changeResourceFactory.create(notes.get(0), atrScope.get().getUser());
}
protected String createGroup(String name) throws Exception {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java
index d1c6763..f9c4808 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java
@@ -165,7 +165,7 @@
}
Iterable<UiAction.Description> descs =
- uiActions.from(changeViews, changeResourceFactory.create(ctl));
+ uiActions.from(changeViews, changeResourceFactory.create(ctl.getNotes(), ctl.getUser()));
// The followup action is a client-side only operation that does not
// have a server side handler. It must be manually registered into the
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index dddedc3..56eb46b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -1345,7 +1345,9 @@
&& userProvider.get().isIdentifiedUser()) {
actionJson.addRevisionActions(
- changeInfo, out, new RevisionResource(changeResourceFactory.create(ctl), in));
+ changeInfo,
+ out,
+ new RevisionResource(changeResourceFactory.create(ctl.getNotes(), ctl.getUser()), in));
}
if (gpgApi.isEnabled() && has(PUSH_CERTIFICATES)) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
index 4e7d96a..4166bf7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeResource.java
@@ -17,6 +17,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableList;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
import com.google.gerrit.extensions.restapi.RestResource;
@@ -35,18 +36,23 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.TypeLiteral;
import com.google.inject.assistedinject.Assisted;
+import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class ChangeResource implements RestResource, HasETag {
+ private static final Logger log = LoggerFactory.getLogger(ChangeResource.class);
+
/**
* JSON format version number for ETag computations.
*
@@ -59,7 +65,7 @@
new TypeLiteral<RestView<ChangeResource>>() {};
public interface Factory {
- ChangeResource create(ChangeControl ctl);
+ ChangeResource create(ChangeNotes notes, CurrentUser user);
}
private static final String ZERO_ID_STRING = ObjectId.zeroId().name();
@@ -70,7 +76,9 @@
private final PatchSetUtil patchSetUtil;
private final PermissionBackend permissionBackend;
private final StarredChangesUtil starredChangesUtil;
- private final ChangeControl control;
+ private final ProjectCache projectCache;
+ private final ChangeNotes notes;
+ private final CurrentUser user;
@Inject
ChangeResource(
@@ -80,37 +88,40 @@
PatchSetUtil patchSetUtil,
PermissionBackend permissionBackend,
StarredChangesUtil starredChangesUtil,
- @Assisted ChangeControl control) {
+ ProjectCache projectCache,
+ @Assisted ChangeNotes notes,
+ @Assisted CurrentUser user) {
this.db = db;
this.accountCache = accountCache;
this.approvalUtil = approvalUtil;
this.patchSetUtil = patchSetUtil;
this.permissionBackend = permissionBackend;
this.starredChangesUtil = starredChangesUtil;
- this.control = control;
+ this.projectCache = projectCache;
+ this.notes = notes;
+ this.user = user;
}
public PermissionBackend.ForChange permissions() {
- return permissionBackend.user(control.getUser()).change(getNotes());
+ return permissionBackend.user(user).change(notes);
}
public CurrentUser getUser() {
- return control.getUser();
+ return user;
}
public Change.Id getId() {
- return control.getId();
+ return notes.getChangeId();
}
/** @return true if {@link #getUser()} is the change's owner. */
public boolean isUserOwner() {
- CurrentUser user = control.getUser();
Account.Id owner = getChange().getOwner();
return user.isIdentifiedUser() && user.asIdentifiedUser().getAccountId().equals(owner);
}
public Change getChange() {
- return control.getChange();
+ return notes.getChange();
}
public Project.NameKey getProject() {
@@ -118,7 +129,7 @@
}
public ChangeNotes getNotes() {
- return control.getNotes();
+ return notes;
}
// This includes all information relevant for ETag computation
@@ -143,7 +154,7 @@
}
try {
patchSetUtil
- .byChange(db.get(), getNotes())
+ .byChange(db.get(), notes)
.stream()
.map(ps -> ps.getUploader())
.forEach(accounts::add);
@@ -155,7 +166,7 @@
// set of accounts that posted a message is too expensive. However everyone who posts a
// message is automatically added as reviewer. Hence if we include removed reviewers we can
// be sure that we have all accounts that posted messages on the change.
- accounts.addAll(approvalUtil.getReviewers(db.get(), getNotes()).all());
+ accounts.addAll(approvalUtil.getReviewers(db.get(), notes).all());
} catch (OrmException e) {
// This ETag will be invalidated if it loads next time.
}
@@ -163,7 +174,7 @@
ObjectId noteId;
try {
- noteId = getNotes().loadRevision();
+ noteId = notes.loadRevision();
} catch (OrmException e) {
noteId = null; // This ETag will be invalidated if it loads next time.
}
@@ -171,14 +182,21 @@
// TODO(dborowitz): Include more NoteDb and other related refs, e.g. drafts
// and edits.
- for (ProjectState p : control.getProjectControl().getProjectState().tree()) {
+ Iterable<ProjectState> projectStateTree;
+ try {
+ projectStateTree = projectCache.checkedGet(getProject()).tree();
+ } catch (IOException e) {
+ log.error(String.format("could not load project %s while computing etag", getProject()));
+ projectStateTree = ImmutableList.of();
+ }
+
+ for (ProjectState p : projectStateTree) {
hashObjectId(h, p.getConfig().getRevision(), buf);
}
}
@Override
public String getETag() {
- CurrentUser user = control.getUser();
Hasher h = Hashing.murmur3_128().newHasher();
if (user.isIdentifiedUser()) {
h.putString(starredChangesUtil.getObjectId(user.getAccountId(), getId()).name(), UTF_8);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
index 3556b42..b363b83 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java
@@ -31,7 +31,6 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.QueryChanges;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -98,7 +97,7 @@
if (!canRead(change)) {
throw new ResourceNotFoundException(id);
}
- return changeResourceFactory.create(controlFor(change));
+ return changeResourceFactory.create(change, user.get());
}
public ChangeResource parse(Change.Id id)
@@ -114,15 +113,15 @@
if (!canRead(change)) {
throw new ResourceNotFoundException(toIdString(id));
}
- return changeResourceFactory.create(controlFor(change));
+ return changeResourceFactory.create(change, user.get());
}
private static IdString toIdString(Change.Id id) {
return IdString.fromDecoded(id.toString());
}
- public ChangeResource parse(ChangeControl control) {
- return changeResourceFactory.create(control);
+ public ChangeResource parse(ChangeNotes notes, CurrentUser user) {
+ return changeResourceFactory.create(notes, user);
}
@SuppressWarnings("unchecked")
@@ -134,13 +133,4 @@
private boolean canRead(ChangeNotes notes) throws PermissionBackendException {
return permissionBackend.user(user).change(notes).database(db).test(ChangePermission.READ);
}
-
- private ChangeControl controlFor(ChangeNotes notes) throws ResourceNotFoundException {
- try {
- return changeControlFactory.controlFor(notes, user.get());
- } catch (NoSuchChangeException e) {
- throw new ResourceNotFoundException(
- String.format("Change %s not found", notes.getChangeId()));
- }
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
index b9294ad..2a7bd4b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRevisionActions.java
@@ -72,7 +72,7 @@
ReviewDb db = dbProvider.get();
ChangeSet cs = mergeSuperSet.get().completeChangeSet(db, rsrc.getChange(), user);
for (ChangeData cd : cs.changes()) {
- changeResourceFactory.create(cd.changeControl()).prepareETag(h, user);
+ changeResourceFactory.create(cd.notes(), user).prepareETag(h, user);
}
h.putBoolean(cs.furtherHiddenChanges());
} catch (IOException | OrmException | PermissionBackendException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index ed4dfd1..81edada 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -97,7 +97,6 @@
import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -146,7 +145,7 @@
private static final int DEFAULT_ROBOT_COMMENT_SIZE_LIMIT_IN_BYTES = 1024 * 1024;
private final Provider<ReviewDb> db;
- private final ChangesCollection changes;
+ private final ChangeResource.Factory changeResourceFactory;
private final ChangeData.Factory changeDataFactory;
private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil;
@@ -162,13 +161,12 @@
private final Config gerritConfig;
private final WorkInProgressOp.Factory workInProgressOpFactory;
private final ProjectCache projectCache;
- private final ChangeControl.GenericFactory changeControlFactory;
@Inject
PostReview(
Provider<ReviewDb> db,
RetryHelper retryHelper,
- ChangesCollection changes,
+ ChangeResource.Factory changeResourceFactory,
ChangeData.Factory changeDataFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
@@ -183,11 +181,10 @@
NotifyUtil notifyUtil,
@GerritServerConfig Config gerritConfig,
WorkInProgressOp.Factory workInProgressOpFactory,
- ProjectCache projectCache,
- ChangeControl.GenericFactory changeControlFactory) {
+ ProjectCache projectCache) {
super(retryHelper);
this.db = db;
- this.changes = changes;
+ this.changeResourceFactory = changeResourceFactory;
this.changeDataFactory = changeDataFactory;
this.commentsUtil = commentsUtil;
this.psUtil = psUtil;
@@ -203,7 +200,6 @@
this.gerritConfig = gerritConfig;
this.workInProgressOpFactory = workInProgressOpFactory;
this.projectCache = projectCache;
- this.changeControlFactory = changeControlFactory;
}
@Override
@@ -473,8 +469,8 @@
String.format("on_behalf_of account %s cannot see change", reviewer.getAccountId()));
}
- ChangeControl ctl = changeControlFactory.controlFor(rev.getNotes(), reviewer);
- return new RevisionResource(changes.parse(ctl), rev.getPatchSet());
+ return new RevisionResource(
+ changeResourceFactory.create(rev.getNotes(), reviewer), rev.getPatchSet());
}
private void checkLabels(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 465a1b3..db705e0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -163,7 +163,8 @@
rebasedCommit = rebaseCommit(ctx, original, baseCommit, newCommitMessage);
Base base =
rebaseUtil.parseBase(
- new RevisionResource(changeResourceFactory.create(ctl), originalPatchSet),
+ new RevisionResource(
+ changeResourceFactory.create(ctl.getNotes(), ctl.getUser()), originalPatchSet),
baseCommitId.name());
rebasedPatchSetId =
