Store users that starred a change in change index
Within notedb each change that is starred by a user is tracked by a
refs/starred-changes/XX/YYYY/ZZZZ ref in the All-Users meta repository
where XX/YYYY is the sharded change ID and ZZZZ is the account ID.
With this storage format finding all users that have starred a
change is cheap because we just need to list all refs that have
refs/starred-changes/XX/YYYY/ as prefix and looking up refs with a
prefix that ends on '/' is optimized in jgit. However to find all
changes that were starred by a user we must scan the complete
refs/starred-changes/ namespace and check which of the refs end with
the account ID. This results in bad performance when there are many
starred changes. Having the users that starred a change stored in the
change index makes this lookup cheap.
Looking up all changes that were starred by a user is needed for
supporting the "is:starred" query operator and for populating the
"starred" field in ChangeInfo.
Change-Id: Iecc9ca8ef133b0d0f86f1471b9ed31be78a43f6c
Signed-off-by: Edwin Kempin <ekempin@google.com>
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 8f7e899..840f8b4 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
@@ -114,6 +114,7 @@
private static final String PATCH_SET_FIELD = ChangeField.PATCH_SET.getName();
private static final String REVIEWEDBY_FIELD =
ChangeField.REVIEWEDBY.getName();
+ private static final String STARREDBY_FIELD = ChangeField.STARREDBY.getName();
static Term idTerm(ChangeData cd) {
return QueryBuilder.intTerm(LEGACY_ID.getName(), cd.getId().get());
@@ -413,6 +414,9 @@
if (fields.contains(REVIEWEDBY_FIELD)) {
decodeReviewedBy(doc, cd);
}
+ if (fields.contains(STARREDBY_FIELD)) {
+ decodeStarredBy(doc, cd);
+ }
return cd;
}
@@ -466,6 +470,16 @@
}
}
+ private void decodeStarredBy(Document doc, ChangeData cd) {
+ IndexableField[] starredBy = doc.getFields(STARREDBY_FIELD);
+ Set<Account.Id> accounts =
+ Sets.newHashSetWithExpectedSize(starredBy.length);
+ for (IndexableField r : starredBy) {
+ accounts.add(new Account.Id(r.numericValue().intValue()));
+ }
+ cd.setStarredBy(accounts);
+ }
+
private static <T> List<T> decodeProtos(Document doc, String fieldName,
ProtobufCodec<T> codec) {
BytesRef[] bytesRefs = doc.getBinaryValues(fieldName);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
index 50b20f0..fb75091b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
@@ -338,7 +338,7 @@
FluentIterable.from(
starredQuery != null
? starredQuery
- : starredChangesUtil.query(accountId))
+ : starredChangesUtil.queryFromIndex(accountId))
.toSet();
} finally {
starredQuery = null;
@@ -356,7 +356,7 @@
public void asyncStarredChanges() {
if (starredChanges == null && starredChangesUtil != null) {
- starredQuery = starredChangesUtil.query(accountId);
+ starredQuery = starredChangesUtil.queryFromIndex(accountId);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
index 6413414..49d3689 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -15,18 +15,24 @@
package com.google.gerrit.server;
import com.google.common.base.Function;
-import com.google.common.base.Predicate;
import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterators;
import com.google.common.collect.Lists;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.StarredChange;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.notedb.NotesMigration;
+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.gwtorm.server.ListResultSet;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
@@ -65,26 +71,33 @@
private final NotesMigration migration;
private final Provider<ReviewDb> dbProvider;
private final PersonIdent serverIdent;
+ private final ChangeIndexer indexer;
+ private final Provider<InternalChangeQuery> queryProvider;
@Inject
StarredChangesUtil(GitRepositoryManager repoManager,
AllUsersName allUsers,
NotesMigration migration,
Provider<ReviewDb> dbProvider,
- @GerritPersonIdent PersonIdent serverIdent) {
+ @GerritPersonIdent PersonIdent serverIdent,
+ ChangeIndexer indexer,
+ Provider<InternalChangeQuery> queryProvider) {
this.repoManager = repoManager;
this.allUsers = allUsers;
this.migration = migration;
this.dbProvider = dbProvider;
this.serverIdent = serverIdent;
+ this.indexer = indexer;
+ this.queryProvider = queryProvider;
}
- public void star(Account.Id accountId, Change.Id changeId)
- throws OrmException {
+ public void star(Account.Id accountId, Project.NameKey project,
+ Change.Id changeId) throws OrmException, IOException {
dbProvider.get().starredChanges()
.insert(Collections.singleton(new StarredChange(
new StarredChange.Key(accountId, changeId))));
if (!migration.writeAccounts()) {
+ indexer.index(dbProvider.get(), project, changeId);
return;
}
try (Repository repo = repoManager.openRepository(allUsers);
@@ -98,6 +111,7 @@
RefUpdate.Result result = u.update(rw);
switch (result) {
case NEW:
+ indexer.index(dbProvider.get(), project, changeId);
return;
case FAST_FORWARD:
case FORCED:
@@ -128,12 +142,13 @@
}
}
- public void unstar(Account.Id accountId, Change.Id changeId)
- throws OrmException {
+ public void unstar(Account.Id accountId, Project.NameKey project,
+ Change.Id changeId) throws OrmException, IOException {
dbProvider.get().starredChanges()
.delete(Collections.singleton(new StarredChange(
new StarredChange.Key(accountId, changeId))));
if (!migration.writeAccounts()) {
+ indexer.index(dbProvider.get(), project, changeId);
return;
}
try (Repository repo = repoManager.openRepository(allUsers);
@@ -146,6 +161,7 @@
RefUpdate.Result result = u.delete();
switch (result) {
case FORCED:
+ indexer.index(dbProvider.get(), project, changeId);
return;
case FAST_FORWARD:
case IO_FAILURE:
@@ -168,10 +184,12 @@
}
}
- public void unstarAll(Change.Id changeId) throws OrmException {
+ public void unstarAll(Project.NameKey project, Change.Id changeId)
+ throws OrmException, IOException, NoSuchChangeException {
dbProvider.get().starredChanges().delete(
dbProvider.get().starredChanges().byChange(changeId));
if (!migration.writeAccounts()) {
+ indexer.index(dbProvider.get(), project, changeId);
return;
}
try (Repository repo = repoManager.openRepository(allUsers);
@@ -180,7 +198,7 @@
batchUpdate.setAllowNonFastForwards(true);
batchUpdate.setRefLogIdent(serverIdent);
batchUpdate.setRefLogMessage("Unstar change " + changeId.get(), true);
- for (Account.Id accountId : byChange(changeId)) {
+ for (Account.Id accountId : byChangeFromIndex(changeId)) {
String refName = RefNames.refsStarredChanges(changeId, accountId);
Ref ref = repo.getRefDatabase().getRef(refName);
batchUpdate.addCommand(new ReceiveCommand(ref.getObjectId(),
@@ -194,13 +212,14 @@
changeId.get(), command.getRefName(), command.getResult()));
}
}
+ indexer.index(dbProvider.get(), project, changeId);
} catch (IOException e) {
throw new OrmException(
String.format("Unstar change %d failed", changeId.get()), e);
}
}
- public Iterable<Account.Id> byChange(Change.Id changeId)
+ public Set<Account.Id> byChange(Change.Id changeId)
throws OrmException {
if (!migration.readAccounts()) {
return FluentIterable
@@ -210,7 +229,7 @@
public Account.Id apply(StarredChange in) {
return in.getAccountId();
}
- });
+ }).toSet();
}
return FluentIterable
.from(getRefNames(RefNames.refsStarredChangesPrefix(changeId)))
@@ -219,29 +238,40 @@
public Account.Id apply(String refPart) {
return Account.Id.parse(refPart);
}
- });
+ }).toSet();
+ }
+
+ public Set<Account.Id> byChangeFromIndex(Change.Id changeId)
+ throws OrmException, NoSuchChangeException {
+ Set<String> fields = ImmutableSet.of(
+ ChangeField.ID.getName(),
+ ChangeField.STARREDBY.getName());
+ List<ChangeData> changeData = queryProvider.get().setRequestedFields(fields)
+ .byLegacyChangeId(changeId);
+ if (changeData.size() != 1) {
+ throw new NoSuchChangeException(changeId);
+ }
+ return changeData.get(0).starredBy();
}
@Deprecated
- public ResultSet<Change.Id> query(final Account.Id accountId) {
+ public ResultSet<Change.Id> queryFromIndex(final Account.Id accountId) {
try {
if (!migration.readAccounts()) {
return new ChangeIdResultSet(
dbProvider.get().starredChanges().byAccount(accountId));
}
+ Set<String> fields = ImmutableSet.of(
+ ChangeField.ID.getName());
+ List<ChangeData> changeData =
+ queryProvider.get().setRequestedFields(fields).byIsStarred(accountId);
return new ListResultSet<>(FluentIterable
- .from(getRefNames(RefNames.REFS_STARRED_CHANGES))
- .filter(new Predicate<String>() {
+ .from(changeData)
+ .transform(new Function<ChangeData, Change.Id>() {
@Override
- public boolean apply(String refPart) {
- return refPart.endsWith("/" + accountId.get());
- }
- })
- .transform(new Function<String, Change.Id>() {
- @Override
- public Change.Id apply(String changeId) {
- return Change.Id.parse(changeId);
+ public Change.Id apply(ChangeData cd) {
+ return cd.getId();
}
}).toList());
} catch (OrmException | RuntimeException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
index f8a23d8..1ed29a7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
@@ -42,6 +42,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.IOException;
+
@Singleton
public class StarredChanges implements
ChildCollection<AccountResource, AccountResource.StarredChange>,
@@ -130,12 +132,13 @@
@Override
public Response<?> apply(AccountResource rsrc, EmptyInput in)
- throws AuthException, OrmException {
+ throws AuthException, OrmException, IOException {
if (self.get() != rsrc.getUser()) {
throw new AuthException("not allowed to add starred change");
}
try {
- starredChangesUtil.star(self.get().getAccountId(), change.getId());
+ starredChangesUtil.star(self.get().getAccountId(), change.getProject(),
+ change.getId());
} catch (OrmDuplicateKeyException e) {
return Response.none();
}
@@ -177,12 +180,12 @@
@Override
public Response<?> apply(AccountResource.StarredChange rsrc,
- EmptyInput in) throws AuthException, OrmException {
+ EmptyInput in) throws AuthException, OrmException, IOException {
if (self.get() != rsrc.getUser()) {
throw new AuthException("not allowed remove starred change");
}
starredChangesUtil.unstar(self.get().getAccountId(),
- rsrc.getChange().getId());
+ rsrc.getChange().getProject(), rsrc.getChange().getId());
return Response.none();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
index 55d077f..b3126d3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
@@ -193,7 +193,7 @@
IdString.fromUrl(id));
starredChangesCreate.setChange(rsrc);
starredChangesCreate.apply(account, new StarredChanges.EmptyInput());
- } catch (OrmException e) {
+ } catch (OrmException | IOException e) {
throw new RestApiException("Cannot star change", e);
}
}
@@ -207,7 +207,7 @@
new AccountResource.StarredChange(account.getUser(), rsrc);
starredChangesDelete.apply(starredChange,
new StarredChanges.EmptyInput());
- } catch (OrmException e) {
+ } catch (OrmException | IOException e) {
throw new RestApiException("Cannot unstar change", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java
index d90fc73..6ac17b2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.BatchUpdate.RepoContext;
import com.google.gerrit.server.git.BatchUpdateReviewDb;
+import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -78,8 +79,8 @@
}
@Override
- public boolean updateChange(ChangeContext ctx)
- throws RestApiException, OrmException {
+ public boolean updateChange(ChangeContext ctx) throws RestApiException,
+ OrmException, IOException, NoSuchChangeException {
checkState(ctx.getOrder() == BatchUpdate.Order.DB_BEFORE_REPO,
"must use DeleteDraftChangeOp with DB_BEFORE_REPO");
checkState(id == null, "cannot reuse DeleteDraftChangeOp");
@@ -117,7 +118,7 @@
// Non-atomic operation on Accounts table; not much we can do to make it
// atomic.
- starredChangesUtil.unstarAll(id);
+ starredChangesUtil.unstarAll(change.getProject(), id);
ctx.deleteChange();
return true;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
index 59d4290..2edff4b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
@@ -35,6 +35,7 @@
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
+import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -99,8 +100,8 @@
}
@Override
- public boolean updateChange(ChangeContext ctx)
- throws RestApiException, OrmException, IOException {
+ public boolean updateChange(ChangeContext ctx) throws RestApiException,
+ OrmException, IOException, NoSuchChangeException {
patchSet = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
if (patchSet == null) {
return false; // Nothing to do.
@@ -148,7 +149,8 @@
}
private void deleteOrUpdateDraftChange(ChangeContext ctx)
- throws OrmException, RestApiException {
+ throws OrmException, RestApiException, IOException,
+ NoSuchChangeException {
Change c = ctx.getChange();
if (deletedOnlyPatchSet()) {
deleteChangeOp = deleteChangeOpProvider.get();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/SchemaUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/SchemaUtil.java
index 53ff0e3..5ead80f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/SchemaUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/SchemaUtil.java
@@ -73,6 +73,16 @@
}
@SafeVarargs
+ public static <V> Schema<V> schema(Schema<V> schema,
+ FieldDef<V, ?>... moreFields) {
+ return new Schema<>(
+ new ImmutableList.Builder<FieldDef<V, ?>>()
+ .addAll(schema.getFields().values())
+ .addAll(ImmutableList.copyOf(moreFields))
+ .build());
+ }
+
+ @SafeVarargs
public static <V> Schema<V> schema(FieldDef<V, ?>... fields) {
return schema(ImmutableList.copyOf(fields));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
index fdeb654..67694ac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -576,6 +576,23 @@
}
};
+ /** Users who have starred this change. */
+ public static final FieldDef<ChangeData, Iterable<Integer>> STARREDBY =
+ new FieldDef.Repeatable<ChangeData, Integer>(
+ ChangeQueryBuilder.FIELD_STARREDBY, FieldType.INTEGER, true) {
+ @Override
+ public Iterable<Integer> get(ChangeData input, FillArgs args)
+ throws OrmException {
+ return Iterables.transform(input.starredBy(),
+ new Function<Account.Id, Integer>() {
+ @Override
+ public Integer apply(Account.Id accountId) {
+ return accountId.get();
+ }
+ });
+ }
+ };
+
/** Opaque group identifiers for this change's patch sets. */
public static final FieldDef<ChangeData, Iterable<String>> GROUP =
new FieldDef.Repeatable<ChangeData, String>(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index 48df562..0ca9922 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -186,14 +186,26 @@
/**
* Synchronously index a change.
*
- * @param change change to index.
* @param db review database.
+ * @param change change to index.
*/
public void index(ReviewDb db, Change change) throws IOException {
index(changeDataFactory.create(db, change));
}
/**
+ * Synchronously index a change.
+ *
+ * @param db review database.
+ * @param project the project to which the change belongs.
+ * @param changeId ID of the change to index.
+ */
+ public void index(ReviewDb db, Project.NameKey project, Change.Id changeId)
+ throws IOException {
+ index(changeDataFactory.create(db, project, changeId));
+ }
+
+ /**
* Start deleting a change.
*
* @param id change to delete.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index b39a0b7..9363f62 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -96,8 +96,11 @@
ChangeField.COMMITTER,
ChangeField.DRAFTBY);
+ @Deprecated
static final Schema<ChangeData> V27 = schema(V26.getFields().values());
+ static final Schema<ChangeData> V28 = schema(V27, ChangeField.STARREDBY);
+
public static final ChangeSchemaDefinitions INSTANCE =
new ChangeSchemaDefinitions();
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 b244ec4..4e1891a 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
@@ -33,6 +33,7 @@
import com.google.gerrit.server.patch.PatchListEntry;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
+import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -305,10 +306,10 @@
// BCC anyone who has starred this change.
//
for (Account.Id accountId : args.starredChangesUtil
- .byChange(change.getId())) {
+ .byChangeFromIndex(change.getId())) {
super.add(RecipientType.BCC, accountId);
}
- } catch (OrmException err) {
+ } catch (OrmException | NoSuchChangeException err) {
// Just don't BCC everyone. Better to send a partial message to those
// we already have queued up then to fail deliver entirely to people
// who have a lower interest in the change.
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 40fadb4..452d51f 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
@@ -44,6 +44,7 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.change.MergeabilityCache;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
@@ -296,7 +297,7 @@
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, project, id);
+ null, null, null, null, null, null, null, null, project, id);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd;
}
@@ -315,6 +316,7 @@
private final PatchListCache patchListCache;
private final NotesMigration notesMigration;
private final MergeabilityCache mergeabilityCache;
+ private final StarredChangesUtil starredChangesUtil;
private final Change.Id legacyId;
private DataSource<ChangeData> returnedBySource;
private Project.NameKey project;
@@ -338,6 +340,7 @@
private Set<Account.Id> editsByUser;
private Set<Account.Id> reviewedBy;
private Set<Account.Id> draftsByUser;
+ private Set<Account.Id> starredByUser;
private PersonIdent author;
private PersonIdent committer;
@@ -356,6 +359,7 @@
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
+ StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted Project.NameKey project,
@Assisted Change.Id id) {
@@ -373,6 +377,7 @@
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
+ this.starredChangesUtil = starredChangesUtil;
this.project = project;
this.legacyId = id;
}
@@ -392,6 +397,7 @@
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
+ StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted Change c) {
this.db = db;
@@ -408,6 +414,7 @@
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
+ this.starredChangesUtil = starredChangesUtil;
legacyId = c.getId();
change = c;
project = c.getProject();
@@ -428,6 +435,7 @@
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
+ StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted ChangeNotes cn) {
this.db = db;
@@ -444,6 +452,7 @@
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
+ this.starredChangesUtil = starredChangesUtil;
legacyId = cn.getChangeId();
change = cn.getChange();
project = cn.getProjectName();
@@ -465,6 +474,7 @@
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
+ StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted ChangeControl c) {
this.db = db;
@@ -481,6 +491,7 @@
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
+ this.starredChangesUtil = starredChangesUtil;
legacyId = c.getId();
change = c.getChange();
changeControl = c;
@@ -503,6 +514,7 @@
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
+ StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted Change.Id id) {
checkState(!notesMigration.readChanges(),
@@ -521,6 +533,7 @@
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
+ this.starredChangesUtil = starredChangesUtil;
this.legacyId = id;
this.project = null;
}
@@ -1005,6 +1018,17 @@
this.reviewedBy = reviewedBy;
}
+ public Set<Account.Id> starredBy() throws OrmException {
+ if (starredByUser == null) {
+ starredByUser = starredChangesUtil.byChange(legacyId);
+ }
+ return starredByUser;
+ }
+
+ public void setStarredBy(Set<Account.Id> starredByUser) {
+ this.starredByUser = starredByUser;
+ }
+
@AutoValue
abstract static class ReviewedByEvent {
private static ReviewedByEvent create(ChangeMessage msg) {
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 cb27de4..2679f29 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
@@ -419,7 +419,7 @@
@Operator
public Predicate<ChangeData> has(String value) throws QueryParseException {
if ("star".equalsIgnoreCase(value)) {
- return new IsStarredByPredicate(args);
+ return starredby(self());
}
if ("draft".equalsIgnoreCase(value)) {
@@ -435,7 +435,7 @@
@Operator
public Predicate<ChangeData> is(String value) throws QueryParseException {
if ("starred".equalsIgnoreCase(value)) {
- return new IsStarredByPredicate(args);
+ return starredby(self());
}
if ("watched".equalsIgnoreCase(value)) {
@@ -648,17 +648,25 @@
@Operator
public Predicate<ChangeData> starredby(String who)
throws QueryParseException, OrmException {
- if ("self".equals(who)) {
- return new IsStarredByPredicate(args);
- }
- Set<Account.Id> m = parseAccount(who);
- List<IsStarredByPredicate> p = Lists.newArrayListWithCapacity(m.size());
- for (Account.Id id : m) {
- p.add(new IsStarredByPredicate(args.asUser(id)));
+ return starredby(parseAccount(who));
+ }
+
+ private Predicate<ChangeData> starredby(Set<Account.Id> who)
+ throws QueryParseException {
+ List<Predicate<ChangeData>> p = Lists.newArrayListWithCapacity(who.size());
+ for (Account.Id id : who) {
+ p.add(starredby(id));
}
return Predicate.or(p);
}
+ private Predicate<ChangeData> starredby(Account.Id who)
+ throws QueryParseException {
+ return args.getSchema().hasField(ChangeField.STARREDBY)
+ ? new IsStarredByPredicate(who)
+ : new IsStarredByLegacyPredicate(args.asUser(who));
+ }
+
@Operator
public Predicate<ChangeData> watchedby(String who)
throws QueryParseException, OrmException {
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 bb72a1b..37cdfaf 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
@@ -28,6 +28,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
@@ -297,6 +298,10 @@
return query(and(project(project), or(groupPredicates)));
}
+ public List<ChangeData> byIsStarred(Account.Id id) throws OrmException {
+ return query(new IsStarredByPredicate(id));
+ }
+
public List<ChangeData> query(Predicate<ChangeData> p) throws OrmException {
try {
return qp.queryChanges(p).changes();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java
new file mode 100644
index 0000000..718c3f6
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java
@@ -0,0 +1,71 @@
+// Copyright (C) 2010 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.query.change;
+
+import com.google.common.collect.Lists;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.query.OrPredicate;
+import com.google.gerrit.server.query.Predicate;
+import com.google.gerrit.server.query.QueryParseException;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
+
+import java.util.List;
+import java.util.Set;
+
+@Deprecated
+class IsStarredByLegacyPredicate extends OrPredicate<ChangeData> {
+ private static String describe(CurrentUser user) {
+ if (user.isIdentifiedUser()) {
+ return user.getAccountId().toString();
+ }
+ return user.toString();
+ }
+
+ private static List<Predicate<ChangeData>> predicates(Set<Change.Id> ids) {
+ List<Predicate<ChangeData>> r = Lists.newArrayListWithCapacity(ids.size());
+ for (Change.Id id : ids) {
+ r.add(new LegacyChangeIdPredicate(id));
+ }
+ return r;
+ }
+
+ private final CurrentUser user;
+
+ IsStarredByLegacyPredicate(Arguments args) throws QueryParseException {
+ super(predicates(args.getIdentifiedUser().getStarredChanges()));
+ this.user = args.getIdentifiedUser();
+ }
+
+ @Override
+ public boolean match(final ChangeData object) {
+ return user.getStarredChanges().contains(object.getId());
+ }
+
+ @Override
+ public int getCost() {
+ return 0;
+ }
+
+ @Override
+ public String toString() {
+ String val = describe(user);
+ if (val.indexOf(' ') < 0) {
+ return ChangeQueryBuilder.FIELD_STARREDBY + ":" + val;
+ } else {
+ return ChangeQueryBuilder.FIELD_STARREDBY + ":\"" + val + "\"";
+ }
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java
index 5ff859a..50c54f6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java
@@ -14,57 +14,31 @@
package com.google.gerrit.server.query.change;
-import com.google.common.collect.Lists;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.query.OrPredicate;
-import com.google.gerrit.server.query.Predicate;
-import com.google.gerrit.server.query.QueryParseException;
-import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.index.IndexPredicate;
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gwtorm.server.OrmException;
-import java.util.List;
-import java.util.Set;
+class IsStarredByPredicate extends IndexPredicate<ChangeData> {
+ private final Account.Id accountId;
-class IsStarredByPredicate extends OrPredicate<ChangeData> {
- private static String describe(CurrentUser user) {
- if (user.isIdentifiedUser()) {
- return user.getAccountId().toString();
- }
- return user.toString();
- }
-
- private static List<Predicate<ChangeData>> predicates(Set<Change.Id> ids) {
- List<Predicate<ChangeData>> r = Lists.newArrayListWithCapacity(ids.size());
- for (Change.Id id : ids) {
- r.add(new LegacyChangeIdPredicate(id));
- }
- return r;
- }
-
- private final CurrentUser user;
-
- IsStarredByPredicate(Arguments args) throws QueryParseException {
- super(predicates(args.getIdentifiedUser().getStarredChanges()));
- this.user = args.getIdentifiedUser();
+ IsStarredByPredicate(Account.Id accountId) {
+ super(ChangeField.STARREDBY, accountId.toString());
+ this.accountId = accountId;
}
@Override
- public boolean match(final ChangeData object) {
- return user.getStarredChanges().contains(object.getId());
+ public boolean match(ChangeData cd) throws OrmException {
+ return cd.starredBy().contains(accountId);
}
@Override
public int getCost() {
- return 0;
+ return 1;
}
@Override
public String toString() {
- String val = describe(user);
- if (val.indexOf(' ') < 0) {
- return ChangeQueryBuilder.FIELD_STARREDBY + ":" + val;
- } else {
- return ChangeQueryBuilder.FIELD_STARREDBY + ":\"" + val + "\"";
- }
+ return ChangeQueryBuilder.FIELD_STARREDBY + ":" + accountId;
}
}
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 129c155..6e2f9c9 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
@@ -1173,6 +1173,23 @@
}
@Test
+ public void byStarredBy() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ Change change1 = insert(repo, newChange(repo));
+ Change change2 = insert(repo, newChange(repo));
+ insert(repo, newChange(repo));
+
+ gApi.accounts().self().starChange(change1.getId().toString());
+ gApi.accounts().self().starChange(change2.getId().toString());
+
+ int user2 = accountManager.authenticate(AuthRequest.forUser("anotheruser"))
+ .getAccountId().get();
+
+ assertQuery("starredby:self", change2, change1);
+ assertQuery("starredby:" + user2);
+ }
+
+ @Test
public void byFrom() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change1 = insert(repo, newChange(repo));
