Merge "Enable aliases for change query has operands"

Documentation/config-gerrit.txt

@@ -2377,6 +2377,40 @@ groups either. This means there is no danger of ambiguous group names
 when this parameter is removed and the system group uses the default
 name again.
 
+[[has-operand-alias]]
+=== Section has operand alias
+
+'has' operand aliasing allows global aliases to be defined for query
+'has' operands. Currently only change queries are supported. The alias
+name is the git config key name, and the 'has' operand being aliased
+is the git config value.
+
+For example:
+
+----
+[has-operand-alias "change"]
+  oldtopic = topic
+----
+
+This section is particularly useful to alias 'has' operands (which may
+be long and clunky as they include a plugin name in them) to shorter
+operands without the plugin name. Admins should take care to choose
+shorter operands that are unique and unlikely to conflict in the future.
+
+Aliases are resolved dynamically at invocation time to the currently
+loaded version of plugins. If a referenced plugin is not loaded, or
+does not define the command, "unsupported operand" is returned to the
+user.
+
+Aliases will override existing 'has' operands. In case of multiple
+aliases with same name, the last one defined will be used.
+
+When the target of an alias does not exist, the 'has' operand with the
+name of the alias will be used (if present). This enables an admin to
+configure the system to override a core 'has' operand with an operand
+provided by a plugin when present and otherwise fall back to the 'has'
+operand provided by core.
+
 [[http]]
 === Section http
 

java/com/google/gerrit/server/config/HasOperandAliasConfig.java

@@ -0,0 +1,46 @@
+// Copyright (C) 2020 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.config;
+
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.HashMap;
+import java.util.Map;
+import org.eclipse.jgit.lib.Config;
+
+@Singleton
+public class HasOperandAliasConfig {
+  private static final String SECTION = "has-operand-alias";
+  private static final String SUBSECTION_CHANGE = "change";
+  private final Config cfg;
+  private final Map<String, String> changeQueryHasOperandAliases;
+
+  @Inject
+  HasOperandAliasConfig(@GerritServerConfig Config cfg) {
+    this.cfg = cfg;
+    changeQueryHasOperandAliases = new HashMap<>();
+    loadChangeHasOperandAliases();
+  }
+
+  public Map<String, String> getChangeQueryHasOperandAliases() {
+    return changeQueryHasOperandAliases;
+  }
+
+  private void loadChangeHasOperandAliases() {
+    for (String name : cfg.getNames(SECTION, SUBSECTION_CHANGE)) {
+      changeQueryHasOperandAliases.put(name, cfg.getString(SECTION, SUBSECTION_CHANGE, name));
+    }
+  }
+}

java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -67,6 +67,7 @@ import com.google.gerrit.server.change.MergeabilityComputationBehavior;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.HasOperandAliasConfig;
 import com.google.gerrit.server.config.OperatorAliasConfig;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.index.change.ChangeField;
@@ -232,6 +233,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
     final Provider<AnonymousUser> anonymousUserProvider;
     final OperatorAliasConfig operatorAliasConfig;
     final boolean indexMergeable;
+    final HasOperandAliasConfig hasOperandAliasConfig;
 
     private final Provider<CurrentUser> self;
 
@@ -265,7 +267,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
         GroupMembers groupMembers,
         Provider<AnonymousUser> anonymousUserProvider,
         OperatorAliasConfig operatorAliasConfig,
-        @GerritServerConfig Config gerritConfig) {
+        @GerritServerConfig Config gerritConfig,
+        HasOperandAliasConfig hasOperandAliasConfig) {
       this(
           queryProvider,
           rewriter,
@@ -294,7 +297,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
           groupMembers,
           anonymousUserProvider,
           operatorAliasConfig,
-          MergeabilityComputationBehavior.fromConfig(gerritConfig).includeInIndex());
+          MergeabilityComputationBehavior.fromConfig(gerritConfig).includeInIndex(),
+          hasOperandAliasConfig);
     }
 
     private Arguments(
@@ -325,7 +329,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
         GroupMembers groupMembers,
         Provider<AnonymousUser> anonymousUserProvider,
         OperatorAliasConfig operatorAliasConfig,
-        boolean indexMergeable) {
+        boolean indexMergeable,
+        HasOperandAliasConfig hasOperandAliasConfig) {
       this.queryProvider = queryProvider;
       this.rewriter = rewriter;
       this.opFactories = opFactories;
@@ -354,6 +359,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
       this.anonymousUserProvider = anonymousUserProvider;
       this.operatorAliasConfig = operatorAliasConfig;
       this.indexMergeable = indexMergeable;
+      this.hasOperandAliasConfig = hasOperandAliasConfig;
     }
 
     Arguments asUser(CurrentUser otherUser) {
@@ -385,7 +391,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
           groupMembers,
           anonymousUserProvider,
           operatorAliasConfig,
-          indexMergeable);
+          indexMergeable,
+          hasOperandAliasConfig);
     }
 
     Arguments asUser(Account.Id otherId) {
@@ -426,6 +433,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
   }
 
   private final Arguments args;
+  protected Map<String, String> hasOperandAliases = Collections.emptyMap();
 
   @Inject
   ChangeQueryBuilder(Arguments args) {
@@ -441,6 +449,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
 
   private void setupAliases() {
     setOperatorAliases(args.operatorAliasConfig.getChangeQueryOperatorAliases());
+    hasOperandAliases = args.hasOperandAliasConfig.getChangeQueryHasOperandAliases();
   }
 
   public Arguments getArgs() {
@@ -518,6 +527,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
 
   @Operator
   public Predicate<ChangeData> has(String value) throws QueryParseException {
+    value = hasOperandAliases.getOrDefault(value, value);
     if ("star".equalsIgnoreCase(value)) {
       return starredby(self());
     }

javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java

@@ -22,6 +22,7 @@ import static java.util.stream.Collectors.toList;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.UseClockStep;
@@ -30,7 +31,9 @@ import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
 import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Patch;
 import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -247,6 +250,39 @@ public class QueryChangeIT extends AbstractDaemonTest {
     assertThat(e).hasMessageThat().isEqualTo("invalid regular expression: [A");
   }
 
+  @Test
+  @SuppressWarnings("unchecked")
+  @GerritConfig(name = "has-operand-alias.change.unaddressedaliastest", value = "unresolved")
+  public void hasOperandAliasQuery() throws Exception {
+    String cId1 = createChange().getChangeId();
+    String cId2 = createChange().getChangeId();
+    int numericId1 = gApi.changes().id(cId1).get()._number;
+    int numericId2 = gApi.changes().id(cId2).get()._number;
+
+    ReviewInput input = new ReviewInput();
+    ReviewInput.CommentInput comment = new ReviewInput.CommentInput();
+    comment.line = 1;
+    comment.message = "comment";
+    comment.unresolved = true;
+    input.comments = ImmutableMap.of(Patch.COMMIT_MSG, ImmutableList.of(comment));
+    gApi.changes().id(cId2).current().review(input);
+
+    QueryChanges queryChanges = queryChangesProvider.get();
+    queryChanges.addQuery("is:open repo:" + project.get());
+    queryChanges.addQuery("has:unaddressedaliastest repo:" + project.get());
+
+    List<List<ChangeInfo>> result =
+        (List<List<ChangeInfo>>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+    assertThat(result).hasSize(2);
+    assertThat(result.get(0)).hasSize(2);
+    assertThat(result.get(1)).hasSize(1);
+
+    List<Integer> firstResultIds =
+        ImmutableList.of(result.get(0).get(0)._number, result.get(0).get(1)._number);
+    assertThat(firstResultIds).containsExactly(numericId1, numericId2);
+    assertThat(result.get(1).get(0)._number).isEqualTo(numericId2);
+  }
+
   private static void assertNoChangeHasMoreChangesSet(List<ChangeInfo> results) {
     for (ChangeInfo info : results) {
       assertThat(info._moreChanges).isNull();

javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java

@@ -54,7 +54,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
             null,
             null,
             null,
-            new Config()));
+            new Config(),
+            null));
   }
 
   @Operator