Merge changes from topic "gr-change-view-to-ts"
* changes:
Convert gr-change-view to typescript
Rename files to preserve history
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index a1197ef..404f5e4 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -108,6 +108,7 @@
import com.google.gerrit.server.RequestInfo;
import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.audit.ExtendedHttpAuditEvent;
+import com.google.gerrit.server.cache.PerThreadCache;
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.GroupAuditService;
@@ -327,7 +328,7 @@
try (TraceContext traceContext = enableTracing(req, res)) {
List<IdString> path = splitPath(req);
- try {
+ try (PerThreadCache ignored = PerThreadCache.create()) {
RequestInfo requestInfo = createRequestInfo(traceContext, requestUri(req), path);
globals.requestListeners.runEach(l -> l.onRequest(requestInfo));
diff --git a/java/com/google/gerrit/server/AnonymousUser.java b/java/com/google/gerrit/server/AnonymousUser.java
index c96d61a..91d2d05 100644
--- a/java/com/google/gerrit/server/AnonymousUser.java
+++ b/java/com/google/gerrit/server/AnonymousUser.java
@@ -27,6 +27,12 @@
}
@Override
+ public Object getCacheKey() {
+ // Treat all anonymous users as a single user
+ return "anonymous";
+ }
+
+ @Override
public String toString() {
return "ANONYMOUS";
}
diff --git a/java/com/google/gerrit/server/CurrentUser.java b/java/com/google/gerrit/server/CurrentUser.java
index 43d3c7b..825b34f 100644
--- a/java/com/google/gerrit/server/CurrentUser.java
+++ b/java/com/google/gerrit/server/CurrentUser.java
@@ -91,6 +91,12 @@
*/
public abstract GroupMembership getEffectiveGroups();
+ /**
+ * Returns a unique identifier for this user that is intended to be used as a cache key. Returned
+ * object should to implement {@code equals()} and {@code hashCode()} for effective caching.
+ */
+ public abstract Object getCacheKey();
+
/** Unique name of the user on this server, if one has been assigned. */
public Optional<String> getUserName() {
return Optional.empty();
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java
index ec2eb81..75c7cda 100644
--- a/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -413,6 +413,11 @@
return effectiveGroups;
}
+ @Override
+ public Object getCacheKey() {
+ return getAccountId();
+ }
+
public PersonIdent newRefLogIdent() {
return newRefLogIdent(new Date(), TimeZone.getDefault());
}
diff --git a/java/com/google/gerrit/server/InternalUser.java b/java/com/google/gerrit/server/InternalUser.java
index 821a0c6..381819d 100644
--- a/java/com/google/gerrit/server/InternalUser.java
+++ b/java/com/google/gerrit/server/InternalUser.java
@@ -36,6 +36,11 @@
}
@Override
+ public String getCacheKey() {
+ return "internal";
+ }
+
+ @Override
public boolean isInternalUser() {
return true;
}
diff --git a/java/com/google/gerrit/server/PeerDaemonUser.java b/java/com/google/gerrit/server/PeerDaemonUser.java
index 8a8b67a..b27e05c 100644
--- a/java/com/google/gerrit/server/PeerDaemonUser.java
+++ b/java/com/google/gerrit/server/PeerDaemonUser.java
@@ -40,6 +40,11 @@
return GroupMembership.EMPTY;
}
+ @Override
+ public Object getCacheKey() {
+ return getRemoteAddress();
+ }
+
public SocketAddress getRemoteAddress() {
return peer;
}
diff --git a/java/com/google/gerrit/server/cache/PerThreadCache.java b/java/com/google/gerrit/server/cache/PerThreadCache.java
new file mode 100644
index 0000000..b4f79d1
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/PerThreadCache.java
@@ -0,0 +1,146 @@
+// Copyright (C) 2018 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.server.cache;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+import com.google.gerrit.common.Nullable;
+import java.util.Map;
+import java.util.function.Supplier;
+
+/**
+ * Caches object instances for a request as {@link ThreadLocal} in the serving thread.
+ *
+ * <p>This class is intended to cache objects that have a high instantiation cost, are specific to
+ * the current request and potentially need to be instantiated multiple times while serving a
+ * request.
+ *
+ * <p>This is different from the key-value storage in {@code CurrentUser}: {@code CurrentUser}
+ * offers a key-value storage by providing thread-safe {@code get} and {@code put} methods. Once the
+ * value is retrieved through {@code get} there is not thread-safety anymore - apart from the
+ * retrieved object guarantees. Depending on the implementation of {@code CurrentUser}, it might be
+ * shared between the request serving thread as well as sub- or background treads.
+ *
+ * <p>In comparison to that, this class guarantees thread safety even on non-thread-safe objects as
+ * its cache is tied to the serving thread only. While allowing to cache non-thread-safe objects, it
+ * has the downside of not sharing any objects with background threads or executors.
+ *
+ * <p>Lastly, this class offers a cache, that requires callers to also provide a {@code Supplier} in
+ * case the object is not present in the cache, while {@code CurrentUser} provides a storage where
+ * just retrieving stored values is a valid operation.
+ *
+ * <p>To prevent OOM errors on requests that would cache a lot of objects, this class enforces an
+ * internal limit after which no new elements are cached. All {@code get} calls are served by
+ * invoking the {@code Supplier} after that.
+ */
+public class PerThreadCache implements AutoCloseable {
+ private static final ThreadLocal<PerThreadCache> CACHE = new ThreadLocal<>();
+ /**
+ * Cache at maximum 25 values per thread. This value was chosen arbitrarily. Some endpoints (like
+ * ListProjects) break the assumption that the data cached in a request is limited. To prevent
+ * this class from accumulating an unbound number of objects, we enforce this limit.
+ */
+ private static final int PER_THREAD_CACHE_SIZE = 25;
+
+ /**
+ * Unique key for key-value mappings stored in PerThreadCache. The key is based on the value's
+ * class and a list of identifiers that in combination uniquely set the object apart form others
+ * of the same class.
+ */
+ public static final class Key<T> {
+ private final Class<T> clazz;
+ private final ImmutableList<Object> identifiers;
+
+ /**
+ * Returns a key based on the value's class and an identifier that uniquely identify the value.
+ * The identifier needs to implement {@code equals()} and {@hashCode()}.
+ */
+ public static <T> Key<T> create(Class<T> clazz, Object identifier) {
+ return new Key<>(clazz, ImmutableList.of(identifier));
+ }
+
+ /**
+ * Returns a key based on the value's class and a set of identifiers that uniquely identify the
+ * value. Identifiers need to implement {@code equals()} and {@hashCode()}.
+ */
+ public static <T> Key<T> create(Class<T> clazz, Object... identifiers) {
+ return new Key<>(clazz, ImmutableList.copyOf(identifiers));
+ }
+
+ private Key(Class<T> clazz, ImmutableList<Object> identifiers) {
+ this.clazz = clazz;
+ this.identifiers = identifiers;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(clazz, identifiers);
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof Key)) {
+ return false;
+ }
+ Key<?> other = (Key<?>) o;
+ return this.clazz == other.clazz && this.identifiers.equals(other.identifiers);
+ }
+ }
+
+ public static PerThreadCache create() {
+ checkState(CACHE.get() == null, "called create() twice on the same request");
+ PerThreadCache cache = new PerThreadCache();
+ CACHE.set(cache);
+ return cache;
+ }
+
+ @Nullable
+ public static PerThreadCache get() {
+ return CACHE.get();
+ }
+
+ 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(PER_THREAD_CACHE_SIZE);
+
+ private PerThreadCache() {}
+
+ /**
+ * Returns an instance of {@code T} that was either loaded from the cache or obtained from the
+ * 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();
+ if (cache.size() < PER_THREAD_CACHE_SIZE) {
+ cache.put(key, value);
+ }
+ }
+ return value;
+ }
+
+ @Override
+ public void close() {
+ CACHE.remove();
+ }
+}
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
index 49df653..d10c139 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
@@ -34,6 +34,7 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PeerDaemonUser;
import com.google.gerrit.server.account.CapabilityCollection;
+import com.google.gerrit.server.cache.PerThreadCache;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
@@ -80,8 +81,15 @@
@Override
public WithUser absentUser(Account.Id id) {
- IdentifiedUser identifiedUser = identifiedUserFactory.create(requireNonNull(id, "user"));
- return new WithUserImpl(identifiedUser);
+ requireNonNull(id, "user");
+ CurrentUser user = currentUser.get();
+ if (user.isIdentifiedUser() && id.equals(user.asIdentifiedUser().getAccountId())) {
+ // What looked liked an absent user is actually the current caller. Use the per-request
+ // singleton IdentifiedUser instead of constructing a new object to leverage caching in member
+ // variables of IdentifiedUser.
+ return new WithUserImpl(user.asIdentifiedUser());
+ }
+ return new WithUserImpl(identifiedUserFactory.create(requireNonNull(id, "user")));
}
@Override
@@ -101,7 +109,11 @@
public ForProject project(Project.NameKey project) {
try {
ProjectState state = projectCache.get(project).orElseThrow(illegalState(project));
- return projectControlFactory.create(user, state).asForProject();
+ ProjectControl control =
+ PerThreadCache.getOrCompute(
+ PerThreadCache.Key.create(ProjectControl.class, project, user.getCacheKey()),
+ () -> projectControlFactory.create(user, state));
+ return control.asForProject();
} catch (Exception e) {
Throwable cause = e.getCause() != null ? e.getCause() : e;
return FailedPermissionBackend.project(
diff --git a/java/com/google/gerrit/server/query/change/GroupBackedUser.java b/java/com/google/gerrit/server/query/change/GroupBackedUser.java
index dac555d..3960813 100644
--- a/java/com/google/gerrit/server/query/change/GroupBackedUser.java
+++ b/java/com/google/gerrit/server/query/change/GroupBackedUser.java
@@ -64,4 +64,9 @@
public String getLoggableName() {
return "GroupBackedUser with memberships: " + groups.getKnownGroups();
}
+
+ @Override
+ public Object getCacheKey() {
+ return groups.getKnownGroups();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/server/permissions/BUILD b/javatests/com/google/gerrit/acceptance/server/permissions/BUILD
new file mode 100644
index 0000000..e89e8d1
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/permissions/BUILD
@@ -0,0 +1,7 @@
+load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+ srcs = glob(["*IT.java"]),
+ group = "server_permissions",
+ labels = ["server"],
+)
diff --git a/javatests/com/google/gerrit/acceptance/server/permissions/GroupBackedUserPermissionIT.java b/javatests/com/google/gerrit/acceptance/server/permissions/GroupBackedUserPermissionIT.java
new file mode 100644
index 0000000..d68d681
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/permissions/GroupBackedUserPermissionIT.java
@@ -0,0 +1,184 @@
+// Copyright (C) 2020 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.server.permissions;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.permissionKey;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Permission;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.GroupBackend;
+import com.google.gerrit.server.group.testing.TestGroupBackend;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.query.change.GroupBackedUser;
+import com.google.inject.AbstractModule;
+import com.google.inject.Module;
+import javax.inject.Inject;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Tests that permission logic used by {@link GroupBackedUser} works as expected. */
+public class GroupBackedUserPermissionIT extends AbstractDaemonTest {
+ @Inject private ChangeOperations changeOperations;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private PermissionBackend permissionBackend;
+ @Inject private ChangeNotes.Factory changeNotesFactory;
+
+ private final TestGroupBackend testGroupBackend = new TestGroupBackend();
+ private final AccountGroup.UUID externalGroup = AccountGroup.uuid("testbackend:test");
+
+ @Before
+ public void setUp() {
+ // Allow only read on refs/heads/master by default
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .remove(permissionKey(Permission.READ).ref("refs/*").group(ANONYMOUS_USERS))
+ .add(allow(Permission.READ).ref("refs/heads/master").group(ANONYMOUS_USERS))
+ .update();
+ }
+
+ @Override
+ public Module createModule() {
+ /** Binding a {@link TestGroupBackend} to test adding external groups * */
+ return new AbstractModule() {
+ @Override
+ protected void configure() {
+ DynamicSet.bind(binder(), GroupBackend.class).toInstance(testGroupBackend);
+ }
+ };
+ }
+
+ @Test
+ public void defaultRefFilter_changeVisibilityIsAgnosticOfProvidedGroups() throws Exception {
+ GroupBackedUser user =
+ new GroupBackedUser(ImmutableSet.of(ANONYMOUS_USERS, REGISTERED_USERS, externalGroup));
+ Change.Id changeOnMaster = changeOperations.newChange().project(project).create();
+ Change.Id changeOnRefsMetaConfig =
+ changeOperations.newChange().project(project).branch("refs/meta/config").create();
+ // Check that only the change on the default branch is visible
+ assertThat(getVisibleRefNames(user))
+ .containsExactly(
+ "HEAD",
+ "refs/heads/master",
+ RefNames.changeMetaRef(changeOnMaster),
+ RefNames.patchSetRef(PatchSet.id(changeOnMaster, 1)));
+ // Grant access
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/meta/config").group(externalGroup))
+ .update();
+ // Check that both changes are visible now
+ assertThat(getVisibleRefNames(user))
+ .containsExactly(
+ "HEAD",
+ "refs/heads/master",
+ "refs/meta/config",
+ RefNames.changeMetaRef(changeOnMaster),
+ RefNames.patchSetRef(PatchSet.id(changeOnMaster, 1)),
+ RefNames.changeMetaRef(changeOnRefsMetaConfig),
+ RefNames.patchSetRef(PatchSet.id(changeOnRefsMetaConfig, 1)));
+ }
+
+ @Test
+ public void defaultRefFilter_refVisibilityIsAgnosticOfProvidedGroups() throws Exception {
+ GroupBackedUser user =
+ new GroupBackedUser(ImmutableSet.of(ANONYMOUS_USERS, REGISTERED_USERS, externalGroup));
+ // Check that refs/meta/config isn't visible by default
+ assertThat(getVisibleRefNames(user)).containsExactly("HEAD", "refs/heads/master");
+ // Grant access
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/meta/config").group(externalGroup))
+ .update();
+ // Check that refs/meta/config became visible
+ assertThat(getVisibleRefNames(user))
+ .containsExactly("HEAD", "refs/heads/master", "refs/meta/config");
+ }
+
+ @Test
+ public void changeVisibility_changeOnInvisibleBranchNotVisible() throws Exception {
+ // Create a change that is not visible to members of 'externalGroup'
+ Change.Id invisibleChange =
+ changeOperations.newChange().project(project).branch("refs/meta/config").create();
+ GroupBackedUser user =
+ new GroupBackedUser(ImmutableSet.of(ANONYMOUS_USERS, REGISTERED_USERS, externalGroup));
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () ->
+ permissionBackend
+ .user(user)
+ .change(changeNotesFactory.create(project, invisibleChange))
+ .check(ChangePermission.READ));
+ assertThat(thrown).hasMessageThat().isEqualTo("read not permitted");
+ }
+
+ @Test
+ public void changeVisibility_changeOnBranchVisibleToAnonymousIsVisible() throws Exception {
+ Change.Id changeId = changeOperations.newChange().project(project).create();
+ GroupBackedUser user =
+ new GroupBackedUser(ImmutableSet.of(ANONYMOUS_USERS, REGISTERED_USERS, externalGroup));
+ permissionBackend
+ .user(user)
+ .change(changeNotesFactory.create(project, changeId))
+ .check(ChangePermission.READ);
+ }
+
+ @Test
+ public void changeVisibility_changeOnBranchVisibleToRegisteredUsersIsVisible() throws Exception {
+ Change.Id changeId = changeOperations.newChange().project(project).create();
+ GroupBackedUser user =
+ new GroupBackedUser(ImmutableSet.of(ANONYMOUS_USERS, REGISTERED_USERS, externalGroup));
+ blockAnonymousRead();
+ permissionBackend
+ .user(user)
+ .change(changeNotesFactory.create(project, changeId))
+ .check(ChangePermission.READ);
+ }
+
+ private ImmutableList<String> getVisibleRefNames(CurrentUser user) throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ return permissionBackend.user(user).project(project)
+ .filter(
+ repo.getRefDatabase().getRefs(), repo, PermissionBackend.RefFilterOptions.defaults())
+ .stream()
+ .map(Ref::getName)
+ .collect(toImmutableList());
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
new file mode 100644
index 0000000..5d420d3
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/PerThreadCacheTest.java
@@ -0,0 +1,103 @@
+// Copyright (C) 2018 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.server.cache;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import java.util.function.Supplier;
+import org.junit.Test;
+
+public class PerThreadCacheTest {
+
+ @SuppressWarnings("TruthIncompatibleType")
+ @Test
+ public void key_respectsClass() {
+ assertThat(PerThreadCache.Key.create(String.class))
+ .isEqualTo(PerThreadCache.Key.create(String.class));
+ assertThat(PerThreadCache.Key.create(String.class))
+ .isNotEqualTo(
+ /* expected: Key<String>, actual: Key<Integer> */ PerThreadCache.Key.create(
+ Integer.class));
+ }
+
+ @Test
+ public void key_respectsIdentifiers() {
+ assertThat(PerThreadCache.Key.create(String.class, "id1"))
+ .isEqualTo(PerThreadCache.Key.create(String.class, "id1"));
+ assertThat(PerThreadCache.Key.create(String.class, "id1"))
+ .isNotEqualTo(PerThreadCache.Key.create(String.class, "id2"));
+ }
+
+ @Test
+ public void endToEndCache() {
+ try (PerThreadCache ignored = PerThreadCache.create()) {
+ PerThreadCache cache = PerThreadCache.get();
+ PerThreadCache.Key<String> key1 = PerThreadCache.Key.create(String.class);
+
+ String value1 = cache.get(key1, () -> "value1");
+ assertThat(value1).isEqualTo("value1");
+
+ Supplier<String> neverCalled =
+ () -> {
+ throw new IllegalStateException("this method must not be called");
+ };
+ assertThat(cache.get(key1, neverCalled)).isEqualTo("value1");
+ }
+ }
+
+ @Test
+ public void cleanUp() {
+ PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class);
+ try (PerThreadCache ignored = PerThreadCache.create()) {
+ PerThreadCache cache = PerThreadCache.get();
+ String value1 = cache.get(key, () -> "value1");
+ assertThat(value1).isEqualTo("value1");
+ }
+
+ // Create a second cache and assert that it is not connected to the first one.
+ // This ensures that the cleanup is actually working.
+ try (PerThreadCache ignored = PerThreadCache.create()) {
+ PerThreadCache cache = PerThreadCache.get();
+ String value1 = cache.get(key, () -> "value2");
+ assertThat(value1).isEqualTo("value2");
+ }
+ }
+
+ @Test
+ public void doubleInstantiationFails() {
+ try (PerThreadCache ignored = PerThreadCache.create()) {
+ IllegalStateException thrown =
+ assertThrows(IllegalStateException.class, () -> PerThreadCache.create());
+ assertThat(thrown).hasMessageThat().contains("called create() twice on the same request");
+ }
+ }
+
+ @Test
+ public void enforceMaxSize() {
+ try (PerThreadCache cache = PerThreadCache.create()) {
+ // Fill the cache
+ for (int i = 0; i < 50; i++) {
+ PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, i);
+ cache.get(key, () -> "cached value");
+ }
+ // Assert that the value was not persisted
+ PerThreadCache.Key<String> key = PerThreadCache.Key.create(String.class, 1000);
+ cache.get(key, () -> "new value");
+ String value = cache.get(key, () -> "directly served");
+ assertThat(value).isEqualTo("directly served");
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
index 1cdca1b..de23ef4 100644
--- a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
+++ b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
@@ -101,6 +101,11 @@
}
@Override
+ public Object getCacheKey() {
+ return new Object();
+ }
+
+ @Override
public boolean isIdentifiedUser() {
return true;
}
diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
index 64f9392..81cb732 100644
--- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java
+++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
@@ -1198,6 +1198,11 @@
}
@Override
+ public Object getCacheKey() {
+ return new Object();
+ }
+
+ @Override
public Optional<String> getUserName() {
return Optional.ofNullable(username);
}
diff --git a/plugins/replication b/plugins/replication
index 63fb2b4..a6a6ec5 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 63fb2b4ba85380d798acbdc076e8673353507569
+Subproject commit a6a6ec5982e41a0ee9bfe24a46be96d4f13fcaaa
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
index 90dac88..78b6cda 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
@@ -29,6 +29,15 @@
removeScrollLock,
} from '@polymer/iron-overlay-behavior/iron-scroll-manager';
+interface ShowAlertEventDetail {
+ message: string;
+ dismissOnNavigation?: boolean;
+}
+
+interface ReloadEventDetail {
+ clearPatchset?: boolean;
+}
+
const HOVER_CLASS = 'hovered';
const HIDE_CLASS = 'hide';
@@ -193,6 +202,19 @@
* Hovercard elements are created outside of <gr-app>, so if you want to fire
* events, then you probably want to do that through the target element.
*/
+
+ dispatchEventThroughTarget(eventName: string): void;
+
+ dispatchEventThroughTarget(
+ eventName: 'show-alert',
+ detail: ShowAlertEventDetail
+ ): void;
+
+ dispatchEventThroughTarget(
+ eventName: 'reload',
+ detail: ReloadEventDetail
+ ): void;
+
dispatchEventThroughTarget(eventName: string, detail?: unknown) {
if (!detail) detail = {};
if (this._target)
diff --git a/polygerrit-ui/app/test/common-test-setup.js b/polygerrit-ui/app/test/common-test-setup.js
index 500187a..eead4f8 100644
--- a/polygerrit-ui/app/test/common-test-setup.js
+++ b/polygerrit-ui/app/test/common-test-setup.js
@@ -14,7 +14,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
+// This should be the first import to install handler before any other code
+import './source-map-support-install.js';
// TODO(dmfilippov): remove bundled-polymer.js imports when the following issue
// https://github.com/Polymer/polymer-resin/issues/9 is resolved.
import '../scripts/bundled-polymer.js';
diff --git a/polygerrit-ui/app/test/source-map-support-install.js b/polygerrit-ui/app/test/source-map-support-install.js
new file mode 100644
index 0000000..a8f147382
--- /dev/null
+++ b/polygerrit-ui/app/test/source-map-support-install.js
@@ -0,0 +1,20 @@
+/**
+ * @license
+ * Copyright (C) 2020 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.
+ */
+
+// The karma.conf.js file loads required module before any other modules
+// The source-map-support.js can't be imported with import ... statement
+window.sourceMapSupport.install();
diff --git a/polygerrit-ui/grep-patch-karma.js b/polygerrit-ui/grep-patch-karma.js
new file mode 100644
index 0000000..adf5171
--- /dev/null
+++ b/polygerrit-ui/grep-patch-karma.js
@@ -0,0 +1,47 @@
+/**
+ * @license
+ * Copyright (C) 2020 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.
+ */
+
+// The IntelliJ (and probably other IDEs) passes test names as a regexp in
+// the format:
+// --grep=/some regexp.../
+// But mochajs doesn't expect the '/' characters before and after the regexp.
+// The code below patches input args and removes '/' if they exists.
+function installPatch(karma) {
+ const originalKarmaStart = karma.start;
+
+ karma.start = function(config, ...args) {
+ const regexpGrepPrefix = '--grep=/';
+ const regexpGrepSuffix = '/';
+ if (config && config.args) {
+ for (let i = 0; i < config.args.length; i++) {
+ const arg = config.args[i];
+ if (arg.startsWith(regexpGrepPrefix) && arg.endsWith(regexpGrepSuffix)) {
+ const regexpText = arg.slice(regexpGrepPrefix.length, -regexpGrepPrefix.length);
+ config.args[i] = '--grep=' + regexpText;
+ }
+ }
+ }
+ originalKarmaStart.apply(this, [config, ...args]);
+ }
+
+}
+
+const karma = window.__karma__;
+if (karma && karma.start && !karma.__grep_patch_installed__) {
+ karma.__grep_patch_installed__ = true;
+ installPatch(karma);
+}
diff --git a/polygerrit-ui/karma.conf.js b/polygerrit-ui/karma.conf.js
index 879a5c8..fb87675 100644
--- a/polygerrit-ui/karma.conf.js
+++ b/polygerrit-ui/karma.conf.js
@@ -43,6 +43,20 @@
}
}
+function runInIde() {
+ // A simple detection of IDE.
+ // Default browserNoActivityTimeout is 30 seconds. An IDE usually
+ // runs karma in background and send commands when a user wants to
+ // execute test. If interval between user executed tests is bigger than
+ // browserNoActivityTimeout, the IDE reports error and doesn't restart
+ // server.
+ // We want to increase browserNoActivityTimeout when tests run in IDE.
+ // Wd don't want to increase it in other cases, oterhise hanging tests
+ // can slow down CI.
+ return !runUnderBazel &&
+ process.argv.some(arg => arg.toLowerCase().contains('intellij'));
+}
+
module.exports = function(config) {
const localDirName = path.resolve(__dirname, '../.ts-out/polygerrit-ui/app');
const rootDir = runUnderBazel ?
@@ -58,7 +72,10 @@
const testFilesPattern = (typeof config.testFiles == 'string') ?
testFilesLocationPattern + config.testFiles :
testFilesLocationPattern + '*_test.js';
+ // Special patch for grep parameters (see details in the grep-patch-karam.js)
+ const additionalFiles = runUnderBazel ? [] : ['polygerrit-ui/grep-patch-karma.js'];
config.set({
+ browserNoActivityTimeout: runInIde ? 60 * 60 * 1000 : 30 * 1000,
// base path that will be used to resolve all patterns (eg. files, exclude)
basePath: '../',
plugins: [
@@ -76,6 +93,8 @@
// list of files / patterns to load in the browser
files: [
+ ...additionalFiles,
+ getUiDevNpmFilePath('source-map-support/browser-source-map-support.js'),
getUiDevNpmFilePath('accessibility-developer-tools/dist/js/axs_testing.js'),
getUiDevNpmFilePath('sinon/pkg/sinon.js'),
{ pattern: testFilesPattern, type: 'module' },
diff --git a/polygerrit-ui/package.json b/polygerrit-ui/package.json
index 7de55aa..91b8579 100644
--- a/polygerrit-ui/package.json
+++ b/polygerrit-ui/package.json
@@ -15,7 +15,8 @@
"karma-mocha-reporter": "^2.2.5",
"lodash": "^4.17.15",
"mocha": "7.2.0",
- "sinon": "^9.0.2"
+ "sinon": "^9.0.2",
+ "source-map-support": "^0.5.19"
},
"license": "Apache-2.0",
"private": true
diff --git a/polygerrit-ui/yarn.lock b/polygerrit-ui/yarn.lock
index dfc5a43..a70ded8 100644
--- a/polygerrit-ui/yarn.lock
+++ b/polygerrit-ui/yarn.lock
@@ -3741,7 +3741,7 @@
socket.io-client "2.1.1"
socket.io-parser "~3.2.0"
-source-map-support@~0.5.12:
+source-map-support@^0.5.19, source-map-support@~0.5.12:
version "0.5.19"
resolved "https://registry.yarnpkg.com/source-map-support/-/source-map-support-0.5.19.tgz#a98b62f86dcaf4f67399648c085291ab9e8fed61"
integrity sha512-Wonm7zOCIJzBGQdB+thsPar0kYuCIzYvxZwlBa87yi/Mdjv7Tip2cyVbLj5o0cFPN4EVkuTwb3GDDyUx2DGnGw==
