Merge "Perform group lookups by name from the group index"
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java
index ea2e556..ef0030f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/ValidatorConfig.java
@@ -20,26 +20,28 @@
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.ProjectConfigEntry;
+import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.project.RefPatternMatcher;
+import com.google.gerrit.server.query.group.InternalGroupQuery;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
+import com.google.inject.Provider;
import java.util.Arrays;
+import java.util.Optional;
import java.util.regex.Pattern;
import java.util.stream.Stream;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class ValidatorConfig {
private static final Logger log = LoggerFactory.getLogger(ValidatorConfig.class);
private static final String KEY_PROJECT = "project";
private static final String KEY_REF = "ref";
private final ConfigFactory configFactory;
- private final GroupCache groupCache;
+ private final GroupByNameFinder groupByNameFinder;
public static AbstractModule module() {
return new AbstractModule() {
@@ -65,14 +67,15 @@
null,
false,
"Only refs that match this regex will be validated."));
+ bind(GroupByNameFinder.class).to(GroupByNameFromIndexFinder.class);
}
};
}
@Inject
- public ValidatorConfig(ConfigFactory configFactory, GroupCache groupCache) {
+ public ValidatorConfig(ConfigFactory configFactory, GroupByNameFinder groupByNameFinder) {
this.configFactory = configFactory;
- this.groupCache = groupCache;
+ this.groupByNameFinder = groupByNameFinder;
}
public boolean isEnabledForRef(
@@ -165,10 +168,32 @@
}
private AccountGroup.UUID groupUUID(String groupNameOrUUID) {
- AccountGroup group = groupCache.get(new AccountGroup.NameKey(groupNameOrUUID));
- if (group == null) {
- return new AccountGroup.UUID(groupNameOrUUID);
+ Optional<InternalGroup> group =
+ groupByNameFinder.get(new AccountGroup.NameKey(groupNameOrUUID));
+ return group.map(InternalGroup::getGroupUUID).orElse(new AccountGroup.UUID(groupNameOrUUID));
+ }
+
+ interface GroupByNameFinder {
+ Optional<InternalGroup> get(AccountGroup.NameKey groupName);
+ }
+
+ static class GroupByNameFromIndexFinder implements GroupByNameFinder {
+
+ private final Provider<InternalGroupQuery> groupQueryProvider;
+
+ @Inject
+ GroupByNameFromIndexFinder(Provider<InternalGroupQuery> groupQueryProvider) {
+ this.groupQueryProvider = groupQueryProvider;
}
- return group.getGroupUUID();
+
+ @Override
+ public Optional<InternalGroup> get(AccountGroup.NameKey groupName) {
+ try {
+ return groupQueryProvider.get().byName(groupName);
+ } catch (OrmException e) {
+ log.warn(String.format("Cannot lookup group %s by name", groupName.get()), e);
+ }
+ return Optional.empty();
+ }
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java
index bdf3c0f..b385259 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/EmailAwareValidatorConfigTest.java
@@ -18,7 +18,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
-
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Test;
@@ -109,9 +108,7 @@
}
private ValidatorConfig getConfig(String defaultConfig) throws ConfigInvalidException {
- ValidatorConfig config =
- new ValidatorConfig(
- new FakeConfigFactory(projectName, defaultConfig), new FakeGroupCacheUUIDByName());
- return config;
+ return new ValidatorConfig(
+ new FakeConfigFactory(projectName, defaultConfig), new FakeGroupByNameFinder());
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java
new file mode 100644
index 0000000..69f1b74
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupByNameFinder.java
@@ -0,0 +1,40 @@
+// Copyright (C) 2017 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.googlesource.gerrit.plugins.uploadvalidator;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.group.InternalGroup;
+import java.util.Objects;
+import java.util.Optional;
+
+public class FakeGroupByNameFinder implements ValidatorConfig.GroupByNameFinder {
+
+ private final Optional<InternalGroup> onlyGroup;
+
+ public FakeGroupByNameFinder() {
+ onlyGroup = Optional.empty();
+ }
+
+ public FakeGroupByNameFinder(AccountGroup accountGroup) {
+ onlyGroup =
+ Optional.of(InternalGroup.create(accountGroup, ImmutableSet.of(), ImmutableSet.of()));
+ }
+
+ @Override
+ public Optional<InternalGroup> get(AccountGroup.NameKey groupName) {
+ return onlyGroup.filter(group -> Objects.equals(group.getNameKey(), groupName));
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupCacheUUIDByName.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupCacheUUIDByName.java
deleted file mode 100644
index 00fef07..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/FakeGroupCacheUUIDByName.java
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright (C) 2017 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.googlesource.gerrit.plugins.uploadvalidator;
-
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountGroup.Id;
-import com.google.gerrit.reviewdb.client.AccountGroup.NameKey;
-import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
-import com.google.gerrit.server.account.GroupCache;
-import com.google.gerrit.server.group.InternalGroup;
-import java.util.Optional;
-
-public class FakeGroupCacheUUIDByName implements GroupCache {
- private AccountGroup accountGroup;
-
- public FakeGroupCacheUUIDByName(AccountGroup accountGroup) {
- this.accountGroup = accountGroup;
- }
-
- public FakeGroupCacheUUIDByName() {
- // TODO Auto-generated constructor stub
- }
-
- @Override
- public AccountGroup get(Id groupId) {
- return null;
- }
-
- @Override
- public AccountGroup get(NameKey name) {
- return accountGroup != null && accountGroup.getNameKey().equals(name) ? accountGroup : null;
- }
-
- @Override
- public Optional<InternalGroup> get(UUID uuid) {
- return null;
- }
-
- @Override
- public ImmutableList<AccountGroup> all() {
- return null;
- }
-
- @Override
- public void onCreateGroup(NameKey newGroupName) {}
-
- @Override
- public void evict(
- AccountGroup.UUID groupUuid, AccountGroup.Id groupId, AccountGroup.NameKey groupName) {}
-
- @Override
- public void evictAfterRename(NameKey oldName, NameKey newName) {}
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java
index 601fcf6..e6e1be7 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/ProjectAwareValidatorConfigTest.java
@@ -18,7 +18,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
-
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Test;
@@ -100,9 +99,7 @@
private ValidatorConfig getConfig(String defaultConfig, Project.NameKey projName)
throws ConfigInvalidException {
- ValidatorConfig config =
- new ValidatorConfig(
- new FakeConfigFactory(projName, defaultConfig), new FakeGroupCacheUUIDByName());
- return config;
+ return new ValidatorConfig(
+ new FakeConfigFactory(projName, defaultConfig), new FakeGroupByNameFinder());
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java
index ac4094f..acc2bdc 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/RefAwareValidatorConfigTest.java
@@ -18,7 +18,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
-
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Test;
@@ -105,9 +104,7 @@
}
private ValidatorConfig getConfig(String defaultConfig) throws ConfigInvalidException {
- ValidatorConfig config =
- new ValidatorConfig(
- new FakeConfigFactory(projectName, defaultConfig), new FakeGroupCacheUUIDByName());
- return config;
+ return new ValidatorConfig(
+ new FakeConfigFactory(projectName, defaultConfig), new FakeGroupByNameFinder());
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java
index f7ed871..4942391 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/SkipValidationTest.java
@@ -29,7 +29,7 @@
@Test
public void dontSkipByDefault() throws Exception {
ValidatorConfig validatorConfig =
- new ValidatorConfig(new FakeConfigFactory(projectName, ""), new FakeGroupCacheUUIDByName());
+ new ValidatorConfig(new FakeConfigFactory(projectName, ""), new FakeGroupByNameFinder());
assertThat(validatorConfig.isEnabledForRef(anyUser, projectName, "anyRef", "anyOp")).isTrue();
}
@@ -44,7 +44,7 @@
ValidatorConfig validatorConfig =
new ValidatorConfig(
- new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName());
+ new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder());
assertThat(
validatorConfig.isEnabledForRef(
@@ -63,7 +63,7 @@
ValidatorConfig validatorConfig =
new ValidatorConfig(
new FakeConfigFactory(projectName, config),
- new FakeGroupCacheUUIDByName(
+ new FakeGroupByNameFinder(
new AccountGroup(
new AccountGroup.NameKey("testGroupName"),
new AccountGroup.Id(1),
@@ -86,7 +86,7 @@
ValidatorConfig validatorConfig =
new ValidatorConfig(
- new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName());
+ new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder());
assertThat(
validatorConfig.isEnabledForRef(
@@ -104,7 +104,7 @@
ValidatorConfig validatorConfig =
new ValidatorConfig(
- new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName());
+ new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder());
assertThat(validatorConfig.isEnabledForRef(anyUser, projectName, "anyRef", "anotherOp"))
.isTrue();
@@ -120,7 +120,7 @@
ValidatorConfig validatorConfig =
new ValidatorConfig(
- new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName());
+ new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder());
assertThat(
validatorConfig.isEnabledForRef(
@@ -138,7 +138,7 @@
ValidatorConfig validatorConfig =
new ValidatorConfig(
- new FakeConfigFactory(projectName, config), new FakeGroupCacheUUIDByName());
+ new FakeConfigFactory(projectName, config), new FakeGroupByNameFinder());
assertThat(
validatorConfig.isEnabledForRef(
