opendev/gerrit
Code Issues Proposed changes

Store users that starred a change in change index

Browse Source 
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>
This commit is contained in:
Edwin Kempin
2016-04-13 14:33:29 +02:00
parent dfe73dc408
commit 8be51b8a57
18 changed files with 280 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@@ -114,6 +114,7 @@ public class LuceneChangeIndex implements ChangeIndex {
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 @@ public class LuceneChangeIndex implements ChangeIndex {
if (fields.contains(REVIEWEDBY_FIELD)) {
decodeReviewedBy(doc, cd);
}
if (fields.contains(STARREDBY_FIELD)) {
decodeStarredBy(doc, cd);
}
return cd;
}
@@ -466,6 +470,16 @@ public class LuceneChangeIndex implements ChangeIndex {
}
}
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);

4
gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
View File

@@ -338,7 +338,7 @@ public class IdentifiedUser extends CurrentUser {
FluentIterable.from(
starredQuery != null
? starredQuery
: starredChangesUtil.query(accountId))
: starredChangesUtil.queryFromIndex(accountId))
.toSet();
} finally {
starredQuery = null;
@@ -356,7 +356,7 @@ public class IdentifiedUser extends CurrentUser {
public void asyncStarredChanges() {
if (starredChanges == null && starredChangesUtil != null) {
starredQuery = starredChangesUtil.query(accountId);
starredQuery = starredChangesUtil.queryFromIndex(accountId);
}
}

74
gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
View File

@@ -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 @@ public class StarredChangesUtil {
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 @@ public class StarredChangesUtil {
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 class StarredChangesUtil {
}
}
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 @@ public class StarredChangesUtil {
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 class StarredChangesUtil {
}
}
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 @@ public class StarredChangesUtil {
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 @@ public class StarredChangesUtil {
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 class StarredChangesUtil {
public Account.Id apply(StarredChange in) {
return in.getAccountId();
}
});
}).toSet();
}
return FluentIterable
.from(getRefNames(RefNames.refsStarredChangesPrefix(changeId)))
@@ -219,29 +238,40 @@ public class StarredChangesUtil {
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) {

11
gerrit-server/src/main/java/com/google/gerrit/server/account/StarredChanges.java
View File

@@ -42,6 +42,8 @@ import com.google.inject.Singleton;
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 @@ public class StarredChanges implements
@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 @@ public class StarredChanges implements
@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();
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
View File

@@ -193,7 +193,7 @@ public class AccountApiImpl implements AccountApi {
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 @@ public class AccountApiImpl implements AccountApi {
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);
}
}

7
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java
View File

@@ -31,6 +31,7 @@ import com.google.gerrit.server.git.BatchUpdate;
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 @@ class DeleteDraftChangeOp extends BatchUpdate.Op {
}
@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 @@ class DeleteDraftChangeOp extends BatchUpdate.Op {
// 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;

8
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
View File

@@ -35,6 +35,7 @@ import com.google.gerrit.server.git.BatchUpdate.RepoContext;
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 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
}
@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 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
}
private void deleteOrUpdateDraftChange(ChangeContext ctx)
throws OrmException, RestApiException {
throws OrmException, RestApiException, IOException,
NoSuchChangeException {
Change c = ctx.getChange();
if (deletedOnlyPatchSet()) {
deleteChangeOp = deleteChangeOpProvider.get();

10
gerrit-server/src/main/java/com/google/gerrit/server/index/SchemaUtil.java
View File

@@ -72,6 +72,16 @@ public class SchemaUtil {
return new Schema<>(ImmutableList.copyOf(fields));
}
@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));

17
gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
View File

@@ -576,6 +576,23 @@ public class ChangeField {
}
};
/** 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>(

14
gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexer.java
View File

@@ -186,13 +186,25 @@ public class ChangeIndexer {
/**
* 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.
*

3
gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
View File

@@ -96,8 +96,11 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
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();

5
gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
View File

@@ -33,6 +33,7 @@ import com.google.gerrit.server.patch.PatchList;
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 @@ public abstract class ChangeEmail extends NotificationEmail {
// 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.

26
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
View File

@@ -44,6 +44,7 @@ import com.google.gerrit.server.CurrentUser;
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 class ChangeData {
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 @@ public class ChangeData {
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 @@ public class ChangeData {
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 @@ public class ChangeData {
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted Project.NameKey project,
@Assisted Change.Id id) {
@@ -373,6 +377,7 @@ public class ChangeData {
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
this.starredChangesUtil = starredChangesUtil;
this.project = project;
this.legacyId = id;
}
@@ -392,6 +397,7 @@ public class ChangeData {
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted Change c) {
this.db = db;
@@ -408,6 +414,7 @@ public class ChangeData {
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
this.starredChangesUtil = starredChangesUtil;
legacyId = c.getId();
change = c;
project = c.getProject();
@@ -428,6 +435,7 @@ public class ChangeData {
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted ChangeNotes cn) {
this.db = db;
@@ -444,6 +452,7 @@ public class ChangeData {
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 @@ public class ChangeData {
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted ChangeControl c) {
this.db = db;
@@ -481,6 +491,7 @@ public class ChangeData {
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
this.starredChangesUtil = starredChangesUtil;
legacyId = c.getId();
change = c.getChange();
changeControl = c;
@@ -503,6 +514,7 @@ public class ChangeData {
PatchListCache patchListCache,
NotesMigration notesMigration,
MergeabilityCache mergeabilityCache,
StarredChangesUtil starredChangesUtil,
@Assisted ReviewDb db,
@Assisted Change.Id id) {
checkState(!notesMigration.readChanges(),
@@ -521,6 +533,7 @@ public class ChangeData {
this.patchListCache = patchListCache;
this.notesMigration = notesMigration;
this.mergeabilityCache = mergeabilityCache;
this.starredChangesUtil = starredChangesUtil;
this.legacyId = id;
this.project = null;
}
@@ -1005,6 +1018,17 @@ public class ChangeData {
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) {

26
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -419,7 +419,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@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 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@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 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@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 {

5
gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
View File

@@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableSet;
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 @@ public class InternalChangeQuery {
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();

71
gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java Normal file
View File

@@ -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 + "\"";
}
}
}

52
gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java
View File

@@ -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;
}
}

17
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -1172,6 +1172,23 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery("draftby:" + user2);
}
@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");