Remove ChangeAccess.byKey and byKeyRange

All callers of byKeyRange passed key.max() as the right endpoint, so
this can be safely replaced with a prefix query against the secondary
index. The number of callers notwithstanding, this was pretty
straightforward, although it does lead to the almost-recursive state
of calling InternalChangeQuery from ChangeQueryBuilder when expanding
the conflicts predicate.

Change-Id: I073d6ab179d3d5a5d8f0157aa3594fd3ea2cc81c
This commit is contained in:
Dave Borowitz
2015-01-13 17:43:06 -08:00
parent dfc07f63bb
commit f9c88308de
19 changed files with 100 additions and 107 deletions

View File

@@ -45,11 +45,13 @@ import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.Util; import com.google.gerrit.server.project.Util;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.testutil.ConfigSuite; import com.google.gerrit.testutil.ConfigSuite;
import com.google.gerrit.testutil.TempFileUtil; import com.google.gerrit.testutil.TempFileUtil;
import com.google.gson.Gson; import com.google.gson.Gson;
import com.google.gwtorm.server.SchemaFactory; import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.util.Providers; import com.google.inject.util.Providers;
import org.apache.http.HttpStatus; import org.apache.http.HttpStatus;
@@ -111,6 +113,9 @@ public abstract class AbstractDaemonTest {
@Inject @Inject
protected ChangeIndexer indexer; protected ChangeIndexer indexer;
@Inject
protected Provider<InternalChangeQuery> queryProvider;
protected Git git; protected Git git;
protected GerritServer server; protected GerritServer server;
protected TestAccount admin; protected TestAccount admin;

View File

@@ -33,7 +33,10 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
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.gwtorm.server.OrmException;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject; import com.google.inject.assistedinject.AssistedInject;
@@ -101,6 +104,7 @@ public class PushOneCommit {
private final ChangeNotes.Factory notesFactory; private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final Provider<InternalChangeQuery> queryProvider;
private final ReviewDb db; private final ReviewDb db;
private final PersonIdent i; private final PersonIdent i;
@@ -113,25 +117,30 @@ public class PushOneCommit {
@AssistedInject @AssistedInject
PushOneCommit(ChangeNotes.Factory notesFactory, PushOneCommit(ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
@Assisted ReviewDb db, @Assisted ReviewDb db,
@Assisted PersonIdent i) { @Assisted PersonIdent i) {
this(notesFactory, approvalsUtil, db, i, SUBJECT, FILE_NAME, FILE_CONTENT); this(notesFactory, approvalsUtil, queryProvider,
db, i, SUBJECT, FILE_NAME, FILE_CONTENT);
} }
@AssistedInject @AssistedInject
PushOneCommit(ChangeNotes.Factory notesFactory, PushOneCommit(ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
@Assisted ReviewDb db, @Assisted ReviewDb db,
@Assisted PersonIdent i, @Assisted PersonIdent i,
@Assisted("subject") String subject, @Assisted("subject") String subject,
@Assisted("fileName") String fileName, @Assisted("fileName") String fileName,
@Assisted("content") String content) { @Assisted("content") String content) {
this(notesFactory, approvalsUtil, db, i, subject, fileName, content, null); this(notesFactory, approvalsUtil, queryProvider,
db, i, subject, fileName, content, null);
} }
@AssistedInject @AssistedInject
PushOneCommit(ChangeNotes.Factory notesFactory, PushOneCommit(ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
@Assisted ReviewDb db, @Assisted ReviewDb db,
@Assisted PersonIdent i, @Assisted PersonIdent i,
@Assisted("subject") String subject, @Assisted("subject") String subject,
@@ -141,6 +150,7 @@ public class PushOneCommit {
this.db = db; this.db = db;
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.queryProvider = queryProvider;
this.i = i; this.i = i;
this.subject = subject; this.subject = subject;
this.fileName = fileName; this.fileName = fileName;
@@ -200,9 +210,13 @@ public class PushOneCommit {
this.resSubj = subject; this.resSubj = subject;
} }
public PatchSet.Id getPatchSetId() throws OrmException { public ChangeData getChange() throws OrmException {
return Iterables.getOnlyElement( return Iterables.getOnlyElement(
db.changes().byKey(new Change.Key(commit.getChangeId()))).currentPatchSetId(); queryProvider.get().byKeyPrefix(commit.getChangeId()));
}
public PatchSet.Id getPatchSetId() throws OrmException {
return getChange().change().currentPatchSetId();
} }
public String getChangeId() { public String getChangeId() {
@@ -220,8 +234,7 @@ public class PushOneCommit {
public void assertChange(Change.Status expectedStatus, public void assertChange(Change.Status expectedStatus,
String expectedTopic, TestAccount... expectedReviewers) String expectedTopic, TestAccount... expectedReviewers)
throws OrmException { throws OrmException {
Change c = Change c = getChange().change();
Iterables.getOnlyElement(db.changes().byKey(new Change.Key(commit.getChangeId())).toList());
assertThat(resSubj).isEqualTo(c.getSubject()); assertThat(resSubj).isEqualTo(c.getSubject());
assertThat(expectedStatus).isEqualTo(c.getStatus()); assertThat(expectedStatus).isEqualTo(c.getStatus());
assertThat(expectedTopic).isEqualTo(Strings.emptyToNull(c.getTopic())); assertThat(expectedTopic).isEqualTo(Strings.emptyToNull(c.getTopic()));

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.acceptance.edit; package com.google.gerrit.acceptance.edit;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.cloneProject; import static com.google.gerrit.acceptance.GitUtil.cloneProject;
import static com.google.gerrit.acceptance.GitUtil.createProject; import static com.google.gerrit.acceptance.GitUtil.createProject;
@@ -27,7 +28,6 @@ import static org.junit.Assert.fail;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
@@ -620,8 +620,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
} }
private Change getChange(String changeId) throws Exception { private Change getChange(String changeId) throws Exception {
return Iterables.getOnlyElement(db.changes() return getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change();
.byKey(new Change.Key(changeId)));
} }
private PatchSet getCurrentPatchSet(String changeId) throws Exception { private PatchSet getCurrentPatchSet(String changeId) throws Exception {

View File

@@ -155,8 +155,8 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
"other content", r.getChangeId()); "other content", r.getChangeId());
r.assertOkStatus(); r.assertOkStatus();
r.assertChange(Change.Status.MERGED, null, admin); r.assertChange(Change.Status.MERGED, null, admin);
Change c = Iterables.getOnlyElement(db.changes().byKey( Change c = Iterables.getOnlyElement(
new Change.Key(r.getChangeId())).toList()); queryProvider.get().byKeyPrefix(r.getChangeId())).change();
assertThat(db.patchSets().byChange(c.getId()).toList()).hasSize(2); assertThat(db.patchSets().byChange(c.getId()).toList()).hasSize(2);
assertSubmitApproval(r.getPatchSetId()); assertSubmitApproval(r.getPatchSetId());
assertCommit(project, "refs/heads/master"); assertCommit(project, "refs/heads/master");

View File

@@ -14,13 +14,13 @@
package com.google.gerrit.acceptance.rest.change; package com.google.gerrit.acceptance.rest.change;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assert_; import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.acceptance.GitUtil.cloneProject; import static com.google.gerrit.acceptance.GitUtil.cloneProject;
import static com.google.gerrit.extensions.common.ListChangesOption.CURRENT_REVISION; import static com.google.gerrit.extensions.common.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.extensions.common.ListChangesOption.DETAILED_LABELS; import static com.google.gerrit.extensions.common.ListChangesOption.DETAILED_LABELS;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.GitUtil;
@@ -40,7 +40,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.PutConfig; import com.google.gerrit.server.project.PutConfig;
import com.google.gson.reflect.TypeToken; import com.google.gson.reflect.TypeToken;
@@ -74,9 +73,6 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
@Inject @Inject
private ApprovalsUtil approvalsUtil; private ApprovalsUtil approvalsUtil;
@Inject
private ChangeIndexer indexer;
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
project = new Project.NameKey("p2"); project = new Project.NameKey("p2");
@@ -158,7 +154,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
protected void submitStatusOnly(String changeId) protected void submitStatusOnly(String changeId)
throws IOException, OrmException { throws IOException, OrmException {
approve(changeId); approve(changeId);
Change c = db.changes().byKey(new Change.Key(changeId)).toList().get(0); Change c = queryProvider.get().byKeyPrefix(changeId).get(0).change();
c.setStatus(Change.Status.SUBMITTED); c.setStatus(Change.Status.SUBMITTED);
db.changes().update(Collections.singleton(c)); db.changes().update(Collections.singleton(c));
db.patchSetApprovals().insert(Collections.singleton( db.patchSetApprovals().insert(Collections.singleton(
@@ -214,7 +210,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
protected void assertSubmitter(String changeId, int psId) protected void assertSubmitter(String changeId, int psId)
throws OrmException { throws OrmException {
ChangeNotes cn = notesFactory.create( ChangeNotes cn = notesFactory.create(
Iterables.getOnlyElement(db.changes().byKey(new Change.Key(changeId)))); getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change());
PatchSetApproval submitter = approvalsUtil.getSubmitter( PatchSetApproval submitter = approvalsUtil.getSubmitter(
db, cn, new PatchSet.Id(cn.getChangeId(), psId)); db, cn, new PatchSet.Id(cn.getChangeId(), psId));
assertThat(submitter.isSubmit()).isTrue(); assertThat(submitter.isSubmit()).isTrue();

View File

@@ -23,7 +23,6 @@ import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.RestSession; import com.google.gerrit.acceptance.RestSession;
import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeStatus; import com.google.gerrit.extensions.common.ChangeStatus;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -98,10 +97,8 @@ public class DeleteDraftChangeIT extends AbstractDaemonTest {
private RestResponse publishPatchSet(String changeId) throws IOException, private RestResponse publishPatchSet(String changeId) throws IOException,
OrmException { OrmException {
PatchSet patchSet = db.patchSets() PatchSet patchSet = Iterables.getOnlyElement(
.get(Iterables.getOnlyElement(db.changes() queryProvider.get().byKeyPrefix(changeId)).currentPatchSet();
.byKey(new Change.Key(changeId)))
.currentPatchSetId());
return adminSession.post("/changes/" return adminSession.post("/changes/"
+ changeId + changeId
+ "/revisions/" + "/revisions/"

View File

@@ -24,8 +24,8 @@ import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.RestSession; import com.google.gerrit.acceptance.RestSession;
import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeStatus; import com.google.gerrit.extensions.common.ChangeStatus;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import org.apache.http.HttpStatus; import org.apache.http.HttpStatus;
@@ -72,13 +72,11 @@ public class DeleteDraftPatchSetIT extends AbstractDaemonTest {
assertThat(c.status).isEqualTo(ChangeStatus.DRAFT); assertThat(c.status).isEqualTo(ChangeStatus.DRAFT);
RestResponse r = deletePatchSet(changeId, ps, adminSession); RestResponse r = deletePatchSet(changeId, ps, adminSession);
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_NO_CONTENT); assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_NO_CONTENT);
Change change = Iterables.getOnlyElement(db.changes().byKey( assertThat(getChange(changeId).patches().size()).isEqualTo(1);
new Change.Key(changeId)).toList());
assertThat(db.patchSets().byChange(change.getId()).toList()).hasSize(1);
ps = getCurrentPatchSet(changeId); ps = getCurrentPatchSet(changeId);
r = deletePatchSet(changeId, ps, adminSession); r = deletePatchSet(changeId, ps, adminSession);
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_NO_CONTENT); assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_NO_CONTENT);
assertThat(db.changes().byKey(new Change.Key(changeId)).toList()).isEmpty(); assertThat(queryProvider.get().byKeyPrefix(changeId)).isEmpty();
} }
private String createDraftChangeWith2PS() throws GitAPIException, private String createDraftChangeWith2PS() throws GitAPIException,
@@ -91,10 +89,11 @@ public class DeleteDraftPatchSetIT extends AbstractDaemonTest {
} }
private PatchSet getCurrentPatchSet(String changeId) throws OrmException { private PatchSet getCurrentPatchSet(String changeId) throws OrmException {
return db.patchSets() return getChange(changeId).currentPatchSet();
.get(Iterables.getOnlyElement(db.changes() }
.byKey(new Change.Key(changeId)))
.currentPatchSetId()); private ChangeData getChange(String changeId) throws OrmException {
return Iterables.getOnlyElement(queryProvider.get().byKeyPrefix(changeId));
} }
private static RestResponse deletePatchSet(String changeId, private static RestResponse deletePatchSet(String changeId,

View File

@@ -31,6 +31,7 @@ import com.google.gerrit.server.change.GetRelated.ChangeAndCommit;
import com.google.gerrit.server.change.GetRelated.RelatedInfo; import com.google.gerrit.server.change.GetRelated.RelatedInfo;
import com.google.gerrit.server.edit.ChangeEditModifier; import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -165,7 +166,7 @@ public class GetRelatedIT extends AbstractDaemonTest {
Commit c3 = createCommit(git, admin.getIdent(), "subject: 3"); Commit c3 = createCommit(git, admin.getIdent(), "subject: 3");
pushHead(git, "refs/for/master", false); pushHead(git, "refs/for/master", false);
Change ch2 = getChange(c2); Change ch2 = getChange(c2).change();
editModifier.createEdit(ch2, getPatchSet(ch2)); editModifier.createEdit(ch2, getPatchSet(ch2));
editModifier.modifyFile(editUtil.byChange(ch2).get(), "a.txt", editModifier.modifyFile(editUtil.byChange(ch2).get(), "a.txt",
RestSession.newRawInput(new byte[] {'a'})); RestSession.newRawInput(new byte[] {'a'}));
@@ -198,15 +199,15 @@ public class GetRelatedIT extends AbstractDaemonTest {
} }
private PatchSet.Id getPatchSetId(Commit c) throws OrmException { private PatchSet.Id getPatchSetId(Commit c) throws OrmException {
return getChange(c).currentPatchSetId(); return getChange(c).change().currentPatchSetId();
} }
private PatchSet getPatchSet(Change c) throws OrmException { private PatchSet getPatchSet(Change c) throws OrmException {
return db.patchSets().get(c.currentPatchSetId()); return db.patchSets().get(c.currentPatchSetId());
} }
private Change getChange(Commit c) throws OrmException { private ChangeData getChange(Commit c) throws OrmException {
return Iterables.getOnlyElement( return Iterables.getOnlyElement(
db.changes().byKey(new Change.Key(c.getChangeId()))); queryProvider.get().byKeyPrefix(c.getChangeId()));
} }
} }

View File

@@ -26,13 +26,6 @@ public interface ChangeAccess extends Access<Change, Change.Id> {
@PrimaryKey("changeId") @PrimaryKey("changeId")
Change get(Change.Id id) throws OrmException; Change get(Change.Id id) throws OrmException;
@Query("WHERE changeKey = ?")
ResultSet<Change> byKey(Change.Key key) throws OrmException;
@Query("WHERE changeKey >= ? AND changeKey <= ?")
ResultSet<Change> byKeyRange(Change.Key reva, Change.Key revb)
throws OrmException;
@Query @Query
ResultSet<Change> all() throws OrmException; ResultSet<Change> all() throws OrmException;
} }

View File

@@ -67,12 +67,6 @@ ON account_project_watches (project_name);
-- @PrimaryKey covers: byProject -- @PrimaryKey covers: byProject
-- *********************************************************************
-- ChangeAccess
CREATE INDEX changes_key
ON changes (change_key);
-- ********************************************************************* -- *********************************************************************
-- ChangeMessageAccess -- ChangeMessageAccess
-- @PrimaryKey covers: byChange -- @PrimaryKey covers: byChange

View File

@@ -74,13 +74,6 @@ ON account_project_watches (project_name)
-- @PrimaryKey covers: byProject -- @PrimaryKey covers: byProject
-- *********************************************************************
-- ChangeAccess
CREATE INDEX changes_key
ON changes (change_key)
#
-- ********************************************************************* -- *********************************************************************
-- ChangeMessageAccess -- ChangeMessageAccess
-- @PrimaryKey covers: byChange -- @PrimaryKey covers: byChange

View File

@@ -115,12 +115,6 @@ ON account_project_watches (project_name);
-- @PrimaryKey covers: byProject -- @PrimaryKey covers: byProject
-- *********************************************************************
-- ChangeAccess
CREATE INDEX changes_key
ON changes (change_key);
-- ********************************************************************* -- *********************************************************************
-- ChangeMessageAccess -- ChangeMessageAccess
-- @PrimaryKey covers: byChange -- @PrimaryKey covers: byChange

View File

@@ -526,12 +526,7 @@ public class ChangeUtil {
// Try isolated changeId // Try isolated changeId
if (!id.contains("~")) { if (!id.contains("~")) {
Change.Key key = new Change.Key(id); return asChanges(queryProvider.get().byKeyPrefix(id));
if (key.get().length() == 41) {
return db.get().changes().byKey(key).toList();
} else {
return db.get().changes().byKeyRange(key, key.max()).toList();
}
} }
// Try change triplet // Try change triplet

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.change.DeleteDraftPatchSet.Input; import com.google.gerrit.server.change.DeleteDraftPatchSet.Input;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
@@ -49,16 +50,19 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
protected final Provider<ReviewDb> dbProvider; protected final Provider<ReviewDb> dbProvider;
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
private final ChangeUtil changeUtil; private final ChangeUtil changeUtil;
private final ChangeIndexer indexer;
private final boolean allowDrafts; private final boolean allowDrafts;
@Inject @Inject
public DeleteDraftPatchSet(Provider<ReviewDb> dbProvider, public DeleteDraftPatchSet(Provider<ReviewDb> dbProvider,
PatchSetInfoFactory patchSetInfoFactory, PatchSetInfoFactory patchSetInfoFactory,
ChangeUtil changeUtil, ChangeUtil changeUtil,
ChangeIndexer indexer,
@GerritServerConfig Config cfg) { @GerritServerConfig Config cfg) {
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.patchSetInfoFactory = patchSetInfoFactory; this.patchSetInfoFactory = patchSetInfoFactory;
this.changeUtil = changeUtil; this.changeUtil = changeUtil;
this.indexer = indexer;
this.allowDrafts = cfg.getBoolean("change", "allowDrafts", true); this.allowDrafts = cfg.getBoolean("change", "allowDrafts", true);
} }
@@ -153,10 +157,11 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
} }
} }
private static void updateChange(final ReviewDb db, private void updateChange(final ReviewDb db,
final Change change, final PatchSetInfo psInfo) Change change, final PatchSetInfo psInfo)
throws OrmException { throws OrmException, IOException {
db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() { change = db.changes().atomicUpdate(change.getId(),
new AtomicUpdate<Change>() {
@Override @Override
public Change update(Change c) { public Change update(Change c) {
if (psInfo != null) { if (psInfo != null) {
@@ -166,5 +171,6 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
return c; return c;
} }
}); });
indexer.index(db, change);
} }
} }

View File

@@ -26,6 +26,7 @@ public class BasicChangeRewrites extends QueryRewriter<ChangeData> {
private static final ChangeQueryBuilder BUILDER = new ChangeQueryBuilder( private static final ChangeQueryBuilder BUILDER = new ChangeQueryBuilder(
new ChangeQueryBuilder.Arguments( new ChangeQueryBuilder.Arguments(
new InvalidProvider<ReviewDb>(), new InvalidProvider<ReviewDb>(),
new InvalidProvider<InternalChangeQuery>(),
new InvalidProvider<ChangeQueryRewriter>(), new InvalidProvider<ChangeQueryRewriter>(),
null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
null, null, null, null, null, null, null, null)); null, null, null, null, null, null, null, null));

View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.query.change; package com.google.gerrit.server.query.change;
import static com.google.gerrit.server.query.change.ChangeData.asChanges;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
@@ -125,6 +127,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@VisibleForTesting @VisibleForTesting
public static class Arguments { public static class Arguments {
final Provider<ReviewDb> db; final Provider<ReviewDb> db;
final Provider<InternalChangeQuery> queryProvider;
final Provider<ChangeQueryRewriter> rewriter; final Provider<ChangeQueryRewriter> rewriter;
final IdentifiedUser.GenericFactory userFactory; final IdentifiedUser.GenericFactory userFactory;
final CapabilityControl.Factory capabilityControlFactory; final CapabilityControl.Factory capabilityControlFactory;
@@ -150,6 +153,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@Inject @Inject
@VisibleForTesting @VisibleForTesting
public Arguments(Provider<ReviewDb> db, public Arguments(Provider<ReviewDb> db,
Provider<InternalChangeQuery> queryProvider,
Provider<ChangeQueryRewriter> rewriter, Provider<ChangeQueryRewriter> rewriter,
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
Provider<CurrentUser> self, Provider<CurrentUser> self,
@@ -170,16 +174,18 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
ConflictsCache conflictsCache, ConflictsCache conflictsCache,
TrackingFooters trackingFooters, TrackingFooters trackingFooters,
@GerritServerConfig Config cfg) { @GerritServerConfig Config cfg) {
this(db, rewriter, userFactory, self, capabilityControlFactory, this(db, queryProvider, rewriter, userFactory, self,
changeControlGenericFactory, changeDataFactory, fillArgs, plcUtil, capabilityControlFactory, changeControlGenericFactory,
accountResolver, groupBackend, allProjectsName, patchListCache, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend,
repoManager, projectCache, listChildProjects, indexes, allProjectsName, patchListCache, repoManager, projectCache,
submitStrategyFactory, conflictsCache, trackingFooters, listChildProjects, indexes, submitStrategyFactory, conflictsCache,
trackingFooters,
cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true)); cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true));
} }
private Arguments( private Arguments(
Provider<ReviewDb> db, Provider<ReviewDb> db,
Provider<InternalChangeQuery> queryProvider,
Provider<ChangeQueryRewriter> rewriter, Provider<ChangeQueryRewriter> rewriter,
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
Provider<CurrentUser> self, Provider<CurrentUser> self,
@@ -201,6 +207,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
TrackingFooters trackingFooters, TrackingFooters trackingFooters,
boolean allowsDrafts) { boolean allowsDrafts) {
this.db = db; this.db = db;
this.queryProvider = queryProvider;
this.rewriter = rewriter; this.rewriter = rewriter;
this.userFactory = userFactory; this.userFactory = userFactory;
this.self = self; this.self = self;
@@ -224,7 +231,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
} }
Arguments asUser(CurrentUser otherUser) { Arguments asUser(CurrentUser otherUser) {
return new Arguments(db, rewriter, userFactory, return new Arguments(db, queryProvider, rewriter, userFactory,
Providers.of(otherUser), Providers.of(otherUser),
capabilityControlFactory, changeControlGenericFactory, capabilityControlFactory, changeControlGenericFactory,
changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend,
@@ -806,9 +813,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
return Collections.singletonList(args.db.get().changes() return Collections.singletonList(args.db.get().changes()
.get(Change.Id.parse(value))); .get(Change.Id.parse(value)));
} else if (PAT_CHANGE_ID.matcher(value).matches()) { } else if (PAT_CHANGE_ID.matcher(value).matches()) {
Change.Key a = new Change.Key(parseChangeId(value));
List<Change> changes = List<Change> changes =
args.db.get().changes().byKeyRange(a, a.max()).toList(); asChanges(args.queryProvider.get().byKeyPrefix(parseChangeId(value)));
if (changes.isEmpty()) { if (changes.isEmpty()) {
throw error("Change " + value + " not found"); throw error("Change " + value + " not found");
} }

View File

@@ -69,6 +69,13 @@ public class InternalChangeQuery {
return this; return this;
} }
public List<ChangeData> byKey(Change.Key key) throws OrmException {
return byKeyPrefix(key.get());
}
public List<ChangeData> byKeyPrefix(String prefix) throws OrmException {
return query(new ChangeIdPredicate(prefix));
}
public List<ChangeData> byBranchKey(Branch.NameKey branch, Change.Key key) public List<ChangeData> byBranchKey(Branch.NameKey branch, Change.Key key)
throws OrmException { throws OrmException {

View File

@@ -26,8 +26,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
new FakeQueryBuilder.Definition<>( new FakeQueryBuilder.Definition<>(
FakeQueryBuilder.class), FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(null, null, null, null, null, null, new ChangeQueryBuilder.Arguments(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, null, null,
null, null, null, null)); indexes, null, null, null, null));
} }
@Operator @Operator

View File

@@ -30,6 +30,8 @@ import com.google.gerrit.server.change.ReviewerResource;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand; import com.google.gerrit.sshd.SshCommand;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -78,6 +80,9 @@ public class SetReviewersCommand extends SshCommand {
@Inject @Inject
private ReviewDb db; private ReviewDb db;
@Inject
private Provider<InternalChangeQuery> queryProvider;
@Inject @Inject
private ReviewerResource.Factory reviewerFactory; private ReviewerResource.Factory reviewerFactory;
@@ -170,21 +175,10 @@ public class SetReviewersCommand extends SshCommand {
// By newer style changeKey? // By newer style changeKey?
// //
boolean changeKeyParses = false; boolean changeKeyParses = idstr.matches("^I[0-9a-f]*$");
if (idstr.matches("^I[0-9a-fA-F]*$")) {
Change.Key key;
try {
key = Change.Key.parse(idstr);
changeKeyParses = true;
} catch (IllegalArgumentException e) {
key = null;
changeKeyParses = false;
}
if (changeKeyParses) { if (changeKeyParses) {
for (Change change : db.changes().byKeyRange(key, key.max())) { for (ChangeData cd : queryProvider.get().byKeyPrefix(idstr)) {
matchChange(matched, change); matchChange(matched, cd.change());
}
} }
} }