Merge "Introduce a diff model"
diff --git a/java/com/google/gerrit/entities/RefNames.java b/java/com/google/gerrit/entities/RefNames.java
index 9745fc5..66ffa42 100644
--- a/java/com/google/gerrit/entities/RefNames.java
+++ b/java/com/google/gerrit/entities/RefNames.java
@@ -185,6 +185,21 @@
return ref.startsWith(REFS_CHANGES);
}
+ /** True if the provided ref is in {@code refs/sequences/*}. */
+ public static boolean isSequenceRef(String ref) {
+ return ref.startsWith(REFS_SEQUENCES);
+ }
+
+ /** True if the provided ref is in {@code refs/tags/*}. */
+ public static boolean isTagRef(String ref) {
+ return ref.startsWith(REFS_TAGS);
+ }
+
+ /** True if the provided ref is {@link REFS_EXTERNAL_IDS}. */
+ public static boolean isExternalIdRef(String ref) {
+ return REFS_EXTERNAL_IDS.equals(ref);
+ }
+
public static String refsGroups(AccountGroup.UUID groupUuid) {
return REFS_GROUPS + shardUuid(groupUuid.get());
}
@@ -330,6 +345,21 @@
return REFS_CONFIG.equals(ref);
}
+ /** Whether the ref is the version branch, i.e. {@code refs/meta/version}. */
+ public static boolean isVersionRef(String ref) {
+ return REFS_VERSION.equals(ref);
+ }
+
+ /** Whether the ref is an auto-merge ref. */
+ public static boolean isAutoMergeRef(String ref) {
+ return ref.startsWith(REFS_CACHE_AUTOMERGE);
+ }
+
+ /** Whether the ref is an reject commit ref, i.e. {@code refs/meta/reject-commits} */
+ public static boolean isRejectCommitsRef(String ref) {
+ return REFS_REJECT_COMMITS.equals(ref);
+ }
+
/**
* Whether the ref is managed by Gerrit. Covers all Gerrit-internal refs like refs/cache-automerge
* and refs/meta as well as refs/changes. Does not cover user-created refs like branches or custom
diff --git a/java/com/google/gerrit/server/update/context/RefUpdateContext.java b/java/com/google/gerrit/server/update/context/RefUpdateContext.java
new file mode 100644
index 0000000..d1c5ff8
--- /dev/null
+++ b/java/com/google/gerrit/server/update/context/RefUpdateContext.java
@@ -0,0 +1,170 @@
+// Copyright (C) 2023 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.update.context;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.ImmutableList;
+import java.util.ArrayDeque;
+import java.util.Deque;
+
+/**
+ * Passes additional information about an operation to the {@link BatchRefUpdate#execute} method.
+ *
+ * <p>To pass the additional information {@link RefUpdateContext}, wraps a code into an open
+ * RefUpdateContext, e.g.:
+ *
+ * <pre>{@code
+ * try(RefUpdateContext ctx = RefUpdateContext.open(RefUpdateType.CHANGE_MODIFICATION)) {
+ * ...
+ * // some code which modifies a ref using BatchRefUpdate.execute method
+ * }
+ * }</pre>
+ *
+ * When the {@link BatchRefUpdate#execute} method is executed, it can get all opened contexts and
+ * use it for an additional actions, e.g. it can put it in the reflog.
+ *
+ * <p>The information provided by this class is used internally in google.
+ *
+ * <p>The InMemoryRepositoryManager file makes some validation to ensure that RefUpdateContext is
+ * used correctly within the code (see thee validateRefUpdateContext method).
+ *
+ * <p>The class includes only operations from open-source gerrit and can be extended (see {@link
+ * TestActionRefUpdateContext} for example how to extend it).
+ */
+public class RefUpdateContext implements AutoCloseable {
+ private static final ThreadLocal<Deque<RefUpdateContext>> current = new ThreadLocal<>();
+
+ /**
+ * List of possible ref-update types.
+ *
+ * <p>Items in this enum are not fine-grained; different actions are shared the same type (e.g.
+ * {@link #CHANGE_MODIFICATION} includes posting comments, change edits and attention set update).
+ *
+ * <p>It is expected, that each type of operation should include only specific ref(s); check the
+ * validateRefUpdateContext in InMemoryRepositoryManager for relation between RefUpdateType and
+ * ref name.
+ */
+ public enum RefUpdateType {
+ /**
+ * Indicates that the context is implemented as a descendant of the {@link RefUpdateContext} .
+ *
+ * <p>The {@link #getUpdateType()} returns this type for all descendant of {@link
+ * RefUpdateContext}. This type is never returned if the context is exactly {@link
+ * RefUpdateContext}.
+ */
+ OTHER,
+ /**
+ * A ref is updated as a part of change-related operation.
+ *
+ * <p>This covers multiple different cases - creating and uploading changes and patchsets,
+ * comments operations, change edits, etc...
+ */
+ CHANGE_MODIFICATION,
+ /** A ref is updated during merge-change operation. */
+ MERGE_CHANGE,
+ /** A ref is updated as a part of a repo sequence operation. */
+ REPO_SEQ,
+ /** A ref is updated as a part of a repo initialization. */
+ INIT_REPO,
+ /** A ref is udpated as a part of gpg keys modification. */
+ GPG_KEYS_MODIFICATION,
+ /** A ref is updated as a part of group(s) update */
+ GROUPS_UPDATE,
+ /** A ref is updated as a part of account(s) update. */
+ ACCOUNTS_UPDATE,
+ /** A ref is updated as a part of direct push. */
+ DIRECT_PUSH,
+ /** A ref is updated as a part of explicit branch update operation. */
+ BRANCH_MODIFICATION,
+ /** A ref is updated as a part of explicit tag update operation. */
+ TAG_MODIFICATION,
+ /**
+ * A tag is updated as a part of an offline operation.
+ *
+ * <p>Offline operation - an operation which is executed separately from the gerrit server and
+ * can't be triggered by any gerrit API. E.g. schema update.
+ */
+ OFFLINE_OPERATION,
+ /** A tag is updated as a part of an update-superproject flow. */
+ UPDATE_SUPERPROJECT,
+ /** A ref is updated as a part of explicit HEAD update operation. */
+ HEAD_MODIFICATION,
+ /** A ref is updated as a part of versioned meta data change. */
+ VERSIONED_META_DATA_CHANGE,
+ /** A ref is updated as a part of commit-ban operation. */
+ BAN_COMMIT
+ }
+
+ /** Opens a provided context. */
+ protected static <T extends RefUpdateContext> T open(T ctx) {
+ getCurrent().addLast(ctx);
+ return ctx;
+ }
+
+ /** Opens a context of a give type. */
+ public static RefUpdateContext open(RefUpdateType updateType) {
+ checkArgument(updateType != RefUpdateType.OTHER, "The OTHER type is for internal use only.");
+ return open(new RefUpdateContext(updateType));
+ }
+
+ /** Returns the list of opened contexts; the first element is the outermost context. */
+ public static ImmutableList<RefUpdateContext> getOpenedContexts() {
+ return ImmutableList.copyOf(getCurrent());
+ }
+
+ /** Checks if there is an open context of the given type. */
+ public static boolean hasOpen(RefUpdateType type) {
+ return getCurrent().stream().anyMatch(ctx -> ctx.getUpdateType() == type);
+ }
+
+ private final RefUpdateType updateType;
+
+ private RefUpdateContext(RefUpdateType updateType) {
+ this.updateType = updateType;
+ }
+
+ protected RefUpdateContext() {
+ this(RefUpdateType.OTHER);
+ }
+
+ protected static final Deque<RefUpdateContext> getCurrent() {
+ Deque<RefUpdateContext> result = current.get();
+ if (result == null) {
+ result = new ArrayDeque<>();
+ current.set(result);
+ }
+ return result;
+ }
+
+ /**
+ * Returns the type of {@link RefUpdateContext}.
+ *
+ * <p>For descendants, always return {@link RefUpdateType#OTHER}
+ */
+ public final RefUpdateType getUpdateType() {
+ return updateType;
+ }
+
+ /** Closes the current context. */
+ @Override
+ public void close() {
+ Deque<RefUpdateContext> openedContexts = getCurrent();
+ checkState(
+ openedContexts.peekLast() == this, "The current context is different from this context.");
+ openedContexts.removeLast();
+ }
+}
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index e5234fe..fb9e64e 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -5,7 +5,10 @@
testonly = True,
srcs = glob(
["**/*.java"],
- exclude = ["AssertableExecutorService.java"],
+ exclude = [
+ "AssertableExecutorService.java",
+ "TestActionRefUpdateContext.java",
+ ],
),
visibility = ["//visibility:public"],
exports = [
@@ -40,6 +43,7 @@
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/server/util/time",
+ "//java/com/google/gerrit/testing:test-ref-update-context",
"//lib:guava",
"//lib:h2",
"//lib:jgit",
@@ -66,3 +70,14 @@
"//lib/truth",
],
)
+
+java_library(
+ name = "test-ref-update-context",
+ testonly = True,
+ srcs = ["TestActionRefUpdateContext.java"],
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/server",
+ "//lib/errorprone:annotations",
+ ],
+)
diff --git a/java/com/google/gerrit/testing/InMemoryRepositoryManager.java b/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
index 2051ae3..0f70103 100644
--- a/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
+++ b/java/com/google/gerrit/testing/InMemoryRepositoryManager.java
@@ -14,20 +14,55 @@
package com.google.gerrit.testing;
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.ACCOUNTS_UPDATE;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.BAN_COMMIT;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.BRANCH_MODIFICATION;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.DIRECT_PUSH;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.GPG_KEYS_MODIFICATION;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.GROUPS_UPDATE;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.HEAD_MODIFICATION;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.INIT_REPO;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.MERGE_CHANGE;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.OFFLINE_OPERATION;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.REPO_SEQ;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.TAG_MODIFICATION;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.VERSIONED_META_DATA_CHANGE;
+import static java.util.stream.Collectors.toList;
+
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.gpg.PublicKeyStore;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.RepositoryCaseMismatchException;
+import com.google.gerrit.server.update.context.RefUpdateContext;
+import com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType;
import com.google.inject.Inject;
+import java.util.AbstractMap.SimpleImmutableEntry;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.NavigableSet;
+import java.util.Optional;
+import java.util.function.Predicate;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.internal.storage.dfs.DfsObjDatabase;
+import org.eclipse.jgit.internal.storage.dfs.DfsReftableBatchRefUpdate;
import org.eclipse.jgit.internal.storage.dfs.DfsRepository;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
/** Repository manager that uses in-memory repositories. */
public class InMemoryRepositoryManager implements GitRepositoryManager {
@@ -56,6 +91,141 @@
setPerformsAtomicTransactions(true);
}
+ /** Validates that a given ref is updated within the expected context. */
+ private static class RefUpdateContextValidator {
+ /**
+ * A configured singleton for ref context validation.
+ *
+ * <p>Each ref must match no more than 1 special ref from the list below. If ref is not
+ * matched to any special ref predicate, then it is checked against the standard rules - check
+ * the code of the {@link #validateRefUpdateContext} for details.
+ */
+ public static final RefUpdateContextValidator INSTANCE =
+ new RefUpdateContextValidator()
+ .addSpecialRef(RefNames::isSequenceRef, REPO_SEQ)
+ .addSpecialRef(RefNames.HEAD::equals, HEAD_MODIFICATION)
+ .addSpecialRef(RefNames::isRefsChanges, CHANGE_MODIFICATION, MERGE_CHANGE)
+ .addSpecialRef(RefNames::isAutoMergeRef, CHANGE_MODIFICATION)
+ .addSpecialRef(RefNames::isRefsEdit, CHANGE_MODIFICATION, MERGE_CHANGE)
+ .addSpecialRef(RefNames::isTagRef, TAG_MODIFICATION)
+ .addSpecialRef(RefNames::isRejectCommitsRef, BAN_COMMIT)
+ .addSpecialRef(
+ name -> RefNames.isRefsUsers(name) && !RefNames.isRefsEdit(name),
+ VERSIONED_META_DATA_CHANGE,
+ ACCOUNTS_UPDATE,
+ MERGE_CHANGE)
+ .addSpecialRef(
+ RefNames::isConfigRef,
+ VERSIONED_META_DATA_CHANGE,
+ BRANCH_MODIFICATION,
+ MERGE_CHANGE)
+ .addSpecialRef(RefNames::isExternalIdRef, VERSIONED_META_DATA_CHANGE, ACCOUNTS_UPDATE)
+ .addSpecialRef(PublicKeyStore.REFS_GPG_KEYS::equals, GPG_KEYS_MODIFICATION)
+ .addSpecialRef(RefNames::isRefsDraftsComments, CHANGE_MODIFICATION)
+ .addSpecialRef(RefNames::isRefsStarredChanges, CHANGE_MODIFICATION)
+ // A user can create a change for updating a group and then merge it.
+ // The GroupsIT#pushToGroupBranchForReviewForNonAllUsersRepoAndSubmit test verifies
+ // this scenario.
+ .addSpecialRef(RefNames::isGroupRef, GROUPS_UPDATE, MERGE_CHANGE);
+
+ private List<Entry<Predicate<String>, ImmutableList<RefUpdateType>>> specialRefs =
+ new ArrayList<>();
+
+ private RefUpdateContextValidator() {}
+
+ public void validateRefUpdateContext(ReceiveCommand cmd) {
+ if (TestActionRefUpdateContext.isOpen()
+ || RefUpdateContext.hasOpen(OFFLINE_OPERATION)
+ || RefUpdateContext.hasOpen(INIT_REPO)
+ || RefUpdateContext.hasOpen(DIRECT_PUSH)) {
+ // The action can touch any refs in these contexts.
+ return;
+ }
+
+ String refName = cmd.getRefName();
+
+ Optional<ImmutableList<RefUpdateType>> allowedRefUpdateTypes =
+ RefUpdateContextValidator.INSTANCE.getAllowedRefUpdateTypes(refName);
+
+ if (allowedRefUpdateTypes.isPresent()) {
+ checkState(
+ allowedRefUpdateTypes.get().stream().anyMatch(RefUpdateContext::hasOpen)
+ || isTestRepoCall(),
+ "Special ref '%s' is updated outside of the expected operation. Wrap code in the correct RefUpdateContext or fix allowed update types",
+ refName);
+ return;
+ }
+ // It is not one of the special ref - update is possible only within specific contexts.
+ checkState(
+ RefUpdateContext.hasOpen(MERGE_CHANGE)
+ || RefUpdateContext.hasOpen(RefUpdateType.BRANCH_MODIFICATION)
+ || RefUpdateContext.hasOpen(RefUpdateType.UPDATE_SUPERPROJECT)
+ || isTestRepoCall(),
+ "Ordinary ref '%s' is updated outside of the expected operation. Wrap code in the correct RefUpdateContext or add the ref as a special ref.",
+ refName);
+ }
+
+ private RefUpdateContextValidator addSpecialRef(
+ Predicate<String> refNamePredicate, RefUpdateType... validRefUpdateTypes) {
+ specialRefs.add(
+ new SimpleImmutableEntry<Predicate<String>, ImmutableList<RefUpdateType>>(
+ refNamePredicate, ImmutableList.copyOf(validRefUpdateTypes)));
+ return this;
+ }
+
+ private Optional<ImmutableList<RefUpdateType>> getAllowedRefUpdateTypes(String refName) {
+ List<ImmutableList<RefUpdateType>> allowedTypes =
+ specialRefs.stream()
+ .filter(entry -> entry.getKey().test(refName))
+ .map(Entry::getValue)
+ .collect(toList());
+ checkState(
+ allowedTypes.size() <= 1,
+ "refName matches more than 1 predicate. Please fix the specialRefs list, so each reference has no more than one match.");
+ if (allowedTypes.size() == 0) {
+ return Optional.empty();
+ }
+ return Optional.of(allowedTypes.get(0));
+ }
+
+ /**
+ * Returns true if a ref is updated using one of the method in {@link
+ * org.eclipse.jgit.junit.TestRepository}.
+ *
+ * <p>The {@link org.eclipse.jgit.junit.TestRepository} used only in tests and allows to
+ * change refs directly. Wrapping each usage in a test context requires a lot of modification,
+ * so instead we allow any ref updates, which are made using through this class.
+ */
+ private boolean isTestRepoCall() {
+ return Arrays.stream(Thread.currentThread().getStackTrace())
+ .anyMatch(elem -> elem.getClassName().equals("org.eclipse.jgit.junit.TestRepository"));
+ }
+ }
+
+ // The following line will be uncommented in the upcoming changes, after adding
+ // RefUpdateContext to the code.
+ static final boolean VALIDATE_REF_UPDATE_CONTEXT = false;
+
+ @Override
+ protected MemRefDatabase createRefDatabase() {
+ return new MemRefDatabase() {
+ @Override
+ public BatchRefUpdate newBatchUpdate() {
+ DfsObjDatabase odb = getRepository().getObjectDatabase();
+ return new DfsReftableBatchRefUpdate(this, odb) {
+ @Override
+ public void execute(RevWalk rw, ProgressMonitor pm, List<String> options) {
+ if (VALIDATE_REF_UPDATE_CONTEXT) {
+ getCommands().stream()
+ .forEach(RefUpdateContextValidator.INSTANCE::validateRefUpdateContext);
+ }
+ super.execute(rw, pm, options);
+ }
+ };
+ }
+ };
+ }
+
@Override
public Description getDescription() {
return (Description) super.getDescription();
diff --git a/java/com/google/gerrit/testing/TestActionRefUpdateContext.java b/java/com/google/gerrit/testing/TestActionRefUpdateContext.java
new file mode 100644
index 0000000..23ec9aa
--- /dev/null
+++ b/java/com/google/gerrit/testing/TestActionRefUpdateContext.java
@@ -0,0 +1,73 @@
+// Copyright (C) 2023 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.testing;
+
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import com.google.gerrit.server.update.context.RefUpdateContext;
+
+/**
+ * Marks ref updates as a test actions.
+ *
+ * <p>This class should be used in tests only to wrap a portion of test code which directly modifies
+ * references. Usage:
+ *
+ * <pre>{@code
+ * import static ...TestActionRefUpdateContext.openTestRefUpdateContext();
+ *
+ * try(RefUpdateContext ctx=openTestRefUpdateContext()) {
+ * // Some test code, which modifies a reference.
+ * }
+ * }</pre>
+ *
+ * or
+ *
+ * <pre>{@code
+ * import static ...TestActionRefUpdateContext.testRefAction;
+ *
+ * testRefAction(() -> {doSomethingWithRef()});
+ * T result = testRefAction(() -> { return doSomethingWithRef()});
+ * }</pre>
+ */
+public final class TestActionRefUpdateContext extends RefUpdateContext {
+ public static boolean isOpen() {
+ return getCurrent().stream().anyMatch(ctx -> ctx instanceof TestActionRefUpdateContext);
+ }
+
+ public static TestActionRefUpdateContext openTestRefUpdateContext() {
+ return open(new TestActionRefUpdateContext());
+ }
+
+ @CanIgnoreReturnValue
+ public static <V, E extends Exception> V testRefAction(CallableWithException<V, E> c) throws E {
+ try (RefUpdateContext ctx = openTestRefUpdateContext()) {
+ return c.call();
+ }
+ }
+
+ public static <E extends Exception> void testRefAction(RunnableWithException<E> c) throws E {
+ try (RefUpdateContext ctx = openTestRefUpdateContext()) {
+ c.run();
+ }
+ }
+
+ public interface CallableWithException<V, E extends Exception> {
+ V call() throws E;
+ }
+
+ @FunctionalInterface
+ public interface RunnableWithException<E extends Exception> {
+ void run() throws E;
+ }
+}
diff --git a/javatests/com/google/gerrit/server/update/context/BUILD b/javatests/com/google/gerrit/server/update/context/BUILD
new file mode 100644
index 0000000..e580595
--- /dev/null
+++ b/javatests/com/google/gerrit/server/update/context/BUILD
@@ -0,0 +1,14 @@
+load("//tools/bzl:junit.bzl", "junit_tests")
+
+junit_tests(
+ name = "update_context_tests",
+ size = "small",
+ srcs = glob(["*.java"]),
+ deps = [
+ "//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
+ "//java/com/google/gerrit/testing:test-ref-update-context",
+ "//lib/truth",
+ "//lib/truth:truth-java8-extension",
+ ],
+)
diff --git a/javatests/com/google/gerrit/server/update/context/RefUpdateContextTest.java b/javatests/com/google/gerrit/server/update/context/RefUpdateContextTest.java
new file mode 100644
index 0000000..178d67d
--- /dev/null
+++ b/javatests/com/google/gerrit/server/update/context/RefUpdateContextTest.java
@@ -0,0 +1,93 @@
+// Copyright (C) 2023 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.update.context;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.GROUPS_UPDATE;
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.INIT_REPO;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import org.junit.After;
+import org.junit.Test;
+
+public class RefUpdateContextTest {
+ @After
+ public void tearDown() {
+ // Each test should close all opened context to avoid interference with other tests.
+ assertThat(RefUpdateContext.getOpenedContexts()).isEmpty();
+ }
+
+ @Test
+ public void contextNotOpen() {
+ assertThat(RefUpdateContext.getOpenedContexts()).isEmpty();
+ assertThat(RefUpdateContext.hasOpen(INIT_REPO)).isFalse();
+ assertThat(RefUpdateContext.hasOpen(GROUPS_UPDATE)).isFalse();
+ }
+
+ @Test
+ public void singleContext_openedAndClosedCorrectly() {
+ try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ ImmutableList<RefUpdateContext> openedContexts = RefUpdateContext.getOpenedContexts();
+ assertThat(openedContexts).hasSize(1);
+ assertThat(openedContexts.get(0).getUpdateType()).isEqualTo(CHANGE_MODIFICATION);
+ assertThat(RefUpdateContext.hasOpen(CHANGE_MODIFICATION)).isTrue();
+ assertThat(RefUpdateContext.hasOpen(INIT_REPO)).isFalse();
+ }
+
+ assertThat(RefUpdateContext.getOpenedContexts()).isEmpty();
+ assertThat(RefUpdateContext.hasOpen(CHANGE_MODIFICATION)).isFalse();
+ }
+
+ @Test
+ public void nestedContext_openedAndClosedCorrectly() {
+ try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ try (RefUpdateContext nestedCtx = RefUpdateContext.open(INIT_REPO)) {
+ ImmutableList<RefUpdateContext> nestedOpenedContexts = RefUpdateContext.getOpenedContexts();
+ assertThat(nestedOpenedContexts).hasSize(2);
+ assertThat(nestedOpenedContexts.get(0).getUpdateType()).isEqualTo(CHANGE_MODIFICATION);
+ assertThat(nestedOpenedContexts.get(1).getUpdateType()).isEqualTo(INIT_REPO);
+ assertThat(RefUpdateContext.hasOpen(CHANGE_MODIFICATION)).isTrue();
+ assertThat(RefUpdateContext.hasOpen(INIT_REPO)).isTrue();
+ assertThat(RefUpdateContext.hasOpen(GROUPS_UPDATE)).isFalse();
+ }
+ ImmutableList<RefUpdateContext> openedContexts = RefUpdateContext.getOpenedContexts();
+ assertThat(openedContexts).hasSize(1);
+ assertThat(openedContexts.get(0).getUpdateType()).isEqualTo(CHANGE_MODIFICATION);
+ assertThat(RefUpdateContext.hasOpen(CHANGE_MODIFICATION)).isTrue();
+ assertThat(RefUpdateContext.hasOpen(INIT_REPO)).isFalse();
+ assertThat(RefUpdateContext.hasOpen(GROUPS_UPDATE)).isFalse();
+ }
+
+ assertThat(RefUpdateContext.getOpenedContexts()).isEmpty();
+ assertThat(RefUpdateContext.hasOpen(CHANGE_MODIFICATION)).isFalse();
+ assertThat(RefUpdateContext.hasOpen(INIT_REPO)).isFalse();
+ assertThat(RefUpdateContext.hasOpen(GROUPS_UPDATE)).isFalse();
+ }
+
+ @Test
+ public void incorrectCloseOrder_exceptionThrown() {
+ try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ try (RefUpdateContext nestedCtx = RefUpdateContext.open(INIT_REPO)) {
+ assertThrows(Exception.class, () -> ctx.close());
+ ImmutableList<RefUpdateContext> openedContexts = RefUpdateContext.getOpenedContexts();
+ assertThat(openedContexts).hasSize(2);
+ assertThat(RefUpdateContext.hasOpen(CHANGE_MODIFICATION)).isTrue();
+ assertThat(RefUpdateContext.hasOpen(INIT_REPO)).isTrue();
+ }
+ }
+ }
+}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-page.ts b/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
index 3af8207..1d2a272 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-page.ts
@@ -280,6 +280,7 @@
export class PageContext {
/**
* Includes everything: base, path, query and hash.
+ * NOT decoded.
*/
canonicalPath = '';
@@ -287,18 +288,21 @@
* Does not include base path.
* Does not include hash.
* Includes query string.
+ * NOT decoded.
*/
path = '';
- /** Does not include hash. */
+ /** Decoded. Does not include hash. */
querystring = '';
+ /** Decoded. */
hash = '';
/**
* Regular expression matches of capturing groups. The first entry params[0]
* corresponds to the first capturing group. The entire matched string is not
* returned in this array.
+ * Each param is double decoded.
*/
params: string[] = [];
@@ -346,17 +350,24 @@
replaceState() {
window.history.replaceState(this.state, this.title, this.canonicalPath);
}
+
+ match(re: RegExp) {
+ const qsIndex = this.path.indexOf('?');
+ const pathname = qsIndex !== -1 ? this.path.slice(0, qsIndex) : this.path;
+ const matches = re.exec(decodeURIComponent(pathname));
+ if (matches) {
+ this.params = matches
+ .slice(1)
+ .map(match => decodeURIComponentString(match));
+ }
+ return !!matches;
+ }
}
function createRoute(re: RegExp, fn: Function) {
return (ctx: PageContext, next: Function) => {
- const qsIndex = ctx.path.indexOf('?');
- const pathname = qsIndex !== -1 ? ctx.path.slice(0, qsIndex) : ctx.path;
- const matches = re.exec(decodeURIComponent(pathname));
+ const matches = ctx.match(re);
if (matches) {
- ctx.params = matches
- .slice(1)
- .map(match => decodeURIComponentString(match));
fn(ctx, next);
} else {
next();
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 997d9d5..da4a576 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -14,7 +14,6 @@
import {assertIsDefined} from '../../../utils/common-util';
import {
BasePatchSetNum,
- DashboardId,
GroupId,
NumericChangeId,
RevisionPatchSetNum,
@@ -73,6 +72,7 @@
import {
DashboardViewModel,
DashboardViewState,
+ PROJECT_DASHBOARD_ROUTE,
} from '../../../models/views/dashboard';
import {
SettingsViewModel,
@@ -107,7 +107,6 @@
DASHBOARD: /^\/dashboard\/(.+)$/,
CUSTOM_DASHBOARD: /^\/dashboard\/?$/,
- PROJECT_DASHBOARD: /^\/p\/(.+)\/\+\/dashboard\/(.+)/,
LEGACY_PROJECT_DASHBOARD: /^\/projects\/(.+),dashboards\/(.+)/,
AGREEMENTS: /^\/settings\/agreements\/?/,
@@ -635,10 +634,10 @@
ctx => this.handleCustomDashboardRoute(ctx)
);
- this.mapRoute(
- RoutePattern.PROJECT_DASHBOARD,
- 'handleProjectDashboardRoute',
- ctx => this.handleProjectDashboardRoute(ctx)
+ this.mapRouteState(
+ PROJECT_DASHBOARD_ROUTE,
+ this.dashboardViewModel,
+ 'handleProjectDashboardRoute'
);
this.mapRoute(
@@ -1008,19 +1007,6 @@
return Promise.resolve();
}
- handleProjectDashboardRoute(ctx: PageContext) {
- const project = ctx.params[0] as RepoName;
- const state: DashboardViewState = {
- view: GerritView.DASHBOARD,
- project,
- dashboard: decodeURIComponent(ctx.params[1]) as DashboardId,
- };
- // Note that router model view must be updated before view models.
- this.setState(state);
- this.dashboardViewModel.setState(state);
- this.reporting.setRepoName(project);
- }
-
handleLegacyProjectDashboardRoute(ctx: PageContext) {
this.redirect('/p/' + ctx.params[0] + '/+/dashboard/' + ctx.params[1]);
}
diff --git a/polygerrit-ui/app/elements/gr-app-global-var-init.ts b/polygerrit-ui/app/elements/gr-app-global-var-init.ts
index da9c0c9..d66fec0 100644
--- a/polygerrit-ui/app/elements/gr-app-global-var-init.ts
+++ b/polygerrit-ui/app/elements/gr-app-global-var-init.ts
@@ -14,11 +14,27 @@
import {GrAnnotation} from '../embed/diff/gr-diff-highlight/gr-annotation';
import {GrPluginActionContext} from './shared/gr-js-api-interface/gr-plugin-action-context';
import {AppContext, injectAppContext} from '../services/app-context';
-import {Finalizable} from '../services/registry';
import {PluginLoader} from './shared/gr-js-api-interface/gr-plugin-loader';
+import {
+ initVisibilityReporter,
+ initPerformanceReporter,
+ initErrorReporter,
+ initWebVitals,
+} from '../services/gr-reporting/gr-reporting_impl';
+import {Finalizable} from '../services/registry';
-export function initGlobalVariables(appContext: AppContext & Finalizable) {
+export function initGlobalVariables(
+ appContext: AppContext & Finalizable,
+ initializeReporting: boolean
+) {
injectAppContext(appContext);
+ if (initializeReporting) {
+ const reportingService = appContext.reportingService;
+ initVisibilityReporter(reportingService);
+ initPerformanceReporter(reportingService);
+ initWebVitals(reportingService);
+ initErrorReporter(reportingService);
+ }
window.GrAnnotation = GrAnnotation;
window.GrPluginActionContext = GrPluginActionContext;
}
diff --git a/polygerrit-ui/app/elements/gr-app.ts b/polygerrit-ui/app/elements/gr-app.ts
index 645f94a..ded0626 100644
--- a/polygerrit-ui/app/elements/gr-app.ts
+++ b/polygerrit-ui/app/elements/gr-app.ts
@@ -37,12 +37,6 @@
createAppDependencies,
Creator,
} from '../services/app-context-init';
-import {
- initVisibilityReporter,
- initPerformanceReporter,
- initErrorReporter,
- initWebVitals,
-} from '../services/gr-reporting/gr-reporting_impl';
import {html, LitElement} from 'lit';
import {customElement} from 'lit/decorators.js';
import {
@@ -50,14 +44,9 @@
serviceWorkerInstallerToken,
} from '../services/service-worker-installer';
import {pluginLoaderToken} from './shared/gr-js-api-interface/gr-plugin-loader';
+import {getAppContext} from '../services/app-context';
-const appContext = createAppContext();
-initGlobalVariables(appContext);
-const reportingService = appContext.reportingService;
-initVisibilityReporter(reportingService);
-initPerformanceReporter(reportingService);
-initWebVitals(reportingService);
-initErrorReporter(reportingService);
+initGlobalVariables(createAppContext(), true);
installPolymerResin(safeTypesBridge);
@@ -97,7 +86,7 @@
};
for (const [token, creator] of createAppDependencies(
- appContext,
+ getAppContext(),
resolver
)) {
injectDependency(token, creator);
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
index cf279ba..fcebeea 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text_test.ts
@@ -76,12 +76,12 @@
element,
/* HTML */ `
<pre class="plaintext">
- http://google.com/<a
+ <a
href="http://google.com/LinkRewriteMe"
rel="noopener"
target="_blank"
>
- LinkRewriteMe
+ http://google.com/LinkRewriteMe
</a>
</pre>
`
@@ -159,8 +159,15 @@
element,
/* HTML */ `
<pre class="plaintext">
- text with plain link: http://google.com
- text with config link:
+ text with plain link:
+ <a
+ href="http://google.com"
+ rel="noopener"
+ target="_blank"
+ >
+ http://google.com
+ </a>
+ text with config link:
<a
href="http://google.com/LinkRewriteMe"
rel="noopener"
@@ -269,7 +276,14 @@
/* HTML */ `
<pre class="plaintext">
text
- text with plain link: http://google.com
+ text with plain link:
+ <a
+ href="http://google.com"
+ rel="noopener"
+ target="_blank"
+ >
+ http://google.com
+ </a>
text with config link:
<a
href="http://google.com/LinkRewriteMe"
diff --git a/polygerrit-ui/app/models/views/admin_test.ts b/polygerrit-ui/app/models/views/admin_test.ts
index b6089af..0881018 100644
--- a/polygerrit-ui/app/models/views/admin_test.ts
+++ b/polygerrit-ui/app/models/views/admin_test.ts
@@ -3,28 +3,34 @@
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
-import {assert} from '@open-wc/testing';
-import {PageContext} from '../../elements/core/gr-router/gr-page';
import {GerritView} from '../../services/router/router-model';
import '../../test/common-test-setup';
-import {AdminChildView, PLUGIN_LIST_ROUTE} from './admin';
+import {assertRouteFalse, assertRouteState} from '../../test/test-utils';
+import {
+ AdminChildView,
+ AdminViewState,
+ createAdminUrl,
+ PLUGIN_LIST_ROUTE,
+} from './admin';
suite('admin view model', () => {
suite('routes', () => {
test('PLUGIN_LIST', () => {
- const {urlPattern: pattern, createState} = PLUGIN_LIST_ROUTE;
+ assertRouteFalse(PLUGIN_LIST_ROUTE, 'admin/plugins');
+ assertRouteFalse(PLUGIN_LIST_ROUTE, '//admin/plugins');
+ assertRouteFalse(PLUGIN_LIST_ROUTE, '//admin/plugins?');
+ assertRouteFalse(PLUGIN_LIST_ROUTE, '/admin/plugins//');
- assert.isTrue(pattern.test('/admin/plugins'));
- assert.isTrue(pattern.test('/admin/plugins/'));
- assert.isFalse(pattern.test('admin/plugins'));
- assert.isFalse(pattern.test('//admin/plugins'));
- assert.isFalse(pattern.test('//admin/plugins?'));
- assert.isFalse(pattern.test('/admin/plugins//'));
-
- assert.deepEqual(createState(new PageContext('')), {
+ const state: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.PLUGINS,
- });
+ };
+ assertRouteState<AdminViewState>(
+ PLUGIN_LIST_ROUTE,
+ '/admin/plugins',
+ state,
+ createAdminUrl
+ );
});
});
});
diff --git a/polygerrit-ui/app/models/views/dashboard.ts b/polygerrit-ui/app/models/views/dashboard.ts
index 74523db..d2e7995 100644
--- a/polygerrit-ui/app/models/views/dashboard.ts
+++ b/polygerrit-ui/app/models/views/dashboard.ts
@@ -10,7 +10,21 @@
import {encodeURL, getBaseUrl} from '../../utils/url-util';
import {define} from '../dependency';
import {Model} from '../model';
-import {ViewState} from './base';
+import {Route, ViewState} from './base';
+
+export const PROJECT_DASHBOARD_ROUTE: Route<DashboardViewState> = {
+ urlPattern: /^\/p\/(.+)\/\+\/dashboard\/(.+)/,
+ createState: ctx => {
+ const project = (ctx.params[0] ?? '') as RepoName;
+ const dashboard = (ctx.params[1] ?? '') as DashboardId;
+ const state: DashboardViewState = {
+ view: GerritView.DASHBOARD,
+ project,
+ dashboard,
+ };
+ return state;
+ },
+};
export interface DashboardViewState extends ViewState {
view: GerritView.DASHBOARD;
diff --git a/polygerrit-ui/app/models/views/dashboard_test.ts b/polygerrit-ui/app/models/views/dashboard_test.ts
index a7620dd..9509977 100644
--- a/polygerrit-ui/app/models/views/dashboard_test.ts
+++ b/polygerrit-ui/app/models/views/dashboard_test.ts
@@ -5,11 +5,36 @@
*/
import {assert} from '@open-wc/testing';
import {RepoName} from '../../api/rest-api';
+import {GerritView} from '../../services/router/router-model';
import '../../test/common-test-setup';
+import {assertRouteFalse, assertRouteState} from '../../test/test-utils';
import {DashboardId} from '../../types/common';
-import {createDashboardUrl} from './dashboard';
+import {
+ createDashboardUrl,
+ DashboardViewState,
+ PROJECT_DASHBOARD_ROUTE,
+} from './dashboard';
suite('dashboard view state tests', () => {
+ suite('routes', () => {
+ test('PROJECT_DASHBOARD_ROUTE', () => {
+ assertRouteFalse(PROJECT_DASHBOARD_ROUTE, '/p//+/dashboard/qwer');
+ assertRouteFalse(PROJECT_DASHBOARD_ROUTE, '/p/asdf/+/dashboard/');
+
+ const state: DashboardViewState = {
+ view: GerritView.DASHBOARD,
+ project: 'asdf' as RepoName,
+ dashboard: 'qwer' as DashboardId,
+ };
+ assertRouteState(
+ PROJECT_DASHBOARD_ROUTE,
+ '/p/asdf/+/dashboard/qwer',
+ state,
+ createDashboardUrl
+ );
+ });
+ });
+
suite('createDashboardUrl()', () => {
test('self dashboard', () => {
assert.equal(createDashboardUrl({}), '/dashboard/self');
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index aed58d8..365bb16 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -6,7 +6,7 @@
// 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';
-import {AppContext} from '../services/app-context';
+import {getAppContext} from '../services/app-context';
import {Finalizable} from '../services/registry';
import {
createTestAppContext,
@@ -60,7 +60,6 @@
});
let testSetupTimestampMs = 0;
-let appContext: AppContext & Finalizable;
const injectedDependencies: Map<
DependencyToken<unknown>,
@@ -101,11 +100,10 @@
// If the following asserts fails - then window.stub is
// overwritten by some other code.
assert.equal(getCleanupsCount(), 0);
- appContext = createTestAppContext();
- initGlobalVariables(appContext);
+ initGlobalVariables(createTestAppContext(), false);
- finalizers.push(appContext);
- const dependencies = createTestDependencies(appContext, testResolver);
+ finalizers.push(getAppContext());
+ const dependencies = createTestDependencies(getAppContext(), testResolver);
for (const [token, provider] of dependencies) {
injectDependency(token, provider);
}
@@ -124,7 +122,7 @@
// `awaitPluginsLoaded` will rely on that to kick off,
// in testing, we want to kick start this earlier.
testResolver(pluginLoaderToken).loadPlugins([]);
- testOnlyResetGrRestApiSharedObjects(appContext.authService);
+ testOnlyResetGrRestApiSharedObjects(getAppContext().authService);
});
export function removeRequestDependencyListener() {
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index c400d9c..19c3a7b 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -14,6 +14,8 @@
import {Observable} from 'rxjs';
import {filter, take, timeout} from 'rxjs/operators';
import {assert} from '@open-wc/testing';
+import {Route, ViewState} from '../models/views/base';
+import {PageContext} from '../elements/core/gr-router/gr-page';
export {query, queryAll, queryAndAssert} from '../utils/common-util';
export interface MockPromise<T> extends Promise<T> {
@@ -328,3 +330,26 @@
};
return new Proxy(obj, handler) as unknown as T;
}
+
+export function assertRouteState<T extends ViewState>(
+ route: Route<T>,
+ path: string,
+ state: T,
+ createUrl: (state: T) => string
+) {
+ const {urlPattern, createState} = route;
+ const ctx = new PageContext(path);
+ const matches = ctx.match(urlPattern);
+ assert.isTrue(matches);
+ assert.deepEqual(createState(ctx), state);
+ assert.equal(path, createUrl(state));
+}
+
+export function assertRouteFalse<T extends ViewState>(
+ route: Route<T>,
+ path: string
+) {
+ const ctx = new PageContext(path);
+ const matches = ctx.match(route.urlPattern);
+ assert.isFalse(matches);
+}
diff --git a/polygerrit-ui/app/utils/link-util.ts b/polygerrit-ui/app/utils/link-util.ts
index 48e9c07..ccafa5a 100644
--- a/polygerrit-ui/app/utils/link-util.ts
+++ b/polygerrit-ui/app/utils/link-util.ts
@@ -33,6 +33,11 @@
commentLinkInfo =>
commentLinkInfo.enabled !== false && commentLinkInfo.link !== undefined
);
+ // Always linkify URLs starting with https?://
+ enabledRewrites.push({
+ match: '(https?://\\S+[\\w/])',
+ link: '$1',
+ });
return enabledRewrites.flatMap(rewrite => {
const regexp = new RegExp(rewrite.match, 'g');
const partialResults: RewriteResult[] = [];
diff --git a/polygerrit-ui/app/utils/link-util_test.ts b/polygerrit-ui/app/utils/link-util_test.ts
index e4e719b..52b8288 100644
--- a/polygerrit-ui/app/utils/link-util_test.ts
+++ b/polygerrit-ui/app/utils/link-util_test.ts
@@ -12,6 +12,17 @@
}
suite('link rewrites', () => {
+ test('default linking', () => {
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('http://www.google.com', {}),
+ link('http://www.google.com', 'http://www.google.com')
+ );
+ assert.equal(
+ linkifyUrlsAndApplyRewrite('https://www.google.com', {}),
+ link('https://www.google.com', 'https://www.google.com')
+ );
+ });
+
test('without text', () => {
assert.equal(
linkifyUrlsAndApplyRewrite('foo', {
