Merge "AbstractDaemonTest: Make updating project config cleaner & safer"
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index aa9fa36..1d713e8 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1370,14 +1370,6 @@
assertThat(contentEntry.skip).isNull();
}
- protected TestRepository<?> createProjectWithPush(
- String name, @Nullable Project.NameKey parent, SubmitType submitType) throws Exception {
- Project.NameKey project = createProject(name, parent, true, submitType);
- grant(project, "refs/heads/*", Permission.PUSH);
- grant(project, "refs/for/refs/heads/*", Permission.SUBMIT);
- return cloneProject(project);
- }
-
protected void assertPermitted(ChangeInfo info, String label, Integer... expected) {
assertThat(info.permittedLabels).isNotNull();
Collection<String> strs = info.permittedLabels.get(label);
@@ -1407,27 +1399,7 @@
}
}
- protected void assertLabelPermission(
- Project.NameKey project,
- GroupReference groupReference,
- String ref,
- boolean exclusive,
- String labelName,
- int min,
- int max)
- throws IOException {
- ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
- AccessSection accessSection = cfg.getAccessSection(ref);
- assertThat(accessSection).isNotNull();
-
- String permissionName = Permission.LABEL + labelName;
- Permission permission = accessSection.getPermission(permissionName);
- assertPermission(permission, permissionName, exclusive, labelName);
- assertPermissionRule(
- permission.getRule(groupReference), groupReference, Action.ALLOW, false, min, max);
- }
-
- private void assertPermission(
+ protected void assertPermission(
Permission permission,
String expectedName,
boolean expectedExclusive,
@@ -1438,7 +1410,7 @@
assertThat(permission.getLabel()).isEqualTo(expectedLabelName);
}
- private void assertPermissionRule(
+ protected void assertPermissionRule(
PermissionRule rule,
GroupReference expectedGroupReference,
Action expectedAction,
@@ -1596,10 +1568,6 @@
}
}
- protected RevCommit parseCurrentRevision(RevWalk rw, PushOneCommit.Result r) throws Exception {
- return parseCurrentRevision(rw, r.getChangeId());
- }
-
protected RevCommit parseCurrentRevision(RevWalk rw, String changeId) throws Exception {
return rw.parseCommit(
ObjectId.fromString(get(changeId, ListChangesOption.CURRENT_REVISION).currentRevision));
@@ -1625,10 +1593,6 @@
assert_().fail(format, args);
}
- protected void fail() {
- assert_().fail();
- }
-
protected void enableCreateNewChangeForAllNotInTarget() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
u.getConfig()
diff --git a/java/com/google/gerrit/server/cache/PerThreadCache.java b/java/com/google/gerrit/server/cache/PerThreadCache.java
index 81cfc44..3f4dacd 100644
--- a/java/com/google/gerrit/server/cache/PerThreadCache.java
+++ b/java/com/google/gerrit/server/cache/PerThreadCache.java
@@ -87,7 +87,7 @@
if (!(o instanceof Key)) {
return false;
}
- Key other = (Key) o;
+ Key<?> other = (Key<?>) o;
return this.clazz == other.clazz && this.identifiers.equals(other.identifiers);
}
}
@@ -104,7 +104,12 @@
return CACHE.get();
}
- private final Map<Key, Object> cache = Maps.newHashMapWithExpectedSize(10);
+ public static <T> T getOrCompute(Key<T> key, Supplier<T> loader) {
+ PerThreadCache cache = get();
+ return cache != null ? cache.get(key, loader) : loader.get();
+ }
+
+ private final Map<Key<?>, Object> cache = Maps.newHashMapWithExpectedSize(10);
private PerThreadCache() {}
@@ -113,6 +118,7 @@
* provided {@link Supplier}.
*/
public <T> T get(Key<T> key, Supplier<T> loader) {
+ @SuppressWarnings("unchecked")
T value = (T) cache.get(key);
if (value == null) {
value = loader.get();
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
index e096930..065d3a1 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
@@ -154,7 +154,7 @@
@SuppressWarnings({"unchecked"})
@Override
public <K, V> Cache<K, V> build(CacheBinding<K, V> def) {
- long limit = config.getLong("cache", def.name(), "diskLimit", 128 << 20);
+ long limit = config.getLong("cache", def.name(), "diskLimit", def.diskLimit());
if (cacheDir == null || limit <= 0) {
return defaultFactory.build(def);
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
index 93f1683..1ac95fe 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
@@ -109,14 +109,10 @@
try {
ProjectState state = projectCache.checkedGet(project);
if (state != null) {
- PerThreadCache perThreadCache = PerThreadCache.get();
- if (perThreadCache == null) {
- return projectControlFactory.create(user, state).asForProject().database(db);
- }
- PerThreadCache.Key<ProjectControl> cacheKey =
- PerThreadCache.Key.create(ProjectControl.class, project, user.getCacheKey());
ProjectControl control =
- perThreadCache.get(cacheKey, () -> projectControlFactory.create(user, state));
+ PerThreadCache.getOrCompute(
+ PerThreadCache.Key.create(ProjectControl.class, project, user.getCacheKey()),
+ () -> projectControlFactory.create(user, state));
return control.asForProject().database(db);
}
return FailedPermissionBackend.project("not found", new NoSuchProjectException(project));
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index c13c32e..6bf8aaf 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -56,8 +56,11 @@
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.extensions.api.accounts.AccountInput;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
@@ -264,6 +267,26 @@
}
}
+ protected void assertLabelPermission(
+ Project.NameKey project,
+ GroupReference groupReference,
+ String ref,
+ boolean exclusive,
+ String labelName,
+ int min,
+ int max)
+ throws IOException {
+ ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+ AccessSection accessSection = cfg.getAccessSection(ref);
+ assertThat(accessSection).isNotNull();
+
+ String permissionName = Permission.LABEL + labelName;
+ Permission permission = accessSection.getPermission(permissionName);
+ assertPermission(permission, permissionName, exclusive, labelName);
+ assertPermissionRule(
+ permission.getRule(groupReference), groupReference, Action.ALLOW, false, min, max);
+ }
+
@Test
public void createByAccountCreator() throws Exception {
Account.Id accountId = createByAccountCreator(2); // account creation + external ID creation
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index c444fbd..8d1a27d 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -916,7 +916,7 @@
// Verify "sub-group" has been deleted.
try {
gApi.groups().id(uuid.get()).get();
- fail();
+ fail("expected ResourceNotFoundException");
} catch (ResourceNotFoundException e) {
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 1871343..3514e8e 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -518,7 +518,7 @@
in.message = r1.getCommit().getFullMessage();
try {
gApi.changes().id(t1).current().cherryPick(in);
- fail();
+ fail("expected ResourceConflictException");
} catch (ResourceConflictException e) {
assertThat(e.getMessage())
.isEqualTo(
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index d91e0f0..acd1e45 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -36,6 +36,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.ChangeApi;
@@ -918,7 +919,7 @@
testRepo.git().fetch().call();
RevWalk rw = testRepo.getRevWalk();
RevCommit master = rw.parseCommit(getRemoteHead(project, "master"));
- RevCommit patchSet = parseCurrentRevision(rw, change);
+ RevCommit patchSet = parseCurrentRevision(rw, change.getChangeId());
assertThat(rw.isMergedInto(patchSet, master)).isTrue();
assertThat(input.generateLockFailures).containsExactly(false);
@@ -960,13 +961,13 @@
repoA.git().fetch().call();
RevWalk rwA = repoA.getRevWalk();
RevCommit masterA = rwA.parseCommit(getRemoteHead(name("project-a"), "master"));
- RevCommit change1Ps = parseCurrentRevision(rwA, change1);
+ RevCommit change1Ps = parseCurrentRevision(rwA, change1.getChangeId());
assertThat(rwA.isMergedInto(change1Ps, masterA)).isTrue();
repoB.git().fetch().call();
RevWalk rwB = repoB.getRevWalk();
RevCommit masterB = rwB.parseCommit(getRemoteHead(name("project-b"), "master"));
- RevCommit change2Ps = parseCurrentRevision(rwB, change2);
+ RevCommit change2Ps = parseCurrentRevision(rwB, change2.getChangeId());
assertThat(rwB.isMergedInto(change2Ps, masterB)).isTrue();
assertThat(input.generateLockFailures).containsExactly(false);
@@ -1310,4 +1311,12 @@
return out.toString();
}
}
+
+ private TestRepository<?> createProjectWithPush(
+ String name, @Nullable Project.NameKey parent, SubmitType submitType) throws Exception {
+ Project.NameKey project = createProject(name, parent, true, submitType);
+ grant(project, "refs/heads/*", Permission.PUSH);
+ grant(project, "refs/for/refs/heads/*", Permission.SUBMIT);
+ return cloneProject(project);
+ }
}
