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
This commit is contained in:
Prudhvi Akhil Alahari
2021-05-03 10:20:10 +05:30
parent 83b32989bc
commit dbbaa13d46
4 changed files with 222 additions and 0 deletions

View File

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

View File

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

View File

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

View File

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