From 5fef1eb3a0adaa93bb79be179cf2b9ad650926a0 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Fri, 14 Dec 2018 13:25:26 -0800 Subject: [PATCH] Remove ReviewDb from ChangeData Propagate the removal out to some downstream interfaces as well. Ironically, in at least one case, we needed to *add* a Provider to a class, since it was using ChangeData#db() as the argument to another method which hasn't yet had this argument removed. Change-Id: I42b1609e402d7dd552ea67197d21c28051261ee8 --- .../gerrit/acceptance/AbstractDaemonTest.java | 2 +- .../elasticsearch/ElasticChangeIndex.java | 11 ++----- .../gerrit/lucene/LuceneChangeIndex.java | 11 ++----- .../google/gerrit/server/ApprovalCopier.java | 16 +++------- .../google/gerrit/server/ApprovalsUtil.java | 11 ++----- .../google/gerrit/server/PatchSetUtil.java | 6 +--- .../gerrit/server/StarredChangesUtil.java | 8 ++--- .../gerrit/server/change/ChangeJson.java | 13 ++++---- .../server/change/ChangeKindCacheImpl.java | 7 ++-- .../gerrit/server/change/LabelsJson.java | 1 - .../gerrit/server/change/ReviewerJson.java | 4 +-- .../server/edit/ChangeEditModifier.java | 2 +- .../gerrit/server/edit/ChangeEditUtil.java | 2 +- .../gerrit/server/events/EventFactory.java | 26 +++------------ .../events/StreamEventsApiListener.java | 6 +++- .../server/extensions/events/EventUtil.java | 9 ++---- .../google/gerrit/server/git/MergeUtil.java | 9 +----- .../server/git/SearchingChangeCacheImpl.java | 6 ++-- .../gerrit/server/git/receive/ReplaceOp.java | 3 +- .../git/validators/MergeValidators.java | 7 +--- .../index/change/AllChangesIndexer.java | 6 ++-- .../server/index/change/ChangeIndexer.java | 14 ++++---- .../index/change/ReindexAfterRefUpdate.java | 4 +-- .../server/mail/receive/MailProcessor.java | 14 ++++---- .../gerrit/server/mail/send/ChangeEmail.java | 2 +- .../gerrit/server/mail/send/MergedSender.java | 3 +- .../server/permissions/ChangeControl.java | 32 +++++++------------ .../server/permissions/DefaultRefFilter.java | 4 +-- .../gerrit/server/permissions/RefControl.java | 7 ++-- .../server/query/change/ChangeData.java | 29 +++++------------ .../query/change/ConflictsPredicate.java | 2 +- .../query/change/InternalChangeQuery.java | 16 +++------- .../query/change/OutputStreamQuery.java | 2 +- .../restapi/account/DeleteDraftComments.java | 2 +- .../server/restapi/change/DeleteVote.java | 7 +--- .../gerrit/server/restapi/change/Index.java | 13 ++------ .../restapi/change/ListChangeComments.java | 6 +--- .../restapi/change/ListChangeDrafts.java | 6 +--- .../change/ListChangeRobotComments.java | 6 +--- .../server/restapi/change/MarkAsReviewed.java | 11 ++----- .../restapi/change/MarkAsUnreviewed.java | 11 ++----- .../server/restapi/change/Mergeable.java | 2 +- .../gerrit/server/restapi/change/Move.java | 2 +- .../server/restapi/change/PostReview.java | 6 ++-- .../server/restapi/change/PostReviewers.java | 3 +- .../gerrit/server/restapi/change/Submit.java | 2 +- .../server/restapi/change/TestSubmitRule.java | 7 +--- .../server/restapi/change/TestSubmitType.java | 7 +--- .../gerrit/server/restapi/change/Votes.java | 7 +--- .../submit/LocalMergeSuperSetComputation.java | 17 ++++------ .../google/gerrit/server/submit/MergeOp.java | 6 ++-- .../server/submit/SubmitStrategyOp.java | 2 +- .../acceptance/git/RefAdvertisementIT.java | 2 +- .../gerrit/acceptance/git/SubmitOnPushIT.java | 2 +- .../acceptance/rest/change/ActionsIT.java | 2 +- .../server/change/ConsistencyCheckerIT.java | 4 +-- .../server/change/GetRelatedIT.java | 2 +- .../change/AbstractQueryChangesTest.java | 4 +-- plugins/reviewnotes | 2 +- 59 files changed, 132 insertions(+), 304 deletions(-) diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 72b5361901..d6f43b7368 100644 --- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -1130,7 +1130,7 @@ public abstract class AbstractDaemonTest { } protected PatchSet getPatchSet(PatchSet.Id psId) throws OrmException { - return changeDataFactory.create(db, project, psId.getParentKey()).patchSet(psId); + return changeDataFactory.create(project, psId.getParentKey()).patchSet(psId); } protected IdentifiedUser user(TestAccount testAccount) { diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java index 9aa65a7735..eda9d547b5 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java @@ -45,7 +45,6 @@ import com.google.gerrit.reviewdb.client.Change.Id; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.converter.PatchSetApprovalProtoConverter; import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ReviewerByEmailSet; import com.google.gerrit.server.ReviewerSet; import com.google.gerrit.server.StarredChangesUtil; @@ -61,7 +60,6 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import java.io.IOException; import java.util.Collections; @@ -93,20 +91,17 @@ class ElasticChangeIndex extends AbstractElasticIndex private static final String CLOSED_CHANGES = "closed_" + CHANGES; private final ChangeMapping mapping; - private final Provider db; private final ChangeData.Factory changeDataFactory; private final Schema schema; @Inject ElasticChangeIndex( ElasticConfiguration cfg, - Provider db, ChangeData.Factory changeDataFactory, SitePaths sitePaths, ElasticRestClientProvider clientBuilder, @Assisted Schema schema) { super(cfg, sitePaths, schema, clientBuilder, CHANGES); - this.db = db; this.changeDataFactory = changeDataFactory; this.schema = schema; mapping = new ChangeMapping(schema, client.adapter()); @@ -219,13 +214,11 @@ class ElasticChangeIndex extends AbstractElasticIndex int id = source.get(ChangeField.LEGACY_ID.getName()).getAsInt(); // IndexUtils#changeFields ensures either CHANGE or PROJECT is always present. String projectName = requireNonNull(source.get(ChangeField.PROJECT.getName()).getAsString()); - return changeDataFactory.create( - db.get(), new Project.NameKey(projectName), new Change.Id(id)); + return changeDataFactory.create(new Project.NameKey(projectName), new Change.Id(id)); } ChangeData cd = - changeDataFactory.create( - db.get(), CHANGE_CODEC.decode(Base64.decodeBase64(c.getAsString()))); + changeDataFactory.create(CHANGE_CODEC.decode(Base64.decodeBase64(c.getAsString()))); // Any decoding that is done here must also be done in {@link LuceneChangeIndex}. diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java index 51e95ed3c3..056ecb1f43 100644 --- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -49,7 +49,6 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.converter.PatchSetApprovalProtoConverter; import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter; import com.google.gerrit.reviewdb.converter.ProtoConverter; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; @@ -65,7 +64,6 @@ import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmRuntimeException; import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import com.google.protobuf.MessageLite; import java.io.IOException; @@ -145,7 +143,6 @@ public class LuceneChangeIndex implements ChangeIndex { } private final ListeningExecutorService executor; - private final Provider db; private final ChangeData.Factory changeDataFactory; private final Schema schema; private final QueryBuilder queryBuilder; @@ -157,12 +154,10 @@ public class LuceneChangeIndex implements ChangeIndex { @GerritServerConfig Config cfg, SitePaths sitePaths, @IndexExecutor(INTERACTIVE) ListeningExecutorService executor, - Provider db, ChangeData.Factory changeDataFactory, @Assisted Schema schema) throws IOException { this.executor = executor; - this.db = db; this.changeDataFactory = changeDataFactory; this.schema = schema; @@ -450,15 +445,13 @@ public class LuceneChangeIndex implements ChangeIndex { IndexableField cb = Iterables.getFirst(doc.get(CHANGE_FIELD), null); if (cb != null) { BytesRef proto = cb.binaryValue(); - cd = - changeDataFactory.create( - db.get(), CHANGE_CODEC.decode(proto.bytes, proto.offset, proto.length)); + cd = changeDataFactory.create(CHANGE_CODEC.decode(proto.bytes, proto.offset, proto.length)); } else { IndexableField f = Iterables.getFirst(doc.get(idFieldName), null); Change.Id id = new Change.Id(f.numericValue().intValue()); // IndexUtils#changeFields ensures either CHANGE or PROJECT is always present. IndexableField project = doc.get(PROJECT.getName()).iterator().next(); - cd = changeDataFactory.create(db.get(), new Project.NameKey(project.stringValue()), id); + cd = changeDataFactory.create(new Project.NameKey(project.stringValue()), id); } // Any decoding that is done here must also be done in {@link ElasticChangeIndex}. diff --git a/java/com/google/gerrit/server/ApprovalCopier.java b/java/com/google/gerrit/server/ApprovalCopier.java index 2a87d1f10d..163788a1a5 100644 --- a/java/com/google/gerrit/server/ApprovalCopier.java +++ b/java/com/google/gerrit/server/ApprovalCopier.java @@ -26,7 +26,6 @@ import com.google.gerrit.extensions.client.ChangeKind; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.change.ChangeKindCache; import com.google.gerrit.server.change.LabelNormalizer; import com.google.gerrit.server.notedb.ChangeNotes; @@ -74,18 +73,12 @@ public class ApprovalCopier { } Iterable getForPatchSet( - ReviewDb db, - ChangeNotes notes, - PatchSet.Id psId, - @Nullable RevWalk rw, - @Nullable Config repoConfig) + ChangeNotes notes, PatchSet.Id psId, @Nullable RevWalk rw, @Nullable Config repoConfig) throws OrmException { - return getForPatchSet( - db, notes, psId, rw, repoConfig, Collections.emptyList()); + return getForPatchSet(notes, psId, rw, repoConfig, Collections.emptyList()); } Iterable getForPatchSet( - ReviewDb db, ChangeNotes notes, PatchSet.Id psId, @Nullable RevWalk rw, @@ -96,11 +89,10 @@ public class ApprovalCopier { if (ps == null) { return Collections.emptyList(); } - return getForPatchSet(db, notes, ps, rw, repoConfig, dontCopy); + return getForPatchSet(notes, ps, rw, repoConfig, dontCopy); } private Iterable getForPatchSet( - ReviewDb db, ChangeNotes notes, PatchSet ps, @Nullable RevWalk rw, @@ -108,7 +100,7 @@ public class ApprovalCopier { Iterable dontCopy) throws OrmException { requireNonNull(ps, "ps should not be null"); - ChangeData cd = changeDataFactory.create(db, notes); + ChangeData cd = changeDataFactory.create(notes); try { ProjectState project = projectCache.checkedGet(cd.change().getDest().getParentKey()); ListMultimap all = cd.approvals(); diff --git a/java/com/google/gerrit/server/ApprovalsUtil.java b/java/com/google/gerrit/server/ApprovalsUtil.java index 606ca163dd..9505ae8e97 100644 --- a/java/com/google/gerrit/server/ApprovalsUtil.java +++ b/java/com/google/gerrit/server/ApprovalsUtil.java @@ -350,24 +350,19 @@ public class ApprovalsUtil { } public Iterable byPatchSet( - ReviewDb db, - ChangeNotes notes, - PatchSet.Id psId, - @Nullable RevWalk rw, - @Nullable Config repoConfig) + ChangeNotes notes, PatchSet.Id psId, @Nullable RevWalk rw, @Nullable Config repoConfig) throws OrmException { - return copier.getForPatchSet(db, notes, psId, rw, repoConfig); + return copier.getForPatchSet(notes, psId, rw, repoConfig); } public Iterable byPatchSetUser( - ReviewDb db, ChangeNotes notes, PatchSet.Id psId, Account.Id accountId, @Nullable RevWalk rw, @Nullable Config repoConfig) throws OrmException { - return filterApprovals(byPatchSet(db, notes, psId, rw, repoConfig), accountId); + return filterApprovals(byPatchSet(notes, psId, rw, repoConfig), accountId); } public PatchSetApproval getSubmitter(ChangeNotes notes, PatchSet.Id c) { diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java index 9405486b9b..744d199c04 100644 --- a/java/com/google/gerrit/server/PatchSetUtil.java +++ b/java/com/google/gerrit/server/PatchSetUtil.java @@ -28,7 +28,6 @@ import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RevId; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeUpdate; @@ -52,18 +51,15 @@ import org.eclipse.jgit.revwalk.RevWalk; public class PatchSetUtil { private final Provider approvalsUtilProvider; private final ProjectCache projectCache; - private final Provider dbProvider; private final GitRepositoryManager repoManager; @Inject PatchSetUtil( Provider approvalsUtilProvider, ProjectCache projectCache, - Provider dbProvider, GitRepositoryManager repoManager) { this.approvalsUtilProvider = approvalsUtilProvider; this.projectCache = projectCache; - this.dbProvider = dbProvider; this.repoManager = repoManager; } @@ -159,7 +155,7 @@ public class PatchSetUtil { ApprovalsUtil approvalsUtil = approvalsUtilProvider.get(); for (PatchSetApproval ap : - approvalsUtil.byPatchSet(dbProvider.get(), notes, change.currentPatchSetId(), null, null)) { + approvalsUtil.byPatchSet(notes, change.currentPatchSetId(), null, null)) { LabelType type = projectState.getLabelTypes(notes).byLabel(ap.getLabel()); if (type != null && ap.getValue() == 1 diff --git a/java/com/google/gerrit/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java index 8fa5fafcad..ea3cf37dc7 100644 --- a/java/com/google/gerrit/server/StarredChangesUtil.java +++ b/java/com/google/gerrit/server/StarredChangesUtil.java @@ -35,7 +35,6 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; 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.server.change.ChangeResource; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; @@ -167,7 +166,6 @@ public class StarredChangesUtil { private final GitRepositoryManager repoManager; private final GitReferenceUpdated gitRefUpdated; private final AllUsersName allUsers; - private final Provider dbProvider; private final Provider serverIdent; private final ChangeIndexer indexer; private final Provider queryProvider; @@ -177,14 +175,12 @@ public class StarredChangesUtil { GitRepositoryManager repoManager, GitReferenceUpdated gitRefUpdated, AllUsersName allUsers, - Provider dbProvider, @GerritPersonIdent Provider serverIdent, ChangeIndexer indexer, Provider queryProvider) { this.repoManager = repoManager; this.gitRefUpdated = gitRefUpdated; this.allUsers = allUsers; - this.dbProvider = dbProvider; this.serverIdent = serverIdent; this.indexer = indexer; this.queryProvider = queryProvider; @@ -229,7 +225,7 @@ public class StarredChangesUtil { updateLabels(repo, refName, old.objectId(), labels); } - indexer.index(dbProvider.get(), project, changeId); + indexer.index(project, changeId); return ImmutableSortedSet.copyOf(labels); } catch (IOException e) { throw new OrmException( @@ -259,7 +255,7 @@ public class StarredChangesUtil { changeId.get(), command.getRefName(), command.getResult())); } } - indexer.index(dbProvider.get(), project, changeId); + indexer.index(project, changeId); } catch (IOException e) { throw new OrmException(String.format("Unstar change %d failed", changeId.get()), e); } diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java index b39d716278..ee1ced59e5 100644 --- a/java/com/google/gerrit/server/change/ChangeJson.java +++ b/java/com/google/gerrit/server/change/ChangeJson.java @@ -268,11 +268,11 @@ public class ChangeJson { } public ChangeInfo format(ChangeResource rsrc) throws OrmException { - return format(changeDataFactory.create(db.get(), rsrc.getNotes())); + return format(changeDataFactory.create(rsrc.getNotes())); } public ChangeInfo format(Change change) throws OrmException { - return format(changeDataFactory.create(db.get(), change)); + return format(changeDataFactory.create(change)); } public ChangeInfo format(Project.NameKey project, Change.Id id) throws OrmException { @@ -284,7 +284,7 @@ public class ChangeJson { } public ChangeInfo format(RevisionResource rsrc) throws OrmException { - ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes()); + ChangeData cd = changeDataFactory.create(rsrc.getNotes()); return format(cd, Optional.of(rsrc.getPatchSet().getId()), true, ChangeInfo::new); } @@ -328,10 +328,9 @@ public class ChangeJson { if (!has(CHECK)) { throw e; } - return checkOnly(changeDataFactory.create(db.get(), project, id), changeInfoSupplier); + return checkOnly(changeDataFactory.create(project, id), changeInfoSupplier); } - return format( - changeDataFactory.create(db.get(), notes), Optional.empty(), true, changeInfoSupplier); + return format(changeDataFactory.create(notes), Optional.empty(), true, changeInfoSupplier); } private static Collection requirementsFor(ChangeData cd) { @@ -496,7 +495,7 @@ public class ChangeJson { // If any problems were fixed, the ChangeData needs to be reloaded. for (ProblemInfo p : out.problems) { if (p.status == ProblemInfo.Status.FIXED) { - cd = changeDataFactory.create(cd.db(), cd.project(), cd.getId()); + cd = changeDataFactory.create(cd.project(), cd.getId()); break; } } diff --git a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java index a57a9a4235..0c0bcaa2cf 100644 --- a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java +++ b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java @@ -115,7 +115,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache { @Override public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) { - return getChangeKindInternal(this, db, change, patch, changeDataFactory, repoManager); + return getChangeKindInternal(this, change, patch, changeDataFactory, repoManager); } @Override @@ -351,7 +351,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache { @Override public ChangeKind getChangeKind(ReviewDb db, Change change, PatchSet patch) { - return getChangeKindInternal(this, db, change, patch, changeDataFactory, repoManager); + return getChangeKindInternal(this, change, patch, changeDataFactory, repoManager); } @Override @@ -406,7 +406,6 @@ public class ChangeKindCacheImpl implements ChangeKindCache { private static ChangeKind getChangeKindInternal( ChangeKindCache cache, - ReviewDb db, Change change, PatchSet patch, ChangeData.Factory changeDataFactory, @@ -420,7 +419,7 @@ public class ChangeKindCacheImpl implements ChangeKindCache { RevWalk rw = new RevWalk(repo)) { kind = getChangeKindInternal( - cache, rw, repo.getConfig(), changeDataFactory.create(db, change), patch); + cache, rw, repo.getConfig(), changeDataFactory.create(change), patch); } catch (IOException e) { // Do nothing; assume we have a complex change logger.atWarning().withCause(e).log( diff --git a/java/com/google/gerrit/server/change/LabelsJson.java b/java/com/google/gerrit/server/change/LabelsJson.java index 29c14271ba..a4d0a68bdc 100644 --- a/java/com/google/gerrit/server/change/LabelsJson.java +++ b/java/com/google/gerrit/server/change/LabelsJson.java @@ -263,7 +263,6 @@ public class LabelsJson { Map result = new HashMap<>(); for (PatchSetApproval psa : approvalsUtil.byPatchSetUser( - db.get(), lazyLoad ? cd.notes() : notesFactory.createFromIndexedChange(cd.change()), cd.change().currentPatchSetId(), accountId, diff --git a/java/com/google/gerrit/server/change/ReviewerJson.java b/java/com/google/gerrit/server/change/ReviewerJson.java index ef2c926393..fbe793f278 100644 --- a/java/com/google/gerrit/server/change/ReviewerJson.java +++ b/java/com/google/gerrit/server/change/ReviewerJson.java @@ -76,7 +76,7 @@ public class ReviewerJson { ChangeData cd = null; for (ReviewerResource rsrc : rsrcs) { if (cd == null || !cd.getId().equals(rsrc.getChangeId())) { - cd = changeDataFactory.create(db.get(), rsrc.getChangeResource().getNotes()); + cd = changeDataFactory.create(rsrc.getChangeResource().getNotes()); } ReviewerInfo info; if (rsrc.isByEmail()) { @@ -105,7 +105,7 @@ public class ReviewerJson { out, reviewerAccountId, cd, - approvalsUtil.byPatchSetUser(db.get(), cd.notes(), psId, reviewerAccountId, null, null)); + approvalsUtil.byPatchSetUser(cd.notes(), psId, reviewerAccountId, null, null)); } public ReviewerInfo format( diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java index d11f61d9cc..baec1ea5b8 100644 --- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java +++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java @@ -628,6 +628,6 @@ public class ChangeEditModifier { } private void reindex(Change change) throws IOException { - indexer.index(reviewDb.get(), change); + indexer.index(change); } } diff --git a/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 9e267ac9c8..0f97ddab46 100644 --- a/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -230,7 +230,7 @@ public class ChangeEditUtil { try (Repository repo = gitManager.openRepository(change.getProject())) { deleteRef(repo, edit); } - indexer.index(db.get(), change); + indexer.index(change); } private PatchSet getBasePatchSet(ChangeNotes notes, Ref ref) throws IOException { diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java index 25b96c8ab5..3c55e78e83 100644 --- a/java/com/google/gerrit/server/events/EventFactory.java +++ b/java/com/google/gerrit/server/events/EventFactory.java @@ -128,23 +128,7 @@ public class EventFactory { * @param change * @return object suitable for serialization to JSON */ - public ChangeAttribute asChangeAttribute(Change change, ChangeNotes notes) { - try (ReviewDb db = schema.open()) { - return asChangeAttribute(db, change, notes); - } catch (OrmException e) { - logger.atSevere().withCause(e).log("Cannot open database connection"); - return new ChangeAttribute(); - } - } - - /** - * Create a ChangeAttribute for the given change suitable for serialization to JSON. - * - * @param db Review database - * @param change - * @return object suitable for serialization to JSON - */ - public ChangeAttribute asChangeAttribute(ReviewDb db, Change change) { + public ChangeAttribute asChangeAttribute(Change change) { ChangeAttribute a = new ChangeAttribute(); a.project = change.getProject().get(); a.branch = change.getDest().getShortName(); @@ -153,7 +137,7 @@ public class EventFactory { a.number = change.getId().get(); a.subject = change.getSubject(); try { - a.commitMessage = changeDataFactory.create(db, change).commitMessage(); + a.commitMessage = changeDataFactory.create(change).commitMessage(); } catch (Exception e) { logger.atSevere().withCause(e).log( "Error while getting full commit message for change %d", a.number); @@ -171,14 +155,12 @@ public class EventFactory { /** * Create a ChangeAttribute for the given change suitable for serialization to JSON. * - * @param db Review database * @param change * @param notes * @return object suitable for serialization to JSON */ - public ChangeAttribute asChangeAttribute(ReviewDb db, Change change, ChangeNotes notes) - throws OrmException { - ChangeAttribute a = asChangeAttribute(db, change); + public ChangeAttribute asChangeAttribute(Change change, ChangeNotes notes) throws OrmException { + ChangeAttribute a = asChangeAttribute(change); Set hashtags = notes.load().getHashtags(); if (!hashtags.isEmpty()) { a.hashtags = new ArrayList<>(hashtags.size()); diff --git a/java/com/google/gerrit/server/events/StreamEventsApiListener.java b/java/com/google/gerrit/server/events/StreamEventsApiListener.java index 04abb9657c..911f4bf46b 100644 --- a/java/com/google/gerrit/server/events/StreamEventsApiListener.java +++ b/java/com/google/gerrit/server/events/StreamEventsApiListener.java @@ -154,7 +154,11 @@ public class StreamEventsApiListener new Supplier() { @Override public ChangeAttribute get() { - return eventFactory.asChangeAttribute(change, notes); + try { + return eventFactory.asChangeAttribute(change, notes); + } catch (OrmException e) { + throw new RuntimeException(e); + } } }); } diff --git a/java/com/google/gerrit/server/extensions/events/EventUtil.java b/java/com/google/gerrit/server/extensions/events/EventUtil.java index 485ed50e4a..30450dd7c3 100644 --- a/java/com/google/gerrit/server/extensions/events/EventUtil.java +++ b/java/com/google/gerrit/server/extensions/events/EventUtil.java @@ -25,7 +25,6 @@ import com.google.gerrit.reviewdb.client.Account; 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.server.ReviewDb; import com.google.gerrit.server.GpgException; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.change.ChangeJson; @@ -35,7 +34,6 @@ import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; import java.io.IOException; import java.sql.Timestamp; @@ -63,7 +61,6 @@ public class EventUtil { } private final ChangeData.Factory changeDataFactory; - private final Provider db; private final ChangeJson.Factory changeJsonFactory; private final RevisionJson.Factory revisionJsonFactory; @@ -71,10 +68,8 @@ public class EventUtil { EventUtil( ChangeJson.Factory changeJsonFactory, RevisionJson.Factory revisionJsonFactory, - ChangeData.Factory changeDataFactory, - Provider db) { + ChangeData.Factory changeDataFactory) { this.changeDataFactory = changeDataFactory; - this.db = db; this.changeJsonFactory = changeJsonFactory; this.revisionJsonFactory = revisionJsonFactory; } @@ -92,7 +87,7 @@ public class EventUtil { public RevisionInfo revisionInfo(Project.NameKey project, PatchSet ps) throws OrmException, PatchListNotAvailableException, GpgException, IOException, PermissionBackendException { - ChangeData cd = changeDataFactory.create(db.get(), project, ps.getId().getParentKey()); + ChangeData cd = changeDataFactory.create(project, ps.getId().getParentKey()); return revisionJsonFactory.create(CHANGE_OPTIONS).getRevisionInfo(cd, ps); } diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java index 9cfca88e39..cc60b1028d 100644 --- a/java/com/google/gerrit/server/git/MergeUtil.java +++ b/java/com/google/gerrit/server/git/MergeUtil.java @@ -43,7 +43,6 @@ import com.google.gerrit.reviewdb.client.LabelId; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet.Id; import com.google.gerrit.reviewdb.client.PatchSetApproval; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.config.GerritServerConfig; @@ -58,7 +57,6 @@ import com.google.gerrit.server.submit.MergeIdenticalTreeException; import com.google.gerrit.server.submit.MergeSorter; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; import java.io.IOException; @@ -162,7 +160,6 @@ public class MergeUtil { MergeUtil create(ProjectState project, boolean useContentMerge); } - private final Provider db; private final IdentifiedUser.GenericFactory identifiedUserFactory; private final UrlFormatter urlFormatter; private final ApprovalsUtil approvalsUtil; @@ -174,7 +171,6 @@ public class MergeUtil { @AssistedInject MergeUtil( @GerritServerConfig Config serverConfig, - Provider db, IdentifiedUser.GenericFactory identifiedUserFactory, UrlFormatter urlFormatter, ApprovalsUtil approvalsUtil, @@ -182,7 +178,6 @@ public class MergeUtil { @Assisted ProjectState project) { this( serverConfig, - db, identifiedUserFactory, urlFormatter, approvalsUtil, @@ -194,14 +189,12 @@ public class MergeUtil { @AssistedInject MergeUtil( @GerritServerConfig Config serverConfig, - Provider db, IdentifiedUser.GenericFactory identifiedUserFactory, UrlFormatter urlFormatter, ApprovalsUtil approvalsUtil, @Assisted ProjectState project, PluggableCommitMessageGenerator commitMessageGenerator, @Assisted boolean useContentMerge) { - this.db = db; this.identifiedUserFactory = identifiedUserFactory; this.urlFormatter = urlFormatter; this.approvalsUtil = approvalsUtil; @@ -598,7 +591,7 @@ public class MergeUtil { private Iterable safeGetApprovals(ChangeNotes notes, PatchSet.Id psId) { try { - return approvalsUtil.byPatchSet(db.get(), notes, psId, null, null); + return approvalsUtil.byPatchSet(notes, psId, null, null); } catch (OrmException e) { logger.atSevere().withCause(e).log("Can't read approval records for %s", psId); return Collections.emptyList(); diff --git a/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java b/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java index d5e19cc613..53d5befca3 100644 --- a/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java +++ b/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java @@ -24,7 +24,6 @@ import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.reviewdb.client.Change; 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.server.ReviewerSet; import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.index.change.ChangeField; @@ -107,16 +106,15 @@ public class SearchingChangeCacheImpl implements GitReferenceUpdatedListener { *

Returned changes only include the {@code Change} object (with id, branch) and the reviewers. * Additional stored fields are not loaded from the index. * - * @param db database handle to populate missing change data (probably unused). * @param project project to read. * @return list of known changes; empty if no changes. */ - public List getChangeData(ReviewDb db, Project.NameKey project) { + public List getChangeData(Project.NameKey project) { try { List cached = cache.get(project); List cds = new ArrayList<>(cached.size()); for (CachedChange cc : cached) { - ChangeData cd = changeDataFactory.create(db, cc.change()); + ChangeData cd = changeDataFactory.create(cc.change()); cd.setReviewers(cc.reviewers()); cds.add(cd); } diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java index 8c03fdb252..c4714b4560 100644 --- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -255,7 +255,7 @@ public class ReplaceOp implements BatchUpdateOp { groups = prevPs != null ? prevPs.getGroups() : ImmutableList.of(); } - ChangeData cd = changeDataFactory.create(ctx.getDb(), ctx.getNotes()); + ChangeData cd = changeDataFactory.create(ctx.getNotes()); oldRecipients = getRecipientsFromReviewers(cd.reviewers()); ChangeUpdate update = ctx.getUpdate(patchSetId); @@ -452,7 +452,6 @@ public class ReplaceOp implements BatchUpdateOp { if (!approvals.isEmpty()) { for (PatchSetApproval a : approvalsUtil.byPatchSetUser( - ctx.getDb(), ctx.getNotes(), priorPatchSetId, ctx.getAccountId(), diff --git a/java/com/google/gerrit/server/git/validators/MergeValidators.java b/java/com/google/gerrit/server/git/validators/MergeValidators.java index 56a641f2bd..1dd48f1c0d 100644 --- a/java/com/google/gerrit/server/git/validators/MergeValidators.java +++ b/java/com/google/gerrit/server/git/validators/MergeValidators.java @@ -26,7 +26,6 @@ import com.google.gerrit.reviewdb.client.Branch; 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.server.IdentifiedUser; import com.google.gerrit.server.account.AccountProperties; import com.google.gerrit.server.config.AllProjectsName; @@ -46,7 +45,6 @@ import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import java.io.IOException; import java.util.List; import org.eclipse.jgit.errors.ConfigInvalidException; @@ -268,18 +266,15 @@ public class MergeValidators { AccountMergeValidator create(); } - private final Provider dbProvider; private final AllUsersName allUsersName; private final ChangeData.Factory changeDataFactory; private final AccountValidator accountValidator; @Inject public AccountMergeValidator( - Provider dbProvider, AllUsersName allUsersName, ChangeData.Factory changeDataFactory, AccountValidator accountValidator) { - this.dbProvider = dbProvider; this.allUsersName = allUsersName; this.changeDataFactory = changeDataFactory; this.accountValidator = accountValidator; @@ -301,7 +296,7 @@ public class MergeValidators { ChangeData cd = changeDataFactory.create( - dbProvider.get(), destProject.getProject().getNameKey(), patchSetId.getParentKey()); + destProject.getProject().getNameKey(), patchSetId.getParentKey()); try { if (!cd.currentFilePaths().contains(AccountProperties.ACCOUNT_CONFIG)) { return; diff --git a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java index 8f1433edf8..d17a23649a 100644 --- a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java +++ b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java @@ -224,20 +224,20 @@ public class AllChangesIndexer extends SiteIndexer index(db, r)); + notesFactory.scan(repo, project).forEach(r -> index(r)); } catch (RepositoryNotFoundException rnfe) { logger.atSevere().log(rnfe.getMessage()); } return null; } - private void index(ReviewDb db, ChangeNotesResult r) { + private void index(ChangeNotesResult r) { if (r.error().isPresent()) { fail("Failed to read change " + r.id() + " for indexing", true, r.error().get()); return; } try { - indexer.index(changeDataFactory.create(db, r.notes())); + indexer.index(changeDataFactory.create(r.notes())); done.update(1); verboseWriter.println("Reindexed change " + r.id()); } catch (RejectedExecutionException e) { diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java index 0c81bfa048..879f5f6186 100644 --- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java +++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java @@ -223,22 +223,20 @@ public class ChangeIndexer { /** * Synchronously index a change. * - * @param db review database. * @param change change to index. */ - public void index(ReviewDb db, Change change) throws IOException { - index(changeDataFactory.create(db, change)); + public void index(Change change) throws IOException { + index(changeDataFactory.create(change)); } /** * Synchronously index a change. * - * @param db review database. * @param project the project to which the change belongs. * @param changeId ID of the change to index. */ - public void index(ReviewDb db, Project.NameKey project, Change.Id changeId) throws IOException { - index(changeDataFactory.create(db, project, changeId)); + public void index(Project.NameKey project, Change.Id changeId) throws IOException { + index(changeDataFactory.create(project, changeId)); } /** @@ -369,7 +367,7 @@ public class ChangeIndexer { @Override public Void callImpl(Provider db) throws Exception { - ChangeData cd = changeDataFactory.create(db.get(), project, id); + ChangeData cd = changeDataFactory.create(project, id); index(cd); return null; } @@ -415,7 +413,7 @@ public class ChangeIndexer { public Boolean callImpl(Provider db) throws Exception { try { if (stalenessChecker.isStale(id)) { - indexImpl(changeDataFactory.create(db.get(), project, id)); + indexImpl(changeDataFactory.create(project, id)); return true; } } catch (Exception e) { diff --git a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java index e3fb740667..669bb1ea25 100644 --- a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java +++ b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java @@ -27,7 +27,6 @@ import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; 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.server.account.AccountCache; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.GerritServerConfig; @@ -182,11 +181,10 @@ public class ReindexAfterRefUpdate implements GitReferenceUpdatedListener { @Override protected Void impl(RequestContext ctx) throws OrmException, IOException { // Reload change, as some time may have passed since GetChanges. - ReviewDb db = ctx.getReviewDbProvider().get(); try { Change c = notesFactory.createChecked(new Project.NameKey(event.getProjectName()), id).getChange(); - indexerFactory.create(executor, indexes).index(db, c); + indexerFactory.create(executor, indexes).index(c); } catch (NoSuchChangeException e) { indexerFactory.create(executor, indexes).delete(id); } diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java index b39f4e31c5..8fbce1c50d 100644 --- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java +++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java @@ -39,6 +39,7 @@ import com.google.gerrit.reviewdb.client.Comment; import com.google.gerrit.reviewdb.client.PatchLineComment.Status; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.CommentsUtil; @@ -84,6 +85,7 @@ import java.util.Set; public class MailProcessor { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private final Provider dbProvider; private final Emails emails; private final InboundEmailRejectionSender.Factory emailRejectionSender; private final RetryHelper retryHelper; @@ -102,6 +104,7 @@ public class MailProcessor { @Inject public MailProcessor( + Provider dbProvider, Emails emails, InboundEmailRejectionSender.Factory emailRejectionSender, RetryHelper retryHelper, @@ -117,6 +120,7 @@ public class MailProcessor { CommentAdded commentAdded, AccountCache accountCache, UrlFormatter urlFormatter) { + this.dbProvider = dbProvider; this.emails = emails; this.emailRejectionSender = emailRejectionSender; this.retryHelper = retryHelper; @@ -259,7 +263,8 @@ public class MailProcessor { } Op o = new Op(new PatchSet.Id(cd.getId(), metadata.patchSet), parsedComments, message.id()); - BatchUpdate batchUpdate = buf.create(cd.db(), project, ctx.getUser(), TimeUtil.nowTs()); + BatchUpdate batchUpdate = + buf.create(dbProvider.get(), project, ctx.getUser(), TimeUtil.nowTs()); batchUpdate.addOp(cd.getId(), o); batchUpdate.execute(); } @@ -329,12 +334,7 @@ public class MailProcessor { Map approvals = new HashMap<>(); approvalsUtil .byPatchSetUser( - ctx.getDb(), - notes, - psId, - ctx.getAccountId(), - ctx.getRevWalk(), - ctx.getRepoView().getConfig()) + notes, psId, ctx.getAccountId(), ctx.getRevWalk(), ctx.getRepoView().getConfig()) .forEach(a -> approvals.put(a.getLabel(), a.getValue())); // Fire Gerrit event. Note that approvals can't be granted via email, so old and new approvals // are always the same here. diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java index 84576f6ac4..25ce89c6a7 100644 --- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java +++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java @@ -69,7 +69,7 @@ public abstract class ChangeEmail extends NotificationEmail { protected static ChangeData newChangeData( EmailArguments ea, Project.NameKey project, Change.Id id) { - return ea.changeDataFactory.create(ea.db.get(), project, id); + return ea.changeDataFactory.create(project, id); } protected final Change change; diff --git a/java/com/google/gerrit/server/mail/send/MergedSender.java b/java/com/google/gerrit/server/mail/send/MergedSender.java index 34f959c817..b524c832ad 100644 --- a/java/com/google/gerrit/server/mail/send/MergedSender.java +++ b/java/com/google/gerrit/server/mail/send/MergedSender.java @@ -69,8 +69,7 @@ public class MergedSender extends ReplyToChangeSender { Table pos = HashBasedTable.create(); Table neg = HashBasedTable.create(); for (PatchSetApproval ca : - args.approvalsUtil.byPatchSet( - args.db.get(), changeData.notes(), patchSet.getId(), null, null)) { + args.approvalsUtil.byPatchSet(changeData.notes(), patchSet.getId(), null, null)) { LabelType lt = labelTypes.byLabel(ca.getLabelId()); if (lt == null) { continue; diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java index b15854d1c6..7fa667ccbe 100644 --- a/java/com/google/gerrit/server/permissions/ChangeControl.java +++ b/java/com/google/gerrit/server/permissions/ChangeControl.java @@ -14,7 +14,6 @@ package com.google.gerrit.server.permissions; -import static com.google.common.base.Preconditions.checkState; import static com.google.gerrit.server.permissions.DefaultPermissionMappings.labelPermissionName; import static com.google.gerrit.server.permissions.LabelPermission.ForUser.ON_BEHALF_OF; @@ -35,7 +34,6 @@ import com.google.gerrit.server.permissions.PermissionBackend.ForChange; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; import java.util.Collection; import java.util.EnumSet; @@ -76,8 +74,8 @@ class ChangeControl { this.notes = notes; } - ForChange asForChange(@Nullable ChangeData cd, @Nullable Provider db) { - return new ForChangeImpl(cd, db); + ForChange asForChange(@Nullable ChangeData cd) { + return new ForChangeImpl(cd); } private CurrentUser getUser() { @@ -93,8 +91,8 @@ class ChangeControl { } /** Can this user see this change? */ - private boolean isVisible(ReviewDb db, @Nullable ChangeData cd) throws OrmException { - if (getChange().isPrivate() && !isPrivateVisible(db, cd)) { + private boolean isVisible(@Nullable ChangeData cd) throws OrmException { + if (getChange().isPrivate() && !isPrivateVisible(cd)) { return false; } return refControl.isVisible(); @@ -157,9 +155,9 @@ class ChangeControl { } /** Is this user a reviewer for the change? */ - private boolean isReviewer(ReviewDb db, @Nullable ChangeData cd) throws OrmException { + private boolean isReviewer(@Nullable ChangeData cd) throws OrmException { if (getUser().isIdentifiedUser()) { - cd = cd != null ? cd : changeDataFactory.create(db, notes); + cd = cd != null ? cd : changeDataFactory.create(notes); Collection results = cd.reviewers().all(); return results.contains(getUser().getAccountId()); } @@ -207,9 +205,9 @@ class ChangeControl { || getProjectControl().isAdmin(); } - private boolean isPrivateVisible(ReviewDb db, ChangeData cd) throws OrmException { + private boolean isPrivateVisible(ChangeData cd) throws OrmException { return isOwner() - || isReviewer(db, cd) + || isReviewer(cd) || refControl.canPerform(Permission.VIEW_PRIVATE_CHANGES) || getUser().isInternalUser(); } @@ -219,26 +217,20 @@ class ChangeControl { private Map labels; private String resourcePath; - ForChangeImpl(@Nullable ChangeData cd, @Nullable Provider db) { + ForChangeImpl(@Nullable ChangeData cd) { this.cd = cd; - this.db = db; } private ReviewDb db() { if (db != null) { return db.get(); - } else if (cd != null) { - return cd.db(); - } else { - return null; } + return null; } private ChangeData changeData() { if (cd == null) { - ReviewDb reviewDb = db(); - checkState(reviewDb != null, "need ReviewDb"); - cd = changeDataFactory.create(reviewDb, notes); + cd = changeDataFactory.create(notes); } return cd; } @@ -294,7 +286,7 @@ class ChangeControl { try { switch (perm) { case READ: - return isVisible(db(), changeData()); + return isVisible(changeData()); case ABANDON: return canAbandon(); case DELETE: diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java index ea4073da67..d0e347ec11 100644 --- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java +++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java @@ -76,7 +76,6 @@ class DefaultRefFilter { private final TagCache tagCache; private final ChangeNotes.Factory changeNotesFactory; @Nullable private final SearchingChangeCacheImpl changeCache; - private final Provider db; private final GroupCache groupCache; private final PermissionBackend permissionBackend; private final ProjectControl projectControl; @@ -103,7 +102,6 @@ class DefaultRefFilter { this.tagCache = tagCache; this.changeNotesFactory = changeNotesFactory; this.changeCache = changeCache; - this.db = db; this.groupCache = groupCache; this.permissionBackend = permissionBackend; this.skipFullRefEvaluationIfAllRefsAreVisible = @@ -334,7 +332,7 @@ class DefaultRefFilter { Project.NameKey project = projectState.getNameKey(); try { Map visibleChanges = new HashMap<>(); - for (ChangeData cd : changeCache.getChangeData(db.get(), project)) { + for (ChangeData cd : changeCache.getChangeData(project)) { ChangeNotes notes = changeNotesFactory.createFromIndexedChange(cd.change()); if (!projectState.statePermitsRead()) { continue; diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java index e445eb875e..60fd15bfad 100644 --- a/java/com/google/gerrit/server/permissions/RefControl.java +++ b/java/com/google/gerrit/server/permissions/RefControl.java @@ -34,7 +34,6 @@ import com.google.gerrit.server.permissions.PermissionBackend.ForRef; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.util.MagicBranch; import com.google.gwtorm.server.OrmException; -import com.google.inject.util.Providers; import java.util.Collection; import java.util.EnumSet; import java.util.List; @@ -442,7 +441,7 @@ class RefControl { public ForChange change(ChangeData cd) { try { // TODO(hiesel) Force callers to call database() and use db instead of cd.db() - return getProjectControl().controlFor(cd.change()).asForChange(cd, Providers.of(cd.db())); + return getProjectControl().controlFor(cd.change()).asForChange(cd); } catch (OrmException e) { return FailedPermissionBackend.change("unavailable", e); } @@ -457,12 +456,12 @@ class RefControl { "expected change in project %s, not %s", project, change.getProject()); - return getProjectControl().controlFor(notes).asForChange(null, db); + return getProjectControl().controlFor(notes).asForChange(null); } @Override public ForChange indexedChange(ChangeData cd, ChangeNotes notes) { - return getProjectControl().controlFor(notes).asForChange(cd, db); + return getProjectControl().controlFor(notes).asForChange(cd); } @Override diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java index d61f7ca4c6..787980c93d 100644 --- a/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/java/com/google/gerrit/server/query/change/ChangeData.java @@ -46,7 +46,6 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RobotComment; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.CommentsUtil; @@ -196,23 +195,22 @@ public class ChangeData { this.assistedFactory = assistedFactory; } - public ChangeData create(ReviewDb db, Project.NameKey project, Change.Id id) { - return assistedFactory.create(db, project, id, null, null); + public ChangeData create(Project.NameKey project, Change.Id id) { + return assistedFactory.create(project, id, null, null); } - public ChangeData create(ReviewDb db, Change change) { - return assistedFactory.create(db, change.getProject(), change.getId(), change, null); + public ChangeData create(Change change) { + return assistedFactory.create(change.getProject(), change.getId(), change, null); } - public ChangeData create(ReviewDb db, ChangeNotes notes) { + public ChangeData create(ChangeNotes notes) { return assistedFactory.create( - db, notes.getChange().getProject(), notes.getChangeId(), notes.getChange(), notes); + notes.getChange().getProject(), notes.getChangeId(), notes.getChange(), notes); } } public interface AssistedFactory { ChangeData create( - ReviewDb db, Project.NameKey project, Change.Id id, @Nullable Change change, @@ -233,7 +231,7 @@ public class ChangeData { ChangeData cd = new ChangeData( null, null, null, null, null, null, null, null, null, null, null, null, null, null, - null, null, project, id, null, null); + null, project, id, null, null); cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId)); return cd; } @@ -256,7 +254,6 @@ public class ChangeData { private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory; // Required assisted injected fields. - private final ReviewDb db; private final Project.NameKey project; private final Change.Id legacyId; @@ -322,7 +319,6 @@ public class ChangeData { TrackingFooters trackingFooters, PureRevert pureRevert, SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory, - @Assisted ReviewDb db, @Assisted Project.NameKey project, @Assisted Change.Id id, @Assisted @Nullable Change change, @@ -343,11 +339,6 @@ public class ChangeData { this.pureRevert = pureRevert; this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory; - // May be null in tests when created via createForTest above, in which case lazy-loading will - // intentionally fail with NPE. Still not marked @Nullable in the constructor, to force callers - // using Guice to pass a non-null value. - this.db = db; - this.project = project; this.legacyId = id; @@ -367,10 +358,6 @@ public class ChangeData { return this; } - public ReviewDb db() { - return db; - } - public AllUsersName getAllUsersNameForIndexing() { return allUsersName; } @@ -536,7 +523,7 @@ public class ChangeData { try { currentApprovals = ImmutableList.copyOf( - approvalsUtil.byPatchSet(db, notes(), c.currentPatchSetId(), null, null)); + approvalsUtil.byPatchSet(notes(), c.currentPatchSetId(), null, null)); } catch (OrmException e) { if (e.getCause() instanceof NoSuchChangeException) { currentApprovals = Collections.emptyList(); diff --git a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java index 7dc7a0b080..7a399f232b 100644 --- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java +++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java @@ -51,7 +51,7 @@ public class ConflictsPredicate { ChangeData cd; List files; try { - cd = args.changeDataFactory.create(args.db.get(), c); + cd = args.changeDataFactory.create(c); files = cd.currentFilePaths(); } catch (IOException e) { throw new OrmException(e); diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java index 23993ef5a5..dc89c88550 100644 --- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java +++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java @@ -33,7 +33,6 @@ import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; 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.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gwtorm.server.OrmException; @@ -168,11 +167,10 @@ public class InternalChangeQuery extends InternalQuery { } public Iterable byCommitsOnBranchNotMerged( - Repository repo, ReviewDb db, Branch.NameKey branch, Collection hashes) + Repository repo, Branch.NameKey branch, Collection hashes) throws OrmException, IOException { return byCommitsOnBranchNotMerged( repo, - db, branch, hashes, // Account for all commit predicates plus ref, project, status. @@ -181,20 +179,16 @@ public class InternalChangeQuery extends InternalQuery { @VisibleForTesting Iterable byCommitsOnBranchNotMerged( - Repository repo, - ReviewDb db, - Branch.NameKey branch, - Collection hashes, - int indexLimit) + Repository repo, Branch.NameKey branch, Collection hashes, int indexLimit) throws OrmException, IOException { if (hashes.size() > indexLimit) { - return byCommitsOnBranchNotMergedFromDatabase(repo, db, branch, hashes); + return byCommitsOnBranchNotMergedFromDatabase(repo, branch, hashes); } return byCommitsOnBranchNotMergedFromIndex(branch, hashes); } private Iterable byCommitsOnBranchNotMergedFromDatabase( - Repository repo, ReviewDb db, Branch.NameKey branch, Collection hashes) + Repository repo, Branch.NameKey branch, Collection hashes) throws OrmException, IOException { Set changeIds = Sets.newHashSetWithExpectedSize(hashes.size()); String lastPrefix = null; @@ -221,7 +215,7 @@ public class InternalChangeQuery extends InternalQuery { Change c = cn.getChange(); return c.getDest().equals(branch) && c.getStatus() != Change.Status.MERGED; }); - return Lists.transform(notes, n -> changeDataFactory.create(db, n)); + return Lists.transform(notes, n -> changeDataFactory.create(n)); } private Iterable byCommitsOnBranchNotMergedFromIndex( diff --git a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java index a190ac57f7..994b527542 100644 --- a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java +++ b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java @@ -242,7 +242,7 @@ public class OutputStreamQuery { ChangeData d, Map repos, Map revWalks) throws OrmException, IOException { LabelTypes labelTypes = d.getLabelTypes(); - ChangeAttribute c = eventFactory.asChangeAttribute(db, d.change(), d.notes()); + ChangeAttribute c = eventFactory.asChangeAttribute(d.change(), d.notes()); eventFactory.extend(c, d.change()); if (!trackingFooters.isEmpty()) { diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java index 96b9519abb..aab502d3b1 100644 --- a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java +++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java @@ -196,7 +196,7 @@ public class DeleteDraftComments result.change = changeJsonFactory .create(ListChangesOption.SKIP_MERGEABLE) - .format(changeDataFactory.create(ctx.getDb(), ctx.getNotes())); + .format(changeDataFactory.create(ctx.getNotes())); result.deleted = comments.build(); } return dirty; diff --git a/java/com/google/gerrit/server/restapi/change/DeleteVote.java b/java/com/google/gerrit/server/restapi/change/DeleteVote.java index 1f770e67b6..ca811604b8 100644 --- a/java/com/google/gerrit/server/restapi/change/DeleteVote.java +++ b/java/com/google/gerrit/server/restapi/change/DeleteVote.java @@ -178,12 +178,7 @@ public class DeleteVote extends RetryingRestModifyView> { - - private final Provider db; private final PermissionBackend permissionBackend; private final ChangeIndexer indexer; @Inject - Index( - Provider db, - RetryHelper retryHelper, - PermissionBackend permissionBackend, - ChangeIndexer indexer) { + Index(RetryHelper retryHelper, PermissionBackend permissionBackend, ChangeIndexer indexer) { super(retryHelper); - this.db = db; this.permissionBackend = permissionBackend; this.indexer = indexer; } @@ -56,7 +47,7 @@ public class Index extends RetryingRestModifyView { - private final Provider db; private final ChangeData.Factory changeDataFactory; private final Provider commentJson; private final CommentsUtil commentsUtil; @Inject ListChangeComments( - Provider db, ChangeData.Factory changeDataFactory, Provider commentJson, CommentsUtil commentsUtil) { - this.db = db; this.changeDataFactory = changeDataFactory; this.commentJson = commentJson; this.commentsUtil = commentsUtil; @@ -51,7 +47,7 @@ public class ListChangeComments implements RestReadView { @Override public Map> apply(ChangeResource rsrc) throws AuthException, OrmException, PermissionBackendException { - ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes()); + ChangeData cd = changeDataFactory.create(rsrc.getNotes()); return commentJson .get() .setFillAccounts(true) diff --git a/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java b/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java index 280277c287..7bdc1398d0 100644 --- a/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java +++ b/java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java @@ -18,7 +18,6 @@ import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.Comment; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CommentsUtil; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -32,18 +31,15 @@ import java.util.Map; @Singleton public class ListChangeDrafts implements RestReadView { - private final Provider db; private final ChangeData.Factory changeDataFactory; private final Provider commentJson; private final CommentsUtil commentsUtil; @Inject ListChangeDrafts( - Provider db, ChangeData.Factory changeDataFactory, Provider commentJson, CommentsUtil commentsUtil) { - this.db = db; this.changeDataFactory = changeDataFactory; this.commentJson = commentJson; this.commentsUtil = commentsUtil; @@ -55,7 +51,7 @@ public class ListChangeDrafts implements RestReadView { if (!rsrc.getUser().isIdentifiedUser()) { throw new AuthException("Authentication required"); } - ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes()); + ChangeData cd = changeDataFactory.create(rsrc.getNotes()); List drafts = commentsUtil.draftByChangeAuthor(cd.notes(), rsrc.getUser().getAccountId()); return commentJson diff --git a/java/com/google/gerrit/server/restapi/change/ListChangeRobotComments.java b/java/com/google/gerrit/server/restapi/change/ListChangeRobotComments.java index dc92ced396..3ea2ac2aeb 100644 --- a/java/com/google/gerrit/server/restapi/change/ListChangeRobotComments.java +++ b/java/com/google/gerrit/server/restapi/change/ListChangeRobotComments.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.restapi.change; import com.google.gerrit.extensions.common.RobotCommentInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CommentsUtil; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -29,18 +28,15 @@ import java.util.List; import java.util.Map; public class ListChangeRobotComments implements RestReadView { - private final Provider db; private final ChangeData.Factory changeDataFactory; private final Provider commentJson; private final CommentsUtil commentsUtil; @Inject ListChangeRobotComments( - Provider db, ChangeData.Factory changeDataFactory, Provider commentJson, CommentsUtil commentsUtil) { - this.db = db; this.changeDataFactory = changeDataFactory; this.commentJson = commentJson; this.commentsUtil = commentsUtil; @@ -49,7 +45,7 @@ public class ListChangeRobotComments implements RestReadView { @Override public Map> apply(ChangeResource rsrc) throws AuthException, OrmException, PermissionBackendException { - ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes()); + ChangeData cd = changeDataFactory.create(rsrc.getNotes()); return commentJson .get() .setFillAccounts(true) diff --git a/java/com/google/gerrit/server/restapi/change/MarkAsReviewed.java b/java/com/google/gerrit/server/restapi/change/MarkAsReviewed.java index 7c9ba73c62..49c8fb650d 100644 --- a/java/com/google/gerrit/server/restapi/change/MarkAsReviewed.java +++ b/java/com/google/gerrit/server/restapi/change/MarkAsReviewed.java @@ -20,14 +20,12 @@ import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.webui.UiAction; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; @Singleton @@ -35,16 +33,11 @@ public class MarkAsReviewed implements RestModifyView, UiAction { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final Provider dbProvider; private final ChangeData.Factory changeDataFactory; private final StarredChangesUtil stars; @Inject - MarkAsReviewed( - Provider dbProvider, - ChangeData.Factory changeDataFactory, - StarredChangesUtil stars) { - this.dbProvider = dbProvider; + MarkAsReviewed(ChangeData.Factory changeDataFactory, StarredChangesUtil stars) { this.changeDataFactory = changeDataFactory; this.stars = stars; } @@ -67,7 +60,7 @@ public class MarkAsReviewed private boolean isReviewed(ChangeResource rsrc) { try { return changeDataFactory - .create(dbProvider.get(), rsrc.getNotes()) + .create(rsrc.getNotes()) .isReviewedBy(rsrc.getUser().asIdentifiedUser().getAccountId()); } catch (OrmException e) { logger.atSevere().withCause(e).log("failed to check if change is reviewed"); diff --git a/java/com/google/gerrit/server/restapi/change/MarkAsUnreviewed.java b/java/com/google/gerrit/server/restapi/change/MarkAsUnreviewed.java index 6e15dcc486..0651e7b037 100644 --- a/java/com/google/gerrit/server/restapi/change/MarkAsUnreviewed.java +++ b/java/com/google/gerrit/server/restapi/change/MarkAsUnreviewed.java @@ -19,14 +19,12 @@ import com.google.gerrit.extensions.common.Input; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.webui.UiAction; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; @Singleton @@ -34,16 +32,11 @@ public class MarkAsUnreviewed implements RestModifyView, UiAction { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final Provider dbProvider; private final ChangeData.Factory changeDataFactory; private final StarredChangesUtil stars; @Inject - MarkAsUnreviewed( - Provider dbProvider, - ChangeData.Factory changeDataFactory, - StarredChangesUtil stars) { - this.dbProvider = dbProvider; + MarkAsUnreviewed(ChangeData.Factory changeDataFactory, StarredChangesUtil stars) { this.changeDataFactory = changeDataFactory; this.stars = stars; } @@ -66,7 +59,7 @@ public class MarkAsUnreviewed private boolean isReviewed(ChangeResource rsrc) { try { return changeDataFactory - .create(dbProvider.get(), rsrc.getNotes()) + .create(rsrc.getNotes()) .isReviewedBy(rsrc.getUser().asIdentifiedUser().getAccountId()); } catch (OrmException e) { logger.atSevere().withCause(e).log("failed to check if change is reviewed"); diff --git a/java/com/google/gerrit/server/restapi/change/Mergeable.java b/java/com/google/gerrit/server/restapi/change/Mergeable.java index b1963474f5..dcbd8e79c2 100644 --- a/java/com/google/gerrit/server/restapi/change/Mergeable.java +++ b/java/com/google/gerrit/server/restapi/change/Mergeable.java @@ -110,7 +110,7 @@ public class Mergeable implements RestReadView { return result; } - ChangeData cd = changeDataFactory.create(db.get(), resource.getNotes()); + ChangeData cd = changeDataFactory.create(resource.getNotes()); result.submitType = getSubmitType(cd); try (Repository git = gitManager.openRepository(change.getProject())) { diff --git a/java/com/google/gerrit/server/restapi/change/Move.java b/java/com/google/gerrit/server/restapi/change/Move.java index d781370a20..5e676c4db8 100644 --- a/java/com/google/gerrit/server/restapi/change/Move.java +++ b/java/com/google/gerrit/server/restapi/change/Move.java @@ -268,7 +268,7 @@ public class Move extends RetryingRestModifyView approvals = new ArrayList<>(); for (PatchSetApproval psa : approvalsUtil.byPatchSet( - ctx.getDb(), ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) { + ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) { ProjectState projectState = projectCache.checkedGet(project); LabelType type = projectState.getLabelTypes(ctx.getNotes()).byLabel(psa.getLabelId()); // Only keep veto votes, defined as votes where: diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java index e26158fe7c..31bd1ca80e 100644 --- a/java/com/google/gerrit/server/restapi/change/PostReview.java +++ b/java/com/google/gerrit/server/restapi/change/PostReview.java @@ -380,8 +380,7 @@ public class PostReview bu.execute(); // Re-read change to take into account results of the update. - ChangeData cd = - changeDataFactory.create(db.get(), revision.getProject(), revision.getChange().getId()); + ChangeData cd = changeDataFactory.create(revision.getProject(), revision.getChange().getId()); for (ReviewerAddition reviewerResult : reviewerResults) { reviewerResult.gatherResults(cd); } @@ -1146,7 +1145,7 @@ public class PostReview if (ctx.getAccountId().equals(ctx.getChange().getOwner())) { return true; } - ChangeData cd = changeDataFactory.create(db.get(), ctx.getNotes()); + ChangeData cd = changeDataFactory.create(ctx.getNotes()); ReviewerSet reviewers = cd.reviewers(); if (reviewers.byState(REVIEWER).contains(ctx.getAccountId())) { return true; @@ -1357,7 +1356,6 @@ public class PostReview for (PatchSetApproval a : approvalsUtil.byPatchSetUser( - ctx.getDb(), ctx.getNotes(), psId, user.getAccountId(), diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java index 3e6618931e..5b27e3c372 100644 --- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java +++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java @@ -82,8 +82,7 @@ public class PostReviewers } // Re-read change to take into account results of the update. - addition.gatherResults( - changeDataFactory.create(dbProvider.get(), rsrc.getProject(), rsrc.getId())); + addition.gatherResults(changeDataFactory.create(rsrc.getProject(), rsrc.getId())); return addition.result; } } diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java index e46085d8d3..ab99ae17a1 100644 --- a/java/com/google/gerrit/server/restapi/change/Submit.java +++ b/java/com/google/gerrit/server/restapi/change/Submit.java @@ -323,7 +323,7 @@ public class Submit } ReviewDb db = dbProvider.get(); - ChangeData cd = changeDataFactory.create(db, resource.getNotes()); + ChangeData cd = changeDataFactory.create(resource.getNotes()); try { MergeOp.checkSubmitRule(cd, false); } catch (ResourceConflictException e) { diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java index c7eb7812be..7cba8e7762 100644 --- a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java +++ b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java @@ -25,7 +25,6 @@ import com.google.gerrit.extensions.common.TestSubmitRuleInput.Filters; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.RestModifyView; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -38,13 +37,11 @@ import com.google.gerrit.server.rules.PrologRule; import com.google.gerrit.server.rules.RulesCache; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import java.util.LinkedHashMap; import java.util.List; import org.kohsuke.args4j.Option; public class TestSubmitRule implements RestModifyView { - private final Provider db; private final ChangeData.Factory changeDataFactory; private final RulesCache rules; private final AccountLoader.Factory accountInfoFactory; @@ -57,14 +54,12 @@ public class TestSubmitRule implements RestModifyView db, ChangeData.Factory changeDataFactory, RulesCache rules, AccountLoader.Factory infoFactory, ProjectCache projectCache, DefaultSubmitRule defaultSubmitRule, PrologRule prologRule) { - this.db = db; this.changeDataFactory = changeDataFactory; this.rules = rules; this.accountInfoFactory = infoFactory; @@ -95,7 +90,7 @@ public class TestSubmitRule implements RestModifyView records; if (projectState.hasPrologRules() || input.rule != null) { records = ImmutableList.copyOf(prologRule.evaluate(cd, opts)); diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitType.java b/java/com/google/gerrit/server/restapi/change/TestSubmitType.java index c1be1cea00..684d22f380 100644 --- a/java/com/google/gerrit/server/restapi/change/TestSubmitType.java +++ b/java/com/google/gerrit/server/restapi/change/TestSubmitType.java @@ -23,7 +23,6 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleOptions; @@ -31,11 +30,9 @@ import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.rules.RulesCache; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import org.kohsuke.args4j.Option; public class TestSubmitType implements RestModifyView { - private final Provider db; private final ChangeData.Factory changeDataFactory; private final RulesCache rules; private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory; @@ -45,11 +42,9 @@ public class TestSubmitType implements RestModifyView db, ChangeData.Factory changeDataFactory, RulesCache rules, SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) { - this.db = db; this.changeDataFactory = changeDataFactory; this.rules = rules; this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory; @@ -74,7 +69,7 @@ public class TestSubmitType implements RestModifyView { @Singleton public static class List implements RestReadView { - private final Provider db; private final ApprovalsUtil approvalsUtil; @Inject - List(Provider db, ApprovalsUtil approvalsUtil) { - this.db = db; + List(ApprovalsUtil approvalsUtil) { this.approvalsUtil = approvalsUtil; } @@ -85,7 +81,6 @@ public class Votes implements ChildCollection { Map votes = new TreeMap<>(); Iterable byPatchSetUser = approvalsUtil.byPatchSetUser( - db.get(), rsrc.getChangeResource().getNotes(), rsrc.getChange().currentPatchSetId(), rsrc.getReviewerUser().getAccountId(), diff --git a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java index 66463befe4..40755b80f4 100644 --- a/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java +++ b/java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java @@ -153,7 +153,7 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation { walkChangesByHashes(visibleCommits, Collections.emptySet(), or, b); Set nonVisibleHashes = walkChangesByHashes(nonVisibleCommits, visibleHashes, or, b); - ChangeSet partialSet = byCommitsOnBranchNotMerged(or, db, b, visibleHashes, nonVisibleHashes); + ChangeSet partialSet = byCommitsOnBranchNotMerged(or, b, visibleHashes, nonVisibleHashes); Iterables.addAll(visibleChanges, partialSet.changes()); Iterables.addAll(nonVisibleChanges, partialSet.nonVisibleChanges()); } @@ -212,16 +212,12 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation { } private ChangeSet byCommitsOnBranchNotMerged( - OpenRepo or, - ReviewDb db, - Branch.NameKey branch, - Set visibleHashes, - Set nonVisibleHashes) + OpenRepo or, Branch.NameKey branch, Set visibleHashes, Set nonVisibleHashes) throws OrmException, IOException { List potentiallyVisibleChanges = - byCommitsOnBranchNotMerged(or, db, branch, visibleHashes); + byCommitsOnBranchNotMerged(or, branch, visibleHashes); List invisibleChanges = - new ArrayList<>(byCommitsOnBranchNotMerged(or, db, branch, nonVisibleHashes)); + new ArrayList<>(byCommitsOnBranchNotMerged(or, branch, nonVisibleHashes)); List visibleChanges = new ArrayList<>(potentiallyVisibleChanges.size()); for (ChangeData cd : potentiallyVisibleChanges) { if (changeIsVisibleToPredicate.match(cd)) { @@ -234,8 +230,7 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation { } private ImmutableList byCommitsOnBranchNotMerged( - OpenRepo or, ReviewDb db, Branch.NameKey branch, Set hashes) - throws OrmException, IOException { + OpenRepo or, Branch.NameKey branch, Set hashes) throws OrmException, IOException { if (hashes.isEmpty()) { return ImmutableList.of(); } @@ -245,7 +240,7 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation { } ImmutableList result = ImmutableList.copyOf( - queryProvider.get().byCommitsOnBranchNotMerged(or.repo, db, branch, hashes)); + queryProvider.get().byCommitsOnBranchNotMerged(or.repo, branch, hashes)); queryCache.put(k, result); return result; } diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java index 13affc3e32..4ebf5e650d 100644 --- a/java/com/google/gerrit/server/submit/MergeOp.java +++ b/java/com/google/gerrit/server/submit/MergeOp.java @@ -542,12 +542,10 @@ public class MergeOp implements AutoCloseable { private ChangeSet reloadChanges(ChangeSet changeSet) { List visible = new ArrayList<>(changeSet.changes().size()); List nonVisible = new ArrayList<>(changeSet.nonVisibleChanges().size()); - changeSet - .changes() - .forEach(c -> visible.add(changeDataFactory.create(db, c.project(), c.getId()))); + changeSet.changes().forEach(c -> visible.add(changeDataFactory.create(c.project(), c.getId()))); changeSet .nonVisibleChanges() - .forEach(c -> nonVisible.add(changeDataFactory.create(db, c.project(), c.getId()))); + .forEach(c -> nonVisible.add(changeDataFactory.create(c.project(), c.getId()))); return new ChangeSet(visible, nonVisible); } diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java index 9cd30db085..6db14e6e3a 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java @@ -347,7 +347,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp { Map byKey = new HashMap<>(); for (PatchSetApproval psa : args.approvalsUtil.byPatchSet( - ctx.getDb(), ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) { + ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) { byKey.put(psa.getKey(), psa); } diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java index b7af4fed70..4f3a2dc083 100644 --- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java +++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java @@ -470,7 +470,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest { + subject + "\n") .create(); - indexer.index(db, c.getProject(), c.getId()); + indexer.index(c.getProject(), c.getId()); } assertThat(getReceivePackRefs().additionalHaves()).containsExactly(obj(c4, 1)); diff --git a/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index fd6d83520d..882c0ff84b 100644 --- a/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -260,7 +260,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest { testRepo.reset(c1); assertPushOk(pushHead(testRepo, "refs/heads/master", false), "refs/heads/master"); - cd = changeDataFactory.create(db, project, psId1.getParentKey()); + cd = changeDataFactory.create(project, psId1.getParentKey()); Change c = cd.change(); assertThat(c.getStatus()).isEqualTo(Change.Status.MERGED); assertThat(c.currentPatchSetId()).isEqualTo(psId1); diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java index 0f394aa7f2..60a6308cab 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java @@ -393,7 +393,7 @@ public class ActionsIT extends AbstractDaemonTest { visitedCurrentRevisionActionsAssertions(origActions, revisionInfo.actions); // ...via ChangeJson directly. - ChangeData cd = changeDataFactory.create(db, project, changeId); + ChangeData cd = changeDataFactory.create(project, changeId); revisionJsonFactory.create(opts).getRevisionInfo(cd, cd.patchSet(new PatchSet.Id(changeId, 1))); } diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java index a9ac3c187b..33e2db61b4 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java @@ -258,7 +258,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { + "Groups: " + rev + "\n"); - indexer.index(db, c.getProject(), c.getId()); + indexer.index(c.getProject(), c.getId()); ChangeNotes notes = changeNotesFactory.create(c.getProject(), c.getId()); FixInput fix = new FixInput(); @@ -817,7 +817,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { + "Subject: " + subject + "\n"); - indexer.index(db, c.getProject(), c.getId()); + indexer.index(c.getProject(), c.getId()); return ps; } diff --git a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java index 26c04294fe..971a03e723 100644 --- a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java +++ b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java @@ -515,7 +515,7 @@ public class GetRelatedIT extends AbstractDaemonTest { // Pretend PS1,1 was pushed before the groups field was added. clearGroups(psId1_1); - indexer.index(changeDataFactory.create(db, project, psId1_1.getParentKey())); + indexer.index(changeDataFactory.create(project, psId1_1.getParentKey())); // PS1,1 has no groups, so disappeared from related changes. assertRelated(psId2_1); diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 7d8f3c6923..65609c8e46 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1759,7 +1759,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { allUsers.update(draftsRef.getName(), draftsRef.getObjectId()); assertThat(allUsers.getRepository().exactRef(draftsRef.getName())).isNotNull(); - indexer.index(db, project, id); + indexer.index(project, id); assertQuery("draftby:" + userId); } @@ -2261,7 +2261,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { for (int i = 1; i <= 11; i++) { Iterable cds = - queryProvider.get().byCommitsOnBranchNotMerged(repo.getRepository(), db, dest, shas, i); + queryProvider.get().byCommitsOnBranchNotMerged(repo.getRepository(), dest, shas, i); Iterable ids = FluentIterable.from(cds).transform(in -> in.getId().get()); String name = "limit " + i; assertThat(ids).named(name).hasSize(n); diff --git a/plugins/reviewnotes b/plugins/reviewnotes index b98d892034..5d607a4193 160000 --- a/plugins/reviewnotes +++ b/plugins/reviewnotes @@ -1 +1 @@ -Subproject commit b98d8920345aa6183fe98ea9bc15ae0fea5d9b58 +Subproject commit 5d607a4193fb41b4ad8fe01622f7e002fd4208c0