From 5f6222910e04f7c7255c7cd507b38d5bf1d97270 Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Thu, 12 Nov 2015 14:52:50 -0700 Subject: [PATCH] Allow plugins to define change search operators. Create a DynamicBuilder interface in the QueryBuilder class along with supporting methods and enhancements to support defining dynamic search operators that can be altered for each query build. Despite allowing for dynamic modifications, the QueryBuilder class was kept generic. No imports or dependencies on other Gerrit frameworks such as plugin logic were added to the QueryBuilder. Enhance the ChangeQueryBuilder to use a DynamicSet to define DynamicBuilders so that plugins can define search operators. Plugins can define DynamicBuilders and bind them to the DynamicSet on load. Plugin search operators are defined similarly to the way ChangeQueryBuilder operators are defined, by annotating methods in the DynamicBuilder with @Operator. The name of the annotated method determines the name of the search operator. Change-Id: I11daed482562b82cb4dce1d8abd846f06784eb3a --- Documentation/dev-plugins.txt | 42 +++++++++++++++++++ .../server/config/GerritGlobalModule.java | 3 ++ .../gerrit/server/query/QueryBuilder.java | 12 +++--- .../query/change/ChangeQueryBuilder.java | 23 +++++++++- .../gerrit/server/index/FakeQueryBuilder.java | 2 +- plugins/cookbook-plugin | 2 +- 6 files changed, 74 insertions(+), 10 deletions(-) diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt index b5aaf7eb65..e6bf2a4f77 100644 --- a/Documentation/dev-plugins.txt +++ b/Documentation/dev-plugins.txt @@ -586,6 +586,48 @@ $ ssh -p 29418 review.example.com sh ls $ ssh -p 29418 review.example.com sh ps ---- +[[search_operators]] +=== Search Operators === + +Plugins can define new search operators to extend change searching by +implementing the `ChangeQueryBuilder.ChangeOperatorFactory` interface +and registering it to an operator name in the plugin module's +`configure()` method. The search operator name is defined during +registration via the DynamicMap annotation mechanism. The plugin +name will get appended to the annotated name, with an underscore +in between, leading to the final operator name. An example +registration looks like this: + + bind(ChangeOperatorFactory.class) + .annotatedWith(Exports.named("sample")) + .to(SampleOperator.class); + +If this is registered in the `myplugin` plugin, then the resulting +operator will be named `sample_myplugin`. + +The search operator itself is implemented by ensuring that the +`create()` method of the class implementing the +`ChangeQueryBuilder.ChangeOperatorFactory` interface returns a +`Predicate`. Here is a sample operator factory +defintion which creates a `MyPredicate`: + +[source,java] +---- +@Singleton +public class SampleOperator + implements ChangeQueryBuilder.ChangeOperatorFactory { + public static class MyPredicate extends OperatorPredicate { + ... + } + + @Override + public Predicate create(ChangeQueryBuilder builder, String value) + throws QueryParseException { + return new MyPredicate(value); + } +} +---- + [[simple-configuration]] == Simple Configuration in `gerrit.config` diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 3f2621fa2d..6b93a68830 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -125,6 +125,7 @@ import com.google.gerrit.server.project.ProjectNode; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.SectionSortCache; import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.query.change.ChangeQueryBuilder; import com.google.gerrit.server.query.change.ConflictsCacheImpl; import com.google.gerrit.server.ssh.SshAddressesModule; import com.google.gerrit.server.tools.ToolsCatalog; @@ -299,6 +300,8 @@ public class GerritGlobalModule extends FactoryModule { factory(UploadValidators.Factory.class); DynamicSet.setOf(binder(), UploadValidationListener.class); + DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeOperatorFactory.class); + bind(AnonymousUser.class); factory(CommitValidators.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryBuilder.java index c8f99722a0..3a21ce4942 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryBuilder.java @@ -73,6 +73,11 @@ import java.util.Map; * @param type of object the predicates can evaluate in memory. */ public abstract class QueryBuilder { + /** Converts a value string passed to an operator into a {@link Predicate}. */ + public interface OperatorFactory> { + Predicate create(Q builder, String value) throws QueryParseException; + } + /** * Defines the operators known by a QueryBuilder. * @@ -162,7 +167,7 @@ public abstract class QueryBuilder { protected final Definition> builderDef; @SuppressWarnings("rawtypes") - private final Map opFactories; + protected final Map opFactories; @SuppressWarnings({"unchecked", "rawtypes"}) protected QueryBuilder(Definition> def) { @@ -323,11 +328,6 @@ public abstract class QueryBuilder { return new QueryParseException(msg, why); } - /** Converts a value string passed to an operator into a {@link Predicate}. */ - protected interface OperatorFactory> { - Predicate create(Q builder, String value) throws QueryParseException; - } - /** Denotes a method which is a query operator. */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) 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 2fdeea42d4..a025caa3f6 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 @@ -21,6 +21,7 @@ import com.google.common.base.Optional; import com.google.common.collect.Lists; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.common.errors.NotSignedInException; +import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Branch; @@ -55,6 +56,7 @@ import com.google.gerrit.server.project.ListChildProjects; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryBuilder; +import com.google.gerrit.server.query.QueryBuilder.OperatorFactory; import com.google.gerrit.server.query.QueryParseException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -70,6 +72,7 @@ import org.eclipse.jgit.lib.Repository; import java.io.IOException; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -80,6 +83,10 @@ import java.util.regex.Pattern; * Parses a query string meant to be applied to change objects. */ public class ChangeQueryBuilder extends QueryBuilder { + public interface ChangeOperatorFactory + extends OperatorFactory { + } + private static final Pattern PAT_LEGACY_ID = Pattern.compile("^[1-9][0-9]*$"); private static final Pattern PAT_CHANGE_ID = Pattern.compile("^[iI][0-9a-f]{4,}.*$"); @@ -145,6 +152,7 @@ public class ChangeQueryBuilder extends QueryBuilder { final Provider db; final Provider queryProvider; final IndexRewriter rewriter; + final DynamicMap opFactories; final IdentifiedUser.GenericFactory userFactory; final CapabilityControl.Factory capabilityControlFactory; final ChangeControl.GenericFactory changeControlGenericFactory; @@ -172,6 +180,7 @@ public class ChangeQueryBuilder extends QueryBuilder { public Arguments(Provider db, Provider queryProvider, IndexRewriter rewriter, + DynamicMap opFactories, IdentifiedUser.GenericFactory userFactory, Provider self, CapabilityControl.Factory capabilityControlFactory, @@ -192,7 +201,7 @@ public class ChangeQueryBuilder extends QueryBuilder { ConflictsCache conflictsCache, TrackingFooters trackingFooters, @GerritServerConfig Config cfg) { - this(db, queryProvider, rewriter, userFactory, self, + this(db, queryProvider, rewriter, opFactories, userFactory, self, capabilityControlFactory, changeControlGenericFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, allUsersName, patchListCache, repoManager, @@ -206,6 +215,7 @@ public class ChangeQueryBuilder extends QueryBuilder { Provider db, Provider queryProvider, IndexRewriter rewriter, + DynamicMap opFactories, IdentifiedUser.GenericFactory userFactory, Provider self, CapabilityControl.Factory capabilityControlFactory, @@ -229,6 +239,7 @@ public class ChangeQueryBuilder extends QueryBuilder { this.db = db; this.queryProvider = queryProvider; this.rewriter = rewriter; + this.opFactories = opFactories; this.userFactory = userFactory; this.self = self; this.capabilityControlFactory = capabilityControlFactory; @@ -252,7 +263,7 @@ public class ChangeQueryBuilder extends QueryBuilder { } Arguments asUser(CurrentUser otherUser) { - return new Arguments(db, queryProvider, rewriter, userFactory, + return new Arguments(db, queryProvider, rewriter, opFactories, userFactory, Providers.of(otherUser), capabilityControlFactory, changeControlGenericFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, @@ -304,6 +315,7 @@ public class ChangeQueryBuilder extends QueryBuilder { ChangeQueryBuilder(Arguments args) { super(mydef); this.args = args; + setupDynamicOperators(); } @VisibleForTesting @@ -314,6 +326,13 @@ public class ChangeQueryBuilder extends QueryBuilder { this.args = args; } + private void setupDynamicOperators() { + for (DynamicMap.Entry e : args.opFactories) { + String name = e.getExportName() + "_" + e.getPluginName(); + opFactories.put(name, e.getProvider().get()); + } + } + public ChangeQueryBuilder asUser(CurrentUser user) { return new ChangeQueryBuilder(builderDef, args.asUser(user)); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java index 0c8625d965..99d68e83a3 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java @@ -27,7 +27,7 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { FakeQueryBuilder.class), new ChangeQueryBuilder.Arguments(null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, - null, indexes, null, null, null, null)); + null, null, indexes, null, null, null, null)); } @Operator diff --git a/plugins/cookbook-plugin b/plugins/cookbook-plugin index 2efae2e9ad..1b41f7a615 160000 --- a/plugins/cookbook-plugin +++ b/plugins/cookbook-plugin @@ -1 +1 @@ -Subproject commit 2efae2e9ade87f5c4fc303db64e0ea643d17367d +Subproject commit 1b41f7a615ea4eca37878f9945ec820f95885755