summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPrudhvi Akhil Alahari <prudhvi@codeaurora.org>2021-05-03 10:20:10 +0530
committerPrudhvi Akhil Alahari <prudhvi@codeaurora.org>2021-05-04 23:47:55 +0530
commitdbbaa13d46b3f628e25317a62d6b0ffc74e8e58c (patch)
treeac401eaed1c2cad92a70a12699992bad16a15183
parent83b32989bcee95f5f5dea1672529db6b6db4956a (diff)
Fix EqualsLabelPredicate to not fail when calling match() from a plugin
Calling match() on the EqualsLabelPredicate returned from ChangeQueryBuilder.parse() in a plugin fails when in the context of an HTTP query. In HTTP query workflow, ChangeData lazyLoad flag is being set to true when certain conditions are met. But in SSH workflow, ChangeData lazyLoad flag is always set to true. Due to this reason, we observe the issue only through a HTTP query. In [1], ChangeControl was modified to use ChangeNotes, but EqualsLabelPredicate wasn't updated to always load ChangeNotes in order to check permissions for approvers. Fix this issue by setting ChangeData lazy load to true within match() in EqualsLabelPredicate. Also write integration tests for Label Predicate to ensure it continues to work as expected. In this test setup, plugin named "my-plugin" defines a --sample switch which calls match() on the predicate received from ChangeQueryBuilder.parse() which parses a Label operator query. [1] Iac176b8e55e https://gerrit-review.googlesource.com/246154 Change-Id: Icd2541fe26c18a8e61ce855862e0c9814a91f5ef
-rw-r--r--java/com/google/gerrit/acceptance/AbstractPredicateTest.java104
-rw-r--r--java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java1
-rw-r--r--javatests/com/google/gerrit/acceptance/rest/change/PredicateIT.java52
-rw-r--r--javatests/com/google/gerrit/acceptance/ssh/PredicateIT.java65
4 files changed, 222 insertions, 0 deletions
diff --git a/java/com/google/gerrit/acceptance/AbstractPredicateTest.java b/java/com/google/gerrit/acceptance/AbstractPredicateTest.java
new file mode 100644
index 0000000000..c9fd3fbfe4
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/AbstractPredicateTest.java
@@ -0,0 +1,104 @@
+// Copyright (C) 2021 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.acceptance;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.server.DynamicOptions;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.restapi.change.QueryChanges;
+import com.google.gerrit.sshd.commands.Query;
+import com.google.gson.Gson;
+import com.google.gson.reflect.TypeToken;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.Collections;
+import java.util.List;
+import org.kohsuke.args4j.Option;
+
+public abstract class AbstractPredicateTest extends AbstractDaemonTest {
+ public static final String PLUGIN_NAME = "my-plugin";
+ public static final Gson GSON = OutputFormat.JSON.newGson();
+
+ public static class MyInfo extends PluginDefinedInfo {
+ public String message;
+ }
+
+ protected static class PluginModule extends AbstractModule {
+ @Override
+ public void configure() {
+ bind(DynamicOptions.DynamicBean.class)
+ .annotatedWith(Exports.named(Query.class))
+ .to(MyQueryOptions.class);
+ bind(DynamicOptions.DynamicBean.class)
+ .annotatedWith(Exports.named(QueryChanges.class))
+ .to(MyQueryOptions.class);
+ bind(ChangeAttributeFactory.class)
+ .annotatedWith(Exports.named("sample"))
+ .to(AttributeFactory.class);
+ }
+ }
+
+ public static class MyQueryOptions implements DynamicOptions.DynamicBean {
+ @Option(name = "--sample")
+ public boolean sample;
+ }
+
+ protected static class AttributeFactory implements ChangeAttributeFactory {
+ private final Provider<ChangeQueryBuilder> queryBuilderProvider;
+
+ @Inject
+ AttributeFactory(Provider<ChangeQueryBuilder> queryBuilderProvider) {
+ this.queryBuilderProvider = queryBuilderProvider;
+ }
+
+ @Override
+ public PluginDefinedInfo create(
+ ChangeData cd, DynamicOptions.BeanProvider beanProvider, String plugin) {
+ MyQueryOptions options = (MyQueryOptions) beanProvider.getDynamicBean(plugin);
+ MyInfo myInfo = new MyInfo();
+ if (options.sample) {
+ try {
+ Predicate<ChangeData> predicate = queryBuilderProvider.get().parse("label:Code-Review+2");
+ if (predicate.isMatchable() && predicate.asMatchable().match(cd)) {
+ myInfo.message = "matched";
+ } else {
+ myInfo.message = "not matched";
+ }
+ } catch (QueryParseException e) {
+ // ignored
+ }
+ }
+ return myInfo;
+ }
+ }
+
+ protected static List<MyInfo> decodeRawPluginsList(@Nullable Object plugins) {
+ if (plugins == null) {
+ return Collections.emptyList();
+ }
+ checkArgument(plugins instanceof List, "not a list: %s", plugins);
+ return GSON.fromJson(GSON.toJson(plugins), new TypeToken<List<MyInfo>>() {}.getType());
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 569d7cbe54..790bcc5dd6 100644
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -73,6 +73,7 @@ public class EqualsLabelPredicate extends ChangeIndexPredicate {
}
boolean hasVote = false;
+ object.setLazyLoad(true);
for (PatchSetApproval p : object.currentApprovals()) {
if (labelType.matches(p)) {
hasVote = true;
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/PredicateIT.java b/javatests/com/google/gerrit/acceptance/rest/change/PredicateIT.java
new file mode 100644
index 0000000000..5e29538851
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/change/PredicateIT.java
@@ -0,0 +1,52 @@
+// Copyright (C) 2021 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.acceptance.rest.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractPredicateTest;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.entities.Change;
+import com.google.gson.reflect.TypeToken;
+import java.util.List;
+import java.util.Map;
+import org.junit.Test;
+
+public class PredicateIT extends AbstractPredicateTest {
+
+ @Test
+ public void testLabelPredicate() throws Exception {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, PluginModule.class)) {
+ Change.Id changeId = createChange().getChange().getId();
+ approve(String.valueOf(changeId.get()));
+ List<MyInfo> myInfos =
+ pluginInfoFromSingletonList(
+ adminRestSession.get("/changes/?--my-plugin--sample&q=change:" + changeId.get()));
+
+ assertThat(myInfos).hasSize(1);
+ assertThat(myInfos.get(0).name).isEqualTo(PLUGIN_NAME);
+ assertThat(myInfos.get(0).message).isEqualTo("matched");
+ }
+ }
+
+ public List<MyInfo> pluginInfoFromSingletonList(RestResponse res) throws Exception {
+ res.assertOK();
+ List<Map<String, Object>> changeInfos =
+ GSON.fromJson(res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
+
+ assertThat(changeInfos).hasSize(1);
+ return decodeRawPluginsList(changeInfos.get(0).get("plugins"));
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/ssh/PredicateIT.java b/javatests/com/google/gerrit/acceptance/ssh/PredicateIT.java
new file mode 100644
index 0000000000..fc6100b330
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/ssh/PredicateIT.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2021 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.acceptance.ssh;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.io.CharStreams;
+import com.google.gerrit.acceptance.AbstractPredicateTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.entities.Change;
+import com.google.gson.reflect.TypeToken;
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import org.junit.Test;
+
+@NoHttpd
+@UseSsh
+public class PredicateIT extends AbstractPredicateTest {
+
+ @Test
+ public void testLabelPredicate() throws Exception {
+ try (AutoCloseable ignored = installPlugin(PLUGIN_NAME, PluginModule.class)) {
+ Change.Id changeId = createChange().getChange().getId();
+ approve(String.valueOf(changeId.get()));
+ String sshOutput =
+ adminSshSession.exec(
+ "gerrit query --format json --my-plugin--sample change:" + changeId.get());
+ adminSshSession.assertSuccess();
+ List<MyInfo> myInfos = pluginInfoFromSingletonList(sshOutput);
+
+ assertThat(myInfos).hasSize(1);
+ assertThat(myInfos.get(0).name).isEqualTo(PLUGIN_NAME);
+ assertThat(myInfos.get(0).message).isEqualTo("matched");
+ }
+ }
+
+ private static List<MyInfo> pluginInfoFromSingletonList(String sshOutput) throws Exception {
+ List<Map<String, Object>> changeAttrs = new ArrayList<>();
+ for (String line : CharStreams.readLines(new StringReader(sshOutput))) {
+ Map<String, Object> changeAttr =
+ GSON.fromJson(line, new TypeToken<Map<String, Object>>() {}.getType());
+ if (!"stats".equals(changeAttr.get("type"))) {
+ changeAttrs.add(changeAttr);
+ }
+ }
+
+ assertThat(changeAttrs).hasSize(1);
+ return decodeRawPluginsList(changeAttrs.get(0).get("plugins"));
+ }
+}