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 fb0edd2db8..286565ba0b 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 @@ -25,12 +25,12 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.SubmitRecord; +import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.ChangeField; import com.google.gerrit.server.index.ChangeField.ChangeProtoField; -import com.google.gerrit.server.index.ChangeField.SubmitLabelProtoField; +import com.google.gerrit.server.index.ChangeField.PatchSetApprovalProtoField; import com.google.gerrit.server.index.ChangeIndex; import com.google.gerrit.server.index.FieldDef; import com.google.gerrit.server.index.FieldDef.FillArgs; @@ -104,8 +104,7 @@ public class LuceneChangeIndex implements ChangeIndex { public static final String CHANGES_CLOSED = "closed"; private static final String ID_FIELD = ChangeField.LEGACY_ID.getName(); private static final String CHANGE_FIELD = ChangeField.CHANGE.getName(); - private static final String SUBMIT_LABEL_FIELD = - ChangeField.SUBMIT_RECORD_LABEL.getName(); + private static final String APPROVAL_FIELD = ChangeField.APPROVAL.getName(); static interface Factory { LuceneChangeIndex create(Schema schema, String base); @@ -265,7 +264,7 @@ public class LuceneChangeIndex implements ChangeIndex { private static class QuerySource implements ChangeDataSource { private static final ImmutableSet FIELDS = - ImmutableSet.of(ID_FIELD, CHANGE_FIELD, SUBMIT_LABEL_FIELD); + ImmutableSet.of(ID_FIELD, CHANGE_FIELD, APPROVAL_FIELD); private final List indexes; private final Query query; @@ -359,15 +358,15 @@ public class LuceneChangeIndex implements ChangeIndex { cb.bytes, cb.offset, cb.length); ChangeData cd = new ChangeData(change); - BytesRef[] labelsBytes = doc.getBinaryValues(SUBMIT_LABEL_FIELD); - if (labelsBytes != null) { - List labels = - Lists.newArrayListWithCapacity(labelsBytes.length); - for (BytesRef lb : labelsBytes) { - labels.add(SubmitLabelProtoField.CODEC.decode( - lb.bytes, lb.offset, lb.length)); + BytesRef[] approvalsBytes = doc.getBinaryValues(APPROVAL_FIELD); + if (approvalsBytes != null) { + List approvals = + Lists.newArrayListWithCapacity(approvalsBytes.length); + for (BytesRef ab : approvalsBytes) { + approvals.add(PatchSetApprovalProtoField.CODEC.decode( + ab.bytes, ab.offset, ab.length)); } - cd.setSubmitRecordLabels(labels); + cd.setCurrentApprovals(approvals); } return cd; } diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java index c28cc2e764..f740fba97f 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java @@ -15,13 +15,9 @@ package com.google.gerrit.pgm; import static com.google.gerrit.server.schema.DataSourceProvider.Context.MULTI_USER; -import static com.google.inject.Scopes.SINGLETON; import com.google.common.collect.Lists; import com.google.common.collect.Sets; -import com.google.gerrit.common.ChangeHooks; -import com.google.gerrit.common.DisabledChangeHooks; -import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.lifecycle.LifecycleManager; @@ -31,28 +27,8 @@ import com.google.gerrit.pgm.util.SiteProgram; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.rules.PrologModule; -import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.InternalUser; -import com.google.gerrit.server.PluginUser; -import com.google.gerrit.server.account.AccountByEmailCacheImpl; -import com.google.gerrit.server.account.AccountCacheImpl; -import com.google.gerrit.server.account.CapabilityControl; -import com.google.gerrit.server.account.EmailExpander; -import com.google.gerrit.server.account.GroupBackend; -import com.google.gerrit.server.account.GroupCacheImpl; -import com.google.gerrit.server.account.GroupControl; -import com.google.gerrit.server.account.GroupIncludeCacheImpl; -import com.google.gerrit.server.account.IncludingGroupMembership; -import com.google.gerrit.server.account.InternalGroupBackend; -import com.google.gerrit.server.account.UniversalGroupBackend; import com.google.gerrit.server.cache.CacheRemovalListener; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; -import com.google.gerrit.server.config.AuthModule; -import com.google.gerrit.server.config.CanonicalWebUrlModule; -import com.google.gerrit.server.config.CanonicalWebUrlProvider; -import com.google.gerrit.server.config.EmailExpanderProvider; -import com.google.gerrit.server.config.FactoryModule; import com.google.gerrit.server.index.ChangeBatchIndexer; import com.google.gerrit.server.index.ChangeIndex; import com.google.gerrit.server.index.ChangeSchemas; @@ -60,25 +36,14 @@ import com.google.gerrit.server.index.IndexCollection; import com.google.gerrit.server.index.IndexModule; import com.google.gerrit.server.index.IndexModule.IndexType; import com.google.gerrit.server.index.NoIndexModule; -import com.google.gerrit.server.mail.Address; -import com.google.gerrit.server.mail.EmailHeader; -import com.google.gerrit.server.mail.EmailSender; -import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier; import com.google.gerrit.server.patch.PatchListCacheImpl; -import com.google.gerrit.server.plugins.PluginModule; -import com.google.gerrit.server.project.AccessControlModule; -import com.google.gerrit.server.project.CommentLinkInfo; -import com.google.gerrit.server.project.CommentLinkProvider; -import com.google.gerrit.server.project.ProjectCacheImpl; -import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.project.ProjectState; -import com.google.gerrit.server.project.SectionSortCache; -import com.google.gerrit.server.ssh.NoSshKeyCache; import com.google.gerrit.solr.SolrIndexModule; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; +import com.google.inject.AbstractModule; import com.google.inject.Injector; import com.google.inject.Key; +import com.google.inject.Module; import com.google.inject.Provider; import com.google.inject.ProvisionException; import com.google.inject.TypeLiteral; @@ -88,10 +53,8 @@ import org.eclipse.jgit.lib.TextProgressMonitor; import org.eclipse.jgit.util.io.NullOutputStream; import org.kohsuke.args4j.Option; -import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -147,99 +110,33 @@ public class Reindex extends SiteProgram { } private Injector createSysInjector() { - return dbInjector.createChildInjector(new GlobalModule()); - } - - /** - * Subset of GerritGlobalModule including just bindings needed for indexing. - * - * Does not include thread local request scoping behavior of ReviewDbs; - * uses a custom {@link ReviewDbModule} instead. - */ - private class GlobalModule extends FactoryModule { - @SuppressWarnings("rawtypes") - @Override - protected void configure() { - install(PatchListCacheImpl.module()); - install(dbInjector.getInstance(AuthModule.class)); - install(AccountByEmailCacheImpl.module()); - install(AccountCacheImpl.module()); - install(new ReviewDbModule()); - install(ProjectCacheImpl.module()); - install(new PluginModule()); - install(new PrologModule()); - install(new DefaultCacheFactory.Module()); - install(NoSshKeyCache.module()); - install(new SignedTokenEmailTokenVerifier.Module()); - install(GroupCacheImpl.module()); - install(new AccessControlModule()); - install(SectionSortCache.module()); - install(new CanonicalWebUrlModule() { - @Override - protected Class> provider() { - return CanonicalWebUrlProvider.class; - } - }); - - switch (IndexModule.getIndexType(dbInjector)) { - case LUCENE: - install(new LuceneIndexModule(version, threads, outputBase)); - break; - case SOLR: - install(new SolrIndexModule(false, threads, outputBase)); - break; - default: - install(new NoIndexModule()); - } - - // No need to support live plugin removal or other live updates of - // caches behind our back, so don't worry about cache removal. - bind(new TypeLiteral>() {}) - .toInstance(DynamicSet. emptySet()); - - // Assume the current user is always the Gerrit internal user; no - // indexed data should depend on what user is doing the indexing. - bind(CurrentUser.class).to(InternalUser.class); - - bind(ChangeHooks.class).to(DisabledChangeHooks.class); - bind(EmailExpander.class).toProvider(EmailExpanderProvider.class).in( - SINGLETON); - bind(new TypeLiteral>() {}) - .toProvider(CommentLinkProvider.class).in(SINGLETON); - bind(ProjectControl.GenericFactory.class); - - factory(CapabilityControl.Factory.class); - factory(IncludingGroupMembership.Factory.class); - factory(InternalUser.Factory.class); - factory(PluginUser.Factory.class); - factory(ProjectState.Factory.class); - - bind(GroupBackend.class).to(UniversalGroupBackend.class).in(SINGLETON); - install(GroupIncludeCacheImpl.module()); - bind(GroupControl.Factory.class).in(SINGLETON); - bind(GroupControl.GenericFactory.class).in(SINGLETON); - bind(InternalGroupBackend.class).in(SINGLETON); - DynamicSet.setOf(binder(), GroupBackend.class); - DynamicSet.bind(binder(), GroupBackend.class).to(InternalGroupBackend.class); - - bind(EmailSender.class).toInstance(new EmailSender() { - @Override - public boolean isEnabled() { - return false; - } - - @Override - public boolean canEmail(String address) { - return false; - } - - @Override - public void send(Address from, Collection
rcpt, - Map headers, String body) - throws EmailException { - } - }); + List modules = Lists.newArrayList(); + modules.add(PatchListCacheImpl.module()); + AbstractModule changeIndexModule; + switch (IndexModule.getIndexType(dbInjector)) { + case LUCENE: + changeIndexModule = new LuceneIndexModule(version, threads, outputBase); + break; + case SOLR: + changeIndexModule = new SolrIndexModule(false, threads, outputBase); + break; + default: + changeIndexModule = new NoIndexModule(); } + modules.add(changeIndexModule); + modules.add(new ReviewDbModule()); + modules.add(new AbstractModule() { + @SuppressWarnings("rawtypes") + @Override + protected void configure() { + // Plugins are not loaded and we're just running through each change + // once, so don't worry about cache removal. + bind(new TypeLiteral>() {}) + .toInstance(DynamicSet. emptySet()); + install(new DefaultCacheFactory.Module()); + } + }); + return dbInjector.createChildInjector(modules); } private class ReviewDbModule extends LifecycleModule { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index d2debbbe76..8b86f13796 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -380,10 +380,6 @@ public class ChangeJson { if (!standard && !detailed) { return null; } - if (cd.getSubmitRecordLabels() != null && standard && !detailed) { - // Use saved labels to avoid recomputing on the fly. - return labelsFromSavedState(cd.getSubmitRecordLabels()); - } ChangeControl ctl = control(cd); if (ctl == null) { @@ -398,15 +394,6 @@ public class ChangeJson { } } - private Map labelsFromSavedState( - List savedLabels) { - Map labels = Maps.newLinkedHashMap(); - for (SubmitRecord.Label r : savedLabels) { - labels.put(r.label, newLabelFromSubmitRecord(r, true)); - } - return labels; - } - private Map labelsForOpenChange(ChangeData cd, LabelTypes labelTypes, boolean standard, boolean detailed) throws OrmException { @@ -447,34 +434,29 @@ public class ChangeJson { for (SubmitRecord.Label r : rec.labels) { LabelInfo p = labels.get(r.label); if (p == null || p._status.compareTo(r.status) < 0) { - labels.put(r.label, newLabelFromSubmitRecord(r, standard)); + LabelInfo n = new LabelInfo(); + n._status = r.status; + if (standard) { + switch (r.status) { + case OK: + n.approved = accountLoader.get(r.appliedBy); + break; + case REJECT: + n.rejected = accountLoader.get(r.appliedBy); + break; + default: + break; + } + } + + n.optional = n._status == SubmitRecord.Label.Status.MAY ? true : null; + labels.put(r.label, n); } } } return labels; } - private LabelInfo newLabelFromSubmitRecord(SubmitRecord.Label r, - boolean standard) { - LabelInfo n = new LabelInfo(); - n._status = r.status; - if (standard) { - switch (r.status) { - case OK: - n.approved = accountLoader.get(r.appliedBy); - break; - case REJECT: - n.rejected = accountLoader.get(r.appliedBy); - break; - default: - break; - } - } - - n.optional = n._status == SubmitRecord.Label.Status.MAY ? true : null; - return n; - } - private void setLabelScores(LabelType type, LabelInfo label, short score, Account.Id accountId) throws OrmException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/EmailExpanderProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/EmailExpanderProvider.java index d4af4a47f8..a55ef84de5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/EmailExpanderProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/EmailExpanderProvider.java @@ -21,7 +21,7 @@ import com.google.inject.Provider; import org.eclipse.jgit.lib.Config; -public class EmailExpanderProvider implements Provider { +class EmailExpanderProvider implements Provider { private final EmailExpander expander; @Inject diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java index 00e910fc48..20b61e7900 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java @@ -14,9 +14,7 @@ package com.google.gerrit.server.index; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; @@ -24,11 +22,8 @@ import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; -import com.google.gerrit.reviewdb.client.SubmitRecord; import com.google.gerrit.reviewdb.client.TrackingId; import com.google.gerrit.server.ChangeUtil; -import com.google.gerrit.server.project.ChangeControl; -import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeQueryBuilder; import com.google.gerrit.server.query.change.ChangeStatusPredicate; @@ -42,7 +37,6 @@ import java.io.IOException; import java.sql.Timestamp; import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.Set; /** @@ -265,63 +259,28 @@ public class ChangeField { /** Serialized change object, used for pre-populating results. */ public static final ChangeProtoField CHANGE = new ChangeProtoField(); - public static class SubmitLabelProtoField + public static class PatchSetApprovalProtoField extends FieldDef.Repeatable { - public static final ProtobufCodec CODEC = - CodecFactory.encoder(SubmitRecord.Label.class); + public static final ProtobufCodec CODEC = + CodecFactory.encoder(PatchSetApproval.class); - private SubmitLabelProtoField() { - super("_label", FieldType.STORED_ONLY, true); + private PatchSetApprovalProtoField() { + super("_approval", FieldType.STORED_ONLY, true); } @Override public Iterable get(ChangeData input, FillArgs args) throws OrmException { - // Flatten the highest-valued labels to mimic the results from ChangeJson - // with standard labels. - Map labels = Maps.newLinkedHashMap(); - for (SubmitRecord rec : getSubmitRecords(input, args)) { - if (rec.labels == null) { - continue; - } - for (SubmitRecord.Label r : rec.labels) { - SubmitRecord.Label p = labels.get(r.label); - if (p == null || p.status.compareTo(r.status) < 0) { - labels.put(r.label, r); - } - } - } - return toProtos(CODEC, labels.values()); - } - - private List getSubmitRecords(ChangeData input, - FillArgs args) throws OrmException { - ChangeControl ctl; - try { - // Use the ChangeControl for InternalUser. This will give bogus - // results for whether or not the change is submittable, but does - // not affect label calculation. - ctl = args.changeControlFor(input.change(args.db)); - } catch (NoSuchChangeException e) { - throw new OrmException(e); - } - if (ctl == null) { - return ImmutableList.of(); - } - PatchSet ps = input.currentPatchSet(args.db); - if (ps == null) { - return ImmutableList.of(); - } - return ctl.canSubmit(args.db.get(), ps, input, true, true, true); + return toProtos(CODEC, input.currentApprovals(args.db)); } } /** - * Serialized labels from the submit rule evaluator, used for pre-populating + * Serialized approvals for the current patch set, used for pre-populating * results. */ - public static final SubmitLabelProtoField SUBMIT_RECORD_LABEL = - new SubmitLabelProtoField(); + public static final PatchSetApprovalProtoField APPROVAL = + new PatchSetApprovalProtoField(); public static String formatLabel(String label, int value) { return formatLabel(label, value, null); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java index 087837e43a..1ed6c470a0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java @@ -69,7 +69,7 @@ public class ChangeSchemas { ChangeField.COMMIT_MESSAGE, ChangeField.COMMENT, ChangeField.CHANGE, - ChangeField.SUBMIT_RECORD_LABEL); + ChangeField.APPROVAL); private static Schema release(FieldDef... fields) { return new Schema(true, Arrays.asList(fields)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/FieldDef.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/FieldDef.java index e228777a1b..f40f534c47 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/FieldDef.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/FieldDef.java @@ -14,13 +14,9 @@ package com.google.gerrit.server.index; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.InternalUser; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.patch.PatchListCache; -import com.google.gerrit.server.project.ChangeControl; -import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -64,31 +60,14 @@ public abstract class FieldDef { final Provider db; final GitRepositoryManager repoManager; final PatchListCache patchListCache; - final ChangeControl.GenericFactory changeControlFactory; - - private final InternalUser internalUser; @Inject - FillArgs(Provider db, GitRepositoryManager repoManager, - PatchListCache patchListCache, - ChangeControl.GenericFactory changeControlFactory, - InternalUser internalUser) { + FillArgs(Provider db, + GitRepositoryManager repoManager, + PatchListCache patchListCache) { this.db = db; this.repoManager = repoManager; this.patchListCache = patchListCache; - this.changeControlFactory = changeControlFactory; - this.internalUser = internalUser; - } - - /** - * Create a change control. - * - * @param change change object. - * @return change control as seen by Gerrit's internal user; index fields - * may not depend on any per-user state. - */ - ChangeControl changeControlFor(Change change) throws NoSuchChangeException { - return changeControlFactory.controlFor(change, internalUser); } } 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 32971eca71..fbc568b7fd 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 @@ -148,7 +148,6 @@ public class ChangeData { private ChangeControl changeControl; private List messages; private List submitRecords; - private List submitRecordLabels; private boolean patchesLoaded; public ChangeData(final Change.Id id) { @@ -312,6 +311,10 @@ public class ChangeData { return currentApprovals; } + public void setCurrentApprovals(List approvals) { + currentApprovals = approvals; + } + public String commitMessage(GitRepositoryManager repoManager, Provider db) throws IOException, OrmException { if (commitMessage == null) { @@ -460,12 +463,4 @@ public class ChangeData { public List getSubmitRecords() { return submitRecords; } - - public void setSubmitRecordLabels(List labels) { - submitRecordLabels = labels; - } - - public List getSubmitRecordLabels() { - return submitRecordLabels; - } }