Allow permission backend to add extra changes queries
The virtual host module, or other permissions backends, can
potentially restrict the visibility of users to a limited set
of projects or changes.
Allow the permission backend to reduce the cardinality further
of the results returned by the indexing backend for avoiding the memory
and CPU overload caused by filtering a large number of entries.
Release-Notes: Improve performance of changes queries when using the custom permission backends
Change-Id: If144832047b46fcf92bcb06ce650e48ed71ec32b
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index ac9ac98..dbdd26f 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -21,6 +21,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.UsedAt;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.LabelType;
@@ -261,6 +263,19 @@
}
return allowed;
}
+
+ /**
+ * Additional filter for changes query for reducing the cardinality of the results for current
+ * user.
+ *
+ * @return additional query filter to add to all user's change queries, null if no filters are
+ * required.
+ * @since 3.11
+ */
+ @UsedAt(UsedAt.Project.MODULE_VIRTUALHOST)
+ public @Nullable String filterQueryChanges() {
+ return null;
+ }
}
/** PermissionBackend scoped to a user and project. */
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index 812711a..d05cbf6 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.change;
+import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.client.ListChangesOption;
@@ -155,6 +156,7 @@
throws BadRequestException, AuthException, PermissionBackendException {
List<List<ChangeInfo>> out;
try {
+ applyPermissionBackendFilter();
out = query();
} catch (QueryRequiresAuthException e) {
throw new AuthException("Must be signed-in to use this operator", e);
@@ -165,6 +167,22 @@
return Response.ok(out.size() == 1 ? out.get(0) : out);
}
+ private void applyPermissionBackendFilter() {
+ String queryFilter = permissionBackend.currentUser().filterQueryChanges();
+ if (Strings.isNullOrEmpty(queryFilter)) {
+ return;
+ }
+
+ if (queries == null || queries.isEmpty()) {
+ addQuery(queryFilter);
+ return;
+ }
+
+ for (int i = 0; i < queries.size(); i++) {
+ queries.set(i, queries.get(i) + " " + queryFilter);
+ }
+ }
+
private List<List<ChangeInfo>> query()
throws BadRequestException, QueryParseException, PermissionBackendException {
ChangeQueryProcessor queryProcessor = queryProcessorProvider.get();
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangesFilterPermissionBackendIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesFilterPermissionBackendIT.java
new file mode 100644
index 0000000..07d32fe
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesFilterPermissionBackendIT.java
@@ -0,0 +1,147 @@
+// Copyright (C) 2024 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.api.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.permissions.DefaultPermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Scopes;
+import com.google.inject.Singleton;
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.junit.Test;
+
+public class QueryChangesFilterPermissionBackendIT extends AbstractDaemonTest {
+ @Inject private ProjectOperations projectOperations;
+
+ @Singleton
+ public static class TestPermissionBackend extends PermissionBackend {
+ private final DefaultPermissionBackend defaultPermissionBackend;
+ private final AtomicReference<String> extraQueryFilter;
+
+ public static class Module extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(PermissionBackend.class).to(TestPermissionBackend.class).in(Scopes.SINGLETON);
+ }
+ }
+
+ @Inject
+ TestPermissionBackend(DefaultPermissionBackend defaultPermissionBackend) {
+ this.defaultPermissionBackend = defaultPermissionBackend;
+ this.extraQueryFilter = new AtomicReference<>();
+ }
+
+ @Override
+ public WithUser currentUser() {
+ return new TestPermissionWithUser(defaultPermissionBackend.currentUser());
+ }
+
+ @Override
+ public WithUser user(CurrentUser user) {
+ return new TestPermissionWithUser(defaultPermissionBackend.user(user));
+ }
+
+ @Override
+ public WithUser absentUser(Account.Id id) {
+ return new TestPermissionWithUser(defaultPermissionBackend.absentUser(id));
+ }
+
+ public String getExtraQueryFilter() {
+ return extraQueryFilter.get();
+ }
+
+ public void setExtraQueryFilter(String extraQueryFilter) {
+ this.extraQueryFilter.set(extraQueryFilter);
+ }
+
+ class TestPermissionWithUser extends WithUser {
+
+ private final WithUser defaultPermissioBackendWithUser;
+
+ TestPermissionWithUser(WithUser defaultPermissioBackendWithUser) {
+ this.defaultPermissioBackendWithUser = defaultPermissioBackendWithUser;
+ }
+
+ @Override
+ public ForProject project(Project.NameKey project) {
+ return defaultPermissioBackendWithUser.project(project);
+ }
+
+ @Override
+ public void check(GlobalOrPluginPermission perm)
+ throws AuthException, PermissionBackendException {
+ defaultPermissioBackendWithUser.check(perm);
+ }
+
+ @Override
+ public <T extends GlobalOrPluginPermission> Set<T> test(Collection<T> permSet)
+ throws PermissionBackendException {
+ return defaultPermissioBackendWithUser.test(permSet);
+ }
+
+ @Override
+ public BooleanCondition testCond(GlobalOrPluginPermission perm) {
+ return defaultPermissioBackendWithUser.testCond(perm);
+ }
+
+ @Override
+ public String filterQueryChanges() {
+ return extraQueryFilter.get();
+ }
+ }
+ }
+
+ @Override
+ public Module createModule() {
+ return new TestPermissionBackend.Module();
+ }
+
+ @Test
+ public void filterHidenProjectByAuthenticationBackend() throws Exception {
+ String projectChangeId = createChange().getChangeId();
+
+ Project.NameKey hiddenProject = projectOperations.newProject().create();
+ TestRepository<InMemoryRepository> hiddenRepo = cloneProject(hiddenProject, admin);
+ createChange(hiddenRepo);
+
+ assertThat(gApi.changes().query().get()).hasSize(2);
+
+ server
+ .getTestInjector()
+ .getInstance(TestPermissionBackend.class)
+ .setExtraQueryFilter("-project:" + hiddenProject);
+ List<ChangeInfo> projectChanges = gApi.changes().query().get();
+ assertThat(projectChanges.stream().map(c -> c.changeId)).containsExactly(projectChangeId);
+ }
+}
