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 74abe81f51..783af770b9 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 @@ -57,6 +57,7 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.index.ChangeIndexer; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.Util; @@ -187,6 +188,9 @@ public abstract class AbstractDaemonTest { @Inject protected NotesMigration notesMigration; + @Inject + protected ChangeNotes.Factory notesFactory; + @Rule public ExpectedException exception = ExpectedException.none(); @@ -636,7 +640,8 @@ public abstract class AbstractDaemonTest { } protected PatchSet getPatchSet(PatchSet.Id psId) throws OrmException { - return changeDataFactory.create(db, psId.getParentKey()).patchSet(psId); + return changeDataFactory.create(db, project, psId.getParentKey()) + .patchSet(psId); } protected IdentifiedUser user(TestAccount testAccount) { diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java index 3686e9d3ca..af3e5eb135 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java @@ -273,8 +273,9 @@ public class PushOneCommit { private void assertReviewers(Change c, TestAccount... expectedReviewers) throws OrmException { - Iterable actualIds = - approvalsUtil.getReviewers(db, notesFactory.create(db, c)).values(); + Iterable actualIds = approvalsUtil + .getReviewers(db, notesFactory.create(db, c.getProject(), c.getId())) + .values(); assertThat(actualIds).containsExactlyElementsIn( Sets.newHashSet(TestAccount.ids(expectedReviewers))); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java index 5b891107c1..ebca92bbe4 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -52,7 +52,7 @@ import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.ChangeFinder; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.change.GetRevisionActions; import com.google.gerrit.server.change.RevisionResource; @@ -80,7 +80,7 @@ import java.util.Map; public class RevisionIT extends AbstractDaemonTest { @Inject - private ChangeUtil changeUtil; + private ChangeFinder changeFinder; @Inject private GetRevisionActions getRevisionActions; @@ -693,7 +693,7 @@ public class RevisionIT extends AbstractDaemonTest { private RevisionResource parseRevisionResource(PushOneCommit.Result r) throws Exception { PatchSet.Id psId = r.getPatchSetId(); - List ctls = changeUtil.findChanges( + List ctls = changeFinder.find( Integer.toString(psId.getParentKey().get()), atrScope.get().getUser()); assertThat(ctls).hasSize(1); return revisions.parse( diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index 3ea17af999..813c9c9176 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -50,9 +50,6 @@ public class SubmitOnPushIT extends AbstractDaemonTest { @Inject private ApprovalsUtil approvalsUtil; - @Inject - private ChangeNotes.Factory changeNotesFactory; - @Test public void submitOnPush() throws Exception { grant(Permission.SUBMIT, project, "refs/for/refs/heads/master"); @@ -223,8 +220,8 @@ public class SubmitOnPushIT extends AbstractDaemonTest { private PatchSetApproval getSubmitter(PatchSet.Id patchSetId) throws OrmException { - Change c = db.changes().get(patchSetId.getParentKey()); - ChangeNotes notes = changeNotesFactory.create(db, c).load(); + ChangeNotes notes = + notesFactory.create(db, project, patchSetId.getParentKey()).load(); return approvalsUtil.getSubmitter(db, notes, patchSetId); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java index 796d4d6e5e..0e47123f58 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java @@ -34,6 +34,7 @@ import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.edit.ChangeEditModifier; import com.google.gerrit.server.git.ProjectConfig; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.Util; import com.google.inject.Inject; @@ -186,16 +187,16 @@ public class VisibleRefFilterIT extends AbstractDaemonTest { allow(Permission.READ, REGISTERED_USERS, "refs/heads/master"); deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch"); - Change change1 = db.changes().get(c1); + ChangeNotes notes = notesFactory.create(db, project, c1); PatchSet ps1 = getPatchSet(new PatchSet.Id(c1, 1)); // Admin's edit is not visible. setApiUser(admin); - editModifier.createEdit(change1, ps1); + editModifier.createEdit(notes.getChange(), ps1); // User's edit is visible. setApiUser(user); - editModifier.createEdit(change1, ps1); + editModifier.createEdit(notes.getChange(), ps1); assertRefs( "HEAD", @@ -213,12 +214,12 @@ public class VisibleRefFilterIT extends AbstractDaemonTest { deny(Permission.READ, REGISTERED_USERS, "refs/heads/master"); allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch"); - Change change1 = db.changes().get(c1); + ChangeNotes notes = notesFactory.create(db, project, c1); PatchSet ps1 = getPatchSet(new PatchSet.Id(c1, 1)); setApiUser(admin); - editModifier.createEdit(change1, ps1); + editModifier.createEdit(notes.getChange(), ps1); setApiUser(user); - editModifier.createEdit(change1, ps1); + editModifier.createEdit(notes.getChange(), ps1); assertRefs( // Change 1 is visible due to accessDatabase capability, even though diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java index 1d5d0b604f..2e59e841f7 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java @@ -313,8 +313,9 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { protected void assertSubmitter(String changeId, int psId) throws OrmException { - ChangeNotes cn = notesFactory.create(db, - getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change()); + Change c = + getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change(); + ChangeNotes cn = notesFactory.create(db, c.getProject(), c.getId()); PatchSetApproval submitter = approvalsUtil.getSubmitter( db, cn, new PatchSet.Id(cn.getChangeId(), psId)); assertThat(submitter).isNotNull(); @@ -324,8 +325,9 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { protected void assertNoSubmitter(String changeId, int psId) throws OrmException { - ChangeNotes cn = notesFactory.create(db, - getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change()); + Change c = + getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change(); + ChangeNotes cn = notesFactory.create(db, c.getProject(), c.getId()); PatchSetApproval submitter = approvalsUtil.getSubmitter( db, cn, new PatchSet.Id(cn.getChangeId(), psId)); assertThat(submitter).isNull(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java index db93f29178..050230868d 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java @@ -223,7 +223,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { assertThat(p.status).isEqualTo(ProblemInfo.Status.FIXED); assertThat(p.outcome).isEqualTo("Deleted patch set"); - c = db.changes().get(c.getId()); + c = notesFactory.create(db, project, c.getId()).getChange(); assertThat(c.currentPatchSetId().get()).isEqualTo(1); assertThat(getPatchSet(ps1.getId())).isNotNull(); assertThat(getPatchSet(ps2.getId())).isNull(); @@ -271,7 +271,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { assertThat(p.status).isEqualTo(ProblemInfo.Status.FIXED); assertThat(p.outcome).isEqualTo("Deleted patch set"); - c = db.changes().get(c.getId()); + c = notesFactory.create(db, project, c.getId()).getChange(); assertThat(c.currentPatchSetId().get()).isEqualTo(3); assertThat(getPatchSet(ps1.getId())).isNotNull(); assertThat(getPatchSet(ps2.getId())).isNull(); @@ -299,7 +299,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { assertThat(p.outcome) .isEqualTo("Cannot delete patch set; no patch sets would remain"); - c = db.changes().get(c.getId()); + c = notesFactory.create(db, project, c.getId()).getChange(); assertThat(c.currentPatchSetId().get()).isEqualTo(1); assertThat(getPatchSet(ps1.getId())).isNotNull(); } @@ -387,7 +387,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { assertThat(p.status).isEqualTo(ProblemInfo.Status.FIXED); assertThat(p.outcome).isEqualTo("Marked change as merged"); - c = db.changes().get(c.getId()); + c = notesFactory.create(db, project, c.getId()).getChange(); assertThat(c.getStatus()).isEqualTo(Change.Status.MERGED); assertProblems(c); } @@ -476,7 +476,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { assertThat(p.status).isEqualTo(ProblemInfo.Status.FIXED); assertThat(p.outcome).isEqualTo("Inserted as patch set 2"); - c = db.changes().get(c.getId()); + c = notesFactory.create(db, project, c.getId()).getChange(); PatchSet.Id psId2 = new PatchSet.Id(c.getId(), 2); assertThat(c.currentPatchSetId()).isEqualTo(psId2); assertThat(getPatchSet(psId2).getRevision().get()) @@ -518,7 +518,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { assertThat(p.status).isEqualTo(ProblemInfo.Status.FIXED); assertThat(p.outcome).isEqualTo("Inserted as patch set 2"); - c = db.changes().get(c.getId()); + c = notesFactory.create(db, project, c.getId()).getChange(); PatchSet.Id psId2 = new PatchSet.Id(c.getId(), 2); assertThat(c.currentPatchSetId()).isEqualTo(psId2); assertThat(getPatchSet(psId2).getRevision().get()) @@ -607,6 +607,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest { private Change insertChange() throws Exception { Change c = newChange(project, adminId); db.changes().insert(singleton(c)); + indexer.index(db, c); ChangeUpdate u = changeUpdateFactory.create( changeControlFactory.controlFor(c, userFactory.create(adminId))); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java index 3918904f7b..27f8bbef4c 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.server.change; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.acceptance.GitUtil.pushHead; +import static java.util.concurrent.TimeUnit.SECONDS; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; @@ -33,17 +34,34 @@ import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.BatchUpdate.ChangeContext; import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.testutil.TestTimeUtil; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.revwalk.RevCommit; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import java.io.IOException; import java.util.List; public class GetRelatedIT extends AbstractDaemonTest { + private String systemTimeZone; + + @Before + public void setTimeForTesting() { + systemTimeZone = System.setProperty("user.timezone", "US/Eastern"); + TestTimeUtil.resetWithClockStep(1, SECONDS); + } + + @After + public void resetTime() { + TestTimeUtil.useSystemTime(); + System.setProperty("user.timezone", systemTimeZone); + } + @Inject private ChangeEditUtil editUtil; @@ -573,7 +591,8 @@ public class GetRelatedIT extends AbstractDaemonTest { // Pretend PS1,1 was pushed before the groups field was added. clearGroups(psId1_1); - indexer.index(changeDataFactory.create(db, psId1_1.getParentKey())); + indexer.index( + changeDataFactory.create(db, project, psId1_1.getParentKey())); // PS1,1 has no groups, so disappeared from related changes. assertRelated(psId2_1); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java index 387092dcf9..357f268364 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS; import static com.google.gerrit.server.project.Util.category; import static com.google.gerrit.server.project.Util.value; -import static com.google.gerrit.testutil.GerritServerTests.isNoteDbTestEnabled; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; @@ -128,7 +127,7 @@ public class CustomLabelIT extends AbstractDaemonTest { revision(r).review(new ReviewInput().label(P.getName(), 0)); ChangeInfo c = get(r.getChangeId()); LabelInfo q = c.labels.get(P.getName()); - assertThat(q.all).hasSize(isNoteDbTestEnabled() ? 1 : 2); + assertThat(q.all).hasSize(2); assertThat(q.disliked).isNull(); assertThat(q.rejected).isNull(); assertThat(q.blocking).isNull(); diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index de860f1ea4..d80dcf3692 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE; import static com.google.gerrit.server.index.ChangeField.LEGACY_ID; +import static com.google.gerrit.server.index.ChangeField.PROJECT; import static com.google.gerrit.server.index.IndexRewriter.CLOSED_STATUSES; import static com.google.gerrit.server.index.IndexRewriter.OPEN_STATUSES; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -34,6 +35,7 @@ import com.google.gerrit.common.Nullable; 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.config.ConfigUtil; import com.google.gerrit.server.config.GerritServerConfig; @@ -454,7 +456,9 @@ public class LuceneChangeIndex implements ChangeIndex { ChangeProtoField.CODEC.decode(cb.bytes, cb.offset, cb.length)); } else { int id = doc.getField(idFieldName).numericValue().intValue(); - cd = changeDataFactory.create(db.get(), new Change.Id(id)); + Project.NameKey project = + new Project.NameKey(doc.getField(PROJECT.getName()).stringValue()); + cd = changeDataFactory.create(db.get(), project, new Change.Id(id)); } if (fields.contains(PATCH_SET_FIELD)) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java index daf7eba328..af1344e1c1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java @@ -309,8 +309,9 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, .build()); } - private ChangeNotes newNotes(ReviewDb db, Change change) { - return notesFactory.create(db, change); + private ChangeNotes newNotes(ReviewDb db, Change change) + throws OrmException { + return notesFactory.create(db, change.getProject(), change.getId()); } private static Optional hook(Config config, Path path, String name) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java new file mode 100644 index 0000000000..0665bc7105 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java @@ -0,0 +1,102 @@ +// Copyright (C) 2016 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server; + +import com.google.common.base.Optional; +import com.google.common.collect.ImmutableSet; +import com.google.common.primitives.Ints; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.server.change.ChangeTriplet; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.query.change.InternalChangeQuery; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +@Singleton +public class ChangeFinder { + private final Provider queryProvider; + + @Inject + ChangeFinder(Provider queryProvider) { + this.queryProvider = queryProvider; + } + + /** + * Find changes matching the given identifier. + * + * @param id change identifier, either a numeric ID, a Change-Id, or + * project~branch~id triplet. + * @param user user to wrap in controls. + * @return possibly-empty list of controls for all matching changes, + * corresponding to the given user; may or may not be visible. + * @throws OrmException if an error occurred querying the database. + */ + public List find(String id, CurrentUser user) + throws OrmException { + // Use the index to search for changes, but don't return any stored fields, + // to force rereading in case the index is stale. + InternalChangeQuery query = queryProvider.get() + .setRequestedFields(ImmutableSet. of()); + + // Try legacy id + if (!id.isEmpty() && id.charAt(0) != '0') { + Integer n = Ints.tryParse(id); + if (n != null) { + return asChangeControls(query.byLegacyChangeId(new Change.Id(n)), user); + } + } + + // Try isolated changeId + if (!id.contains("~")) { + return asChangeControls(query.byKeyPrefix(id), user); + } + + // Try change triplet + Optional triplet = ChangeTriplet.parse(id); + if (triplet.isPresent()) { + return asChangeControls(query.byBranchKey( + triplet.get().branch(), + triplet.get().id()), + user); + } + + return Collections.emptyList(); + } + + public List find(Change.Id id, CurrentUser user) + throws OrmException { + // Use the index to search for changes, but don't return any stored fields, + // to force rereading in case the index is stale. + InternalChangeQuery query = queryProvider.get() + .setRequestedFields(ImmutableSet. of()); + return asChangeControls(query.byLegacyChangeId(id), user); + } + + private List asChangeControls(List cds, + CurrentUser user) throws OrmException { + List ctls = new ArrayList<>(cds.size()); + for (ChangeData cd : cds) { + ctls.add(cd.changeControl(user)); + } + return ctls; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index f51eeb10c8..c49498de80 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -15,11 +15,7 @@ package com.google.gerrit.server; import com.google.common.base.Function; -import com.google.common.base.Optional; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Ordering; -import com.google.common.primitives.Ints; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; @@ -30,7 +26,6 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.ChangeMessages; -import com.google.gerrit.server.change.ChangeTriplet; import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.UpdateException; @@ -40,8 +35,6 @@ import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gerrit.server.query.change.ChangeData; -import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.util.IdGenerator; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -66,9 +59,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.text.MessageFormat; -import java.util.ArrayList; import java.util.Collections; -import java.util.List; import java.util.Map; @Singleton @@ -164,9 +155,7 @@ public class ChangeUtil { private final Provider user; private final Provider db; private final Sequences seq; - private final Provider queryProvider; private final PatchSetUtil psUtil; - private final ChangeControl.GenericFactory changeControlFactory; private final RevertedSender.Factory revertedSenderFactory; private final ChangeInserter.Factory changeInserterFactory; private final GitRepositoryManager gitManager; @@ -178,9 +167,7 @@ public class ChangeUtil { ChangeUtil(Provider user, Provider db, Sequences seq, - Provider queryProvider, PatchSetUtil psUtil, - ChangeControl.GenericFactory changeControlFactory, RevertedSender.Factory revertedSenderFactory, ChangeInserter.Factory changeInserterFactory, GitRepositoryManager gitManager, @@ -190,9 +177,7 @@ public class ChangeUtil { this.user = user; this.db = db; this.seq = seq; - this.queryProvider = queryProvider; this.psUtil = psUtil; - this.changeControlFactory = changeControlFactory; this.revertedSenderFactory = revertedSenderFactory; this.changeInserterFactory = changeInserterFactory; this.gitManager = gitManager; @@ -285,7 +270,7 @@ public class ChangeUtil { } try { - RevertedSender cm = revertedSenderFactory.create(changeId); + RevertedSender cm = revertedSenderFactory.create(project, changeId); cm.setFrom(user.get().getAccountId()); cm.setChangeMessage(ins.getChangeMessage()); cm.send(); @@ -319,62 +304,6 @@ public class ChangeUtil { } } - /** - * Find changes matching the given identifier. - * - * @param id change identifier, either a numeric ID, a Change-Id, or - * project~branch~id triplet. - * @param user user to wrap in controls. - * @return possibly-empty list of controls for all matching changes, - * corresponding to the given user; may or may not be visible. - * @throws OrmException if an error occurred querying the database. - */ - public List findChanges(String id, CurrentUser user) - throws OrmException { - // Try legacy id - if (!id.isEmpty() && id.charAt(0) != '0') { - Integer n = Ints.tryParse(id); - try { - if (n != null) { - return ImmutableList.of( - changeControlFactory.controlFor(new Change.Id(n), user)); - } - } catch (NoSuchChangeException e) { - return Collections.emptyList(); - } - } - - // Use the index to search for changes, but don't return any stored fields, - // to force rereading in case the index is stale. - InternalChangeQuery query = queryProvider.get() - .setRequestedFields(ImmutableSet. of()); - - // Try isolated changeId - if (!id.contains("~")) { - return asChangeControls(query.byKeyPrefix(id), user); - } - - // Try change triplet - Optional triplet = ChangeTriplet.parse(id); - if (triplet.isPresent()) { - return asChangeControls(query.byBranchKey( - triplet.get().branch(), - triplet.get().id()), - user); - } - - return Collections.emptyList(); - } - - private List asChangeControls(List cds, - CurrentUser user) throws OrmException { - List ctls = new ArrayList<>(cds.size()); - for (ChangeData cd : cds) { - ctls.add(cd.changeControl(user)); - } - return ctls; - } - public static PatchSet.Id nextPatchSetId(PatchSet.Id id) { return new PatchSet.Id(id.getParentKey(), id.get() + 1); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java index 6bddf22e10..def4591865 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java @@ -162,7 +162,8 @@ public class Abandon implements RestModifyView, @Override public void postUpdate(Context ctx) throws OrmException { try { - ReplyToChangeSender cm = abandonedSenderFactory.create(change.getId()); + ReplyToChangeSender cm = + abandonedSenderFactory.create(ctx.getProject(), change.getId()); if (account != null) { cm.setFrom(account.getId()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index 3b36f3af04..562d8814a7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -361,8 +361,8 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { @Override public void run() { try { - CreateChangeSender cm = - createChangeSenderFactory.create(change.getId()); + CreateChangeSender cm = createChangeSenderFactory + .create(change.getProject(), change.getId()); cm.setFrom(change.getOwner()); cm.setPatchSet(patchSet, patchSetInfo); cm.setNotify(notify); 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 c754a48c46..4cef85aaa9 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 @@ -96,6 +96,7 @@ import com.google.gerrit.server.api.accounts.GpgApiAdapter; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.LabelNormalizer; import com.google.gerrit.server.git.MergeUtil; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.project.ChangeControl; @@ -156,6 +157,7 @@ public class ChangeJson { private final Provider checkerProvider; private final ActionJson actionJson; private final GpgApiAdapter gpgApi; + private final ChangeNotes.Factory changeNotesFactory; private AccountLoader accountLoader; private Map> submitRecords; @@ -181,6 +183,7 @@ public class ChangeJson { Provider checkerProvider, ActionJson actionJson, GpgApiAdapter gpgApi, + ChangeNotes.Factory changeNotesFactory, @Assisted Set options) { this.db = db; this.labelNormalizer = ln; @@ -200,6 +203,7 @@ public class ChangeJson { this.checkerProvider = checkerProvider; this.actionJson = actionJson; this.gpgApi = gpgApi; + this.changeNotesFactory = changeNotesFactory; this.options = options.isEmpty() ? EnumSet.noneOf(ListChangesOption.class) : EnumSet.copyOf(options); @@ -218,32 +222,18 @@ public class ChangeJson { return format(changeDataFactory.create(db.get(), change)); } - public ChangeInfo format(Change.Id id) throws OrmException { - Change c; + public ChangeInfo format(Project.NameKey project, Change.Id id) + throws OrmException { + ChangeNotes notes; try { - c = db.get().changes().get(id); + notes = changeNotesFactory.create(db.get(), project, id); } catch (OrmException e) { if (!has(CHECK)) { throw e; } - return checkOnly(changeDataFactory.create(db.get(), id)); + return checkOnly(changeDataFactory.create(db.get(), project, id)); } - return format(changeDataFactory.create(db.get(), c)); - } - - public List format(Collection ids) throws OrmException { - List changes = new ArrayList<>(ids.size()); - List ret = new ArrayList<>(ids.size()); - ReviewDb reviewDb = db.get(); - for (Change.Id id : ids) { - changes.add(changeDataFactory.create(reviewDb, id)); - } - accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); - for (ChangeData cd : changes) { - ret.add(format(cd, Optional. absent(), false)); - } - accountLoader.fill(); - return ret; + return format(changeDataFactory.create(db.get(), notes.getChange())); } public ChangeInfo format(ChangeData cd) throws OrmException { @@ -391,7 +381,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.getId()); + cd = changeDataFactory.create(cd.db(), cd.getProject(), cd.getId()); break; } } 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 96f2e1379c..af3c576e2b 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 @@ -25,11 +25,10 @@ import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.ChangeFinder; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.index.ChangeIndexer; 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; @@ -45,10 +44,9 @@ public class ChangesCollection implements AcceptsPost { private final Provider db; private final Provider user; - private final ChangeControl.GenericFactory changeControlFactory; private final Provider queryFactory; private final DynamicMap> views; - private final ChangeUtil changeUtil; + private final ChangeFinder changeFinder; private final CreateChange createChange; private final ChangeIndexer changeIndexer; @@ -56,18 +54,16 @@ public class ChangesCollection implements ChangesCollection( Provider db, Provider user, - ChangeControl.GenericFactory changeControlFactory, Provider queryFactory, DynamicMap> views, - ChangeUtil changeUtil, + ChangeFinder changeFinder, CreateChange createChange, ChangeIndexer changeIndexer) { this.db = db; this.user = user; - this.changeControlFactory = changeControlFactory; this.queryFactory = queryFactory; this.views = views; - this.changeUtil = changeUtil; + this.changeFinder = changeFinder; this.createChange = createChange; this.changeIndexer = changeIndexer; } @@ -85,7 +81,8 @@ public class ChangesCollection implements @Override public ChangeResource parse(TopLevelResource root, IdString id) throws ResourceNotFoundException, OrmException { - List ctls = changeUtil.findChanges(id.encoded(), user.get()); + List ctls = + changeFinder.find(id.encoded(), user.get()); if (ctls.isEmpty()) { Integer changeId = Ints.tryParse(id.get()); if (changeId != null) { @@ -112,15 +109,23 @@ public class ChangesCollection implements public ChangeResource parse(Change.Id id) throws ResourceNotFoundException, OrmException { - try { - ChangeControl ctl = changeControlFactory.controlFor(id, user.get()); - if (!ctl.isVisible(db.get())) { - throw new ResourceNotFoundException(toIdString(id)); + List ctls = changeFinder.find(id, user.get()); + if (ctls.isEmpty()) { + try { + changeIndexer.delete(id); + } catch (IOException e) { + throw new ResourceNotFoundException(toIdString(id).get(), e); } - return new ChangeResource(ctl); - } catch (NoSuchChangeException e) { throw new ResourceNotFoundException(toIdString(id)); } + if (ctls.size() != 1) { + throw new ResourceNotFoundException("Multiple changes found for " + id); + } + ChangeControl ctl = ctls.get(0); + if (!ctl.isVisible(db.get())) { + throw new ResourceNotFoundException(toIdString(id)); + } + return new ChangeResource(ctl); } private static IdString toIdString(Change.Id id) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java index 82aaecbe28..545ea17a81 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java @@ -84,7 +84,8 @@ public class CherryPick implements RestModifyView all = Lists.newArrayList(db.patchSets().byChange(cid)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java index b280c48789..d47eb8b7db 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java @@ -33,7 +33,7 @@ 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.ChangeUtil; +import com.google.gerrit.server.ChangeFinder; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; @@ -84,7 +84,7 @@ public class CreateChange implements private final ProjectsCollection projectsCollection; private final ChangeInserter.Factory changeInserterFactory; private final ChangeJson.Factory jsonFactory; - private final ChangeUtil changeUtil; + private final ChangeFinder changeFinder; private final BatchUpdate.Factory updateFactory; private final PatchSetUtil psUtil; private final boolean allowDrafts; @@ -98,7 +98,7 @@ public class CreateChange implements ProjectsCollection projectsCollection, ChangeInserter.Factory changeInserterFactory, ChangeJson.Factory json, - ChangeUtil changeUtil, + ChangeFinder changeFinder, BatchUpdate.Factory updateFactory, PatchSetUtil psUtil, @GerritServerConfig Config config) { @@ -110,7 +110,7 @@ public class CreateChange implements this.projectsCollection = projectsCollection; this.changeInserterFactory = changeInserterFactory; this.jsonFactory = json; - this.changeUtil = changeUtil; + this.changeFinder = changeFinder; this.updateFactory = updateFactory; this.psUtil = psUtil; this.allowDrafts = config.getBoolean("change", "allowDrafts", true); @@ -162,7 +162,7 @@ public class CreateChange implements ObjectId parentCommit; List groups; if (input.baseChange != null) { - List ctls = changeUtil.findChanges( + List ctls = changeFinder.find( input.baseChange, rsrc.getControl().getUser()); if (ctls.size() != 1) { throw new InvalidChangeOperationException( @@ -217,7 +217,7 @@ public class CreateChange implements bu.execute(); } ChangeJson json = jsonFactory.create(ChangeJson.NO_OPTIONS); - return Response.created(json.format(changeId)); + return Response.created(json.format(project, changeId)); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java index 243e8553eb..dd499ec76b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java @@ -105,7 +105,8 @@ public class EmailReviewComments implements Runnable, RequestContext { RequestContext old = requestContext.setContext(this); try { - CommentSender cm = commentSenderFactory.create(notes.getChangeId()); + CommentSender cm = commentSenderFactory.create(notes.getProjectName(), + notes.getChangeId()); cm.setFrom(authorId); cm.setPatchSet(patchSet, patchSetInfoFactory.get(notes.getProjectName(), patchSet)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index 822a12f83c..39c27cd93e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -259,7 +259,7 @@ public class PatchSetInserter extends BatchUpdate.Op { if (sendMail) { try { ReplacePatchSetSender cm = replacePatchSetFactory.create( - change.getId()); + ctx.getProject(), change.getId()); cm.setFrom(ctx.getUser().getAccountId()); cm.setPatchSet(patchSet, patchSetInfo); cm.setChangeMessage(changeMessage); 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 f30882c77f..bc83237842 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 @@ -600,6 +600,8 @@ public class PostReview implements RestModifyView ups.add(c); } } + ctx.getUpdate(ctx.getChange().currentPatchSetId()) + .putReviewer(user.getAccountId(), REVIEWER); } private Map scanLabels(ChangeContext ctx, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index bf0c903ccc..a15297d100 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -248,7 +248,7 @@ public class PostReviewers implements RestModifyView indexFuture = - indexer.indexAsync(rsrc.getId()); + indexer.indexAsync(rsrc.getProject(), rsrc.getId()); result.reviewers = Lists.newArrayListWithCapacity(added.size()); for (PatchSetApproval psa : added) { // New reviewers have value 0, don't bother normalizing. @@ -286,7 +286,8 @@ public class PostReviewers implements RestModifyView, private final RebaseChangeOp.Factory rebaseFactory; private final RebaseUtil rebaseUtil; private final ChangeJson.Factory json; + private final ChangeNotes.Factory notesFactory; private final Provider dbProvider; private final Provider queryProvider; private final PatchSetUtil psUtil; @@ -78,6 +80,7 @@ public class Rebase implements RestModifyView, RebaseChangeOp.Factory rebaseFactory, RebaseUtil rebaseUtil, ChangeJson.Factory json, + ChangeNotes.Factory notesFactory, Provider dbProvider, Provider queryProvider, PatchSetUtil psUtil) { @@ -86,6 +89,7 @@ public class Rebase implements RestModifyView, this.rebaseFactory = rebaseFactory; this.rebaseUtil = rebaseUtil; this.json = json; + this.notesFactory = notesFactory; this.dbProvider = dbProvider; this.queryProvider = queryProvider; this.psUtil = psUtil; @@ -120,7 +124,7 @@ public class Rebase implements RestModifyView, .setValidatePolicy(CommitValidators.Policy.GERRIT)); bu.execute(); } - return json.create(OPTIONS).format(change.getId()); + return json.create(OPTIONS).format(change.getProject(), change.getId()); } private String findBaseRev(RevWalk rw, RevisionResource rsrc, @@ -236,11 +240,9 @@ public class Rebase implements RestModifyView, if (rsrc.getChange().getId().equals(id)) { return rsrc.getControl(); } - Change c = dbProvider.get().changes().get(id); - if (c == null) { - return null; - } - return rsrc.getControl().getProjectControl().controlFor(c); + ChangeNotes notes = + notesFactory.create(dbProvider.get(), rsrc.getProject(), id); + return rsrc.getControl().getProjectControl().controlFor(notes); } private boolean hasOneParent(RevWalk rw, PatchSet ps) throws IOException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java index a195e462fe..54b66b6d66 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java @@ -150,7 +150,8 @@ public class Restore implements RestModifyView, @Override public void postUpdate(Context ctx) throws OrmException { try { - ReplyToChangeSender cm = restoredSenderFactory.create(change.getId()); + ReplyToChangeSender cm = + restoredSenderFactory.create(ctx.getProject(), change.getId()); cm.setFrom(caller.getAccountId()); cm.setChangeMessage(message); cm.send(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java index 7e3845cd72..d702fc6026 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java @@ -76,7 +76,8 @@ public class Revert implements RestModifyView, } catch (NoSuchChangeException e) { throw new ResourceNotFoundException(e.getMessage()); } - return json.create(ChangeJson.NO_OPTIONS).format(revertedChangeId); + return json.create(ChangeJson.NO_OPTIONS).format(req.getProject(), + revertedChangeId); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java index 8ba80bd719..72265433e8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java @@ -32,6 +32,7 @@ import com.google.gerrit.extensions.webui.UiAction; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; +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.ChangeMessagesUtil; @@ -44,6 +45,7 @@ import com.google.gerrit.server.git.ChangeSet; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.git.MergeSuperSet; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.change.ChangeData; @@ -116,6 +118,7 @@ public class Submit implements RestModifyView, private final ChangeData.Factory changeDataFactory; private final ChangeMessagesUtil cmUtil; private final ChangeControl.GenericFactory changeControlFactory; + private final ChangeNotes.Factory changeNotesFactory; private final Provider mergeOpProvider; private final MergeSuperSet mergeSuperSet; private final AccountsCollection accounts; @@ -135,6 +138,7 @@ public class Submit implements RestModifyView, ChangeData.Factory changeDataFactory, ChangeMessagesUtil cmUtil, ChangeControl.GenericFactory changeControlFactory, + ChangeNotes.Factory changeNotesFactory, Provider mergeOpProvider, MergeSuperSet mergeSuperSet, AccountsCollection accounts, @@ -146,6 +150,7 @@ public class Submit implements RestModifyView, this.changeDataFactory = changeDataFactory; this.cmUtil = cmUtil; this.changeControlFactory = changeControlFactory; + this.changeNotesFactory = changeNotesFactory; this.mergeOpProvider = mergeOpProvider; this.mergeSuperSet = mergeSuperSet; this.accounts = accounts; @@ -203,7 +208,8 @@ public class Submit implements RestModifyView, try (MergeOp op = mergeOpProvider.get()) { ReviewDb db = dbProvider.get(); op.merge(db, change, caller, true, input); - change = db.changes().get(change.getId()); + change = changeNotesFactory + .create(db, change.getProject(), change.getId()).getChange(); } if (change == null) { @@ -227,17 +233,18 @@ public class Submit implements RestModifyView, /** * @param cs set of changes to be submitted at once + * @param project the name of the project * @param identifiedUser the user who is checking to submit * @return a reason why any of the changes is not submittable or null */ - private String problemsForSubmittingChangeset( - ChangeSet cs, IdentifiedUser identifiedUser) { + private String problemsForSubmittingChangeset(ChangeSet cs, + Project.NameKey project, IdentifiedUser identifiedUser) { try { @SuppressWarnings("resource") ReviewDb db = dbProvider.get(); for (PatchSet.Id psId : cs.patchIds()) { ChangeControl changeControl = changeControlFactory - .controlFor(psId.getParentKey(), identifiedUser); + .controlFor(project, psId.getParentKey(), identifiedUser); ChangeData c = changeDataFactory.create(db, changeControl); if (!changeControl.isVisible(db)) { @@ -337,7 +344,7 @@ public class Submit implements RestModifyView, && topicSize > 1; String submitProblems = problemsForSubmittingChangeset(cs, - resource.getUser()); + resource.getProject(), resource.getUser()); if (submitProblems != null) { return new UiAction.Description() .setLabel(treatWithTopic diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java index 758fdbabc0..09a8452d27 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java @@ -496,7 +496,7 @@ public class EventFactory { p.author = asAccountAttribute(author.getAccount()); } - Change change = db.changes().get(pId.getParentKey()); + Change change = notes.getChange(); List list = patchListCache.get(change, patchSet).toPatchList(pId); for (Patch pe : list) { @@ -506,7 +506,7 @@ public class EventFactory { } } p.kind = changeKindCache.getChangeKind(db, change, patchSet); - } catch (OrmException | IOException e) { + } catch (IOException e) { log.error("Cannot load patch set data for " + patchSet.getId(), e); } catch (PatchSetInfoNotAvailableException e) { log.error(String.format("Cannot get authorEmail for %s.", pId), e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java index 449b39d530..57c2ae276d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java @@ -615,7 +615,7 @@ public class BatchUpdate implements AutoCloseable { bmdu.commit(); } } - indexFutures.add(indexer.indexAsync(id)); + indexFutures.add(indexer.indexAsync(ctx.getProject(), id)); } } } catch (Exception e) { @@ -635,9 +635,6 @@ public class BatchUpdate implements AutoCloseable { ChangeNotes notes = changeNotesFactory.createForNew(c); ChangeContext ctx = new ChangeContext( changeControlFactory.controlFor(notes, user), new BatchUpdateReviewDb(db)); - if (notesMigration.readChanges()) { - ctx.getNotes().load(); - } return ctx; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java index b281c5b139..2e993bd9f9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.git; import com.google.gerrit.common.Nullable; 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.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.mail.MergedSender; @@ -39,7 +40,8 @@ public class EmailMerge implements Runnable, RequestContext { private static final Logger log = LoggerFactory.getLogger(EmailMerge.class); public interface Factory { - EmailMerge create(Change.Id changeId, Account.Id submitter); + EmailMerge create(Project.NameKey project, Change.Id changeId, + Account.Id submitter); } private final ExecutorService sendEmailsExecutor; @@ -47,6 +49,7 @@ public class EmailMerge implements Runnable, RequestContext { private final SchemaFactory schemaFactory; private final ThreadLocalRequestContext requestContext; + private final Project.NameKey project; private final Change.Id changeId; private final Account.Id submitter; private ReviewDb db; @@ -56,12 +59,14 @@ public class EmailMerge implements Runnable, RequestContext { MergedSender.Factory mergedSenderFactory, SchemaFactory schemaFactory, ThreadLocalRequestContext requestContext, + @Assisted Project.NameKey project, @Assisted Change.Id changeId, @Assisted @Nullable Account.Id submitter) { this.sendEmailsExecutor = executor; this.mergedSenderFactory = mergedSenderFactory; this.schemaFactory = schemaFactory; this.requestContext = requestContext; + this.project = project; this.changeId = changeId; this.submitter = submitter; } @@ -74,7 +79,7 @@ public class EmailMerge implements Runnable, RequestContext { public void run() { RequestContext old = requestContext.setContext(this); try { - MergedSender cm = mergedSenderFactory.create(changeId); + MergedSender cm = mergedSenderFactory.create(project, changeId); if (submitter != null) { cm.setFrom(submitter); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java index 0051e2f60f..8d13f7e8ca 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java @@ -31,8 +31,8 @@ import com.google.common.collect.Multimaps; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.common.collect.SortedSetMultimap; -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.PatchSetUtil; import com.google.gerrit.server.change.RevisionResource; @@ -115,18 +115,15 @@ public class GroupCollector { public static GroupCollector create(Multimap changeRefsById, final ReviewDb db, final PatchSetUtil psUtil, - final ChangeNotes.Factory notesFactory) { + final ChangeNotes.Factory notesFactory, final Project.NameKey project) { return new GroupCollector( transformRefs(changeRefsById), new Lookup() { @Override public List lookup(PatchSet.Id psId) throws OrmException { - // TODO(dborowitz): Shouldn't have to look up Change. - Change c = db.changes().get(psId.getParentKey()); - if (c == null) { - return null; - } - ChangeNotes notes = notesFactory.create(db, c); + // TODO(dborowitz): Reuse open repository from caller. + ChangeNotes notes = + notesFactory.create(db, project, psId.getParentKey()); PatchSet ps = psUtil.get(db, notes, psId); return ps != null ? ps.getGroups() : null; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java index b38304227f..b3538b7280 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java @@ -83,7 +83,8 @@ public class MergeSuperSet { public ChangeSet completeChangeSet(ReviewDb db, Change change) throws MissingObjectException, IncorrectObjectTypeException, IOException, OrmException { - ChangeData cd = changeDataFactory.create(db, change.getId()); + ChangeData cd = + changeDataFactory.create(db, change.getProject(), change.getId()); if (Submit.wholeTopicEnabled(cfg)) { return completeChangeSetIncludingTopics(db, new ChangeSet(cd)); } else { @@ -101,7 +102,7 @@ public class MergeSuperSet { try (Repository repo = repoManager.openRepository(project); RevWalk rw = CodeReviewCommit.newRevWalk(repo)) { for (Change.Id cId : pc.get(project)) { - ChangeData cd = changeDataFactory.create(db, cId); + ChangeData cd = changeDataFactory.create(db, project, cId); SubmitTypeRecord str = cd.submitTypeRecord(); if (!str.isOk()) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 3a24db788e..a04b06ac7d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -1486,7 +1486,8 @@ public class ReceiveCommits { final Change changeEnt; try { - changeEnt = db.changes().get(changeId); + changeEnt = + notesFactory.create(db, project.getNameKey(), changeId).getChange(); } catch (OrmException e) { log.error("Cannot lookup existing change " + changeId, e); reject(cmd, "database error"); @@ -1526,8 +1527,8 @@ public class ReceiveCommits { newChanges = Lists.newArrayList(); SetMultimap existing = changeRefsById(); - GroupCollector groupCollector = - GroupCollector.create(refsById, db, psUtil, notesFactory); + GroupCollector groupCollector = GroupCollector.create(refsById, db, psUtil, + notesFactory, project.getNameKey()); rp.getRevWalk().reset(); rp.getRevWalk().sort(RevSort.TOPO); @@ -1837,7 +1838,8 @@ public class ReceiveCommits { changeCtl.getUser().asIdentifiedUser(), false, null); } addMessage(""); - Change c = db.changes().get(rsrc.getChange().getId()); + Change c = notesFactory + .create(db, project.getNameKey(), rsrc.getChange().getId()).getChange(); switch (c.getStatus()) { case MERGED: addMessage("Change " + c.getChangeId() + " merged."); @@ -2377,8 +2379,8 @@ public class ReceiveCommits { @Override public void run() { try { - ReplacePatchSetSender cm = - replacePatchSetFactory.create(change.getId()); + ReplacePatchSetSender cm = replacePatchSetFactory + .create(project.getNameKey(), change.getId()); cm.setFrom(me); cm.setPatchSet(newPatchSet, info); cm.setChangeMessage(msg); @@ -2721,7 +2723,8 @@ public class ReceiveCommits { String refName = cmd.getRefName(); Change.Id cid = psi.getParentKey(); - Change change = db.changes().get(cid); + Change change = + notesFactory.create(db, project.getNameKey(), cid).getChange(); ChangeControl ctl = projectControl.controlFor(change); PatchSet ps = psUtil.get(db, ctl.getNotes(), psi); if (change == null || ps == null) { @@ -2816,8 +2819,8 @@ public class ReceiveCommits { @Override public void run() { try { - MergedSender cm = - mergedSenderFactory.create(ps.getId().getParentKey()); + MergedSender cm = mergedSenderFactory.create(project.getNameKey(), + ps.getId().getParentKey()); cm.setFrom(user.getAccountId()); cm.setPatchSet(ps, info); cm.send(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java index faf37767d9..bf04e211c0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ScanningChangeCacheImpl.java @@ -24,6 +24,8 @@ 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.cache.CacheModule; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; import com.google.gwtorm.server.OrmException; @@ -86,12 +88,18 @@ public class ScanningChangeCacheImpl implements ChangeCache { static class Loader extends CacheLoader> { private final GitRepositoryManager repoManager; + private final NotesMigration notesMigration; + private final ChangeNotes.Factory notesFactory; private final OneOffRequestContext requestContext; @Inject Loader(GitRepositoryManager repoManager, + NotesMigration notesMigration, + ChangeNotes.Factory notesFactory, OneOffRequestContext requestContext) { this.repoManager = repoManager; + this.notesMigration = notesMigration; + this.notesFactory = notesFactory; this.requestContext = requestContext; } @@ -99,13 +107,23 @@ public class ScanningChangeCacheImpl implements ChangeCache { public List load(Project.NameKey key) throws Exception { try (Repository repo = repoManager.openRepository(key); ManualRequestContext ctx = requestContext.open()) { - return scan(repo, ctx.getReviewDbProvider().get()); + return scan(notesMigration, notesFactory, repo, + ctx.getReviewDbProvider().get(), key); } } - } - public static List scan(Repository repo, ReviewDb db) + public static List scan(NotesMigration notesMigration, + ChangeNotes.Factory notesFactory, Repository repo, ReviewDb db, + Project.NameKey project) throws OrmException, IOException { + if (!notesMigration.readChanges()) { + return scanDb(repo, db); + } + + return scanNotedb(notesFactory, repo, db, project); + } + + public static List scanDb(Repository repo, ReviewDb db) throws OrmException, IOException { Map refs = repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES); @@ -124,4 +142,19 @@ public class ScanningChangeCacheImpl implements ChangeCache { } return changes; } + + public static List scanNotedb(ChangeNotes.Factory notesFactory, + Repository repo, ReviewDb db, Project.NameKey project) + throws OrmException, IOException { + Map refs = + repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES); + List changes = new ArrayList<>(refs.size()); + for (Ref r : refs.values()) { + Change.Id id = Change.Id.fromRef(r.getName()); + if (id != null) { + changes.add(notesFactory.create(db, project, id).getChange()); + } + } + return changes; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java index e96453d5ca..e5bc614046 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java @@ -481,7 +481,8 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op { // Assume the change must have been merged at this point, otherwise we would // have failed fast in one of the other steps. try { - args.mergedSenderFactory.create(getId(), submitter.getAccountId()) + args.mergedSenderFactory + .create(ctx.getProject(), getId(), submitter.getAccountId()) .sendAsync(); } catch (Exception e) { log.error("Cannot email merged notification for " + getId(), e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java index 29f67cb83f..f4ef4af326 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java @@ -108,7 +108,7 @@ public class ChangeField { /** Project containing the change. */ public static final FieldDef PROJECT = new FieldDef.Single( - ChangeQueryBuilder.FIELD_PROJECT, FieldType.EXACT, false) { + ChangeQueryBuilder.FIELD_PROJECT, FieldType.EXACT, true) { @Override public String get(ChangeData input, FillArgs args) throws OrmException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java index 84edcbff71..a0860c7afc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java @@ -21,6 +21,7 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.query.change.ChangeData; @@ -128,9 +129,10 @@ public class ChangeIndexer { * @param id change to index. * @return future for the indexing task. */ - public CheckedFuture indexAsync(Change.Id id) { + public CheckedFuture indexAsync(Project.NameKey project, + Change.Id id) { return executor != null - ? submit(new IndexTask(id)) + ? submit(new IndexTask(project, id)) : Futures. immediateCheckedFuture(null); } @@ -140,10 +142,11 @@ public class ChangeIndexer { * @param ids changes to index. * @return future for completing indexing of all changes. */ - public CheckedFuture indexAsync(Collection ids) { + public CheckedFuture indexAsync(Project.NameKey project, + Collection ids) { List> futures = new ArrayList<>(ids.size()); for (Change.Id id : ids) { - futures.add(indexAsync(id)); + futures.add(indexAsync(project, id)); } return allAsList(futures); } @@ -201,9 +204,11 @@ public class ChangeIndexer { } private class IndexTask implements Callable { + private final Project.NameKey project; private final Change.Id id; - private IndexTask(Change.Id id) { + private IndexTask(Project.NameKey project, Change.Id id) { + this.project = project; this.id = id; } @@ -237,8 +242,8 @@ public class ChangeIndexer { }; RequestContext oldCtx = context.setContext(newCtx); try { - ChangeData cd = changeDataFactory.create( - newCtx.getReviewDbProvider().get(), id); + ChangeData cd = changeDataFactory + .create(newCtx.getReviewDbProvider().get(), project, id); index(cd); return null; } finally { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java index 2eb210caca..58a4433718 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java @@ -67,6 +67,7 @@ public class ChangeSchemas { ChangeField.AUTHOR, ChangeField.COMMITTER); + @Deprecated static final Schema V26 = schema( ChangeField.LEGACY_ID, ChangeField.ID, @@ -104,6 +105,8 @@ public class ChangeSchemas { ChangeField.COMMITTER, ChangeField.DRAFTBY); + static final Schema V27 = schema(V26.getFields().values()); + private static Schema schema(Collection> fields) { return new Schema<>(ImmutableList.copyOf(fields)); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java index 53af8ada00..11bbbd8e82 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java @@ -37,6 +37,8 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MultiProgressMonitor; import com.google.gerrit.server.git.MultiProgressMonitor.Task; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.git.ScanningChangeCacheImpl; import com.google.gerrit.server.patch.PatchListLoader; import com.google.gerrit.server.query.change.ChangeData; @@ -116,6 +118,8 @@ public class SiteIndexer { private final GitRepositoryManager repoManager; private final ListeningExecutorService executor; private final ChangeIndexer.Factory indexerFactory; + private final NotesMigration notesMigration; + private final ChangeNotes.Factory notesFactory; private final ThreeWayMergeStrategy mergeStrategy; private int numChanges = -1; @@ -129,12 +133,16 @@ public class SiteIndexer { GitRepositoryManager repoManager, @IndexExecutor(BATCH) ListeningExecutorService executor, ChangeIndexer.Factory indexerFactory, + NotesMigration notesMigration, + ChangeNotes.Factory notesFactory, @GerritServerConfig Config config) { this.schemaFactory = schemaFactory; this.changeDataFactory = changeDataFactory; this.repoManager = repoManager; this.executor = executor; this.indexerFactory = indexerFactory; + this.notesMigration = notesMigration; + this.notesFactory = notesFactory; this.mergeStrategy = MergeUtil.getMergeStrategy(config); } @@ -238,7 +246,8 @@ public class SiteIndexer { try (Repository repo = repoManager.openRepository(project); ReviewDb db = schemaFactory.open()) { Map refs = repo.getRefDatabase().getRefs(ALL); - for (Change c : ScanningChangeCacheImpl.scan(repo, db)) { + for (Change c : ScanningChangeCacheImpl.scan(notesMigration, + notesFactory, repo, db, project)) { Ref r = refs.get(c.currentPatchSetId().toRefName()); if (r != null) { byId.put(r.getObjectId(), changeDataFactory.create(db, c)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java index 409c1559cf..dbd0438483 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/AbandonedSender.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.mail; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -26,13 +27,15 @@ public class AbandonedSender extends ReplyToChangeSender { public static interface Factory extends ReplyToChangeSender.Factory { @Override - AbandonedSender create(Change.Id change); + AbandonedSender create(Project.NameKey project, Change.Id change); } @Inject - public AbandonedSender(EmailArguments ea, @Assisted Change.Id id) + public AbandonedSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id) throws OrmException { - super(ea, "abandon", newChangeData(ea, id)); + super(ea, "abandon", newChangeData(ea, project, id)); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/AddReviewerSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/AddReviewerSender.java index 7a6d204df2..98fc4a023e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/AddReviewerSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/AddReviewerSender.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.mail; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -23,13 +24,15 @@ import com.google.inject.assistedinject.Assisted; /** Asks a user to review a change. */ public class AddReviewerSender extends NewChangeSender { public static interface Factory { - AddReviewerSender create(Change.Id id); + AddReviewerSender create(Project.NameKey project, Change.Id id); } @Inject - public AddReviewerSender(EmailArguments ea, @Assisted Change.Id id) + public AddReviewerSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id) throws OrmException { - super(ea, newChangeData(ea, id)); + super(ea, newChangeData(ea, project, id)); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java index 3da6605760..5c888992bb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java @@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetInfo; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.mail.ProjectWatch.Watchers; import com.google.gerrit.server.patch.PatchList; @@ -56,8 +57,9 @@ import java.util.TreeSet; public abstract class ChangeEmail extends NotificationEmail { private static final Logger log = LoggerFactory.getLogger(ChangeEmail.class); - protected static ChangeData newChangeData(EmailArguments ea, Change.Id id) { - return ea.changeDataFactory.create(ea.db.get(), id); + protected static ChangeData newChangeData(EmailArguments ea, + Project.NameKey project, Change.Id id) { + return ea.changeDataFactory.create(ea.db.get(), project, id); } protected final Change change; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java index 83494cc6a2..c845c28a47 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java @@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.CommentRange; import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.PatchLineCommentsUtil; import com.google.gerrit.server.patch.PatchFile; import com.google.gerrit.server.patch.PatchList; @@ -52,7 +53,7 @@ public class CommentSender extends ReplyToChangeSender { .getLogger(CommentSender.class); public static interface Factory { - CommentSender create(Change.Id id); + CommentSender create(Project.NameKey project, Change.Id id); } private List inlineComments = Collections.emptyList(); @@ -61,8 +62,9 @@ public class CommentSender extends ReplyToChangeSender { @Inject public CommentSender(EmailArguments ea, PatchLineCommentsUtil plcUtil, + @Assisted Project.NameKey project, @Assisted Change.Id id) throws OrmException { - super(ea, "comment", newChangeData(ea, id)); + super(ea, "comment", newChangeData(ea, project, id)); this.plcUtil = plcUtil; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java index e67b8ce2aa..87bfa375d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/CreateChangeSender.java @@ -19,6 +19,7 @@ import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.mail.ProjectWatch.Watchers; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -33,13 +34,15 @@ public class CreateChangeSender extends NewChangeSender { LoggerFactory.getLogger(CreateChangeSender.class); public static interface Factory { - CreateChangeSender create(Change.Id id); + CreateChangeSender create(Project.NameKey project, Change.Id id); } @Inject - public CreateChangeSender(EmailArguments ea, @Assisted Change.Id id) + public CreateChangeSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id) throws OrmException { - super(ea, newChangeData(ea, id)); + super(ea, newChangeData(ea, project, id)); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergeFailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergeFailSender.java index f91b0fd9c0..3dfbcda31f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergeFailSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergeFailSender.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.mail; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -23,13 +24,15 @@ import com.google.inject.assistedinject.Assisted; /** Send notice about a change failing to merged. */ public class MergeFailSender extends ReplyToChangeSender { public static interface Factory { - MergeFailSender create(Change.Id id); + MergeFailSender create(Project.NameKey project, Change.Id id); } @Inject - public MergeFailSender(EmailArguments ea, @Assisted Change.Id id) + public MergeFailSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id) throws OrmException { - super(ea, "merge-failed", newChangeData(ea, id)); + super(ea, "merge-failed", newChangeData(ea, project, id)); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java index 9725e7f34e..7271c181e7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java @@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSetApproval; +import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -31,15 +32,17 @@ import com.google.inject.assistedinject.Assisted; /** Send notice about a change successfully merged. */ public class MergedSender extends ReplyToChangeSender { public static interface Factory { - MergedSender create(Change.Id id); + MergedSender create(Project.NameKey project, Change.Id id); } private final LabelTypes labelTypes; @Inject - public MergedSender(EmailArguments ea, @Assisted Change.Id id) + public MergedSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id) throws OrmException { - super(ea, "merged", newChangeData(ea, id)); + super(ea, "merged", newChangeData(ea, project, id)); labelTypes = changeData.changeControl().getLabelTypes(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplacePatchSetSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplacePatchSetSender.java index 7980845fca..6bee3e8a82 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplacePatchSetSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplacePatchSetSender.java @@ -18,6 +18,7 @@ import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -31,16 +32,18 @@ import java.util.Set; /** Send notice of new patch sets for reviewers. */ public class ReplacePatchSetSender extends ReplyToChangeSender { public static interface Factory { - ReplacePatchSetSender create(Change.Id id); + ReplacePatchSetSender create(Project.NameKey project, Change.Id id); } private final Set reviewers = new HashSet<>(); private final Set extraCC = new HashSet<>(); @Inject - public ReplacePatchSetSender(EmailArguments ea, @Assisted Change.Id id) + public ReplacePatchSetSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id) throws OrmException { - super(ea, "newpatchset", newChangeData(ea, id)); + super(ea, "newpatchset", newChangeData(ea, project, id)); } public void addReviewers(final Collection cc) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java index fbfbe61212..8eadeefa19 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ReplyToChangeSender.java @@ -16,13 +16,14 @@ package com.google.gerrit.server.mail; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.query.change.ChangeData; import com.google.gwtorm.server.OrmException; /** Alert a user to a reply to a change, usually commentary made during review. */ public abstract class ReplyToChangeSender extends ChangeEmail { public static interface Factory { - T create(Change.Id id); + T create(Project.NameKey project, Change.Id id); } protected ReplyToChangeSender(EmailArguments ea, String mc, ChangeData cd) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java index a43c7b665c..0808090245 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RestoredSender.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.mail; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -26,13 +27,15 @@ public class RestoredSender extends ReplyToChangeSender { public static interface Factory extends ReplyToChangeSender.Factory { @Override - RestoredSender create(Change.Id id); + RestoredSender create(Project.NameKey project, Change.Id id); } @Inject - public RestoredSender(EmailArguments ea, @Assisted Change.Id id) + public RestoredSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id) throws OrmException { - super(ea, "restore", newChangeData(ea, id)); + super(ea, "restore", newChangeData(ea, project, id)); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java index dda68ab549..55438fea29 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RevertedSender.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.mail; import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -24,13 +25,15 @@ import com.google.inject.assistedinject.Assisted; /** Send notice about a change being reverted. */ public class RevertedSender extends ReplyToChangeSender { public static interface Factory { - RevertedSender create(Change.Id id); + RevertedSender create(Project.NameKey project, Change.Id id); } @Inject - public RevertedSender(EmailArguments ea, @Assisted Change.Id id) + public RevertedSender(EmailArguments ea, + @Assisted Project.NameKey project, + @Assisted Change.Id id) throws OrmException { - super(ea, "revert", newChangeData(ea, id)); + super(ea, "revert", newChangeData(ea, project, id)); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java index 88f795ddbb..c22d4f8abd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java @@ -50,7 +50,7 @@ public abstract class AbstractChangeNotes extends VersionedMetaData { if (loaded) { return self(); } - if (!migration.enabled()) { + if (!migration.enabled() || changeId == null) { loadDefaults(); return self(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index aa8028a607..56fd25baff 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -116,16 +116,27 @@ public class ChangeNotes extends AbstractChangeNotes { this.allUsersProvider = allUsersProvider; } - public ChangeNotes create(@SuppressWarnings("unused") ReviewDb db, - Change change) { - return new ChangeNotes(repoManager, migration, allUsersProvider, change); + public ChangeNotes create(ReviewDb db, Project.NameKey project, + Change.Id changeId) throws OrmException { + Change change = db.changes().get(changeId); + // TODO: Throw NoSuchChangeException when the change is not found in the + // database + return new ChangeNotes(repoManager, migration, allUsersProvider, project, + change).load(); } - public ChangeNotes createForNew(Change change) { - return new ChangeNotes(repoManager, migration, allUsersProvider, change); + public ChangeNotes createFromIndexedChange(Change change) { + return new ChangeNotes(repoManager, migration, allUsersProvider, + change.getProject(), change); + } + + public ChangeNotes createForNew(Change change) throws OrmException { + return new ChangeNotes(repoManager, migration, allUsersProvider, + change.getProject(), change).load(); } } + private final Project.NameKey project; private final Change change; private ImmutableSortedMap patchSets; private ImmutableListMultimap approvals; @@ -147,10 +158,12 @@ public class ChangeNotes extends AbstractChangeNotes { @VisibleForTesting public ChangeNotes(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersNameProvider allUsersProvider, Change change) { - super(repoManager, migration, change.getId()); + AllUsersNameProvider allUsersProvider, Project.NameKey project, + Change change) { + super(repoManager, migration, change != null ? change.getId() : null); this.allUsers = allUsersProvider.get(); - this.change = new Change(change); + this.project = project; + this.change = change != null ? new Change(change) : null; } public Change getChange() { @@ -283,7 +296,7 @@ public class ChangeNotes extends AbstractChangeNotes { return; } try (RevWalk walk = new RevWalk(reader); - ChangeNotesParser parser = new ChangeNotesParser(change.getProject(), + ChangeNotesParser parser = new ChangeNotesParser(project, change.getId(), rev, walk, repoManager)) { parser.parseAll(); @@ -297,7 +310,7 @@ public class ChangeNotes extends AbstractChangeNotes { noteMap = parser.noteMap; revisionNotes = parser.revisionNotes; change.setKey(new Change.Key(parser.changeId)); - change.setDest(new Branch.NameKey(getProjectName(), parser.branch)); + change.setDest(new Branch.NameKey(project, parser.branch)); change.setTopic(Strings.emptyToNull(parser.topic)); change.setCreatedOn(parser.createdOn); change.setLastUpdatedOn(parser.lastUpdatedOn); @@ -352,6 +365,6 @@ public class ChangeNotes extends AbstractChangeNotes { @Override public Project.NameKey getProjectName() { - return getChange().getProject(); + return project; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index c2d5f13540..dd7d6127fc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -207,7 +207,7 @@ class ChangeNotesParser implements AutoCloseable { } PatchSet.Id psId = parsePatchSetId(commit); - if (currentPatchSetId == null) { + if (currentPatchSetId == null || psId.get() > currentPatchSetId.get()) { currentPatchSetId = psId; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java index 8b5695e725..ca9357eb10 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java @@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; 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.ChangeFinder; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.query.change.ChangeData; @@ -46,20 +47,24 @@ public class ChangeControl { public static class GenericFactory { private final ProjectControl.GenericFactory projectControl; private final Provider db; + private final ChangeNotes.Factory notesFactory; + private final ChangeFinder changeFinder; @Inject - GenericFactory(ProjectControl.GenericFactory p, Provider d) { + GenericFactory( + ProjectControl.GenericFactory p, + Provider d, + ChangeNotes.Factory n, + ChangeFinder f) { projectControl = p; db = d; + notesFactory = n; + changeFinder = f; } - public ChangeControl controlFor(Change.Id changeId, CurrentUser user) - throws NoSuchChangeException, OrmException { - Change change = db.get().changes().get(changeId); - if (change == null) { - throw new NoSuchChangeException(changeId); - } - return controlFor(change, user); + public ChangeControl controlFor(Project.NameKey project, Change.Id changeId, + CurrentUser user) throws NoSuchChangeException, OrmException { + return controlFor(notesFactory.create(db.get(), project, changeId), user); } public ChangeControl controlFor(Change change, CurrentUser user) @@ -87,11 +92,11 @@ public class ChangeControl { public ChangeControl validateFor(Change.Id changeId, CurrentUser user) throws NoSuchChangeException, OrmException { - Change change = db.get().changes().get(changeId); - if (change == null) { + List ctls = changeFinder.find(changeId, user); + if (ctls.size() != 1) { throw new NoSuchChangeException(changeId); } - return validateFor(change, user); + return validateFor(ctls.get(0).getChange(), user); } public ChangeControl validateFor(Change change, CurrentUser user) @@ -121,10 +126,10 @@ public class ChangeControl { this.approvalsUtil = approvalsUtil; } - @SuppressWarnings("unused") - ChangeControl create(RefControl refControl, Change change) - throws OrmException { - return create(refControl, notesFactory.create(db, change)); + ChangeControl create(RefControl refControl, Project.NameKey project, + Change.Id changeId) throws OrmException { + return create(refControl, + notesFactory.create(db, project, changeId)); } ChangeControl create(RefControl refControl, ChangeNotes notes) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java index 367dc1464e..e51bf98755 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java @@ -195,7 +195,8 @@ public class ProjectControl { } public ChangeControl controlFor(Change change) throws OrmException { - return changeControlFactory.create(controlForRef(change.getDest()), change); + return changeControlFactory.create(controlForRef(change.getDest()), + change.getProject(), change.getId()); } public ChangeControl controlFor(ChangeNotes notes) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index 65c7295151..4893de8705 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -33,6 +33,7 @@ import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchLineComment; 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.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; @@ -269,7 +270,7 @@ public class ChangeData { } public interface Factory { - ChangeData create(ReviewDb db, Change.Id id); + ChangeData create(ReviewDb db, Project.NameKey project, Change.Id id); ChangeData create(ReviewDb db, Change c); ChangeData create(ReviewDb db, ChangeControl c); } @@ -283,9 +284,10 @@ public class ChangeData { * @param id change ID * @return instance for testing. */ - public static ChangeData createForTest(Change.Id id, int currentPatchSetId) { + public static ChangeData createForTest(Project.NameKey project, Change.Id id, + int currentPatchSetId) { ChangeData cd = new ChangeData(null, null, null, null, null, null, null, - null, null, null, null, null, null, null, id); + null, null, null, null, null, null, null, project, id); cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId)); return cd; } @@ -305,6 +307,7 @@ public class ChangeData { private final NotesMigration notesMigration; private final MergeabilityCache mergeabilityCache; private final Change.Id legacyId; + private final Project.NameKey project; private ChangeDataSource returnedBySource; private Change change; private ChangeNotes notes; @@ -345,6 +348,7 @@ public class ChangeData { NotesMigration notesMigration, MergeabilityCache mergeabilityCache, @Assisted ReviewDb db, + @Assisted Project.NameKey project, @Assisted Change.Id id) { this.db = db; this.repoManager = repoManager; @@ -360,7 +364,8 @@ public class ChangeData { this.patchListCache = patchListCache; this.notesMigration = notesMigration; this.mergeabilityCache = mergeabilityCache; - legacyId = id; + this.project = project; + this.legacyId = id; } @AssistedInject @@ -396,6 +401,7 @@ public class ChangeData { this.mergeabilityCache = mergeabilityCache; legacyId = c.getId(); change = c; + project = c.getProject(); } @AssistedInject @@ -433,6 +439,7 @@ public class ChangeData { change = c.getChange(); changeControl = c; notes = c.getNotes(); + project = notes.getProjectName(); } public ReviewDb db() { @@ -536,6 +543,10 @@ public class ChangeData { return legacyId; } + public Project.NameKey getProject() { + return project; + } + boolean fastIsVisibleTo(CurrentUser user) { return visibleTo == user; } @@ -566,7 +577,8 @@ public class ChangeData { if (change != null) { changeControl = changeControlFactory.controlFor(change, user); } else { - changeControl = changeControlFactory.controlFor(legacyId, user); + changeControl = + changeControlFactory.controlFor(project, legacyId, user); } } catch (NoSuchChangeException e) { throw new OrmException(e); @@ -591,13 +603,17 @@ public class ChangeData { } public Change reloadChange() throws OrmException { - change = db.changes().get(legacyId); + notes = notesFactory.create(db, project, legacyId); + change = notes.getChange(); + if (change == null) { + throw new OrmException("Unable to load change " + legacyId); + } return change; } public ChangeNotes notes() throws OrmException { if (notes == null) { - notes = notesFactory.create(db, change()); + notes = notesFactory.create(db, project, legacyId); } return notes; } @@ -680,7 +696,7 @@ public class ChangeData { return false; } String sha1 = ps.getRevision().get(); - try (Repository repo = repoManager.openRepository(change().getProject()); + try (Repository repo = repoManager.openRepository(project); RevWalk walk = new RevWalk(repo)) { RevCommit c = walk.parseCommit(ObjectId.fromString(sha1)); commitMessage = c.getFullMessage(); @@ -789,7 +805,7 @@ public class ChangeData { if (ps == null || !changeControl().isPatchVisible(ps, db)) { return null; } - try (Repository repo = repoManager.openRepository(c.getProject())) { + try (Repository repo = repoManager.openRepository(project)) { Ref ref = repo.getRefDatabase().exactRef(c.getDest().get()); SubmitTypeRecord str = submitTypeRecord(); if (!str.isOk()) { @@ -798,7 +814,7 @@ public class ChangeData { return false; } String mergeStrategy = mergeUtilFactory - .create(projectCache.get(c.getProject())) + .create(projectCache.get(project)) .mergeStrategyName(); mergeable = mergeabilityCache.get( ObjectId.fromString(ps.getRevision().get()), @@ -819,7 +835,7 @@ public class ChangeData { } editsByUser = new HashSet<>(); Change.Id id = change.getId(); - try (Repository repo = repoManager.openRepository(change.getProject())) { + try (Repository repo = repoManager.openRepository(project)) { for (String ref : repo.getRefDatabase().getRefs(RefNames.REFS_USERS).keySet()) { if (Change.Id.fromEditRefPart(ref).equals(id)) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeDataResultSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeDataResultSet.java index 52a5b7b01e..a2b3e2391e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeDataResultSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeDataResultSet.java @@ -15,7 +15,6 @@ package com.google.gerrit.server.query.change; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gwtorm.server.ResultSet; import com.google.inject.Provider; @@ -36,16 +35,6 @@ abstract class ChangeDataResultSet extends AbstractResultSet { }; } - static ResultSet patchSet(final ChangeData.Factory factory, - final Provider db, final ResultSet rs) { - return new ChangeDataResultSet(rs, false) { - @Override - ChangeData convert(PatchSet t) { - return factory.create(db.get(), t.getId().getParentKey()); - } - }; - } - private final ResultSet source; private final boolean unique; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index eaed46614d..c502faf1e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -57,6 +57,7 @@ import com.google.gerrit.server.index.IndexCollection; import com.google.gerrit.server.index.IndexConfig; import com.google.gerrit.server.index.IndexRewriter; import com.google.gerrit.server.index.Schema; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ListChildProjects; @@ -161,6 +162,7 @@ public class ChangeQueryBuilder extends QueryBuilder { final IdentifiedUser.GenericFactory userFactory; final CapabilityControl.Factory capabilityControlFactory; final ChangeControl.GenericFactory changeControlGenericFactory; + final ChangeNotes.Factory notesFactory; final ChangeData.Factory changeDataFactory; final FieldDef.FillArgs fillArgs; final PatchLineCommentsUtil plcUtil; @@ -192,6 +194,7 @@ public class ChangeQueryBuilder extends QueryBuilder { Provider self, CapabilityControl.Factory capabilityControlFactory, ChangeControl.GenericFactory changeControlGenericFactory, + ChangeNotes.Factory notesFactory, ChangeData.Factory changeDataFactory, FieldDef.FillArgs fillArgs, PatchLineCommentsUtil plcUtil, @@ -211,12 +214,11 @@ public class ChangeQueryBuilder extends QueryBuilder { Provider listMembers, @GerritServerConfig Config cfg) { this(db, queryProvider, rewriter, opFactories, userFactory, self, - capabilityControlFactory, changeControlGenericFactory, + capabilityControlFactory, changeControlGenericFactory, notesFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, allUsersName, patchListCache, repoManager, - projectCache, listChildProjects, submitDryRun, - conflictsCache, trackingFooters, - indexes != null ? indexes.getSearchIndex() : null, + projectCache, listChildProjects, submitDryRun, conflictsCache, + trackingFooters, indexes != null ? indexes.getSearchIndex() : null, indexConfig, listMembers, cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true)); } @@ -230,6 +232,7 @@ public class ChangeQueryBuilder extends QueryBuilder { Provider self, CapabilityControl.Factory capabilityControlFactory, ChangeControl.GenericFactory changeControlGenericFactory, + ChangeNotes.Factory notesFactory, ChangeData.Factory changeDataFactory, FieldDef.FillArgs fillArgs, PatchLineCommentsUtil plcUtil, @@ -255,6 +258,7 @@ public class ChangeQueryBuilder extends QueryBuilder { this.userFactory = userFactory; this.self = self; this.capabilityControlFactory = capabilityControlFactory; + this.notesFactory = notesFactory; this.changeControlGenericFactory = changeControlGenericFactory; this.changeDataFactory = changeDataFactory; this.fillArgs = fillArgs; @@ -279,7 +283,7 @@ public class ChangeQueryBuilder extends QueryBuilder { Arguments asUser(CurrentUser otherUser) { return new Arguments(db, queryProvider, rewriter, opFactories, userFactory, Providers.of(otherUser), - capabilityControlFactory, changeControlGenericFactory, + capabilityControlFactory, changeControlGenericFactory, notesFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, allUsersName, patchListCache, repoManager, projectCache, listChildProjects, submitDryRun, @@ -729,9 +733,8 @@ public class ChangeQueryBuilder extends QueryBuilder { } public Predicate visibleto(CurrentUser user) { - return new IsVisibleToPredicate(args.db, // - args.changeControlGenericFactory, // - user); + return new IsVisibleToPredicate(args.db, args.notesFactory, + args.changeControlGenericFactory, user); } public Predicate is_visible() throws QueryParseException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByLegacyPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByLegacyPredicate.java index 578a9f75d6..f85a9ed765 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByLegacyPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByLegacyPredicate.java @@ -57,8 +57,10 @@ class HasDraftByLegacyPredicate extends OperatorPredicate implements } List r = new ArrayList<>(ids.size()); - for (Change.Id id : ids) { - r.add(args.changeDataFactory.create(args.db.get(), id)); + // TODO Don't load the changes directly from the database, but provide + // project name + change ID to changeDataFactory, or delete this predicate. + for (Change c : args.db.get().changes().get(ids)) { + r.add(args.changeDataFactory.create(args.db.get(), c)); } return new ListResultSet<>(r); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java index 5ef41fd837..7bd4816368 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java @@ -118,6 +118,10 @@ public class InternalChangeQuery { return query(new ChangeIdPredicate(prefix)); } + public List byLegacyChangeId(Change.Id id) throws OrmException { + return query(new LegacyChangeIdPredicate(id)); + } + public List byBranchKey(Branch.NameKey branch, Change.Key key) throws OrmException { return query(and( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsVisibleToPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsVisibleToPredicate.java index 856a559b38..a99a85c621 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsVisibleToPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsVisibleToPredicate.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.query.change; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.OperatorPredicate; @@ -36,13 +37,15 @@ class IsVisibleToPredicate extends OperatorPredicate { } private final Provider db; + private final ChangeNotes.Factory notesFactory; private final ChangeControl.GenericFactory changeControl; private final CurrentUser user; - IsVisibleToPredicate(Provider db, + IsVisibleToPredicate(Provider db, ChangeNotes.Factory notesFactory, ChangeControl.GenericFactory changeControlFactory, CurrentUser user) { super(ChangeQueryBuilder.FIELD_VISIBLETO, describe(user)); this.db = db; + this.notesFactory = notesFactory; this.changeControl = changeControlFactory; this.user = user; } @@ -58,7 +61,8 @@ class IsVisibleToPredicate extends OperatorPredicate { return false; } - ChangeControl cc = changeControl.controlFor(c, user); + ChangeNotes notes = notesFactory.createFromIndexedChange(c); + ChangeControl cc = changeControl.controlFor(notes, user); if (cc.isVisible(db.get())) { cd.cacheVisibleTo(cc); return true; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryOptions.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryOptions.java index a70f892b02..686eb6d9bb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryOptions.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryOptions.java @@ -15,11 +15,14 @@ package com.google.gerrit.server.query.change; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.gerrit.server.index.ChangeField.CHANGE; +import static com.google.gerrit.server.index.ChangeField.PROJECT; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableSet; import com.google.gerrit.server.index.IndexConfig; +import java.util.HashSet; import java.util.Set; @AutoValue @@ -28,6 +31,14 @@ public abstract class QueryOptions { Set fields) { checkArgument(start >= 0, "start must be nonnegative: %s", start); checkArgument(limit > 0, "limit must be positive: %s", limit); + + // Always include project since it is needed to load the change from notedb. + if (!fields.contains(CHANGE.getName()) + && !fields.contains(PROJECT.getName())) { + fields = new HashSet<>(fields); + fields.add(PROJECT.getName()); + } + return new AutoValue_QueryOptions(config, start, limit, ImmutableSet.copyOf(fields)); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java index 1a6ae02a0f..12882f3878 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java @@ -31,6 +31,7 @@ import com.google.gerrit.server.index.IndexCollection; import com.google.gerrit.server.index.IndexConfig; import com.google.gerrit.server.index.IndexPredicate; import com.google.gerrit.server.index.IndexRewriter; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; @@ -48,6 +49,7 @@ public class QueryProcessor { private final Provider db; private final Provider userProvider; private final ChangeControl.GenericFactory changeControlFactory; + private final ChangeNotes.Factory notesFactory; private final IndexCollection indexes; private final IndexRewriter rewriter; private final IndexConfig indexConfig; @@ -62,6 +64,7 @@ public class QueryProcessor { QueryProcessor(Provider db, Provider userProvider, ChangeControl.GenericFactory changeControlFactory, + ChangeNotes.Factory notesFactory, IndexCollection indexes, IndexRewriter rewriter, IndexConfig indexConfig, @@ -69,6 +72,7 @@ public class QueryProcessor { this.db = db; this.userProvider = userProvider; this.changeControlFactory = changeControlFactory; + this.notesFactory = notesFactory; this.indexes = indexes; this.rewriter = rewriter; this.indexConfig = indexConfig; @@ -138,7 +142,8 @@ public class QueryProcessor { Timer0.Context context = metrics.executionTime.start(); Predicate visibleToMe = enforceVisibility - ? new IsVisibleToPredicate(db, changeControlFactory, userProvider.get()) + ? new IsVisibleToPredicate(db, notesFactory, changeControlFactory, + userProvider.get()) : null; int cnt = queries.size(); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/change/WalkSorterTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/change/WalkSorterTest.java index cb2d5a1244..4f2166d1b8 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/change/WalkSorterTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/change/WalkSorterTest.java @@ -355,7 +355,7 @@ public class WalkSorterTest { throws Exception { Project.NameKey project = tr.getRepository().getDescription().getProject(); Change c = TestChanges.newChange(project, userId); - ChangeData cd = ChangeData.createForTest(c.getId(), 1); + ChangeData cd = ChangeData.createForTest(project, c.getId(), 1); cd.setChange(c); cd.currentPatchSet().setRevision(new RevId(id.name())); cd.setPatchSets(ImmutableList.of(cd.currentPatchSet())); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java index dd03bb043a..a6bcac2977 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java @@ -27,7 +27,7 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { FakeQueryBuilder.class), new ChangeQueryBuilder.Arguments(null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, - null, null, indexes, null, null, null, null, null, null)); + null, null, null, indexes, null, null, null, null, null, null)); } @Operator diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java index 7ba42ae2ab..f8660d4a61 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java @@ -186,7 +186,8 @@ public class AbstractChangeNotesTest extends GerritBaseTests { } protected ChangeNotes newNotes(Change c) throws OrmException { - return new ChangeNotes(repoManager, MIGRATION, allUsers, c).load(); + return new ChangeNotes(repoManager, MIGRATION, allUsers, c.getProject(), c) + .load(); } protected static SubmitRecord submitRecord(String status, diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java index ec6a69e505..bc75bd0f9b 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java @@ -633,6 +633,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest { assertThat(ps2.getRevision().get()).isEqualTo(commit.name()); assertThat(ps2.getUploader()).isEqualTo(otherUser.getAccountId()); assertThat(ps2.getCreatedOn()).isEqualTo(update.getWhen()); + + // comment on ps1, current patch set is still ps2 + update = newUpdate(c, changeOwner); + update.setPatchSetId(ps1.getId()); + update.setChangeMessage("Comment on old patch set."); + update.commit(); + notes = newNotes(c); + assertThat(notes.getChange().currentPatchSetId()).isEqualTo(ps2.getId()); } @Test diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index d97165efbf..4f2a2193c9 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -20,6 +20,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED; import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MINUTES; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.Assert.fail; import com.google.common.base.Function; @@ -59,6 +60,7 @@ import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.index.ChangeField; import com.google.gerrit.server.index.IndexCollection; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.RefControl; @@ -110,6 +112,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { @Inject protected InMemoryRepositoryManager repoManager; @Inject protected InternalChangeQuery internalChangeQuery; @Inject protected NotesMigration notesMigration; + @Inject protected ChangeNotes.Factory notesFactory; @Inject protected PatchSetInserter.Factory patchSetFactory; @Inject protected ProjectControl.GenericFactory projectControlFactory; @Inject protected QueryProcessor queryProcessor; @@ -720,6 +723,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { @Test public void updatedOrderWithSubMinuteResolution() throws Exception { + TestTimeUtil.resetWithClockStep(1, SECONDS); + TestRepository repo = createProject("repo"); ChangeInserter ins1 = newChange(repo); Change change1 = insert(repo, ins1); @@ -731,7 +736,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { gApi.changes().id(change1.getId().get()).current() .review(new ReviewInput()); - change1 = db.changes().get(change1.getId()); + change1 = notesFactory.create(db, change1.getProject(), change1.getId()) + .getChange(); assertThat(lastUpdatedMs(change1)).isGreaterThan(lastUpdatedMs(change2)); assertThat(lastUpdatedMs(change1) - lastUpdatedMs(change2)) diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/ChangeDataTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/ChangeDataTest.java index ca1e2b133b..eb19ebe497 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/ChangeDataTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/ChangeDataTest.java @@ -28,9 +28,9 @@ import org.junit.Test; public class ChangeDataTest { @Test public void setPatchSetsClearsCurrentPatchSet() throws Exception { - ChangeData cd = ChangeData.createForTest(new Change.Id(1), 1); - cd.setChange(TestChanges.newChange( - new Project.NameKey("project"), new Account.Id(1000))); + Project.NameKey project = new Project.NameKey("project"); + ChangeData cd = ChangeData.createForTest(project, new Change.Id(1), 1); + cd.setChange(TestChanges.newChange(project, new Account.Id(1000))); PatchSet curr1 = cd.currentPatchSet(); int currId = curr1.getId().get(); PatchSet ps1 = new PatchSet(new PatchSet.Id(cd.getId(), currId + 1)); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java index 55d0f38c10..5532108534 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; import com.google.gwtorm.server.OrmException; import org.junit.Test; @@ -84,7 +85,8 @@ public class RegexPathPredicateTest { private static ChangeData change(String... files) throws OrmException { Arrays.sort(files); - ChangeData cd = ChangeData.createForTest(new Change.Id(1), 1); + ChangeData cd = ChangeData.createForTest(new Project.NameKey("project"), + new Change.Id(1), 1); cd.setCurrentFilePaths(Arrays.asList(files)); return cd; } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java index c7e3c4b7e8..300b06779c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java @@ -139,8 +139,9 @@ public class TestChanges { expect(ctl.getChange()).andStubReturn(c); expect(ctl.getProject()).andStubReturn(new Project(c.getProject())); expect(ctl.getUser()).andStubReturn(user); - ChangeNotes notes = new ChangeNotes(repoManager, migration, allUsers, c) - .load(); + ChangeNotes notes = + new ChangeNotes(repoManager, migration, allUsers, c.getProject(), c) + .load(); expect(ctl.getNotes()).andStubReturn(notes); expect(ctl.getId()).andStubReturn(c.getId()); EasyMock.replay(ctl); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PatchSetParser.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PatchSetParser.java index 18fed7e34c..ffc4e0559e 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PatchSetParser.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PatchSetParser.java @@ -127,7 +127,7 @@ public class PatchSetParser { if (c == null) { throw error("\"" + changeId + "\" no such change"); } - return notesFactory.create(db.get(), c); + return notesFactory.create(db.get(), c.getProject(), changeId); } private static boolean inProject(Change change, diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java index 076942e618..0718dc6179 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java @@ -20,7 +20,7 @@ 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.server.ReviewDb; -import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.ChangeFinder; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.change.ChangesCollection; @@ -94,7 +94,7 @@ public class SetReviewersCommand extends SshCommand { private ChangesCollection changesCollection; @Inject - private ChangeUtil changeUtil; + private ChangeFinder changeFinder; private Set toRemove = new HashSet<>(); @@ -165,7 +165,7 @@ public class SetReviewersCommand extends SshCommand { private void addChangeImpl(String id) throws UnloggedFailure, OrmException { List matched = - changeUtil.findChanges(id, userProvider.get()); + changeFinder.find(id, userProvider.get()); List toAdd = new ArrayList<>(changes.size()); for (ChangeControl ctl : matched) { if (!changes.containsKey(ctl.getId()) && inProject(ctl.getProject()) diff --git a/plugins/reviewnotes b/plugins/reviewnotes index 34678f04b2..552001eeaf 160000 --- a/plugins/reviewnotes +++ b/plugins/reviewnotes @@ -1 +1 @@ -Subproject commit 34678f04b28c02cda754dd49c9dd99ff4db60415 +Subproject commit 552001eeaf6ffbfdb1f1ecd08cd9b34c94ca7ed8