Merge "Disable the extra font loading for paper-styles"
diff --git a/.bazelversion b/.bazelversion
index 25939d3..3eefcb9 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-0.29.1
+1.0.0
diff --git a/Documentation/user-request-tracing.txt b/Documentation/user-request-tracing.txt
index d422dd9..b26f4c1 100644
--- a/Documentation/user-request-tracing.txt
+++ b/Documentation/user-request-tracing.txt
@@ -22,12 +22,19 @@
`--trace` option. More information about this can be found in
the link:cmd-index.html#trace[Trace] section of the
link:cmd-index.html[SSH command documentation].
-* Git: For Git pushes tracing can be enabled by setting the
- `trace` push option, the trace ID is returned in the command output.
- More information about this can be found in
- the link:user-upload.html#trace[Trace] section of the
- link:user-upload.html[upload documentation]. Tracing for Git requests
- other than Git push is not supported.
+* Git Push (requires usage of git protocol v2): For Git pushes tracing
+ can be enabled by setting the `trace` push option, the trace ID is
+ returned in the command output. More information about this can be
+ found in the link:user-upload.html#trace[Trace] section of the
+ link:user-upload.html[upload documentation].
+* Git Clone/Fetch/Ls-Remote (requires usage of git protocol v2): For
+ Git clone/fetch/ls-remote tracing can be enabled by setting the
+ `trace` server option. Use '-o trace=<TRACE-ID>' for `git fetch` and
+ `git ls-remote`, and '--server-option trace=<TRACE-ID>' for
+ `git clone`. If the `trace` server option is set without a value
+ (without trace ID) a trace ID is generated but the generated trace ID
+ is not returned to the client (hence a trace ID should always be
+ set).
When request tracing is enabled it is possible to provide an ID that
should be used as trace ID. If a trace ID is not provided a trace ID is
diff --git a/WORKSPACE b/WORKSPACE
index 7b843b6..0d54bbe 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -611,18 +611,18 @@
sha1 = "18d4d07010c24405129a6dbb0e92057f8779fb9d",
)
-AUTO_VALUE_VERSION = "1.6.6"
+AUTO_VALUE_VERSION = "1.7"
maven_jar(
name = "auto-value",
artifact = "com.google.auto.value:auto-value:" + AUTO_VALUE_VERSION,
- sha1 = "8831874cb2f4a9e1b196a2973f3cef695f5ce459",
+ sha1 = "fe8387764ed19460eda4f106849c664f51c07121",
)
maven_jar(
name = "auto-value-annotations",
artifact = "com.google.auto.value:auto-value-annotations:" + AUTO_VALUE_VERSION,
- sha1 = "9947ae63d8ec42ea159283baf2e5b9c0ff100909",
+ sha1 = "5be124948ebdc7807df68207f35a0f23ce427f29",
)
declare_nongoogle_deps()
@@ -743,8 +743,8 @@
# Keep this version of Soy synchronized with the version used in Gitiles.
maven_jar(
name = "soy",
- artifact = "com.google.template:soy:2019-09-03",
- sha1 = "40781da0302b4b5d53006dc8bd5a432c7288d807",
+ artifact = "com.google.template:soy:2019-10-08",
+ sha1 = "4518bf8bac2dbbed684849bc209c39c4cb546237",
)
maven_jar(
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index c3823cd..f9116a1 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -16,20 +16,29 @@
import com.google.gerrit.extensions.api.changes.ActionVisitor;
import com.google.gerrit.extensions.config.DownloadScheme;
+import com.google.gerrit.extensions.events.AccountIndexedListener;
import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.extensions.events.CommentAddedListener;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.extensions.events.GroupIndexedListener;
import com.google.gerrit.extensions.events.ProjectIndexedListener;
+import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.PrivateInternals_DynamicMapImpl;
import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.extensions.webui.FileHistoryWebLink;
+import com.google.gerrit.extensions.webui.PatchSetWebLink;
import com.google.gerrit.server.ExceptionHook;
+import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.change.ChangeETagComputation;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.git.validators.CommitValidationListener;
+import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
import com.google.gerrit.server.git.validators.RefOperationValidationListener;
import com.google.gerrit.server.logging.PerformanceLogger;
import com.google.gerrit.server.rules.SubmitRule;
+import com.google.gerrit.server.validators.AccountActivationValidationListener;
import com.google.gerrit.server.validators.ProjectCreationValidationListener;
import com.google.inject.Inject;
import com.google.inject.util.Providers;
@@ -37,7 +46,9 @@
import java.util.List;
public class ExtensionRegistry {
+ private final DynamicSet<AccountIndexedListener> accountIndexedListeners;
private final DynamicSet<ChangeIndexedListener> changeIndexedListeners;
+ private final DynamicSet<GroupIndexedListener> groupIndexedListeners;
private final DynamicSet<ProjectIndexedListener> projectIndexedListeners;
private final DynamicSet<CommitValidationListener> commitValidationListeners;
private final DynamicSet<ExceptionHook> exceptionHooks;
@@ -50,10 +61,20 @@
private final DynamicMap<DownloadScheme> downloadSchemes;
private final DynamicSet<RefOperationValidationListener> refOperationValidationListeners;
private final DynamicSet<CommentAddedListener> commentAddedListeners;
+ private final DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners;
+ private final DynamicSet<FileHistoryWebLink> fileHistoryWebLinks;
+ private final DynamicSet<PatchSetWebLink> patchSetWebLinks;
+ private final DynamicSet<RevisionCreatedListener> revisionCreatedListeners;
+ private final DynamicSet<GroupBackend> groupBackends;
+ private final DynamicSet<AccountActivationValidationListener>
+ accountActivationValidationListeners;
+ private final DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
@Inject
ExtensionRegistry(
+ DynamicSet<AccountIndexedListener> accountIndexedListeners,
DynamicSet<ChangeIndexedListener> changeIndexedListeners,
+ DynamicSet<GroupIndexedListener> groupIndexedListeners,
DynamicSet<ProjectIndexedListener> projectIndexedListeners,
DynamicSet<CommitValidationListener> commitValidationListeners,
DynamicSet<ExceptionHook> exceptionHooks,
@@ -65,8 +86,17 @@
DynamicSet<ActionVisitor> actionVisitors,
DynamicMap<DownloadScheme> downloadSchemes,
DynamicSet<RefOperationValidationListener> refOperationValidationListeners,
- DynamicSet<CommentAddedListener> commentAddedListeners) {
+ DynamicSet<CommentAddedListener> commentAddedListeners,
+ DynamicSet<GitReferenceUpdatedListener> refUpdatedListeners,
+ DynamicSet<FileHistoryWebLink> fileHistoryWebLinks,
+ DynamicSet<PatchSetWebLink> patchSetWebLinks,
+ DynamicSet<RevisionCreatedListener> revisionCreatedListeners,
+ DynamicSet<GroupBackend> groupBackends,
+ DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners,
+ DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners) {
+ this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
+ this.groupIndexedListeners = groupIndexedListeners;
this.projectIndexedListeners = projectIndexedListeners;
this.commitValidationListeners = commitValidationListeners;
this.exceptionHooks = exceptionHooks;
@@ -79,19 +109,35 @@
this.downloadSchemes = downloadSchemes;
this.refOperationValidationListeners = refOperationValidationListeners;
this.commentAddedListeners = commentAddedListeners;
+ this.refUpdatedListeners = refUpdatedListeners;
+ this.fileHistoryWebLinks = fileHistoryWebLinks;
+ this.patchSetWebLinks = patchSetWebLinks;
+ this.revisionCreatedListeners = revisionCreatedListeners;
+ this.groupBackends = groupBackends;
+ this.accountActivationValidationListeners = accountActivationValidationListeners;
+ this.onSubmitValidationListeners = onSubmitValidationListeners;
}
public Registration newRegistration() {
return new Registration();
}
+ @SuppressWarnings("FunctionalInterfaceClash")
public class Registration implements AutoCloseable {
private final List<RegistrationHandle> registrationHandles = new ArrayList<>();
+ public Registration add(AccountIndexedListener accountIndexedListener) {
+ return add(accountIndexedListeners, accountIndexedListener);
+ }
+
public Registration add(ChangeIndexedListener changeIndexedListener) {
return add(changeIndexedListeners, changeIndexedListener);
}
+ public Registration add(GroupIndexedListener groupIndexedListener) {
+ return add(groupIndexedListeners, groupIndexedListener);
+ }
+
public Registration add(ProjectIndexedListener projectIndexedListener) {
return add(projectIndexedListeners, projectIndexedListener);
}
@@ -120,6 +166,10 @@
return add(changeMessageModifiers, changeMessageModifier);
}
+ public Registration add(ChangeMessageModifier changeMessageModifier, String exportName) {
+ return add(changeMessageModifiers, changeMessageModifier, exportName);
+ }
+
public Registration add(ChangeETagComputation changeETagComputation) {
return add(changeETagComputations, changeETagComputation);
}
@@ -140,8 +190,41 @@
return add(commentAddedListeners, commentAddedListener);
}
+ public Registration add(GitReferenceUpdatedListener refUpdatedListener) {
+ return add(refUpdatedListeners, refUpdatedListener);
+ }
+
+ public Registration add(FileHistoryWebLink fileHistoryWebLink) {
+ return add(fileHistoryWebLinks, fileHistoryWebLink);
+ }
+
+ public Registration add(PatchSetWebLink patchSetWebLink) {
+ return add(patchSetWebLinks, patchSetWebLink);
+ }
+
+ public Registration add(RevisionCreatedListener revisionCreatedListener) {
+ return add(revisionCreatedListeners, revisionCreatedListener);
+ }
+
+ public Registration add(GroupBackend groupBackend) {
+ return add(groupBackends, groupBackend);
+ }
+
+ public Registration add(
+ AccountActivationValidationListener accountActivationValidationListener) {
+ return add(accountActivationValidationListeners, accountActivationValidationListener);
+ }
+
+ public Registration add(OnSubmitValidationListener onSubmitValidationListener) {
+ return add(onSubmitValidationListeners, onSubmitValidationListener);
+ }
+
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
- RegistrationHandle registrationHandle = dynamicSet.add("gerrit", extension);
+ return add(dynamicSet, extension, "gerrit");
+ }
+
+ private <T> Registration add(DynamicSet<T> dynamicSet, T extension, String exportname) {
+ RegistrationHandle registrationHandle = dynamicSet.add(exportname, extension);
registrationHandles.add(registrationHandle);
return this;
}
diff --git a/java/com/google/gerrit/acceptance/GitClientVersion.java b/java/com/google/gerrit/acceptance/GitClientVersion.java
new file mode 100644
index 0000000..4c9a32d
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/GitClientVersion.java
@@ -0,0 +1,66 @@
+// Copyright (C) 2019 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;
+
+import static java.util.stream.Collectors.joining;
+
+import java.util.stream.IntStream;
+
+/** Class to parse and represent version of git-core client */
+public class GitClientVersion implements Comparable<GitClientVersion> {
+ private final int v[];
+
+ /**
+ * Constructor to represent instance for minimum supported git-core version
+ *
+ * @param parts version passed as single digits
+ */
+ public GitClientVersion(int... parts) {
+ this.v = parts;
+ }
+
+ /**
+ * Parse the git-core version as returned by git version command
+ *
+ * @param version String returned by git version command
+ */
+ public GitClientVersion(String version) {
+ // "git version x.y.z", at Google "git version x.y.z.gXXXXXXXXXX-goog"
+ String parts[] = version.split(" ")[2].split("\\.");
+ int numParts = Math.min(parts.length, 3); // ignore Google-specific part of the version
+ v = new int[numParts];
+ for (int i = 0; i < numParts; i++) {
+ v[i] = Integer.valueOf(parts[i]);
+ }
+ }
+
+ @Override
+ public int compareTo(GitClientVersion o) {
+ int m = Math.max(v.length, o.v.length);
+ for (int i = 0; i < m; i++) {
+ int l = i < v.length ? v[i] : 0;
+ int r = i < o.v.length ? o.v[i] : 0;
+ if (l != r) {
+ return l < r ? -1 : 1;
+ }
+ }
+ return 0;
+ }
+
+ @Override
+ public String toString() {
+ return IntStream.of(v).mapToObj(String::valueOf).collect(joining("."));
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
index 933c4e1..a095daa 100644
--- a/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
+++ b/java/com/google/gerrit/acceptance/StandaloneSiteTest.java
@@ -15,16 +15,20 @@
package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertWithMessage;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.joining;
import static org.junit.Assert.fail;
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Streams;
+import com.google.common.io.ByteStreams;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.launcher.GerritLauncher;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.util.ManualRequestContext;
@@ -35,6 +39,9 @@
import com.google.inject.Injector;
import com.google.inject.Module;
import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InterruptedIOException;
import java.util.Arrays;
import java.util.Collections;
import org.eclipse.jgit.lib.Config;
@@ -59,10 +66,10 @@
private ServerContext(GerritServer server) throws Exception {
this.server = server;
Injector i = server.getTestInjector();
- if (adminId == null) {
- adminId = i.getInstance(AccountCreator.class).admin().id();
+ if (admin == null) {
+ admin = i.getInstance(AccountCreator.class).admin();
}
- ctx = i.getInstance(OneOffRequestContext.class).openAs(adminId);
+ ctx = i.getInstance(OneOffRequestContext.class).openAs(admin.id());
GerritApi gApi = i.getInstance(GerritApi.class);
try {
@@ -117,7 +124,7 @@
@Rule public RuleChain ruleChain = RuleChain.outerRule(tempSiteDir).around(testRunner);
protected SitePaths sitePaths;
- protected Account.Id adminId;
+ protected TestAccount admin;
private GerritServer.Description serverDesc;
private SystemReader oldSystemReader;
@@ -190,4 +197,33 @@
protected static void runGerrit(Iterable<String>... multiArgs) throws Exception {
runGerrit(Arrays.stream(multiArgs).flatMap(Streams::stream).toArray(String[]::new));
}
+
+ protected static String execute(
+ ImmutableList<String> cmd, File dir, ImmutableMap<String, String> env) throws IOException {
+ ProcessBuilder pb = new ProcessBuilder(cmd);
+ pb.directory(dir).redirectErrorStream(true);
+ pb.environment().putAll(env);
+ Process p = pb.start();
+ byte[] out;
+ try (InputStream in = p.getInputStream()) {
+ out = ByteStreams.toByteArray(in);
+ } finally {
+ p.getOutputStream().close();
+ }
+
+ int status;
+ try {
+ status = p.waitFor();
+ } catch (InterruptedException e) {
+ throw new InterruptedIOException(
+ "interrupted waiting for: " + Joiner.on(' ').join(pb.command()));
+ }
+
+ String result = new String(out, UTF_8);
+ if (status != 0) {
+ throw new IOException(result);
+ }
+
+ return result.trim();
+ }
}
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index ff2a83d..858c173 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.PermissionAwareRepositoryManager;
+import com.google.gerrit.server.git.TracingHook;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
@@ -415,7 +416,11 @@
up.setPreUploadHook(
PreUploadHookChain.newChain(
Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
- next.doFilter(httpRequest, responseWrapper);
+
+ try (TracingHook tracingHook = new TracingHook()) {
+ up.setProtocolV2Hook(tracingHook);
+ next.doFilter(httpRequest, responseWrapper);
+ }
} finally {
groupAuditService.dispatch(
new HttpAuditEvent(
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index 61d609b..9501e52 100644
--- a/java/com/google/gerrit/index/query/QueryProcessor.java
+++ b/java/com/google/gerrit/index/query/QueryProcessor.java
@@ -218,7 +218,7 @@
logger.atFine().log(
"Executing %d %s index queries for %s",
- cnt, schemaDef.getName(), callerFinder.findCaller());
+ cnt, schemaDef.getName(), callerFinder.findCallerLazy());
List<QueryResult<T>> out;
try {
// Parse and rewrite all queries.
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java b/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
index 2c72f56..d75aa17 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
@@ -675,7 +675,8 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
if (revision != null) {
- logger.atFine().log("Reading external ID note map (caller: %s)", callerFinder.findCaller());
+ logger.atFine().log(
+ "Reading external ID note map (caller: %s)", callerFinder.findCallerLazy());
noteMap = NoteMap.read(reader, revision);
} else {
noteMap = NoteMap.newEmptyMap();
diff --git a/java/com/google/gerrit/server/change/testing/TestChangeETagComputation.java b/java/com/google/gerrit/server/change/testing/TestChangeETagComputation.java
new file mode 100644
index 0000000..344b9b3
--- /dev/null
+++ b/java/com/google/gerrit/server/change/testing/TestChangeETagComputation.java
@@ -0,0 +1,30 @@
+// Copyright (C) 2019 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.change.testing;
+
+import com.google.gerrit.server.change.ChangeETagComputation;
+
+public class TestChangeETagComputation {
+
+ public static ChangeETagComputation withETag(String etag) {
+ return (p, id) -> etag;
+ }
+
+ public static ChangeETagComputation withException(RuntimeException e) {
+ return (p, id) -> {
+ throw e;
+ };
+ }
+}
diff --git a/java/com/google/gerrit/server/git/TracingHook.java b/java/com/google/gerrit/server/git/TracingHook.java
new file mode 100644
index 0000000..4191373
--- /dev/null
+++ b/java/com/google/gerrit/server/git/TracingHook.java
@@ -0,0 +1,99 @@
+// 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.git;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.LinkedListMultimap;
+import com.google.common.collect.ListMultimap;
+import com.google.gerrit.server.logging.TraceContext;
+import java.util.List;
+import java.util.Optional;
+import org.eclipse.jgit.transport.FetchV2Request;
+import org.eclipse.jgit.transport.LsRefsV2Request;
+import org.eclipse.jgit.transport.ProtocolV2Hook;
+
+/**
+ * Git hook for ls-refs and fetch that enables Gerrit request tracing if the user sets the 'trace'
+ * server option.
+ *
+ * <p>This hook is only invoked if Git protocol v2 is used.
+ *
+ * <p>If the 'trace' server option is specified without value, this means without providing a trace
+ * ID, a trace ID is generated, but it's not returned to the client. Hence users are advised to
+ * always provide a trace ID.
+ */
+public class TracingHook implements ProtocolV2Hook, AutoCloseable {
+ private TraceContext traceContext;
+
+ @Override
+ public void onLsRefs(LsRefsV2Request req) {
+ maybeStartTrace(req.getServerOptions());
+ }
+
+ @Override
+ public void onFetch(FetchV2Request req) {
+ maybeStartTrace(req.getServerOptions());
+ }
+
+ @Override
+ public void close() {
+ if (traceContext != null) {
+ traceContext.close();
+ }
+ }
+
+ /**
+ * Starts request tracing if 'trace' server option is set.
+ *
+ * @param serverOptionList list of provided server options
+ */
+ private void maybeStartTrace(List<String> serverOptionList) {
+ checkState(traceContext == null, "Trace was already started.");
+
+ Optional<String> traceOption = parseTraceOption(serverOptionList);
+ traceContext =
+ TraceContext.newTrace(
+ traceOption.isPresent(),
+ traceOption.orElse(null),
+ (tagName, traceId) -> {
+ // TODO(ekempin): Return trace ID to client
+ });
+ }
+
+ private Optional<String> parseTraceOption(List<String> serverOptionList) {
+ if (serverOptionList == null || serverOptionList.isEmpty()) {
+ return Optional.empty();
+ }
+
+ ListMultimap<String, String> serverOptions = LinkedListMultimap.create();
+ for (String option : serverOptionList) {
+ int e = option.indexOf('=');
+ if (e > 0) {
+ serverOptions.put(option.substring(0, e), option.substring(e + 1));
+ } else {
+ serverOptions.put(option, "");
+ }
+ }
+
+ List<String> traceValues = serverOptions.get("trace");
+ if (!traceValues.isEmpty()) {
+ return Optional.of(Iterables.getLast(traceValues));
+ }
+
+ return Optional.empty();
+ }
+}
diff --git a/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java b/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java
index 2706eaa..ef008a1 100644
--- a/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java
+++ b/java/com/google/gerrit/server/git/receive/BranchCommitValidator.java
@@ -29,6 +29,8 @@
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidationMessage;
import com.google.gerrit.server.git.validators.CommitValidators;
+import com.google.gerrit.server.logging.TraceContext;
+import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.ssh.SshInfo;
@@ -130,44 +132,48 @@
@Nullable Change change,
boolean skipValidation)
throws IOException {
- ImmutableList.Builder<CommitValidationMessage> messages = new ImmutableList.Builder<>();
- try (CommitReceivedEvent receiveEvent =
- new CommitReceivedEvent(cmd, project, branch.branch(), objectReader, commit, user)) {
- CommitValidators validators;
- if (isMerged) {
- validators =
- commitValidatorsFactory.forMergedCommits(permissions, branch, user.asIdentifiedUser());
- } else {
- validators =
- commitValidatorsFactory.forReceiveCommits(
- permissions,
- branch,
- user.asIdentifiedUser(),
- sshInfo,
- rejectCommits,
- receiveEvent.revWalk,
- change,
- skipValidation);
- }
+ try (TraceTimer traceTimer = TraceContext.newTimer("BranchCommitValidator#validateCommit")) {
+ ImmutableList.Builder<CommitValidationMessage> messages = new ImmutableList.Builder<>();
+ try (CommitReceivedEvent receiveEvent =
+ new CommitReceivedEvent(cmd, project, branch.branch(), objectReader, commit, user)) {
+ CommitValidators validators;
+ if (isMerged) {
+ validators =
+ commitValidatorsFactory.forMergedCommits(
+ permissions, branch, user.asIdentifiedUser());
+ } else {
+ validators =
+ commitValidatorsFactory.forReceiveCommits(
+ permissions,
+ branch,
+ user.asIdentifiedUser(),
+ sshInfo,
+ rejectCommits,
+ receiveEvent.revWalk,
+ change,
+ skipValidation);
+ }
- for (CommitValidationMessage m : validators.validate(receiveEvent)) {
- messages.add(
- new CommitValidationMessage(
- messageForCommit(commit, m.getMessage(), objectReader), m.getType()));
+ for (CommitValidationMessage m : validators.validate(receiveEvent)) {
+ messages.add(
+ new CommitValidationMessage(
+ messageForCommit(commit, m.getMessage(), objectReader), m.getType()));
+ }
+ } catch (CommitValidationException e) {
+ logger.atFine().log("Commit validation failed on %s", commit.name());
+ for (CommitValidationMessage m : e.getMessages()) {
+ // The non-error messages may contain background explanation for the
+ // fatal error, so have to preserve all messages.
+ messages.add(
+ new CommitValidationMessage(
+ messageForCommit(commit, m.getMessage(), objectReader), m.getType()));
+ }
+ cmd.setResult(
+ REJECTED_OTHER_REASON, messageForCommit(commit, e.getMessage(), objectReader));
+ return Result.create(false, messages.build());
}
- } catch (CommitValidationException e) {
- logger.atFine().log("Commit validation failed on %s", commit.name());
- for (CommitValidationMessage m : e.getMessages()) {
- // The non-error messages may contain background explanation for the
- // fatal error, so have to preserve all messages.
- messages.add(
- new CommitValidationMessage(
- messageForCommit(commit, m.getMessage(), objectReader), m.getType()));
- }
- cmd.setResult(REJECTED_OTHER_REASON, messageForCommit(commit, e.getMessage(), objectReader));
- return Result.create(false, messages.build());
+ return Result.create(true, messages.build());
}
- return Result.create(true, messages.build());
}
private String messageForCommit(RevCommit c, String msg, ObjectReader objectReader)
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index ef782d1..07bbade 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2002,45 +2002,48 @@
*/
private boolean requestReplaceAndValidateComments(
ReceiveCommand cmd, boolean checkMergedInto, Change change, RevCommit newCommit) {
- if (change.isClosed()) {
- reject(
- cmd,
- changeFormatter.changeClosed(
- ChangeReportFormatter.Input.builder().setChange(change).build()));
- return false;
- }
+ try (TraceTimer traceTimer = newTimer("requestReplaceAndValidateComments")) {
+ if (change.isClosed()) {
+ reject(
+ cmd,
+ changeFormatter.changeClosed(
+ ChangeReportFormatter.Input.builder().setChange(change).build()));
+ return false;
+ }
- ReplaceRequest req = new ReplaceRequest(change.getId(), newCommit, cmd, checkMergedInto);
- if (replaceByChange.containsKey(req.ontoChange)) {
- reject(cmd, "duplicate request");
- return false;
- }
+ ReplaceRequest req = new ReplaceRequest(change.getId(), newCommit, cmd, checkMergedInto);
+ if (replaceByChange.containsKey(req.ontoChange)) {
+ reject(cmd, "duplicate request");
+ return false;
+ }
- if (magicBranch != null && magicBranch.shouldPublishComments()) {
- List<Comment> drafts =
- commentsUtil.draftByChangeAuthor(notesFactory.createChecked(change), user.getAccountId());
- ImmutableList<CommentForValidation> draftsForValidation =
- drafts.stream()
- .map(
- comment ->
- CommentForValidation.create(
- comment.lineNbr > 0
- ? CommentType.INLINE_COMMENT
- : CommentType.FILE_COMMENT,
- comment.message))
- .collect(toImmutableList());
- ImmutableList<CommentValidationFailure> commentValidationFailures =
- PublishCommentUtil.findInvalidComments(commentValidators, draftsForValidation);
- magicBranch.setCommentsValid(commentValidationFailures.isEmpty());
- commentValidationFailures.forEach(
- failure ->
- addMessage(
- "Comment validation failure: " + failure.getMessage(),
- ValidationMessage.Type.WARNING));
- }
+ if (magicBranch != null && magicBranch.shouldPublishComments()) {
+ List<Comment> drafts =
+ commentsUtil.draftByChangeAuthor(
+ notesFactory.createChecked(change), user.getAccountId());
+ ImmutableList<CommentForValidation> draftsForValidation =
+ drafts.stream()
+ .map(
+ comment ->
+ CommentForValidation.create(
+ comment.lineNbr > 0
+ ? CommentType.INLINE_COMMENT
+ : CommentType.FILE_COMMENT,
+ comment.message))
+ .collect(toImmutableList());
+ ImmutableList<CommentValidationFailure> commentValidationFailures =
+ PublishCommentUtil.findInvalidComments(commentValidators, draftsForValidation);
+ magicBranch.setCommentsValid(commentValidationFailures.isEmpty());
+ commentValidationFailures.forEach(
+ failure ->
+ addMessage(
+ "Comment validation failure: " + failure.getMessage(),
+ ValidationMessage.Type.WARNING));
+ }
- replaceByChange.put(req.ontoChange, req);
- return true;
+ replaceByChange.put(req.ontoChange, req);
+ return true;
+ }
}
private void warnAboutMissingChangeId(List<CreateRequest> newChanges) {
@@ -2333,19 +2336,21 @@
}
private boolean foundInExistingRef(Collection<Ref> existingRefs) {
- for (Ref ref : existingRefs) {
- ChangeNotes notes =
- notesFactory.create(project.getNameKey(), Change.Id.fromRef(ref.getName()));
- Change change = notes.getChange();
- if (change.getDest().equals(magicBranch.dest)) {
- logger.atFine().log("Found change %s from existing refs.", change.getKey());
- // Reindex the change asynchronously, ignoring errors.
- @SuppressWarnings("unused")
- Future<?> possiblyIgnoredError = indexer.indexAsync(project.getNameKey(), change.getId());
- return true;
+ try (TraceTimer traceTimer = newTimer("foundInExistingRef")) {
+ for (Ref ref : existingRefs) {
+ ChangeNotes notes =
+ notesFactory.create(project.getNameKey(), Change.Id.fromRef(ref.getName()));
+ Change change = notes.getChange();
+ if (change.getDest().equals(magicBranch.dest)) {
+ logger.atFine().log("Found change %s from existing refs.", change.getKey());
+ // Reindex the change asynchronously, ignoring errors.
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError = indexer.indexAsync(project.getNameKey(), change.getId());
+ return true;
+ }
}
+ return false;
}
- return false;
}
private RevCommit setUpWalkForSelectingChanges() throws IOException {
@@ -2465,12 +2470,16 @@
}
private ChangeLookup lookupByChangeKey(RevCommit c, Change.Key key) {
- return new ChangeLookup(c, key, queryProvider.get().byBranchKey(magicBranch.dest, key));
+ try (TraceTimer traceTimer = newTimer("lookupByChangeKey")) {
+ return new ChangeLookup(c, key, queryProvider.get().byBranchKey(magicBranch.dest, key));
+ }
}
private ChangeLookup lookupByCommit(RevCommit c) {
- return new ChangeLookup(
- c, null, queryProvider.get().byBranchCommit(magicBranch.dest, c.getName()));
+ try (TraceTimer traceTimer = newTimer("lookupByCommit")) {
+ return new ChangeLookup(
+ c, null, queryProvider.get().byBranchCommit(magicBranch.dest, c.getName()));
+ }
}
/** Represents a commit for which a Change should be created. */
@@ -2734,7 +2743,7 @@
* @throws PermissionBackendException
*/
boolean validateNewPatchSet() throws IOException, PermissionBackendException {
- try (TraceTimer traceTimer = newTimer(ReplaceRequest.class, "validateNewPatchSet")) {
+ try (TraceTimer traceTimer = newTimer("validateNewPatchSet")) {
if (!validateNewPatchSetNoteDb()) {
return false;
}
@@ -2767,59 +2776,63 @@
/** Validates the new PS against permissions and notedb status. */
private boolean validateNewPatchSetNoteDb() throws IOException, PermissionBackendException {
- if (notes == null) {
- reject(inputCommand, "change " + ontoChange + " not found");
- return false;
- }
-
- Change change = notes.getChange();
- priorPatchSet = change.currentPatchSetId();
- if (!revisions.containsValue(priorPatchSet)) {
- reject(inputCommand, "change " + ontoChange + " missing revisions");
- return false;
- }
-
- RevCommit newCommit = receivePack.getRevWalk().parseCommit(newCommitId);
-
- // Not allowed to create a new patch set if the current patch set is locked.
- if (psUtil.isPatchSetLocked(notes)) {
- reject(inputCommand, "cannot add patch set to " + ontoChange + ".");
- return false;
- }
-
- try {
- permissions.change(notes).check(ChangePermission.ADD_PATCH_SET);
- } catch (AuthException no) {
- reject(inputCommand, "cannot add patch set to " + ontoChange + ".");
- return false;
- }
-
- if (change.isClosed()) {
- reject(inputCommand, "change " + ontoChange + " closed");
- return false;
- } else if (revisions.containsKey(newCommit)) {
- reject(inputCommand, "commit already exists (in the change)");
- return false;
- }
-
- for (Ref r : receivePack.getRepository().getRefDatabase().getRefsByPrefix("refs/changes")) {
- if (r.getObjectId().equals(newCommit)) {
- reject(inputCommand, "commit already exists (in the project)");
+ try (TraceTimer traceTimer = newTimer("validateNewPatchSetNoteDb")) {
+ if (notes == null) {
+ reject(inputCommand, "change " + ontoChange + " not found");
return false;
}
- }
- for (RevCommit prior : revisions.keySet()) {
- // Don't allow a change to directly depend upon itself. This is a
- // very common error due to users making a new commit rather than
- // amending when trying to address review comments.
- if (receivePack.getRevWalk().isMergedInto(prior, newCommit)) {
- reject(inputCommand, SAME_CHANGE_ID_IN_MULTIPLE_CHANGES);
+ Change change = notes.getChange();
+ priorPatchSet = change.currentPatchSetId();
+ if (!revisions.containsValue(priorPatchSet)) {
+ reject(inputCommand, "change " + ontoChange + " missing revisions");
return false;
}
- }
- return true;
+ RevCommit newCommit = receivePack.getRevWalk().parseCommit(newCommitId);
+
+ // Not allowed to create a new patch set if the current patch set is locked.
+ if (psUtil.isPatchSetLocked(notes)) {
+ reject(inputCommand, "cannot add patch set to " + ontoChange + ".");
+ return false;
+ }
+
+ try {
+ permissions.change(notes).check(ChangePermission.ADD_PATCH_SET);
+ } catch (AuthException no) {
+ reject(inputCommand, "cannot add patch set to " + ontoChange + ".");
+ return false;
+ }
+
+ if (change.isClosed()) {
+ reject(inputCommand, "change " + ontoChange + " closed");
+ return false;
+ } else if (revisions.containsKey(newCommit)) {
+ reject(inputCommand, "commit already exists (in the change)");
+ return false;
+ }
+
+ for (Ref r : receivePack.getRepository().getRefDatabase().getRefsByPrefix("refs/changes")) {
+ if (r.getObjectId().equals(newCommit)) {
+ reject(inputCommand, "commit already exists (in the project)");
+ return false;
+ }
+ }
+
+ try (TraceTimer traceTimer2 = newTimer("validateNewPatchSetNoteDb#isMergedInto")) {
+ for (RevCommit prior : revisions.keySet()) {
+ // Don't allow a change to directly depend upon itself. This is a
+ // very common error due to users making a new commit rather than
+ // amending when trying to address review comments.
+ if (receivePack.getRevWalk().isMergedInto(prior, newCommit)) {
+ reject(inputCommand, SAME_CHANGE_ID_IN_MULTIPLE_CHANGES);
+ return false;
+ }
+ }
+ }
+
+ return true;
+ }
}
/** Validates whether the WIP change is allowed. Rejects inputCommand if not. */
@@ -2848,39 +2861,41 @@
/** prints a warning if the new PS has the same tree as the previous commit. */
private void sameTreeWarning() throws IOException {
- RevWalk rw = receivePack.getRevWalk();
- RevCommit newCommit = rw.parseCommit(newCommitId);
- RevCommit priorCommit = revisions.inverse().get(priorPatchSet);
+ try (TraceTimer traceTimer = newTimer("sameTreeWarning")) {
+ RevWalk rw = receivePack.getRevWalk();
+ RevCommit newCommit = rw.parseCommit(newCommitId);
+ RevCommit priorCommit = revisions.inverse().get(priorPatchSet);
- if (newCommit.getTree().equals(priorCommit.getTree())) {
- rw.parseBody(newCommit);
- rw.parseBody(priorCommit);
- boolean messageEq =
- Objects.equals(newCommit.getFullMessage(), priorCommit.getFullMessage());
- boolean parentsEq = parentsEqual(newCommit, priorCommit);
- boolean authorEq = authorEqual(newCommit, priorCommit);
- ObjectReader reader = receivePack.getRevWalk().getObjectReader();
+ if (newCommit.getTree().equals(priorCommit.getTree())) {
+ rw.parseBody(newCommit);
+ rw.parseBody(priorCommit);
+ boolean messageEq =
+ Objects.equals(newCommit.getFullMessage(), priorCommit.getFullMessage());
+ boolean parentsEq = parentsEqual(newCommit, priorCommit);
+ boolean authorEq = authorEqual(newCommit, priorCommit);
+ ObjectReader reader = receivePack.getRevWalk().getObjectReader();
- if (messageEq && parentsEq && authorEq) {
- addMessage(
- String.format(
- "warning: no changes between prior commit %s and new commit %s",
- abbreviateName(priorCommit, reader), abbreviateName(newCommit, reader)));
- } else {
- StringBuilder msg = new StringBuilder();
- msg.append("warning: ").append(abbreviateName(newCommit, reader));
- msg.append(":");
- msg.append(" no files changed");
- if (!authorEq) {
- msg.append(", author changed");
+ if (messageEq && parentsEq && authorEq) {
+ addMessage(
+ String.format(
+ "warning: no changes between prior commit %s and new commit %s",
+ abbreviateName(priorCommit, reader), abbreviateName(newCommit, reader)));
+ } else {
+ StringBuilder msg = new StringBuilder();
+ msg.append("warning: ").append(abbreviateName(newCommit, reader));
+ msg.append(":");
+ msg.append(" no files changed");
+ if (!authorEq) {
+ msg.append(", author changed");
+ }
+ if (!messageEq) {
+ msg.append(", message updated");
+ }
+ if (!parentsEq) {
+ msg.append(", was rebased");
+ }
+ addMessage(msg.toString());
}
- if (!messageEq) {
- msg.append(", message updated");
- }
- if (!parentsEq) {
- msg.append(", was rebased");
- }
- addMessage(msg.toString());
}
}
}
@@ -2890,33 +2905,36 @@
* failure.
*/
private boolean newEdit() {
- psId = notes.getChange().currentPatchSetId();
- Optional<ChangeEdit> edit;
+ try (TraceTimer traceTimer = newTimer("newEdit")) {
+ psId = notes.getChange().currentPatchSetId();
+ Optional<ChangeEdit> edit;
- try {
- edit = editUtil.byChange(notes, user);
- } catch (AuthException | IOException e) {
- logger.atSevere().withCause(e).log("Cannot retrieve edit");
- return false;
- }
+ try {
+ edit = editUtil.byChange(notes, user);
+ } catch (AuthException | IOException e) {
+ logger.atSevere().withCause(e).log("Cannot retrieve edit");
+ return false;
+ }
- if (edit.isPresent()) {
- if (edit.get().getBasePatchSet().id().equals(psId)) {
- // replace edit
- cmd =
- new ReceiveCommand(edit.get().getEditCommit(), newCommitId, edit.get().getRefName());
+ if (edit.isPresent()) {
+ if (edit.get().getBasePatchSet().id().equals(psId)) {
+ // replace edit
+ cmd =
+ new ReceiveCommand(
+ edit.get().getEditCommit(), newCommitId, edit.get().getRefName());
+ } else {
+ // delete old edit ref on rebase
+ prev =
+ new ReceiveCommand(
+ edit.get().getEditCommit(), ObjectId.zeroId(), edit.get().getRefName());
+ createEditCommand();
+ }
} else {
- // delete old edit ref on rebase
- prev =
- new ReceiveCommand(
- edit.get().getEditCommit(), ObjectId.zeroId(), edit.get().getRefName());
createEditCommand();
}
- } else {
- createEditCommand();
- }
- return true;
+ return true;
+ }
}
/** Creates a ReceiveCommand for a new edit. */
@@ -2930,15 +2948,18 @@
/** Updates 'this' to add a new patchset. */
private void newPatchSet() throws IOException {
- RevCommit newCommit = receivePack.getRevWalk().parseCommit(newCommitId);
- psId =
- ChangeUtil.nextPatchSetIdFromAllRefsMap(allRefs(), notes.getChange().currentPatchSetId());
- info = patchSetInfoFactory.get(receivePack.getRevWalk(), newCommit, psId);
- cmd = new ReceiveCommand(ObjectId.zeroId(), newCommitId, psId.toRefName());
+ try (TraceTimer traceTimer = newTimer("newPatchSet")) {
+ RevCommit newCommit = receivePack.getRevWalk().parseCommit(newCommitId);
+ psId =
+ ChangeUtil.nextPatchSetIdFromAllRefsMap(
+ allRefs(), notes.getChange().currentPatchSetId());
+ info = patchSetInfoFactory.get(receivePack.getRevWalk(), newCommit, psId);
+ cmd = new ReceiveCommand(ObjectId.zeroId(), newCommitId, psId.toRefName());
+ }
}
void addOps(BatchUpdate bu, @Nullable Task progress) throws IOException {
- try (TraceTimer traceTimer = newTimer(ReplaceRequest.class, "addOps")) {
+ try (TraceTimer traceTimer = newTimer("addOps")) {
if (magicBranch != null && magicBranch.edit) {
bu.addOp(notes.getChangeId(), new ReindexOnlyOp());
if (prev != null) {
@@ -3075,7 +3096,11 @@
}
private void initChangeRefMaps() {
- if (refsByChange == null) {
+ if (refsByChange != null) {
+ return;
+ }
+
+ try (TraceTimer traceTimer = newTimer("initChangeRefMaps")) {
int estRefsPerChange = 4;
refsById = MultimapBuilder.hashKeys().arrayListValues().build();
refsByChange =
@@ -3317,7 +3342,7 @@
}
logger.atFine().log(
- "Auto-closing %s changes with existing patch sets and %s with new patch sets",
+ "Auto-closing %d changes with existing patch sets and %d with new patch sets",
existingPatchSets, newPatchSets);
bu.execute();
} catch (IOException | StorageException | PermissionBackendException e) {
diff --git a/java/com/google/gerrit/server/logging/CallerFinder.java b/java/com/google/gerrit/server/logging/CallerFinder.java
index 73ffeb5..bd7e608 100644
--- a/java/com/google/gerrit/server/logging/CallerFinder.java
+++ b/java/com/google/gerrit/server/logging/CallerFinder.java
@@ -142,7 +142,7 @@
/**
* The minimum number of calls known to have occurred between the first call to the target class
- * and the call of {@link #findCaller()}. If in doubt, specify zero here to avoid accidentally
+ * and the call of {@link #findCallerLazy()}. If in doubt, specify zero here to avoid accidentally
* skipping past the caller.
*
* @return the number of stack elements to skip when computing the caller
@@ -195,7 +195,7 @@
public abstract CallerFinder build();
}
- public LazyArg<String> findCaller() {
+ public LazyArg<String> findCallerLazy() {
return lazy(
() ->
targets().stream()
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index dae78c0..7af204e 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -18,11 +18,14 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.MoreObjects.ToStringHelper;
import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.LazyArg;
+import com.google.common.flogger.LazyArgs;
import com.google.gerrit.common.Nullable;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
+import java.util.Comparator;
import java.util.Optional;
/** Metadata that is provided to {@link PerformanceLogger}s as context for performance records. */
@@ -141,7 +144,8 @@
public abstract Optional<String> username();
/**
- * Returns a string representation of this instance that is suitable for logging.
+ * Returns a string representation of this instance that is suitable for logging. This is wrapped
+ * in a {@link LazyArg} because it is expensive to evaluate.
*
* <p>{@link #toString()} formats the {@link Optional} fields as {@code key=Optional[value]} or
* {@code key=Optional.empty}. Since this class has many optional fields from which usually only a
@@ -178,7 +182,14 @@
*
* @return string representation of this instance that is suitable for logging
*/
- public String toStringForLogging() {
+ LazyArg<String> toStringForLoggingLazy() {
+ // Don't use a lambda because different compilers generate different method names for lambdas,
+ // e.g. "lambda$myFunction$0" vs. just "lambda$0" in Eclipse. We need to identify the method
+ // by name to skip it and avoid infinite recursion.
+ return LazyArgs.lazy(this::toStringForLoggingImpl);
+ }
+
+ private String toStringForLoggingImpl() {
// Append class name.
String className = getClass().getSimpleName();
if (className.startsWith("AutoValue_")) {
@@ -188,15 +199,16 @@
// Append key-value pairs for field which are set.
Method[] methods = Metadata.class.getDeclaredMethods();
- Arrays.<Method>sort(methods, (m1, m2) -> m1.getName().compareTo(m2.getName()));
+ Arrays.sort(methods, Comparator.comparing(Method::getName));
for (Method method : methods) {
if (Modifier.isStatic(method.getModifiers())) {
// skip static method
continue;
}
- if (method.getName().equals(Thread.currentThread().getStackTrace()[1].getMethodName())) {
- // skip this method (toStringForLogging() method)
+ if (method.getName().equals("toStringForLoggingLazy")
+ || method.getName().equals("toStringForLoggingImpl")) {
+ // Don't call myself in infinite recursion.
continue;
}
diff --git a/java/com/google/gerrit/server/logging/TraceContext.java b/java/com/google/gerrit/server/logging/TraceContext.java
index 30c5250..21a4ce6 100644
--- a/java/com/google/gerrit/server/logging/TraceContext.java
+++ b/java/com/google/gerrit/server/logging/TraceContext.java
@@ -196,13 +196,13 @@
this(
() ->
logger.atFine().log(
- "Starting timer for %s (%s)", operation, metadata.toStringForLogging()),
+ "Starting timer for %s (%s)", operation, metadata.toStringForLoggingLazy()),
elapsedMs -> {
LoggingContext.getInstance()
.addPerformanceLogRecord(
() -> PerformanceLogRecord.create(operation, elapsedMs, metadata));
logger.atFine().log(
- "%s (%s) done (%d ms)", operation, metadata.toStringForLogging(), elapsedMs);
+ "%s (%s) done (%d ms)", operation, metadata.toStringForLoggingLazy(), elapsedMs);
});
}
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index 9a2ecdd..694bb82 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -395,7 +395,7 @@
withForce,
projectControl.getProject().getName(),
refName,
- callerFinder.findCaller());
+ callerFinder.findCallerLazy());
return false;
}
@@ -408,7 +408,7 @@
withForce,
projectControl.getProject().getName(),
refName,
- callerFinder.findCaller());
+ callerFinder.findCallerLazy());
return true;
}
}
@@ -420,7 +420,7 @@
withForce,
projectControl.getProject().getName(),
refName,
- callerFinder.findCaller());
+ callerFinder.findCallerLazy());
return false;
}
diff --git a/java/com/google/gerrit/sshd/commands/Upload.java b/java/com/google/gerrit/sshd/commands/Upload.java
index 41323dd..87ae493 100644
--- a/java/com/google/gerrit/sshd/commands/Upload.java
+++ b/java/com/google/gerrit/sshd/commands/Upload.java
@@ -21,6 +21,7 @@
import com.google.gerrit.server.RequestInfo;
import com.google.gerrit.server.RequestListener;
import com.google.gerrit.server.git.PermissionAwareRepositoryManager;
+import com.google.gerrit.server.git.TracingHook;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.validators.UploadValidationException;
@@ -83,13 +84,14 @@
for (UploadPackInitializer initializer : uploadPackInitializers) {
initializer.init(projectState.getNameKey(), up);
}
- try (TraceContext traceContext = TraceContext.open()) {
+ try (TraceContext traceContext = TraceContext.open();
+ TracingHook tracingHook = new TracingHook()) {
RequestInfo requestInfo =
RequestInfo.builder(RequestInfo.RequestType.GIT_UPLOAD, user, traceContext)
.project(projectState.getNameKey())
.build();
requestListeners.runEach(l -> l.onRequest(requestInfo));
-
+ up.setProtocolV2Hook(tracingHook);
up.upload(in, out, err);
session.setPeerAgent(up.getPeerUserAgent());
} catch (UploadValidationException e) {
diff --git a/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java b/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
index fc42474..d8ed29a 100644
--- a/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
+++ b/javatests/com/google/gerrit/acceptance/TestGroupBackendTest.java
@@ -16,18 +16,16 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.UniversalGroupBackend;
import com.google.gerrit.server.group.testing.TestGroupBackend;
import com.google.inject.Inject;
import org.junit.Test;
public class TestGroupBackendTest extends AbstractDaemonTest {
- @Inject private DynamicSet<GroupBackend> groupBackends;
@Inject private UniversalGroupBackend universalGroupBackend;
+ @Inject private ExtensionRegistry extensionRegistry;
private final TestGroupBackend testGroupBackend = new TestGroupBackend();
private final AccountGroup.UUID testUUID = AccountGroup.uuid("testbackend:test");
@@ -39,11 +37,8 @@
@Test
public void universalGroupBackendHandlesTestGroup() throws Exception {
- RegistrationHandle registrationHandle = groupBackends.add("gerrit", testGroupBackend);
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(testGroupBackend)) {
assertThat(universalGroupBackend.handles(testUUID)).isTrue();
- } finally {
- registrationHandle.remove();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 47bc7b3..6c61ae9 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -57,6 +57,8 @@
import com.google.common.util.concurrent.AtomicLongMap;
import com.google.common.util.concurrent.Runnables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.Sandboxed;
@@ -98,7 +100,6 @@
import com.google.gerrit.extensions.events.AccountIndexedListener;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -189,7 +190,6 @@
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
public class AccountIT extends AbstractDaemonTest {
@@ -207,8 +207,6 @@
@Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
@Inject private AccountIndexer accountIndexer;
- @Inject private DynamicSet<AccountIndexedListener> accountIndexedListeners;
- @Inject private DynamicSet<GitReferenceUpdatedListener> refUpdateListeners;
@Inject private ExternalIdNotes.Factory extIdNotesFactory;
@Inject private ExternalIds externalIds;
@Inject private GitReferenceUpdated gitReferenceUpdated;
@@ -222,6 +220,7 @@
@Inject private StalenessChecker stalenessChecker;
@Inject private VersionedAuthorizedKeys.Accessor authorizedKeys;
@Inject private PermissionBackend permissionBackend;
+ @Inject private ExtensionRegistry extensionRegistry;
@Inject protected Emails emails;
@@ -231,42 +230,8 @@
@Inject private AccountOperations accountOperations;
- @Inject
- private DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners;
-
@Inject protected GroupOperations groupOperations;
- private AccountIndexedCounter accountIndexedCounter;
- private RegistrationHandle accountIndexEventCounterHandle;
- private RefUpdateCounter refUpdateCounter;
- private RegistrationHandle refUpdateCounterHandle;
-
- @Before
- public void addAccountIndexEventCounter() {
- accountIndexedCounter = new AccountIndexedCounter();
- accountIndexEventCounterHandle = accountIndexedListeners.add("gerrit", accountIndexedCounter);
- }
-
- @After
- public void removeAccountIndexEventCounter() {
- if (accountIndexEventCounterHandle != null) {
- accountIndexEventCounterHandle.remove();
- }
- }
-
- @Before
- public void addRefUpdateCounter() {
- refUpdateCounter = new RefUpdateCounter();
- refUpdateCounterHandle = refUpdateListeners.add("gerrit", refUpdateCounter);
- }
-
- @After
- public void removeRefUpdateCounter() {
- if (refUpdateCounterHandle != null) {
- refUpdateCounterHandle.remove();
- }
- }
-
@After
public void clearPublicKeyStore() throws Exception {
try (Repository repo = repoManager.openRepository(allUsers)) {
@@ -315,11 +280,14 @@
@Test
public void createByAccountCreator() throws Exception {
- Account.Id accountId = createByAccountCreator(1);
- refUpdateCounter.assertRefUpdateFor(
- RefUpdateCounter.projectRef(allUsers, RefNames.refsUsers(accountId)),
- RefUpdateCounter.projectRef(allUsers, RefNames.REFS_EXTERNAL_IDS),
- RefUpdateCounter.projectRef(allUsers, RefNames.REFS_SEQUENCES + Sequences.NAME_ACCOUNTS));
+ RefUpdateCounter refUpdateCounter = new RefUpdateCounter();
+ try (Registration registration = extensionRegistry.newRegistration().add(refUpdateCounter)) {
+ Account.Id accountId = createByAccountCreator(1);
+ refUpdateCounter.assertRefUpdateFor(
+ RefUpdateCounter.projectRef(allUsers, RefNames.refsUsers(accountId)),
+ RefUpdateCounter.projectRef(allUsers, RefNames.REFS_EXTERNAL_IDS),
+ RefUpdateCounter.projectRef(allUsers, RefNames.REFS_SEQUENCES + Sequences.NAME_ACCOUNTS));
+ }
}
@Test
@@ -338,42 +306,54 @@
}
private Account.Id createByAccountCreator(int expectedAccountReindexCalls) throws Exception {
- String name = "foo";
- TestAccount foo = accountCreator.create(name);
- AccountInfo info = gApi.accounts().id(foo.id().get()).get();
- assertThat(info.username).isEqualTo(name);
- assertThat(info.name).isEqualTo(name);
- accountIndexedCounter.assertReindexOf(foo, expectedAccountReindexCalls);
- assertUserBranch(foo.id(), name, null);
- return foo.id();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String name = "foo";
+ TestAccount foo = accountCreator.create(name);
+ AccountInfo info = gApi.accounts().id(foo.id().get()).get();
+ assertThat(info.username).isEqualTo(name);
+ assertThat(info.name).isEqualTo(name);
+ accountIndexedCounter.assertReindexOf(foo, expectedAccountReindexCalls);
+ assertUserBranch(foo.id(), name, null);
+ return foo.id();
+ }
}
@Test
public void createAnonymousCowardByAccountCreator() throws Exception {
- TestAccount anonymousCoward = accountCreator.create();
- accountIndexedCounter.assertReindexOf(anonymousCoward);
- assertUserBranchWithoutAccountConfig(anonymousCoward.id());
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestAccount anonymousCoward = accountCreator.create();
+ accountIndexedCounter.assertReindexOf(anonymousCoward);
+ assertUserBranchWithoutAccountConfig(anonymousCoward.id());
+ }
}
@Test
public void create() throws Exception {
- AccountInput input = new AccountInput();
- input.username = "foo";
- input.name = "Foo";
- input.email = "foo@example.com";
- AccountInfo accountInfo = gApi.accounts().create(input).get();
- assertThat(accountInfo._accountId).isNotNull();
- assertThat(accountInfo.username).isEqualTo(input.username);
- assertThat(accountInfo.name).isEqualTo(input.name);
- assertThat(accountInfo.email).isEqualTo(input.email);
- assertThat(accountInfo.status).isNull();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ AccountInput input = new AccountInput();
+ input.username = "foo";
+ input.name = "Foo";
+ input.email = "foo@example.com";
+ AccountInfo accountInfo = gApi.accounts().create(input).get();
+ assertThat(accountInfo._accountId).isNotNull();
+ assertThat(accountInfo.username).isEqualTo(input.username);
+ assertThat(accountInfo.name).isEqualTo(input.name);
+ assertThat(accountInfo.email).isEqualTo(input.email);
+ assertThat(accountInfo.status).isNull();
- Account.Id accountId = Account.id(accountInfo._accountId);
- accountIndexedCounter.assertReindexOf(accountId, 1);
- assertThat(externalIds.byAccount(accountId))
- .containsExactly(
- ExternalId.createUsername(input.username, accountId, null),
- ExternalId.createEmail(accountId, input.email));
+ Account.Id accountId = Account.id(accountInfo._accountId);
+ accountIndexedCounter.assertReindexOf(accountId, 1);
+ assertThat(externalIds.byAccount(accountId))
+ .containsExactly(
+ ExternalId.createUsername(input.username, accountId, null),
+ ExternalId.createEmail(accountId, input.email));
+ }
}
@Test
@@ -517,69 +497,93 @@
@Test
public void get() throws Exception {
- AccountInfo info = gApi.accounts().id("admin").get();
- assertThat(info.name).isEqualTo("Administrator");
- assertThat(info.email).isEqualTo("admin@example.com");
- assertThat(info.username).isEqualTo("admin");
- accountIndexedCounter.assertNoReindex();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ AccountInfo info = gApi.accounts().id("admin").get();
+ assertThat(info.name).isEqualTo("Administrator");
+ assertThat(info.email).isEqualTo("admin@example.com");
+ assertThat(info.username).isEqualTo("admin");
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void getByIntId() throws Exception {
- AccountInfo info = gApi.accounts().id("admin").get();
- AccountInfo infoByIntId = gApi.accounts().id(info._accountId).get();
- assertThat(info.name).isEqualTo(infoByIntId.name);
- accountIndexedCounter.assertNoReindex();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ AccountInfo info = gApi.accounts().id("admin").get();
+ AccountInfo infoByIntId = gApi.accounts().id(info._accountId).get();
+ assertThat(info.name).isEqualTo(infoByIntId.name);
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void self() throws Exception {
- AccountInfo info = gApi.accounts().self().get();
- assertUser(info, admin);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ AccountInfo info = gApi.accounts().self().get();
+ assertUser(info, admin);
- info = gApi.accounts().id("self").get();
- assertUser(info, admin);
- accountIndexedCounter.assertNoReindex();
+ info = gApi.accounts().id("self").get();
+ assertUser(info, admin);
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void active() throws Exception {
- int id = gApi.accounts().id("user").get()._accountId;
- assertThat(gApi.accounts().id("user").getActive()).isTrue();
- gApi.accounts().id("user").setActive(false);
- accountIndexedCounter.assertReindexOf(user);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ int id = gApi.accounts().id("user").get()._accountId;
+ assertThat(gApi.accounts().id("user").getActive()).isTrue();
+ gApi.accounts().id("user").setActive(false);
+ accountIndexedCounter.assertReindexOf(user);
- // Inactive users may only be resolved by ID.
- ResourceNotFoundException thrown =
- assertThrows(ResourceNotFoundException.class, () -> gApi.accounts().id("user"));
- assertThat(thrown)
- .hasMessageThat()
- .isEqualTo(
- "Account 'user' only matches inactive accounts. To use an inactive account, retry"
- + " with one of the following exact account IDs:\n"
- + id
- + ": User <user@example.com>");
- assertThat(gApi.accounts().id(id).getActive()).isFalse();
+ // Inactive users may only be resolved by ID.
+ ResourceNotFoundException thrown =
+ assertThrows(ResourceNotFoundException.class, () -> gApi.accounts().id("user"));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "Account 'user' only matches inactive accounts. To use an inactive account, retry"
+ + " with one of the following exact account IDs:\n"
+ + id
+ + ": User <user@example.com>");
+ assertThat(gApi.accounts().id(id).getActive()).isFalse();
- gApi.accounts().id(id).setActive(true);
- assertThat(gApi.accounts().id("user").getActive()).isTrue();
- accountIndexedCounter.assertReindexOf(user);
+ gApi.accounts().id(id).setActive(true);
+ assertThat(gApi.accounts().id("user").getActive()).isTrue();
+ accountIndexedCounter.assertReindexOf(user);
+ }
}
@Test
public void shouldAllowQueryByEmailForInactiveUser() throws Exception {
- Account.Id activatableAccountId =
- accountOperations.newAccount().inactive().preferredEmail("foo@activatable.com").create();
- accountIndexedCounter.assertReindexOf(activatableAccountId, 1);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ Account.Id activatableAccountId =
+ accountOperations.newAccount().inactive().preferredEmail("foo@activatable.com").create();
+ accountIndexedCounter.assertReindexOf(activatableAccountId, 1);
+ }
gApi.changes().query("owner:foo@activatable.com").get();
}
@Test
public void shouldAllowQueryByUserNameForInactiveUser() throws Exception {
- Account.Id activatableAccountId =
- accountOperations.newAccount().inactive().username("foo").create();
- accountIndexedCounter.assertReindexOf(activatableAccountId, 1);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ Account.Id activatableAccountId =
+ accountOperations.newAccount().inactive().username("foo").create();
+ accountIndexedCounter.assertReindexOf(activatableAccountId, 1);
+ }
gApi.changes().query("owner:foo").get();
}
@@ -590,27 +594,26 @@
accountOperations.newAccount().inactive().preferredEmail("foo@activatable.com").create();
Account.Id deactivatableAccountId =
accountOperations.newAccount().preferredEmail("foo@deactivatable.com").create();
- RegistrationHandle registrationHandle =
- accountActivationValidationListeners.add(
- "gerrit",
- new AccountActivationValidationListener() {
- @Override
- public void validateActivation(AccountState account) throws ValidationException {
- String preferredEmail = account.account().preferredEmail();
- if (preferredEmail == null || !preferredEmail.endsWith("@activatable.com")) {
- throw new ValidationException("not allowed to active account");
- }
- }
- @Override
- public void validateDeactivation(AccountState account) throws ValidationException {
- String preferredEmail = account.account().preferredEmail();
- if (preferredEmail == null || !preferredEmail.endsWith("@deactivatable.com")) {
- throw new ValidationException("not allowed to deactive account");
- }
- }
- });
- try {
+ AccountActivationValidationListener listener =
+ new AccountActivationValidationListener() {
+ @Override
+ public void validateActivation(AccountState account) throws ValidationException {
+ String preferredEmail = account.account().preferredEmail();
+ if (preferredEmail == null || !preferredEmail.endsWith("@activatable.com")) {
+ throw new ValidationException("not allowed to active account");
+ }
+ }
+
+ @Override
+ public void validateDeactivation(AccountState account) throws ValidationException {
+ String preferredEmail = account.account().preferredEmail();
+ if (preferredEmail == null || !preferredEmail.endsWith("@deactivatable.com")) {
+ throw new ValidationException("not allowed to deactive account");
+ }
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
/* Test account that can be activated, but not deactivated */
// Deactivate account that is already inactive
ResourceConflictException thrown =
@@ -660,8 +663,6 @@
() -> gApi.accounts().id(deactivatableAccountId.get()).setActive(true));
assertThat(thrown).hasMessageThat().isEqualTo("not allowed to active account");
assertThat(accountOperations.account(deactivatableAccountId).get().active()).isFalse();
- } finally {
- registrationHandle.remove();
}
}
@@ -688,84 +689,96 @@
@Test
public void starUnstarChange() throws Exception {
- PushOneCommit.Result r = createChange();
- String triplet = project.get() + "~master~" + r.getChangeId();
- refUpdateCounter.clear();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ RefUpdateCounter refUpdateCounter = new RefUpdateCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter).add(refUpdateCounter)) {
+ PushOneCommit.Result r = createChange();
+ String triplet = project.get() + "~master~" + r.getChangeId();
+ refUpdateCounter.clear();
- gApi.accounts().self().starChange(triplet);
- ChangeInfo change = info(triplet);
- assertThat(change.starred).isTrue();
- assertThat(change.stars).contains(DEFAULT_LABEL);
- refUpdateCounter.assertRefUpdateFor(
- RefUpdateCounter.projectRef(
- allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
+ gApi.accounts().self().starChange(triplet);
+ ChangeInfo change = info(triplet);
+ assertThat(change.starred).isTrue();
+ assertThat(change.stars).contains(DEFAULT_LABEL);
+ refUpdateCounter.assertRefUpdateFor(
+ RefUpdateCounter.projectRef(
+ allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
- gApi.accounts().self().unstarChange(triplet);
- change = info(triplet);
- assertThat(change.starred).isNull();
- assertThat(change.stars).isNull();
- refUpdateCounter.assertRefUpdateFor(
- RefUpdateCounter.projectRef(
- allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
+ gApi.accounts().self().unstarChange(triplet);
+ change = info(triplet);
+ assertThat(change.starred).isNull();
+ assertThat(change.stars).isNull();
+ refUpdateCounter.assertRefUpdateFor(
+ RefUpdateCounter.projectRef(
+ allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
- accountIndexedCounter.assertNoReindex();
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void starUnstarChangeWithLabels() throws Exception {
- PushOneCommit.Result r = createChange();
- String triplet = project.get() + "~master~" + r.getChangeId();
- refUpdateCounter.clear();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ RefUpdateCounter refUpdateCounter = new RefUpdateCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter).add(refUpdateCounter)) {
+ PushOneCommit.Result r = createChange();
+ String triplet = project.get() + "~master~" + r.getChangeId();
+ refUpdateCounter.clear();
- assertThat(gApi.accounts().self().getStars(triplet)).isEmpty();
- assertThat(gApi.accounts().self().getStarredChanges()).isEmpty();
+ assertThat(gApi.accounts().self().getStars(triplet)).isEmpty();
+ assertThat(gApi.accounts().self().getStarredChanges()).isEmpty();
- gApi.accounts()
- .self()
- .setStars(triplet, new StarsInput(ImmutableSet.of(DEFAULT_LABEL, "red", "blue")));
- ChangeInfo change = info(triplet);
- assertThat(change.starred).isTrue();
- assertThat(change.stars).containsExactly("blue", "red", DEFAULT_LABEL).inOrder();
- assertThat(gApi.accounts().self().getStars(triplet))
- .containsExactly("blue", "red", DEFAULT_LABEL)
- .inOrder();
- List<ChangeInfo> starredChanges = gApi.accounts().self().getStarredChanges();
- assertThat(starredChanges).hasSize(1);
- ChangeInfo starredChange = starredChanges.get(0);
- assertThat(starredChange._number).isEqualTo(r.getChange().getId().get());
- assertThat(starredChange.starred).isTrue();
- assertThat(starredChange.stars).containsExactly("blue", "red", DEFAULT_LABEL).inOrder();
- refUpdateCounter.assertRefUpdateFor(
- RefUpdateCounter.projectRef(
- allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
+ gApi.accounts()
+ .self()
+ .setStars(triplet, new StarsInput(ImmutableSet.of(DEFAULT_LABEL, "red", "blue")));
+ ChangeInfo change = info(triplet);
+ assertThat(change.starred).isTrue();
+ assertThat(change.stars).containsExactly("blue", "red", DEFAULT_LABEL).inOrder();
+ assertThat(gApi.accounts().self().getStars(triplet))
+ .containsExactly("blue", "red", DEFAULT_LABEL)
+ .inOrder();
+ List<ChangeInfo> starredChanges = gApi.accounts().self().getStarredChanges();
+ assertThat(starredChanges).hasSize(1);
+ ChangeInfo starredChange = starredChanges.get(0);
+ assertThat(starredChange._number).isEqualTo(r.getChange().getId().get());
+ assertThat(starredChange.starred).isTrue();
+ assertThat(starredChange.stars).containsExactly("blue", "red", DEFAULT_LABEL).inOrder();
+ refUpdateCounter.assertRefUpdateFor(
+ RefUpdateCounter.projectRef(
+ allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
- gApi.accounts()
- .self()
- .setStars(
- triplet,
- new StarsInput(ImmutableSet.of("yellow"), ImmutableSet.of(DEFAULT_LABEL, "blue")));
- change = info(triplet);
- assertThat(change.starred).isNull();
- assertThat(change.stars).containsExactly("red", "yellow").inOrder();
- assertThat(gApi.accounts().self().getStars(triplet)).containsExactly("red", "yellow").inOrder();
- starredChanges = gApi.accounts().self().getStarredChanges();
- assertThat(starredChanges).hasSize(1);
- starredChange = starredChanges.get(0);
- assertThat(starredChange._number).isEqualTo(r.getChange().getId().get());
- assertThat(starredChange.starred).isNull();
- assertThat(starredChange.stars).containsExactly("red", "yellow").inOrder();
- refUpdateCounter.assertRefUpdateFor(
- RefUpdateCounter.projectRef(
- allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
+ gApi.accounts()
+ .self()
+ .setStars(
+ triplet,
+ new StarsInput(ImmutableSet.of("yellow"), ImmutableSet.of(DEFAULT_LABEL, "blue")));
+ change = info(triplet);
+ assertThat(change.starred).isNull();
+ assertThat(change.stars).containsExactly("red", "yellow").inOrder();
+ assertThat(gApi.accounts().self().getStars(triplet))
+ .containsExactly("red", "yellow")
+ .inOrder();
+ starredChanges = gApi.accounts().self().getStarredChanges();
+ assertThat(starredChanges).hasSize(1);
+ starredChange = starredChanges.get(0);
+ assertThat(starredChange._number).isEqualTo(r.getChange().getId().get());
+ assertThat(starredChange.starred).isNull();
+ assertThat(starredChange.stars).containsExactly("red", "yellow").inOrder();
+ refUpdateCounter.assertRefUpdateFor(
+ RefUpdateCounter.projectRef(
+ allUsers, RefNames.refsStarredChanges(Change.id(change._number), admin.id())));
- accountIndexedCounter.assertNoReindex();
+ accountIndexedCounter.assertNoReindex();
- requestScopeOperations.setApiUser(user.id());
- AuthException thrown =
- assertThrows(
- AuthException.class,
- () -> gApi.accounts().id(Integer.toString((admin.id().get()))).getStars(triplet));
- assertThat(thrown).hasMessageThat().contains("not allowed to get stars of another account");
+ requestScopeOperations.setApiUser(user.id());
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () -> gApi.accounts().id(Integer.toString((admin.id().get()))).getStars(triplet));
+ assertThat(thrown).hasMessageThat().contains("not allowed to get stars of another account");
+ }
}
@Test
@@ -825,65 +838,81 @@
@Test
public void ignoreChangeBySetStars() throws Exception {
- TestAccount user2 = accountCreator.user2();
- accountIndexedCounter.clear();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestAccount user2 = accountCreator.user2();
+ accountIndexedCounter.clear();
- PushOneCommit.Result r = createChange();
+ PushOneCommit.Result r = createChange();
- AddReviewerInput in = new AddReviewerInput();
- in.reviewer = user.email();
- gApi.changes().id(r.getChangeId()).addReviewer(in);
+ AddReviewerInput in = new AddReviewerInput();
+ in.reviewer = user.email();
+ gApi.changes().id(r.getChangeId()).addReviewer(in);
- in = new AddReviewerInput();
- in.reviewer = user2.email();
- gApi.changes().id(r.getChangeId()).addReviewer(in);
+ in = new AddReviewerInput();
+ in.reviewer = user2.email();
+ gApi.changes().id(r.getChangeId()).addReviewer(in);
- requestScopeOperations.setApiUser(user.id());
- gApi.accounts().self().setStars(r.getChangeId(), new StarsInput(ImmutableSet.of(IGNORE_LABEL)));
+ requestScopeOperations.setApiUser(user.id());
+ gApi.accounts()
+ .self()
+ .setStars(r.getChangeId(), new StarsInput(ImmutableSet.of(IGNORE_LABEL)));
- sender.clear();
- requestScopeOperations.setApiUser(admin.id());
- gApi.changes().id(r.getChangeId()).abandon();
- List<Message> messages = sender.getMessages();
- assertThat(messages).hasSize(1);
- assertThat(messages.get(0).rcpt()).containsExactly(user2.getEmailAddress());
- accountIndexedCounter.assertNoReindex();
+ sender.clear();
+ requestScopeOperations.setApiUser(admin.id());
+ gApi.changes().id(r.getChangeId()).abandon();
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ assertThat(messages.get(0).rcpt()).containsExactly(user2.getEmailAddress());
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void addReviewerToIgnoredChange() throws Exception {
- PushOneCommit.Result r = createChange();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ PushOneCommit.Result r = createChange();
- requestScopeOperations.setApiUser(user.id());
- gApi.accounts().self().setStars(r.getChangeId(), new StarsInput(ImmutableSet.of(IGNORE_LABEL)));
+ requestScopeOperations.setApiUser(user.id());
+ gApi.accounts()
+ .self()
+ .setStars(r.getChangeId(), new StarsInput(ImmutableSet.of(IGNORE_LABEL)));
- sender.clear();
- requestScopeOperations.setApiUser(admin.id());
+ sender.clear();
+ requestScopeOperations.setApiUser(admin.id());
- AddReviewerInput in = new AddReviewerInput();
- in.reviewer = user.email();
- gApi.changes().id(r.getChangeId()).addReviewer(in);
- List<Message> messages = sender.getMessages();
- assertThat(messages).hasSize(1);
- Message message = messages.get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
- assertMailReplyTo(message, admin.email());
- accountIndexedCounter.assertNoReindex();
+ AddReviewerInput in = new AddReviewerInput();
+ in.reviewer = user.email();
+ gApi.changes().id(r.getChangeId()).addReviewer(in);
+ List<Message> messages = sender.getMessages();
+ assertThat(messages).hasSize(1);
+ Message message = messages.get(0);
+ assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertMailReplyTo(message, admin.email());
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void suggestAccounts() throws Exception {
- String adminUsername = "admin";
- List<AccountInfo> result = gApi.accounts().suggestAccounts().withQuery(adminUsername).get();
- assertThat(result).hasSize(1);
- assertThat(result.get(0).username).isEqualTo(adminUsername);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String adminUsername = "admin";
+ List<AccountInfo> result = gApi.accounts().suggestAccounts().withQuery(adminUsername).get();
+ assertThat(result).hasSize(1);
+ assertThat(result.get(0).username).isEqualTo(adminUsername);
- List<AccountInfo> resultShortcutApi = gApi.accounts().suggestAccounts(adminUsername).get();
- assertThat(resultShortcutApi).hasSize(result.size());
+ List<AccountInfo> resultShortcutApi = gApi.accounts().suggestAccounts(adminUsername).get();
+ assertThat(resultShortcutApi).hasSize(result.size());
- List<AccountInfo> emptyResult = gApi.accounts().suggestAccounts("unknown").get();
- assertThat(emptyResult).isEmpty();
- accountIndexedCounter.assertNoReindex();
+ List<AccountInfo> emptyResult = gApi.accounts().suggestAccounts("unknown").get();
+ assertThat(emptyResult).isEmpty();
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
@@ -983,17 +1012,21 @@
@Test
public void addEmail() throws Exception {
- List<String> emails = ImmutableList.of("new.email@example.com", "new.email@example.systems");
- Set<String> currentEmails = getEmails();
- for (String email : emails) {
- assertThat(currentEmails).doesNotContain(email);
- EmailInput input = newEmailInput(email);
- gApi.accounts().self().addEmail(input);
- accountIndexedCounter.assertReindexOf(admin);
- }
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ List<String> emails = ImmutableList.of("new.email@example.com", "new.email@example.systems");
+ Set<String> currentEmails = getEmails();
+ for (String email : emails) {
+ assertThat(currentEmails).doesNotContain(email);
+ EmailInput input = newEmailInput(email);
+ gApi.accounts().self().addEmail(input);
+ accountIndexedCounter.assertReindexOf(admin);
+ }
- requestScopeOperations.resetCurrentApiUser();
- assertThat(getEmails()).containsAtLeastElementsIn(emails);
+ requestScopeOperations.resetCurrentApiUser();
+ assertThat(getEmails()).containsAtLeastElementsIn(emails);
+ }
}
@Test
@@ -1011,13 +1044,17 @@
// Non-supported TLD (see tlds-alpha-by-domain.txt)
"new.email@example.africa");
- for (String email : emails) {
- EmailInput input = newEmailInput(email);
- BadRequestException thrown =
- assertThrows(BadRequestException.class, () -> gApi.accounts().self().addEmail(input));
- assertWithMessage(email).that(thrown).hasMessageThat().isEqualTo("invalid email address");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ for (String email : emails) {
+ EmailInput input = newEmailInput(email);
+ BadRequestException thrown =
+ assertThrows(BadRequestException.class, () -> gApi.accounts().self().addEmail(input));
+ assertWithMessage(email).that(thrown).hasMessageThat().isEqualTo("invalid email address");
+ }
+ accountIndexedCounter.assertNoReindex();
}
- accountIndexedCounter.assertNoReindex();
}
@Test
@@ -1097,62 +1134,74 @@
@Test
public void addEmailAndSetPreferred() throws Exception {
- String email = "foo.bar@example.com";
- EmailInput input = new EmailInput();
- input.email = email;
- input.noConfirmation = true;
- input.preferred = true;
- gApi.accounts().self().addEmail(input);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String email = "foo.bar@example.com";
+ EmailInput input = new EmailInput();
+ input.email = email;
+ input.noConfirmation = true;
+ input.preferred = true;
+ gApi.accounts().self().addEmail(input);
- // Account is reindexed twice; once on adding the new email,
- // and then again on setting the email preferred.
- accountIndexedCounter.assertReindexOf(admin, 2);
+ // Account is reindexed twice; once on adding the new email,
+ // and then again on setting the email preferred.
+ accountIndexedCounter.assertReindexOf(admin, 2);
- String preferred = gApi.accounts().self().get().email;
- assertThat(preferred).isEqualTo(email);
+ String preferred = gApi.accounts().self().get().email;
+ assertThat(preferred).isEqualTo(email);
+ }
}
@Test
public void deleteEmail() throws Exception {
- String email = "foo.bar@example.com";
- EmailInput input = newEmailInput(email);
- gApi.accounts().self().addEmail(input);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String email = "foo.bar@example.com";
+ EmailInput input = newEmailInput(email);
+ gApi.accounts().self().addEmail(input);
- requestScopeOperations.resetCurrentApiUser();
- assertThat(getEmails()).contains(email);
+ requestScopeOperations.resetCurrentApiUser();
+ assertThat(getEmails()).contains(email);
- accountIndexedCounter.clear();
- gApi.accounts().self().deleteEmail(input.email);
- accountIndexedCounter.assertReindexOf(admin);
+ accountIndexedCounter.clear();
+ gApi.accounts().self().deleteEmail(input.email);
+ accountIndexedCounter.assertReindexOf(admin);
- requestScopeOperations.resetCurrentApiUser();
- assertThat(getEmails()).doesNotContain(email);
+ requestScopeOperations.resetCurrentApiUser();
+ assertThat(getEmails()).doesNotContain(email);
+ }
}
@Test
public void deletePreferredEmail() throws Exception {
- String previous = gApi.accounts().self().get().email;
- String email = "foo.bar.baz@example.com";
- EmailInput input = new EmailInput();
- input.email = email;
- input.noConfirmation = true;
- input.preferred = true;
- gApi.accounts().self().addEmail(input);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String previous = gApi.accounts().self().get().email;
+ String email = "foo.bar.baz@example.com";
+ EmailInput input = new EmailInput();
+ input.email = email;
+ input.noConfirmation = true;
+ input.preferred = true;
+ gApi.accounts().self().addEmail(input);
- // Account is reindexed twice; once on adding the new email,
- // and then again on setting the email preferred.
- accountIndexedCounter.assertReindexOf(admin, 2);
+ // Account is reindexed twice; once on adding the new email,
+ // and then again on setting the email preferred.
+ accountIndexedCounter.assertReindexOf(admin, 2);
- // The new preferred email is set
- assertThat(gApi.accounts().self().get().email).isEqualTo(email);
+ // The new preferred email is set
+ assertThat(gApi.accounts().self().get().email).isEqualTo(email);
- accountIndexedCounter.clear();
- gApi.accounts().self().deleteEmail(input.email);
- accountIndexedCounter.assertReindexOf(admin);
+ accountIndexedCounter.clear();
+ gApi.accounts().self().deleteEmail(input.email);
+ accountIndexedCounter.assertReindexOf(admin);
- requestScopeOperations.resetCurrentApiUser();
- assertThat(getEmails()).containsExactly(previous);
- assertThat(gApi.accounts().self().get().email).isNull();
+ requestScopeOperations.resetCurrentApiUser();
+ assertThat(getEmails()).containsExactly(previous);
+ assertThat(gApi.accounts().self().get().email).isNull();
+ }
}
@Test
@@ -1178,64 +1227,77 @@
@Test
public void deleteEmailFromCustomExternalIdSchemes() throws Exception {
- String email = "foo.bar@example.com";
- String extId1 = "foo:bar";
- String extId2 = "foo:baz";
- accountsUpdateProvider
- .get()
- .update(
- "Add External IDs",
- admin.id(),
- u ->
- u.addExternalId(
- ExternalId.createWithEmail(ExternalId.Key.parse(extId1), admin.id(), email))
- .addExternalId(
- ExternalId.createWithEmail(
- ExternalId.Key.parse(extId2), admin.id(), email)));
- accountIndexedCounter.assertReindexOf(admin);
- assertThat(
- gApi.accounts().self().getExternalIds().stream().map(e -> e.identity).collect(toSet()))
- .containsAtLeast(extId1, extId2);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String email = "foo.bar@example.com";
+ String extId1 = "foo:bar";
+ String extId2 = "foo:baz";
+ accountsUpdateProvider
+ .get()
+ .update(
+ "Add External IDs",
+ admin.id(),
+ u ->
+ u.addExternalId(
+ ExternalId.createWithEmail(
+ ExternalId.Key.parse(extId1), admin.id(), email))
+ .addExternalId(
+ ExternalId.createWithEmail(
+ ExternalId.Key.parse(extId2), admin.id(), email)));
+ accountIndexedCounter.assertReindexOf(admin);
+ assertThat(
+ gApi.accounts().self().getExternalIds().stream()
+ .map(e -> e.identity)
+ .collect(toSet()))
+ .containsAtLeast(extId1, extId2);
- requestScopeOperations.resetCurrentApiUser();
- assertThat(getEmails()).contains(email);
+ requestScopeOperations.resetCurrentApiUser();
+ assertThat(getEmails()).contains(email);
- gApi.accounts().self().deleteEmail(email);
- accountIndexedCounter.assertReindexOf(admin);
+ gApi.accounts().self().deleteEmail(email);
+ accountIndexedCounter.assertReindexOf(admin);
- requestScopeOperations.resetCurrentApiUser();
- assertThat(getEmails()).doesNotContain(email);
- assertThat(
- gApi.accounts().self().getExternalIds().stream().map(e -> e.identity).collect(toSet()))
- .containsNoneOf(extId1, extId2);
+ requestScopeOperations.resetCurrentApiUser();
+ assertThat(getEmails()).doesNotContain(email);
+ assertThat(
+ gApi.accounts().self().getExternalIds().stream()
+ .map(e -> e.identity)
+ .collect(toSet()))
+ .containsNoneOf(extId1, extId2);
+ }
}
@Test
public void deleteEmailOfOtherUser() throws Exception {
- String email = "foo.bar@example.com";
- EmailInput input = new EmailInput();
- input.email = email;
- input.noConfirmation = true;
- gApi.accounts().id(user.id().get()).addEmail(input);
- accountIndexedCounter.assertReindexOf(user);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String email = "foo.bar@example.com";
+ EmailInput input = new EmailInput();
+ input.email = email;
+ input.noConfirmation = true;
+ gApi.accounts().id(user.id().get()).addEmail(input);
+ accountIndexedCounter.assertReindexOf(user);
- requestScopeOperations.setApiUser(user.id());
- assertThat(getEmails()).contains(email);
+ requestScopeOperations.setApiUser(user.id());
+ assertThat(getEmails()).contains(email);
- // admin can delete email of user
- requestScopeOperations.setApiUser(admin.id());
- gApi.accounts().id(user.id().get()).deleteEmail(email);
- accountIndexedCounter.assertReindexOf(user);
+ // admin can delete email of user
+ requestScopeOperations.setApiUser(admin.id());
+ gApi.accounts().id(user.id().get()).deleteEmail(email);
+ accountIndexedCounter.assertReindexOf(user);
- requestScopeOperations.setApiUser(user.id());
- assertThat(getEmails()).doesNotContain(email);
+ requestScopeOperations.setApiUser(user.id());
+ assertThat(getEmails()).doesNotContain(email);
- // user cannot delete email of admin
- AuthException thrown =
- assertThrows(
- AuthException.class,
- () -> gApi.accounts().id(admin.id().get()).deleteEmail(admin.email()));
- assertThat(thrown).hasMessageThat().contains("modify account not permitted");
+ // user cannot delete email of admin
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () -> gApi.accounts().id(admin.id().get()).deleteEmail(admin.email()));
+ assertThat(thrown).hasMessageThat().contains("modify account not permitted");
+ }
}
@Test
@@ -1301,17 +1363,21 @@
public void putStatus() throws Exception {
List<String> statuses = ImmutableList.of("OOO", "Busy");
AccountInfo info;
- for (String status : statuses) {
- gApi.accounts().self().setStatus(status);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ for (String status : statuses) {
+ gApi.accounts().self().setStatus(status);
+ info = gApi.accounts().self().get();
+ assertUser(info, admin, status);
+ accountIndexedCounter.assertReindexOf(admin);
+ }
+
+ gApi.accounts().self().setStatus(null);
info = gApi.accounts().self().get();
- assertUser(info, admin, status);
+ assertUser(info, admin);
accountIndexedCounter.assertReindexOf(admin);
}
-
- gApi.accounts().self().setStatus(null);
- info = gApi.accounts().self().get();
- assertUser(info, admin);
- accountIndexedCounter.assertReindexOf(admin);
}
@Test
@@ -1347,61 +1413,65 @@
@Test
public void fetchUserBranch() throws Exception {
- requestScopeOperations.setApiUser(user.id());
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ requestScopeOperations.setApiUser(user.id());
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, user);
- String userRefName = RefNames.refsUsers(user.id());
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, user);
+ String userRefName = RefNames.refsUsers(user.id());
- // remove default READ permissions
- try (ProjectConfigUpdate u = updateProject(allUsers)) {
- u.getConfig()
- .getAccessSection(RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", true)
- .remove(new Permission(Permission.READ));
- u.save();
+ // remove default READ permissions
+ try (ProjectConfigUpdate u = updateProject(allUsers)) {
+ u.getConfig()
+ .getAccessSection(RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", true)
+ .remove(new Permission(Permission.READ));
+ u.save();
+ }
+
+ // deny READ permission that is inherited from All-Projects
+ projectOperations
+ .project(allUsers)
+ .forUpdate()
+ .add(deny(Permission.READ).ref(RefNames.REFS + "*").group(ANONYMOUS_USERS))
+ .update();
+
+ // fetching user branch without READ permission fails
+ assertThrows(TransportException.class, () -> fetch(allUsersRepo, userRefName + ":userRef"));
+
+ // allow each user to read its own user branch
+ projectOperations
+ .project(allUsers)
+ .forUpdate()
+ .add(
+ allow(Permission.READ)
+ .ref(RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}")
+ .group(REGISTERED_USERS))
+ .update();
+
+ // fetch user branch using refs/users/YY/XXXXXXX
+ fetch(allUsersRepo, userRefName + ":userRef");
+ Ref userRef = allUsersRepo.getRepository().exactRef("userRef");
+ assertThat(userRef).isNotNull();
+
+ // fetch user branch using refs/users/self
+ fetch(allUsersRepo, RefNames.REFS_USERS_SELF + ":userSelfRef");
+ Ref userSelfRef = allUsersRepo.getRepository().getRefDatabase().exactRef("userSelfRef");
+ assertThat(userSelfRef).isNotNull();
+ assertThat(userSelfRef.getObjectId()).isEqualTo(userRef.getObjectId());
+
+ accountIndexedCounter.assertNoReindex();
+
+ // fetching user branch of another user fails
+ String otherUserRefName = RefNames.refsUsers(admin.id());
+ TransportException thrown =
+ assertThrows(
+ TransportException.class,
+ () -> fetch(allUsersRepo, otherUserRefName + ":otherUserRef"));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Remote does not have " + otherUserRefName + " available for fetch.");
}
-
- // deny READ permission that is inherited from All-Projects
- projectOperations
- .project(allUsers)
- .forUpdate()
- .add(deny(Permission.READ).ref(RefNames.REFS + "*").group(ANONYMOUS_USERS))
- .update();
-
- // fetching user branch without READ permission fails
- assertThrows(TransportException.class, () -> fetch(allUsersRepo, userRefName + ":userRef"));
-
- // allow each user to read its own user branch
- projectOperations
- .project(allUsers)
- .forUpdate()
- .add(
- allow(Permission.READ)
- .ref(RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}")
- .group(REGISTERED_USERS))
- .update();
-
- // fetch user branch using refs/users/YY/XXXXXXX
- fetch(allUsersRepo, userRefName + ":userRef");
- Ref userRef = allUsersRepo.getRepository().exactRef("userRef");
- assertThat(userRef).isNotNull();
-
- // fetch user branch using refs/users/self
- fetch(allUsersRepo, RefNames.REFS_USERS_SELF + ":userSelfRef");
- Ref userSelfRef = allUsersRepo.getRepository().getRefDatabase().exactRef("userSelfRef");
- assertThat(userSelfRef).isNotNull();
- assertThat(userSelfRef.getObjectId()).isEqualTo(userRef.getObjectId());
-
- accountIndexedCounter.assertNoReindex();
-
- // fetching user branch of another user fails
- String otherUserRefName = RefNames.refsUsers(admin.id());
- TransportException thrown =
- assertThrows(
- TransportException.class,
- () -> fetch(allUsersRepo, otherUserRefName + ":otherUserRef"));
- assertThat(thrown)
- .hasMessageThat()
- .contains("Remote does not have " + otherUserRefName + " available for fetch.");
}
@Test
@@ -1419,535 +1489,599 @@
@Test
public void pushToUserBranch() throws Exception {
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
- allUsersRepo.reset("userRef");
- PushOneCommit push = pushFactory.create(admin.newIdent(), allUsersRepo);
- push.to(RefNames.refsUsers(admin.id())).assertOkStatus();
- accountIndexedCounter.assertReindexOf(admin);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
+ allUsersRepo.reset("userRef");
+ PushOneCommit push = pushFactory.create(admin.newIdent(), allUsersRepo);
+ push.to(RefNames.refsUsers(admin.id())).assertOkStatus();
+ accountIndexedCounter.assertReindexOf(admin);
- push = pushFactory.create(admin.newIdent(), allUsersRepo);
- push.to(RefNames.REFS_USERS_SELF).assertOkStatus();
- accountIndexedCounter.assertReindexOf(admin);
+ push = pushFactory.create(admin.newIdent(), allUsersRepo);
+ push.to(RefNames.REFS_USERS_SELF).assertOkStatus();
+ accountIndexedCounter.assertReindexOf(admin);
+ }
}
@Test
public void pushToUserBranchForReview() throws Exception {
- String userRefName = RefNames.refsUsers(admin.id());
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, userRefName + ":userRef");
- allUsersRepo.reset("userRef");
- PushOneCommit push = pushFactory.create(admin.newIdent(), allUsersRepo);
- PushOneCommit.Result r = push.to(MagicBranch.NEW_CHANGE + userRefName);
- r.assertOkStatus();
- accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRefName);
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
- gApi.changes().id(r.getChangeId()).current().submit();
- accountIndexedCounter.assertReindexOf(admin);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String userRefName = RefNames.refsUsers(admin.id());
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRefName + ":userRef");
+ allUsersRepo.reset("userRef");
+ PushOneCommit push = pushFactory.create(admin.newIdent(), allUsersRepo);
+ PushOneCommit.Result r = push.to(MagicBranch.NEW_CHANGE + userRefName);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRefName);
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+ accountIndexedCounter.assertReindexOf(admin);
- push = pushFactory.create(admin.newIdent(), allUsersRepo);
- r = push.to(MagicBranch.NEW_CHANGE + RefNames.REFS_USERS_SELF);
- r.assertOkStatus();
- accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRefName);
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
- gApi.changes().id(r.getChangeId()).current().submit();
- accountIndexedCounter.assertReindexOf(admin);
+ push = pushFactory.create(admin.newIdent(), allUsersRepo);
+ r = push.to(MagicBranch.NEW_CHANGE + RefNames.REFS_USERS_SELF);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRefName);
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+ accountIndexedCounter.assertReindexOf(admin);
+ }
}
@Test
public void pushAccountConfigToUserBranchForReviewAndSubmit() throws Exception {
- String userRef = RefNames.refsUsers(admin.id());
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String userRef = RefNames.refsUsers(admin.id());
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS, "out-of-office");
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS, "out-of-office");
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(MagicBranch.NEW_CHANGE + userRef);
- r.assertOkStatus();
- accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
- gApi.changes().id(r.getChangeId()).current().submit();
- accountIndexedCounter.assertReindexOf(admin);
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+ accountIndexedCounter.assertReindexOf(admin);
- AccountInfo info = gApi.accounts().self().get();
- assertThat(info.email).isEqualTo(admin.email());
- assertThat(info.name).isEqualTo(admin.fullName());
- assertThat(info.status).isEqualTo("out-of-office");
+ AccountInfo info = gApi.accounts().self().get();
+ assertThat(info.email).isEqualTo(admin.email());
+ assertThat(info.name).isEqualTo(admin.fullName());
+ assertThat(info.status).isEqualTo("out-of-office");
+ }
}
@Test
public void pushAccountConfigWithPrefEmailThatDoesNotExistAsExtIdToUserBranchForReviewAndSubmit()
throws Exception {
- TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
- String userRef = RefNames.refsUsers(foo.id());
- accountIndexedCounter.clear();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
+ String userRef = RefNames.refsUsers(foo.id());
+ accountIndexedCounter.clear();
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- String email = "some.email@example.com";
- Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_PREFERRED_EMAIL, email);
+ String email = "some.email@example.com";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_PREFERRED_EMAIL, email);
- PushOneCommit.Result r =
- pushFactory
- .create(
- foo.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(MagicBranch.NEW_CHANGE + userRef);
- r.assertOkStatus();
- accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ foo.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
- requestScopeOperations.setApiUser(foo.id());
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
- gApi.changes().id(r.getChangeId()).current().submit();
+ requestScopeOperations.setApiUser(foo.id());
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
- accountIndexedCounter.assertReindexOf(foo);
+ accountIndexedCounter.assertReindexOf(foo);
- AccountInfo info = gApi.accounts().self().get();
- assertThat(info.email).isEqualTo(email);
- assertThat(info.name).isEqualTo(foo.fullName());
+ AccountInfo info = gApi.accounts().self().get();
+ assertThat(info.email).isEqualTo(email);
+ assertThat(info.name).isEqualTo(foo.fullName());
+ }
}
@Test
public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfConfigIsInvalid()
throws Exception {
- String userRef = RefNames.refsUsers(admin.id());
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String userRef = RefNames.refsUsers(admin.id());
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- "invalid config")
- .to(MagicBranch.NEW_CHANGE + userRef);
- r.assertOkStatus();
- accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ "invalid config")
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
- ResourceConflictException thrown =
- assertThrows(
- ResourceConflictException.class,
- () -> gApi.changes().id(r.getChangeId()).current().submit());
- assertThat(thrown)
- .hasMessageThat()
- .contains(
- String.format(
- "invalid account configuration: commit '%s' has an invalid '%s' file for account"
- + " '%s': Invalid config file %s in commit %s",
- r.getCommit().name(),
- AccountProperties.ACCOUNT_CONFIG,
- admin.id(),
- AccountProperties.ACCOUNT_CONFIG,
- r.getCommit().name()));
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(r.getChangeId()).current().submit());
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains(
+ String.format(
+ "invalid account configuration: commit '%s' has an invalid '%s' file for account"
+ + " '%s': Invalid config file %s in commit %s",
+ r.getCommit().name(),
+ AccountProperties.ACCOUNT_CONFIG,
+ admin.id(),
+ AccountProperties.ACCOUNT_CONFIG,
+ r.getCommit().name()));
+ }
}
@Test
public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfPreferredEmailIsInvalid()
throws Exception {
- String userRef = RefNames.refsUsers(admin.id());
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String userRef = RefNames.refsUsers(admin.id());
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- String noEmail = "no.email";
- Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_PREFERRED_EMAIL, noEmail);
+ String noEmail = "no.email";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_PREFERRED_EMAIL, noEmail);
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(MagicBranch.NEW_CHANGE + userRef);
- r.assertOkStatus();
- accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
- ResourceConflictException thrown =
- assertThrows(
- ResourceConflictException.class,
- () -> gApi.changes().id(r.getChangeId()).current().submit());
- assertThat(thrown)
- .hasMessageThat()
- .contains(
- String.format(
- "invalid account configuration: invalid preferred email '%s' for account '%s'",
- noEmail, admin.id()));
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(r.getChangeId()).current().submit());
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains(
+ String.format(
+ "invalid account configuration: invalid preferred email '%s' for account '%s'",
+ noEmail, admin.id()));
+ }
}
@Test
public void pushAccountConfigToUserBranchForReviewIsRejectedOnSubmitIfOwnAccountIsDeactivated()
throws Exception {
- String userRef = RefNames.refsUsers(admin.id());
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ String userRef = RefNames.refsUsers(admin.id());
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- Config ac = getAccountConfig(allUsersRepo);
- ac.setBoolean(AccountProperties.ACCOUNT, null, AccountProperties.KEY_ACTIVE, false);
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setBoolean(AccountProperties.ACCOUNT, null, AccountProperties.KEY_ACTIVE, false);
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(MagicBranch.NEW_CHANGE + userRef);
- r.assertOkStatus();
- accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
- ResourceConflictException thrown =
- assertThrows(
- ResourceConflictException.class,
- () -> gApi.changes().id(r.getChangeId()).current().submit());
- assertThat(thrown)
- .hasMessageThat()
- .contains("invalid account configuration: cannot deactivate own account");
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(r.getChangeId()).current().submit());
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("invalid account configuration: cannot deactivate own account");
+ }
}
@Test
public void pushAccountConfigToUserBranchForReviewDeactivateOtherAccount() throws Exception {
- projectOperations
- .allProjectsForUpdate()
- .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
- .update();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
- TestAccount foo = accountCreator.create(name("foo"));
- assertThat(gApi.accounts().id(foo.id().get()).getActive()).isTrue();
- String userRef = RefNames.refsUsers(foo.id());
- accountIndexedCounter.clear();
+ TestAccount foo = accountCreator.create(name("foo"));
+ assertThat(gApi.accounts().id(foo.id().get()).getActive()).isTrue();
+ String userRef = RefNames.refsUsers(foo.id());
+ accountIndexedCounter.clear();
- projectOperations
- .project(allUsers)
- .forUpdate()
- .add(allow(Permission.PUSH).ref(userRef).group(adminGroupUuid()))
- .add(allowLabel("Code-Review").ref(userRef).group(adminGroupUuid()).range(-2, 2))
- .add(allow(Permission.SUBMIT).ref(userRef).group(adminGroupUuid()))
- .update();
+ projectOperations
+ .project(allUsers)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref(userRef).group(adminGroupUuid()))
+ .add(allowLabel("Code-Review").ref(userRef).group(adminGroupUuid()).range(-2, 2))
+ .add(allow(Permission.SUBMIT).ref(userRef).group(adminGroupUuid()))
+ .update();
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- Config ac = getAccountConfig(allUsersRepo);
- ac.setBoolean(AccountProperties.ACCOUNT, null, AccountProperties.KEY_ACTIVE, false);
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setBoolean(AccountProperties.ACCOUNT, null, AccountProperties.KEY_ACTIVE, false);
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(MagicBranch.NEW_CHANGE + userRef);
- r.assertOkStatus();
- accountIndexedCounter.assertNoReindex();
- assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(MagicBranch.NEW_CHANGE + userRef);
+ r.assertOkStatus();
+ accountIndexedCounter.assertNoReindex();
+ assertThat(r.getChange().change().getDest().branch()).isEqualTo(userRef);
- gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
- gApi.changes().id(r.getChangeId()).current().submit();
- accountIndexedCounter.assertReindexOf(foo);
+ gApi.changes().id(r.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(r.getChangeId()).current().submit();
+ accountIndexedCounter.assertReindexOf(foo);
- assertThat(gApi.accounts().id(foo.id().get()).getActive()).isFalse();
+ assertThat(gApi.accounts().id(foo.id().get()).getActive()).isFalse();
+ }
}
@Test
public void pushWatchConfigToUserBranch() throws Exception {
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
- allUsersRepo.reset("userRef");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
+ allUsersRepo.reset("userRef");
- Config wc = new Config();
- wc.setString(
- ProjectWatches.PROJECT,
- project.get(),
- ProjectWatches.KEY_NOTIFY,
- ProjectWatches.NotifyValue.create(null, EnumSet.of(NotifyType.ALL_COMMENTS)).toString());
- PushOneCommit push =
- pushFactory.create(
- admin.newIdent(),
- allUsersRepo,
- "Add project watch",
- ProjectWatches.WATCH_CONFIG,
- wc.toText());
- push.to(RefNames.REFS_USERS_SELF).assertOkStatus();
- accountIndexedCounter.assertReindexOf(admin);
+ Config wc = new Config();
+ wc.setString(
+ ProjectWatches.PROJECT,
+ project.get(),
+ ProjectWatches.KEY_NOTIFY,
+ ProjectWatches.NotifyValue.create(null, EnumSet.of(NotifyType.ALL_COMMENTS)).toString());
+ PushOneCommit push =
+ pushFactory.create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Add project watch",
+ ProjectWatches.WATCH_CONFIG,
+ wc.toText());
+ push.to(RefNames.REFS_USERS_SELF).assertOkStatus();
+ accountIndexedCounter.assertReindexOf(admin);
- String invalidNotifyValue = "]invalid[";
- wc.setString(
- ProjectWatches.PROJECT, project.get(), ProjectWatches.KEY_NOTIFY, invalidNotifyValue);
- push =
- pushFactory.create(
- admin.newIdent(),
- allUsersRepo,
- "Add invalid project watch",
- ProjectWatches.WATCH_CONFIG,
- wc.toText());
- PushOneCommit.Result r = push.to(RefNames.REFS_USERS_SELF);
- r.assertErrorStatus("invalid account configuration");
- r.assertMessage(
- String.format(
- "%s: Invalid project watch of account %d for project %s: %s",
- ProjectWatches.WATCH_CONFIG, admin.id().get(), project.get(), invalidNotifyValue));
+ String invalidNotifyValue = "]invalid[";
+ wc.setString(
+ ProjectWatches.PROJECT, project.get(), ProjectWatches.KEY_NOTIFY, invalidNotifyValue);
+ push =
+ pushFactory.create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Add invalid project watch",
+ ProjectWatches.WATCH_CONFIG,
+ wc.toText());
+ PushOneCommit.Result r = push.to(RefNames.REFS_USERS_SELF);
+ r.assertErrorStatus("invalid account configuration");
+ r.assertMessage(
+ String.format(
+ "%s: Invalid project watch of account %d for project %s: %s",
+ ProjectWatches.WATCH_CONFIG, admin.id().get(), project.get(), invalidNotifyValue));
+ }
}
@Test
public void pushAccountConfigToUserBranch() throws Exception {
- TestAccount oooUser = accountCreator.create("away", "away@mail.invalid", "Ambrose Way");
- requestScopeOperations.setApiUser(oooUser.id());
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestAccount oooUser = accountCreator.create("away", "away@mail.invalid", "Ambrose Way");
+ requestScopeOperations.setApiUser(oooUser.id());
- // Must clone as oooUser to ensure the push is allowed.
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, oooUser);
- fetch(allUsersRepo, RefNames.refsUsers(oooUser.id()) + ":userRef");
- allUsersRepo.reset("userRef");
+ // Must clone as oooUser to ensure the push is allowed.
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, oooUser);
+ fetch(allUsersRepo, RefNames.refsUsers(oooUser.id()) + ":userRef");
+ allUsersRepo.reset("userRef");
- Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS, "out-of-office");
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS, "out-of-office");
- accountIndexedCounter.clear();
- pushFactory
- .create(
- oooUser.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(RefNames.refsUsers(oooUser.id()))
- .assertOkStatus();
+ accountIndexedCounter.clear();
+ pushFactory
+ .create(
+ oooUser.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(RefNames.refsUsers(oooUser.id()))
+ .assertOkStatus();
- accountIndexedCounter.assertReindexOf(oooUser);
+ accountIndexedCounter.assertReindexOf(oooUser);
- AccountInfo info = gApi.accounts().self().get();
- assertThat(info.email).isEqualTo(oooUser.email());
- assertThat(info.name).isEqualTo(oooUser.fullName());
- assertThat(info.status).isEqualTo("out-of-office");
+ AccountInfo info = gApi.accounts().self().get();
+ assertThat(info.email).isEqualTo(oooUser.email());
+ assertThat(info.name).isEqualTo(oooUser.fullName());
+ assertThat(info.status).isEqualTo("out-of-office");
+ }
}
@Test
public void pushAccountConfigToUserBranchIsRejectedIfConfigIsInvalid() throws Exception {
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
- allUsersRepo.reset("userRef");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
+ allUsersRepo.reset("userRef");
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- "invalid config")
- .to(RefNames.REFS_USERS_SELF);
- r.assertErrorStatus("invalid account configuration");
- r.assertMessage(
- String.format(
- "commit '%s' has an invalid '%s' file for account '%s':"
- + " Invalid config file %s in commit %s",
- r.getCommit().name(),
- AccountProperties.ACCOUNT_CONFIG,
- admin.id(),
- AccountProperties.ACCOUNT_CONFIG,
- r.getCommit().name()));
- accountIndexedCounter.assertNoReindex();
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ "invalid config")
+ .to(RefNames.REFS_USERS_SELF);
+ r.assertErrorStatus("invalid account configuration");
+ r.assertMessage(
+ String.format(
+ "commit '%s' has an invalid '%s' file for account '%s':"
+ + " Invalid config file %s in commit %s",
+ r.getCommit().name(),
+ AccountProperties.ACCOUNT_CONFIG,
+ admin.id(),
+ AccountProperties.ACCOUNT_CONFIG,
+ r.getCommit().name()));
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void pushAccountConfigToUserBranchIsRejectedIfPreferredEmailIsInvalid() throws Exception {
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
- allUsersRepo.reset("userRef");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
+ allUsersRepo.reset("userRef");
- String noEmail = "no.email";
- Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_PREFERRED_EMAIL, noEmail);
+ String noEmail = "no.email";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_PREFERRED_EMAIL, noEmail);
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(RefNames.REFS_USERS_SELF);
- r.assertErrorStatus("invalid account configuration");
- r.assertMessage(
- String.format("invalid preferred email '%s' for account '%s'", noEmail, admin.id()));
- accountIndexedCounter.assertNoReindex();
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(RefNames.REFS_USERS_SELF);
+ r.assertErrorStatus("invalid account configuration");
+ r.assertMessage(
+ String.format("invalid preferred email '%s' for account '%s'", noEmail, admin.id()));
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void pushAccountConfigToUserBranchInvalidPreferredEmailButNotChanged() throws Exception {
- TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
- String userRef = RefNames.refsUsers(foo.id());
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
+ String userRef = RefNames.refsUsers(foo.id());
- String noEmail = "no.email";
- accountsUpdateProvider
- .get()
- .update("Set Preferred Email", foo.id(), u -> u.setPreferredEmail(noEmail));
- accountIndexedCounter.clear();
+ String noEmail = "no.email";
+ accountsUpdateProvider
+ .get()
+ .update("Set Preferred Email", foo.id(), u -> u.setPreferredEmail(noEmail));
+ accountIndexedCounter.clear();
- projectOperations
- .project(allUsers)
- .forUpdate()
- .add(allow(Permission.PUSH).ref(userRef).group(REGISTERED_USERS))
- .update();
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ projectOperations
+ .project(allUsers)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref(userRef).group(REGISTERED_USERS))
+ .update();
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- String status = "in vacation";
- Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS, status);
+ String status = "in vacation";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_STATUS, status);
- pushFactory
- .create(
- foo.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(userRef)
- .assertOkStatus();
- accountIndexedCounter.assertReindexOf(foo);
+ pushFactory
+ .create(
+ foo.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(userRef)
+ .assertOkStatus();
+ accountIndexedCounter.assertReindexOf(foo);
- AccountInfo info = gApi.accounts().id(foo.id().get()).get();
- assertThat(info.email).isEqualTo(noEmail);
- assertThat(info.name).isEqualTo(foo.fullName());
- assertThat(info.status).isEqualTo(status);
+ AccountInfo info = gApi.accounts().id(foo.id().get()).get();
+ assertThat(info.email).isEqualTo(noEmail);
+ assertThat(info.name).isEqualTo(foo.fullName());
+ assertThat(info.status).isEqualTo(status);
+ }
}
@Test
public void pushAccountConfigToUserBranchIfPreferredEmailDoesNotExistAsExtId() throws Exception {
- TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
- String userRef = RefNames.refsUsers(foo.id());
- accountIndexedCounter.clear();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestAccount foo = accountCreator.create(name("foo"), name("foo") + "@example.com", "Foo");
+ String userRef = RefNames.refsUsers(foo.id());
+ accountIndexedCounter.clear();
- projectOperations
- .project(allUsers)
- .forUpdate()
- .add(allow(Permission.PUSH).ref(userRef).group(adminGroupUuid()))
- .update();
+ projectOperations
+ .project(allUsers)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref(userRef).group(adminGroupUuid()))
+ .update();
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers, foo);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- String email = "some.email@example.com";
- Config ac = getAccountConfig(allUsersRepo);
- ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_PREFERRED_EMAIL, email);
+ String email = "some.email@example.com";
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setString(AccountProperties.ACCOUNT, null, AccountProperties.KEY_PREFERRED_EMAIL, email);
- pushFactory
- .create(
- foo.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(userRef)
- .assertOkStatus();
- accountIndexedCounter.assertReindexOf(foo);
+ pushFactory
+ .create(
+ foo.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(userRef)
+ .assertOkStatus();
+ accountIndexedCounter.assertReindexOf(foo);
- AccountInfo info = gApi.accounts().id(foo.id().get()).get();
- assertThat(info.email).isEqualTo(email);
- assertThat(info.name).isEqualTo(foo.fullName());
+ AccountInfo info = gApi.accounts().id(foo.id().get()).get();
+ assertThat(info.email).isEqualTo(email);
+ assertThat(info.name).isEqualTo(foo.fullName());
+ }
}
@Test
public void pushAccountConfigToUserBranchIsRejectedIfOwnAccountIsDeactivated() throws Exception {
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
- allUsersRepo.reset("userRef");
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, RefNames.refsUsers(admin.id()) + ":userRef");
+ allUsersRepo.reset("userRef");
- Config ac = getAccountConfig(allUsersRepo);
- ac.setBoolean(AccountProperties.ACCOUNT, null, AccountProperties.KEY_ACTIVE, false);
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setBoolean(AccountProperties.ACCOUNT, null, AccountProperties.KEY_ACTIVE, false);
- PushOneCommit.Result r =
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(RefNames.REFS_USERS_SELF);
- r.assertErrorStatus("invalid account configuration");
- r.assertMessage("cannot deactivate own account");
- accountIndexedCounter.assertNoReindex();
+ PushOneCommit.Result r =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(RefNames.REFS_USERS_SELF);
+ r.assertErrorStatus("invalid account configuration");
+ r.assertMessage("cannot deactivate own account");
+ accountIndexedCounter.assertNoReindex();
+ }
}
@Test
public void pushAccountConfigToUserBranchDeactivateOtherAccount() throws Exception {
- projectOperations
- .allProjectsForUpdate()
- .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
- .update();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
- TestAccount foo = accountCreator.create(name("foo"));
- assertThat(gApi.accounts().id(foo.id().get()).getActive()).isTrue();
- String userRef = RefNames.refsUsers(foo.id());
- accountIndexedCounter.clear();
+ TestAccount foo = accountCreator.create(name("foo"));
+ assertThat(gApi.accounts().id(foo.id().get()).getActive()).isTrue();
+ String userRef = RefNames.refsUsers(foo.id());
+ accountIndexedCounter.clear();
- projectOperations
- .project(allUsers)
- .forUpdate()
- .add(allow(Permission.PUSH).ref(userRef).group(adminGroupUuid()))
- .update();
+ projectOperations
+ .project(allUsers)
+ .forUpdate()
+ .add(allow(Permission.PUSH).ref(userRef).group(adminGroupUuid()))
+ .update();
- TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
- fetch(allUsersRepo, userRef + ":userRef");
- allUsersRepo.reset("userRef");
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ fetch(allUsersRepo, userRef + ":userRef");
+ allUsersRepo.reset("userRef");
- Config ac = getAccountConfig(allUsersRepo);
- ac.setBoolean(AccountProperties.ACCOUNT, null, AccountProperties.KEY_ACTIVE, false);
+ Config ac = getAccountConfig(allUsersRepo);
+ ac.setBoolean(AccountProperties.ACCOUNT, null, AccountProperties.KEY_ACTIVE, false);
- pushFactory
- .create(
- admin.newIdent(),
- allUsersRepo,
- "Update account config",
- AccountProperties.ACCOUNT_CONFIG,
- ac.toText())
- .to(userRef)
- .assertOkStatus();
- accountIndexedCounter.assertReindexOf(foo);
+ pushFactory
+ .create(
+ admin.newIdent(),
+ allUsersRepo,
+ "Update account config",
+ AccountProperties.ACCOUNT_CONFIG,
+ ac.toText())
+ .to(userRef)
+ .assertOkStatus();
+ accountIndexedCounter.assertReindexOf(foo);
- assertThat(gApi.accounts().id(foo.id().get()).getActive()).isFalse();
+ assertThat(gApi.accounts().id(foo.id().get()).getActive()).isFalse();
+ }
}
@Test
@@ -2150,103 +2284,127 @@
@Test
public void addOtherUsersGpgKey_Conflict() throws Exception {
- // Both users have a matching external ID for this key.
- addExternalIdEmail(admin, "test5@example.com");
- accountsUpdateProvider
- .get()
- .update(
- "Add External ID",
- user.id(),
- u -> u.addExternalId(ExternalId.create("foo", "myId", user.id())));
- accountIndexedCounter.assertReindexOf(user);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ // Both users have a matching external ID for this key.
+ addExternalIdEmail(admin, "test5@example.com");
+ accountIndexedCounter.clear();
+ accountsUpdateProvider
+ .get()
+ .update(
+ "Add External ID",
+ user.id(),
+ u -> u.addExternalId(ExternalId.create("foo", "myId", user.id())));
+ accountIndexedCounter.assertReindexOf(user);
- TestKey key = validKeyWithSecondUserId();
- addGpgKey(key.getPublicKeyArmored());
- requestScopeOperations.setApiUser(user.id());
+ TestKey key = validKeyWithSecondUserId();
+ addGpgKey(key.getPublicKeyArmored());
+ requestScopeOperations.setApiUser(user.id());
- ResourceConflictException thrown =
- assertThrows(
- ResourceConflictException.class, () -> addGpgKey(user, key.getPublicKeyArmored()));
- assertThat(thrown).hasMessageThat().contains("GPG key already associated with another account");
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class, () -> addGpgKey(user, key.getPublicKeyArmored()));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("GPG key already associated with another account");
+ }
}
@Test
public void listGpgKeys() throws Exception {
- List<TestKey> keys = allValidKeys();
- List<String> toAdd = new ArrayList<>(keys.size());
- for (TestKey key : keys) {
- addExternalIdEmail(admin, PushCertificateIdent.parse(key.getFirstUserId()).getEmailAddress());
- toAdd.add(key.getPublicKeyArmored());
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ List<TestKey> keys = allValidKeys();
+ List<String> toAdd = new ArrayList<>(keys.size());
+ for (TestKey key : keys) {
+ addExternalIdEmail(
+ admin, PushCertificateIdent.parse(key.getFirstUserId()).getEmailAddress());
+ toAdd.add(key.getPublicKeyArmored());
+ }
+ accountIndexedCounter.clear();
+ gApi.accounts().self().putGpgKeys(toAdd, ImmutableList.of());
+ assertKeys(keys);
+ accountIndexedCounter.assertReindexOf(admin);
}
- gApi.accounts().self().putGpgKeys(toAdd, ImmutableList.of());
- assertKeys(keys);
- accountIndexedCounter.assertReindexOf(admin);
}
@Test
public void deleteGpgKey() throws Exception {
- TestKey key = validKeyWithoutExpiration();
- String id = key.getKeyIdString();
- addExternalIdEmail(admin, "test1@example.com");
- addGpgKey(key.getPublicKeyArmored());
- assertKeys(key);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ TestKey key = validKeyWithoutExpiration();
+ String id = key.getKeyIdString();
+ addExternalIdEmail(admin, "test1@example.com");
+ addGpgKey(key.getPublicKeyArmored());
+ assertKeys(key);
+ accountIndexedCounter.clear();
- sender.clear();
- gApi.accounts().self().gpgKey(id).delete();
- accountIndexedCounter.assertReindexOf(admin);
- assertKeys();
- assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).body()).contains("GPG keys have been deleted");
+ sender.clear();
+ gApi.accounts().self().gpgKey(id).delete();
+ accountIndexedCounter.assertReindexOf(admin);
+ assertKeys();
+ assertThat(sender.getMessages()).hasSize(1);
+ assertThat(sender.getMessages().get(0).body()).contains("GPG keys have been deleted");
- ResourceNotFoundException thrown =
- assertThrows(
- ResourceNotFoundException.class, () -> gApi.accounts().self().gpgKey(id).get());
- assertThat(thrown).hasMessageThat().contains(id);
+ ResourceNotFoundException thrown =
+ assertThrows(
+ ResourceNotFoundException.class, () -> gApi.accounts().self().gpgKey(id).get());
+ assertThat(thrown).hasMessageThat().contains(id);
+ }
}
@Test
public void addAndRemoveGpgKeys() throws Exception {
- for (TestKey key : allValidKeys()) {
- addExternalIdEmail(admin, PushCertificateIdent.parse(key.getFirstUserId()).getEmailAddress());
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ for (TestKey key : allValidKeys()) {
+ addExternalIdEmail(
+ admin, PushCertificateIdent.parse(key.getFirstUserId()).getEmailAddress());
+ }
+ accountIndexedCounter.clear();
+ TestKey key1 = validKeyWithoutExpiration();
+ TestKey key2 = validKeyWithExpiration();
+ TestKey key5 = validKeyWithSecondUserId();
+
+ Map<String, GpgKeyInfo> infos =
+ gApi.accounts()
+ .self()
+ .putGpgKeys(
+ ImmutableList.of(key1.getPublicKeyArmored(), key2.getPublicKeyArmored()),
+ ImmutableList.of(key5.getKeyIdString()));
+ assertThat(infos.keySet()).containsExactly(key1.getKeyIdString(), key2.getKeyIdString());
+ assertKeys(key1, key2);
+ accountIndexedCounter.assertReindexOf(admin);
+
+ infos =
+ gApi.accounts()
+ .self()
+ .putGpgKeys(
+ ImmutableList.of(key5.getPublicKeyArmored()),
+ ImmutableList.of(key1.getKeyIdString()));
+ assertThat(infos.keySet()).containsExactly(key1.getKeyIdString(), key5.getKeyIdString());
+ assertKeyMapContains(key5, infos);
+ assertThat(infos.get(key1.getKeyIdString()).key).isNull();
+ assertKeys(key2, key5);
+ accountIndexedCounter.assertReindexOf(admin);
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.accounts()
+ .self()
+ .putGpgKeys(
+ ImmutableList.of(key2.getPublicKeyArmored()),
+ ImmutableList.of(key2.getKeyIdString())));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Cannot both add and delete key: " + keyToString(key2.getPublicKey()));
}
- TestKey key1 = validKeyWithoutExpiration();
- TestKey key2 = validKeyWithExpiration();
- TestKey key5 = validKeyWithSecondUserId();
-
- Map<String, GpgKeyInfo> infos =
- gApi.accounts()
- .self()
- .putGpgKeys(
- ImmutableList.of(key1.getPublicKeyArmored(), key2.getPublicKeyArmored()),
- ImmutableList.of(key5.getKeyIdString()));
- assertThat(infos.keySet()).containsExactly(key1.getKeyIdString(), key2.getKeyIdString());
- assertKeys(key1, key2);
- accountIndexedCounter.assertReindexOf(admin);
-
- infos =
- gApi.accounts()
- .self()
- .putGpgKeys(
- ImmutableList.of(key5.getPublicKeyArmored()),
- ImmutableList.of(key1.getKeyIdString()));
- assertThat(infos.keySet()).containsExactly(key1.getKeyIdString(), key5.getKeyIdString());
- assertKeyMapContains(key5, infos);
- assertThat(infos.get(key1.getKeyIdString()).key).isNull();
- assertKeys(key2, key5);
- accountIndexedCounter.assertReindexOf(admin);
-
- BadRequestException thrown =
- assertThrows(
- BadRequestException.class,
- () ->
- gApi.accounts()
- .self()
- .putGpgKeys(
- ImmutableList.of(key2.getPublicKeyArmored()),
- ImmutableList.of(key2.getKeyIdString())));
- assertThat(thrown)
- .hasMessageThat()
- .contains("Cannot both add and delete key: " + keyToString(key2.getPublicKey()));
}
@Test
@@ -2259,110 +2417,118 @@
@Test
@UseSsh
public void sshKeys() throws Exception {
- // The test account should initially have exactly one ssh key
- List<SshKeyInfo> info = gApi.accounts().self().listSshKeys();
- assertThat(info).hasSize(1);
- assertSequenceNumbers(info);
- SshKeyInfo key = info.get(0);
- KeyPair keyPair = sshKeys.getKeyPair(admin);
- String initial = TestSshKeys.publicKey(keyPair, admin.email());
- assertThat(key.sshPublicKey).isEqualTo(initial);
- accountIndexedCounter.assertNoReindex();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ // The test account should initially have exactly one ssh key
+ List<SshKeyInfo> info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(1);
+ assertSequenceNumbers(info);
+ SshKeyInfo key = info.get(0);
+ KeyPair keyPair = sshKeys.getKeyPair(admin);
+ String initial = TestSshKeys.publicKey(keyPair, admin.email());
+ assertThat(key.sshPublicKey).isEqualTo(initial);
+ accountIndexedCounter.assertNoReindex();
- // Add a new key
- sender.clear();
- String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
- gApi.accounts().self().addSshKey(newKey);
- info = gApi.accounts().self().listSshKeys();
- assertThat(info).hasSize(2);
- assertSequenceNumbers(info);
- accountIndexedCounter.assertReindexOf(admin);
- assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
+ // Add a new key
+ sender.clear();
+ String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
+ gApi.accounts().self().addSshKey(newKey);
+ info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(2);
+ assertSequenceNumbers(info);
+ accountIndexedCounter.assertReindexOf(admin);
+ assertThat(sender.getMessages()).hasSize(1);
+ assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
- // Add an existing key (the request succeeds, but the key isn't added again)
- sender.clear();
- gApi.accounts().self().addSshKey(initial);
- info = gApi.accounts().self().listSshKeys();
- assertThat(info).hasSize(2);
- assertSequenceNumbers(info);
- accountIndexedCounter.assertNoReindex();
- // TODO: Issue 10769: Adding an already existing key should not result in a notification email
- assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
+ // Add an existing key (the request succeeds, but the key isn't added again)
+ sender.clear();
+ gApi.accounts().self().addSshKey(initial);
+ info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(2);
+ assertSequenceNumbers(info);
+ accountIndexedCounter.assertNoReindex();
+ // TODO: Issue 10769: Adding an already existing key should not result in a notification email
+ assertThat(sender.getMessages()).hasSize(1);
+ assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
- // Add another new key
- sender.clear();
- String newKey2 = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
- gApi.accounts().self().addSshKey(newKey2);
- info = gApi.accounts().self().listSshKeys();
- assertThat(info).hasSize(3);
- assertSequenceNumbers(info);
- accountIndexedCounter.assertReindexOf(admin);
- assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
+ // Add another new key
+ sender.clear();
+ String newKey2 = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
+ gApi.accounts().self().addSshKey(newKey2);
+ info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(3);
+ assertSequenceNumbers(info);
+ accountIndexedCounter.assertReindexOf(admin);
+ assertThat(sender.getMessages()).hasSize(1);
+ assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added");
- // Delete second key
- sender.clear();
- gApi.accounts().self().deleteSshKey(2);
- info = gApi.accounts().self().listSshKeys();
- assertThat(info).hasSize(2);
- assertThat(info.get(0).seq).isEqualTo(1);
- assertThat(info.get(1).seq).isEqualTo(3);
- accountIndexedCounter.assertReindexOf(admin);
+ // Delete second key
+ sender.clear();
+ gApi.accounts().self().deleteSshKey(2);
+ info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(2);
+ assertThat(info.get(0).seq).isEqualTo(1);
+ assertThat(info.get(1).seq).isEqualTo(3);
+ accountIndexedCounter.assertReindexOf(admin);
- assertThat(sender.getMessages()).hasSize(1);
- assertThat(sender.getMessages().get(0).body()).contains("SSH keys have been deleted");
+ assertThat(sender.getMessages()).hasSize(1);
+ assertThat(sender.getMessages().get(0).body()).contains("SSH keys have been deleted");
- // Mark first key as invalid
- assertThat(info.get(0).valid).isTrue();
- authorizedKeys.markKeyInvalid(admin.id(), 1);
- info = gApi.accounts().self().listSshKeys();
- assertThat(info).hasSize(2);
- assertThat(info.get(0).seq).isEqualTo(1);
- assertThat(info.get(0).valid).isFalse();
- assertThat(info.get(1).seq).isEqualTo(3);
- accountIndexedCounter.assertReindexOf(admin);
+ // Mark first key as invalid
+ assertThat(info.get(0).valid).isTrue();
+ authorizedKeys.markKeyInvalid(admin.id(), 1);
+ info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(2);
+ assertThat(info.get(0).seq).isEqualTo(1);
+ assertThat(info.get(0).valid).isFalse();
+ assertThat(info.get(1).seq).isEqualTo(3);
+ accountIndexedCounter.assertReindexOf(admin);
+ }
}
@Test
@UseSsh
public void adminCanAddOrRemoveSshKeyOnOtherAccount() throws Exception {
- // The test account should initially have exactly one ssh key
- List<SshKeyInfo> info = gApi.accounts().self().listSshKeys();
- assertThat(info).hasSize(1);
- assertSequenceNumbers(info);
- SshKeyInfo key = info.get(0);
- KeyPair keyPair = sshKeys.getKeyPair(admin);
- String initial = TestSshKeys.publicKey(keyPair, admin.email());
- assertThat(key.sshPublicKey).isEqualTo(initial);
- accountIndexedCounter.assertNoReindex();
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ // The test account should initially have exactly one ssh key
+ List<SshKeyInfo> info = gApi.accounts().self().listSshKeys();
+ assertThat(info).hasSize(1);
+ assertSequenceNumbers(info);
+ SshKeyInfo key = info.get(0);
+ KeyPair keyPair = sshKeys.getKeyPair(admin);
+ String initial = TestSshKeys.publicKey(keyPair, admin.email());
+ assertThat(key.sshPublicKey).isEqualTo(initial);
+ accountIndexedCounter.assertNoReindex();
- // Add a new key
- sender.clear();
- String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), user.email());
- gApi.accounts().id(user.username()).addSshKey(newKey);
- info = gApi.accounts().id(user.username()).listSshKeys();
- assertThat(info).hasSize(2);
- assertSequenceNumbers(info);
- accountIndexedCounter.assertReindexOf(user);
+ // Add a new key
+ sender.clear();
+ String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), user.email());
+ gApi.accounts().id(user.username()).addSshKey(newKey);
+ info = gApi.accounts().id(user.username()).listSshKeys();
+ assertThat(info).hasSize(2);
+ assertSequenceNumbers(info);
+ accountIndexedCounter.assertReindexOf(user);
- assertThat(sender.getMessages()).hasSize(1);
- Message message = sender.getMessages().get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
- assertThat(message.body()).contains("new SSH keys have been added");
+ assertThat(sender.getMessages()).hasSize(1);
+ Message message = sender.getMessages().get(0);
+ assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.body()).contains("new SSH keys have been added");
- // Delete key
- sender.clear();
- gApi.accounts().id(user.username()).deleteSshKey(1);
- info = gApi.accounts().id(user.username()).listSshKeys();
- assertThat(info).hasSize(1);
- accountIndexedCounter.assertReindexOf(user);
+ // Delete key
+ sender.clear();
+ gApi.accounts().id(user.username()).deleteSshKey(1);
+ info = gApi.accounts().id(user.username()).listSshKeys();
+ assertThat(info).hasSize(1);
+ accountIndexedCounter.assertReindexOf(user);
- assertThat(sender.getMessages()).hasSize(1);
- message = sender.getMessages().get(0);
- assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
- assertThat(message.body()).contains("SSH keys have been deleted");
+ assertThat(sender.getMessages()).hasSize(1);
+ message = sender.getMessages().get(0);
+ assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
+ assertThat(message.body()).contains("SSH keys have been deleted");
+ }
}
@Test
@@ -2385,20 +2551,24 @@
// reindex is tested by {@link AbstractQueryAccountsTest#reindex}
@Test
public void reindexPermissions() throws Exception {
- // admin can reindex any account
- requestScopeOperations.setApiUser(admin.id());
- gApi.accounts().id(user.username()).index();
- accountIndexedCounter.assertReindexOf(user);
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ // admin can reindex any account
+ requestScopeOperations.setApiUser(admin.id());
+ gApi.accounts().id(user.username()).index();
+ accountIndexedCounter.assertReindexOf(user);
- // user can reindex own account
- requestScopeOperations.setApiUser(user.id());
- gApi.accounts().self().index();
- accountIndexedCounter.assertReindexOf(user);
+ // user can reindex own account
+ requestScopeOperations.setApiUser(user.id());
+ gApi.accounts().self().index();
+ accountIndexedCounter.assertReindexOf(user);
- // user cannot reindex any account
- AuthException thrown =
- assertThrows(AuthException.class, () -> gApi.accounts().id(admin.username()).index());
- assertThat(thrown).hasMessageThat().contains("modify account not permitted");
+ // user cannot reindex any account
+ AuthException thrown =
+ assertThrows(AuthException.class, () -> gApi.accounts().id(admin.username()).index());
+ assertThat(thrown).hasMessageThat().contains("modify account not permitted");
+ }
}
@Test
@@ -3266,17 +3436,21 @@
}
private void addExternalIdEmail(TestAccount account, String email) throws Exception {
- requireNonNull(email);
- accountsUpdateProvider
- .get()
- .update(
- "Add Email",
- account.id(),
- u ->
- u.addExternalId(
- ExternalId.createWithEmail(name("test"), email, account.id(), email)));
- accountIndexedCounter.assertReindexOf(account);
- requestScopeOperations.setApiUser(account.id());
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ requireNonNull(email);
+ accountsUpdateProvider
+ .get()
+ .update(
+ "Add Email",
+ account.id(),
+ u ->
+ u.addExternalId(
+ ExternalId.createWithEmail(name("test"), email, account.id(), email)));
+ accountIndexedCounter.assertReindexOf(account);
+ requestScopeOperations.setApiUser(account.id());
+ }
}
private Map<String, GpgKeyInfo> addGpgKey(String armored) throws Exception {
@@ -3284,12 +3458,16 @@
}
private Map<String, GpgKeyInfo> addGpgKey(TestAccount account, String armored) throws Exception {
- Map<String, GpgKeyInfo> gpgKeys =
- gApi.accounts()
- .id(account.username())
- .putGpgKeys(ImmutableList.of(armored), ImmutableList.<String>of());
- accountIndexedCounter.assertReindexOf(gApi.accounts().id(account.username()).get());
- return gpgKeys;
+ AccountIndexedCounter accountIndexedCounter = new AccountIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(accountIndexedCounter)) {
+ Map<String, GpgKeyInfo> gpgKeys =
+ gApi.accounts()
+ .id(account.username())
+ .putGpgKeys(ImmutableList.of(armored), ImmutableList.<String>of());
+ accountIndexedCounter.assertReindexOf(gApi.accounts().id(account.username()).get());
+ return gpgKeys;
+ }
}
private Map<String, GpgKeyInfo> addGpgKeyNoReindex(String armored) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 70d5e99..9767e08 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -161,6 +161,7 @@
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.testing.TestChangeETagComputation;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -2256,7 +2257,8 @@
PushOneCommit.Result r = createChange();
String oldETag = parseResource(r).getETag();
- try (Registration registration = extensionRegistry.newRegistration().add((p, id) -> "foo")) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(TestChangeETagComputation.withETag("foo"))) {
assertThat(parseResource(r).getETag()).isNotEqualTo(oldETag);
}
@@ -2268,7 +2270,8 @@
PushOneCommit.Result r = createChange();
String oldETag = parseResource(r).getETag();
- try (Registration registration = extensionRegistry.newRegistration().add((p, id) -> null)) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(TestChangeETagComputation.withETag(null))) {
assertThat(parseResource(r).getETag()).isEqualTo(oldETag);
}
}
@@ -2282,9 +2285,8 @@
extensionRegistry
.newRegistration()
.add(
- (p, id) -> {
- throw new StorageException("exception during test");
- })) {
+ TestChangeETagComputation.withException(
+ new StorageException("exception during test")))) {
assertThat(parseResource(r).getETag()).isEqualTo(oldETag);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 599a2b9..ba8bb7a 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -38,6 +38,8 @@
import com.google.common.truth.Correspondence;
import com.google.common.util.concurrent.AtomicLongMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
@@ -66,8 +68,6 @@
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.GroupOptionsInfo;
import com.google.gerrit.extensions.events.GroupIndexedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -127,7 +127,6 @@
public class GroupsIT extends AbstractDaemonTest {
@Inject @ServerInitiated private GroupsUpdate groupsUpdate;
@Inject private AccountOperations accountOperations;
- @Inject private DynamicSet<GroupIndexedListener> groupIndexedListeners;
@Inject private GroupIncludeCache groupIncludeCache;
@Inject private GroupIndexer groupIndexer;
@Inject private GroupOperations groupOperations;
@@ -138,6 +137,7 @@
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private Sequences seq;
@Inject private StalenessChecker stalenessChecker;
+ @Inject private ExtensionRegistry extensionRegistry;
@After
public void consistencyCheck() throws Exception {
@@ -1334,9 +1334,7 @@
restartAsSlave();
GroupIndexedCounter groupIndexedCounter = new GroupIndexedCounter();
- RegistrationHandle groupIndexEventCounterHandle =
- groupIndexedListeners.add("gerrit", groupIndexedCounter);
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(groupIndexedCounter)) {
// Running the reindexer right after startup should not need to reindex any group since
// reindexing was already done on startup.
slaveGroupIndexer.run();
@@ -1372,8 +1370,6 @@
}
slaveGroupIndexer.run();
groupIndexedCounter.assertReindexOf(groupUuid);
- } finally {
- groupIndexEventCounterHandle.remove();
}
}
@@ -1391,14 +1387,10 @@
restartAsSlave();
GroupIndexedCounter groupIndexedCounter = new GroupIndexedCounter();
- RegistrationHandle groupIndexEventCounterHandle =
- groupIndexedListeners.add("gerrit", groupIndexedCounter);
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(groupIndexedCounter)) {
// No group indexing happened on startup. All groups should be reindexed now.
slaveGroupIndexer.run();
groupIndexedCounter.assertReindexOf(expectedGroups);
- } finally {
- groupIndexEventCounterHandle.remove();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 2087f939..47eec0d 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -40,6 +40,8 @@
import com.google.common.collect.Iterators;
import com.google.common.collect.ListMultimap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.TestAccount;
@@ -75,8 +77,6 @@
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
import com.google.gerrit.extensions.events.ChangeIndexedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
@@ -117,11 +117,10 @@
import org.junit.Test;
public class RevisionIT extends AbstractDaemonTest {
- @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
- @Inject private DynamicSet<PatchSetWebLink> patchSetLinks;
@Inject private GetRevisionActions getRevisionActions;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
@Test
public void reviewTriplet() throws Exception {
@@ -1079,25 +1078,22 @@
// Using the API returns the correct value, and reindexes as well.
CountDownLatch reindexed = new CountDownLatch(1);
- RegistrationHandle handle =
- changeIndexedListeners.add(
- "gerrit",
- new ChangeIndexedListener() {
- @Override
- public void onChangeIndexed(String projectName, int id) {
- if (id == id2.get()) {
- reindexed.countDown();
- }
- }
+ ChangeIndexedListener listener =
+ new ChangeIndexedListener() {
+ @Override
+ public void onChangeIndexed(String projectName, int id) {
+ if (id == id2.get()) {
+ reindexed.countDown();
+ }
+ }
- @Override
- public void onChangeDeleted(int id) {}
- });
- try {
+ @Override
+ public void onChangeDeleted(int id) {}
+ };
+
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
assertMergeable(r2.getChangeId(), true);
reindexed.await();
- } finally {
- handle.remove();
}
List<ChangeInfo> changes = search.call();
@@ -1315,10 +1311,14 @@
@Test
public void commit() throws Exception {
WebLinkInfo expectedWebLinkInfo = new WebLinkInfo("foo", "imageUrl", "url");
- RegistrationHandle handle =
- patchSetLinks.add("gerrit", (projectName, commit) -> expectedWebLinkInfo);
-
- try {
+ PatchSetWebLink link =
+ new PatchSetWebLink() {
+ @Override
+ public WebLinkInfo getPatchSetWebLink(String projectName, String commit) {
+ return expectedWebLinkInfo;
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(link)) {
PushOneCommit.Result r = createChange();
RevCommit c = r.getCommit();
@@ -1340,8 +1340,6 @@
assertThat(webLinkInfo.imageUrl).isEqualTo(expectedWebLinkInfo.imageUrl);
assertThat(webLinkInfo.url).isEqualTo(expectedWebLinkInfo.url);
assertThat(webLinkInfo.target).isEqualTo(expectedWebLinkInfo.target);
- } finally {
- handle.remove();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index e8ab515..8afa0e0 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -50,6 +50,8 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -82,8 +84,6 @@
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -150,12 +150,11 @@
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
private static String NEW_CHANGE_INDICATOR = " [NEW]";
private LabelType patchSetLock;
- @Inject private DynamicSet<CommitValidationListener> commitValidators;
-
@Before
public void setUpPatchSetLock() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
@@ -2206,24 +2205,16 @@
@GerritConfig(name = "receive.maxBatchCommits", value = "2")
@Test
public void maxBatchCommitsWithDefaultValidator() throws Exception {
- TestValidator validator = new TestValidator();
- RegistrationHandle handle = commitValidators.add("test-validator", validator);
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(new TestValidator())) {
testMaxBatchCommits();
- } finally {
- handle.remove();
}
}
@GerritConfig(name = "receive.maxBatchCommits", value = "2")
@Test
public void maxBatchCommitsWithValidateAllCommitsValidator() throws Exception {
- TestValidator validator = new TestValidator(true);
- RegistrationHandle handle = commitValidators.add("test-validator", validator);
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(new TestValidator())) {
testMaxBatchCommits();
- } finally {
- handle.remove();
}
}
@@ -2281,10 +2272,7 @@
public void skipValidation() throws Exception {
String master = "refs/heads/master";
TestValidator validator = new TestValidator();
- RegistrationHandle handle = commitValidators.add("test-validator", validator);
- RegistrationHandle handle2 = null;
-
- try {
+ try (Registration registration = extensionRegistry.newRegistration().add(validator)) {
// Validation listener is called on normal push
PushOneCommit push =
pushFactory.create(admin.newIdent(), testRepo, "change1", "a.txt", "content");
@@ -2313,20 +2301,16 @@
// Validation listener that needs to validate all commits gets called even
// when the skip option is used.
TestValidator validator2 = new TestValidator(true);
- handle2 = commitValidators.add("test-validator-2", validator2);
- PushOneCommit push4 =
- pushFactory.create(admin.newIdent(), testRepo, "change2", "b.txt", "content");
- push4.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
- r = push4.to(master);
- r.assertOkStatus();
- // First listener was not called; its count remains the same.
- assertThat(validator.count()).isEqualTo(1);
- // Second listener was called.
- assertThat(validator2.count()).isEqualTo(1);
- } finally {
- handle.remove();
- if (handle2 != null) {
- handle2.remove();
+ try (Registration registration2 = extensionRegistry.newRegistration().add(validator2)) {
+ PushOneCommit push4 =
+ pushFactory.create(admin.newIdent(), testRepo, "change2", "b.txt", "content");
+ push4.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
+ r = push4.to(master);
+ r.assertOkStatus();
+ // First listener was not called; its count remains the same.
+ assertThat(validator.count()).isEqualTo(1);
+ // Second listener was called.
+ assertThat(validator2.count()).isEqualTo(1);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
index f8176a5..f633842 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/AbstractReindexTests.java
@@ -74,13 +74,13 @@
.containsExactly(changeId);
// Query account index
assertThat(gApi.accounts().query("admin").get().stream().map(a -> a._accountId))
- .containsExactly(adminId.get());
+ .containsExactly(admin.id().get());
// Query group index
assertThat(
gApi.groups().query("Group").withOption(MEMBERS).get().stream()
.flatMap(g -> g.members.stream())
.map(a -> a._accountId))
- .containsExactly(adminId.get());
+ .containsExactly(admin.id().get());
// Query project index
assertThat(gApi.projects().query(project.get()).get().stream().map(p -> p.name))
.containsExactly(project.get());
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index c0083d2..154fabf 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -42,6 +42,8 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
@@ -67,8 +69,6 @@
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.events.ChangeIndexedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -118,7 +118,6 @@
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.RefSpec;
-import org.junit.After;
import org.junit.Test;
@NoHttpd
@@ -131,21 +130,11 @@
}
@Inject private ApprovalsUtil approvalsUtil;
- @Inject private DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
- @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
@Inject private IdentifiedUser.GenericFactory userFactory;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private Submit submitHandler;
-
- private RegistrationHandle onSubmitValidatorHandle;
-
- @After
- public void removeOnSubmitValidator() {
- if (onSubmitValidatorHandle != null) {
- onSubmitValidatorHandle.remove();
- }
- }
+ @Inject private ExtensionRegistry extensionRegistry;
protected abstract SubmitType getSubmitType();
@@ -821,23 +810,28 @@
@Test
public void submitWithValidation() throws Throwable {
AtomicBoolean called = new AtomicBoolean(false);
- this.addOnSubmitValidationListener(
- args -> {
- called.set(true);
- HashSet<String> refs = Sets.newHashSet(args.getCommands().keySet());
- assertThat(refs).contains("refs/heads/master");
- refs.remove("refs/heads/master");
- if (!refs.isEmpty()) {
- // Some submit strategies need to insert new patchset.
- assertThat(refs).hasSize(1);
- assertThat(refs.iterator().next()).startsWith(RefNames.REFS_CHANGES);
+ OnSubmitValidationListener listener =
+ new OnSubmitValidationListener() {
+ @Override
+ public void preBranchUpdate(Arguments args) throws ValidationException {
+ called.set(true);
+ HashSet<String> refs = Sets.newHashSet(args.getCommands().keySet());
+ assertThat(refs).contains("refs/heads/master");
+ refs.remove("refs/heads/master");
+ if (!refs.isEmpty()) {
+ // Some submit strategies need to insert new patchset.
+ assertThat(refs).hasSize(1);
+ assertThat(refs.iterator().next()).startsWith(RefNames.REFS_CHANGES);
+ }
}
- });
+ };
- PushOneCommit.Result change = createChange();
- approve(change.getChangeId());
- submit(change.getChangeId());
- assertThat(called.get()).isTrue();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ PushOneCommit.Result change = createChange();
+ approve(change.getChangeId());
+ submit(change.getChangeId());
+ assertThat(called.get()).isTrue();
+ }
}
@Test
@@ -872,34 +866,39 @@
// Since there are 2 repos, first submit attempt will fail, the second will
// succeed.
List<String> projectsCalled = new ArrayList<>(4);
- this.addOnSubmitValidationListener(
- args -> {
- String master = "refs/heads/master";
- assertThat(args.getCommands()).containsKey(master);
- ReceiveCommand cmd = args.getCommands().get(master);
- ObjectId newMasterId = cmd.getNewId();
- try (Repository repo = repoManager.openRepository(args.getProject())) {
- assertThat(repo.exactRef(master).getObjectId()).isEqualTo(cmd.getOldId());
- assertThat(args.getRef(master)).hasValue(newMasterId);
- args.getRevWalk().parseBody(args.getRevWalk().parseCommit(newMasterId));
- } catch (IOException e) {
- throw new AssertionError("failed checking new ref value", e);
+ OnSubmitValidationListener listener =
+ new OnSubmitValidationListener() {
+ @Override
+ public void preBranchUpdate(Arguments args) throws ValidationException {
+ String master = "refs/heads/master";
+ assertThat(args.getCommands()).containsKey(master);
+ ReceiveCommand cmd = args.getCommands().get(master);
+ ObjectId newMasterId = cmd.getNewId();
+ try (Repository repo = repoManager.openRepository(args.getProject())) {
+ assertThat(repo.exactRef(master).getObjectId()).isEqualTo(cmd.getOldId());
+ assertThat(args.getRef(master)).hasValue(newMasterId);
+ args.getRevWalk().parseBody(args.getRevWalk().parseCommit(newMasterId));
+ } catch (IOException e) {
+ throw new AssertionError("failed checking new ref value", e);
+ }
+ projectsCalled.add(args.getProject().get());
+ if (projectsCalled.size() == 2) {
+ throw new ValidationException("time to fail");
+ }
}
- projectsCalled.add(args.getProject().get());
- if (projectsCalled.size() == 2) {
- throw new ValidationException("time to fail");
- }
- });
- submitWithConflict(change4.getChangeId(), "time to fail");
- assertThat(projectsCalled).containsExactly(keyA.get(), keyB.get());
- for (PushOneCommit.Result change : changes) {
- change.assertChange(Change.Status.NEW, name(topic), admin);
- }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ submitWithConflict(change4.getChangeId(), "time to fail");
+ assertThat(projectsCalled).containsExactly(keyA.get(), keyB.get());
+ for (PushOneCommit.Result change : changes) {
+ change.assertChange(Change.Status.NEW, name(topic), admin);
+ }
- submit(change4.getChangeId());
- assertThat(projectsCalled).containsExactly(keyA.get(), keyB.get(), keyA.get(), keyB.get());
- for (PushOneCommit.Result change : changes) {
- change.assertChange(Change.Status.MERGED, name(topic), admin);
+ submit(change4.getChangeId());
+ assertThat(projectsCalled).containsExactly(keyA.get(), keyB.get(), keyA.get(), keyB.get());
+ for (PushOneCommit.Result change : changes) {
+ change.assertChange(Change.Status.MERGED, name(topic), admin);
+ }
}
}
@@ -1228,9 +1227,8 @@
PushOneCommit.Result changeOtherBranch = createChange("refs/for/dev");
ChangeIndexedListener changeIndexedListener = mock(ChangeIndexedListener.class);
- RegistrationHandle registrationHandle =
- changeIndexedListeners.add("gerrit", changeIndexedListener);
- try {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedListener)) {
// submit a change, this should trigger asynchronous reindexing of the open changes on the
// same branch
approve(change1.getChangeId());
@@ -1257,8 +1255,6 @@
// open changes on other branches don't get reindexed
verify(changeIndexedListener, times(0))
.onChangeScheduledForIndexing(project.get(), changeOtherBranch.getChange().getId().get());
- } finally {
- registrationHandle.remove();
}
}
@@ -1438,11 +1434,6 @@
return getRemoteLog(project, "master");
}
- protected void addOnSubmitValidationListener(OnSubmitValidationListener listener) {
- assertThat(onSubmitValidatorHandle).isNull();
- onSubmitValidatorHandle = onSubmitValidationListeners.add("gerrit", listener);
- }
-
private String getLatestDiff(Repository repo) throws Throwable {
ObjectId oldTreeId = repo.resolve("HEAD~1^{tree}");
ObjectId newTreeId = repo.resolve("HEAD^{tree}");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index aeb5a4b..f2b8468 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -38,6 +38,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.change.RevisionJson;
+import com.google.gerrit.server.change.testing.TestChangeETagComputation;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
@@ -195,7 +196,8 @@
String change = createChange().getChangeId();
String oldETag = getETag(change);
- try (Registration registration = extensionRegistry.newRegistration().add((p, id) -> "foo")) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(TestChangeETagComputation.withETag("foo"))) {
assertThat(getETag(change)).isNotEqualTo(oldETag);
}
@@ -207,7 +209,8 @@
String change = createChange().getChangeId();
String oldETag = getETag(change);
- try (Registration registration = extensionRegistry.newRegistration().add((p, id) -> null)) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(TestChangeETagComputation.withETag(null))) {
assertThat(getETag(change)).isEqualTo(oldETag);
}
}
@@ -221,9 +224,8 @@
extensionRegistry
.newRegistration()
.add(
- (p, id) -> {
- throw new StorageException("exception during test");
- })) {
+ TestChangeETagComputation.withException(
+ new StorageException("exception during test")))) {
assertThat(getETag(change)).isEqualTo(oldETag);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
index 16b7690..6f519f1 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
@@ -19,6 +19,8 @@
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -29,9 +31,6 @@
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
-import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.submit.CommitMergeStatus;
import com.google.inject.Inject;
import java.util.List;
@@ -40,8 +39,8 @@
import org.junit.Test;
public class SubmitByCherryPickIT extends AbstractSubmit {
- @Inject private DynamicSet<ChangeMessageModifier> changeMessageModifiers;
@Inject private ProjectOperations projectOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
@Override
protected SubmitType getSubmitType() {
@@ -89,15 +88,13 @@
@Test
public void changeMessageOnSubmit() throws Throwable {
PushOneCommit.Result change = createChange();
- RegistrationHandle handle =
- changeMessageModifiers.add(
- "gerrit",
- (newCommitMessage, original, mergeTip, destination) ->
- newCommitMessage + "Custom: " + destination.branch());
- try {
+ try (Registration registration =
+ extensionRegistry
+ .newRegistration()
+ .add(
+ (newCommitMessage, original, mergeTip, destination) ->
+ newCommitMessage + "Custom: " + destination.branch())) {
submit(change.getChangeId());
- } finally {
- handle.remove();
}
testRepo.git().fetch().setRemote("origin").call();
ChangeInfo info = get(change.getChangeId(), CURRENT_REVISION);
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
index 1808480..0cdc0e9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java
@@ -20,6 +20,8 @@
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -28,24 +30,20 @@
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.registration.DynamicItem;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
-import java.util.ArrayDeque;
-import java.util.Deque;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
public class SubmitByRebaseAlwaysIT extends AbstractSubmitByRebase {
- @Inject private DynamicSet<ChangeMessageModifier> changeMessageModifiers;
@Inject private DynamicItem<UrlFormatter> urlFormatter;
@Inject private ProjectOperations projectOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
@Override
protected SubmitType getSubmitType() {
@@ -99,7 +97,8 @@
ChangeMessageModifier modifier3 =
(msg, orig, tip, dest) -> msg + "Dest: " + dest.shortName() + "\n";
- try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2, modifier3)) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(modifier1).add(modifier2).add(modifier3)) {
ImmutableList<PushOneCommit.Result> changes = submitWithRebase(admin);
ChangeData cd1 = changes.get(0).getChange();
ChangeData cd2 = changes.get(1).getChange();
@@ -128,7 +127,8 @@
throw new IllegalStateException("boom");
};
ChangeMessageModifier modifier2 = (msg, orig, tip, dest) -> msg + "A-footer: value\n";
- try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2)) {
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(modifier1).add(modifier2)) {
ResourceConflictException thrown =
assertThrows(ResourceConflictException.class, () -> submitWithRebase());
Throwable cause = Throwables.getRootCause(thrown);
@@ -141,7 +141,11 @@
public void changeMessageModifierReturningNullShortCircuits() throws Throwable {
ChangeMessageModifier modifier1 = (msg, orig, tip, dest) -> null;
ChangeMessageModifier modifier2 = (msg, orig, tip, dest) -> msg + "A-footer: value\n";
- try (AutoCloseable ignored = installChangeMessageModifiers(modifier1, modifier2)) {
+ try (Registration registration =
+ extensionRegistry
+ .newRegistration()
+ .add(modifier1, "modifier-1")
+ .add(modifier2, "modifier-2")) {
ResourceConflictException thrown =
assertThrows(ResourceConflictException.class, () -> submitWithRebase());
Throwable cause = Throwables.getRootCause(thrown);
@@ -155,18 +159,6 @@
}
}
- private AutoCloseable installChangeMessageModifiers(ChangeMessageModifier... modifiers) {
- Deque<RegistrationHandle> handles = new ArrayDeque<>(modifiers.length);
- for (int i = 0; i < modifiers.length; i++) {
- handles.push(changeMessageModifiers.add("modifier-" + (i + 1), modifiers[i]));
- }
- return () -> {
- while (!handles.isEmpty()) {
- handles.pop().remove();
- }
- };
- }
-
private void assertLatestRevisionHasFooters(PushOneCommit.Result change) throws Throwable {
RevCommit c = getCurrentCommit(change);
assertThat(c.getFooterLines(FooterConstants.CHANGE_ID)).isNotEmpty();
diff --git a/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java b/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java
index 7590532..a3c1722 100644
--- a/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/config/IndexChangesIT.java
@@ -21,87 +21,84 @@
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ChangeIndexedCounter;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.events.ChangeIndexedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.server.restapi.config.IndexChanges;
import com.google.inject.Inject;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
public class IndexChangesIT extends AbstractDaemonTest {
- @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
@Inject private ProjectOperations projectOperations;
-
- private ChangeIndexedCounter changeIndexedCounter;
- private RegistrationHandle changeIndexedCounterHandle;
-
- @Before
- public void addChangeIndexedCounter() {
- changeIndexedCounter = new ChangeIndexedCounter();
- changeIndexedCounterHandle = changeIndexedListeners.add("gerrit", changeIndexedCounter);
- }
-
- @After
- public void removeChangeIndexedCounter() {
- if (changeIndexedCounterHandle != null) {
- changeIndexedCounterHandle.remove();
- }
- }
+ @Inject private ExtensionRegistry extensionRegistry;
@Test
public void indexRequestFromNonAdminRejected() throws Exception {
- String changeId = createChange().getChangeId();
- IndexChanges.Input in = new IndexChanges.Input();
- in.changes = ImmutableSet.of(changeId);
- changeIndexedCounter.clear();
- userRestSession.post("/config/server/index.changes", in).assertForbidden();
- assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(0);
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ String changeId = createChange().getChangeId();
+ IndexChanges.Input in = new IndexChanges.Input();
+ in.changes = ImmutableSet.of(changeId);
+ changeIndexedCounter.clear();
+ userRestSession.post("/config/server/index.changes", in).assertForbidden();
+ assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(0);
+ }
}
@Test
public void indexVisibleChange() throws Exception {
- String changeId = createChange().getChangeId();
- IndexChanges.Input in = new IndexChanges.Input();
- in.changes = ImmutableSet.of(changeId);
- changeIndexedCounter.clear();
- adminRestSession.post("/config/server/index.changes", in).assertOK();
- assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1);
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ String changeId = createChange().getChangeId();
+ IndexChanges.Input in = new IndexChanges.Input();
+ in.changes = ImmutableSet.of(changeId);
+ changeIndexedCounter.clear();
+ adminRestSession.post("/config/server/index.changes", in).assertOK();
+ assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1);
+ }
}
@Test
public void indexNonVisibleChange() throws Exception {
- String changeId = createChange().getChangeId();
- ChangeInfo changeInfo = info(changeId);
- projectOperations
- .project(project)
- .forUpdate()
- .add(block(Permission.READ).ref("refs/heads/master").group(REGISTERED_USERS))
- .update();
- IndexChanges.Input in = new IndexChanges.Input();
- changeIndexedCounter.clear();
- in.changes = ImmutableSet.of(changeId);
- adminRestSession.post("/config/server/index.changes", in).assertOK();
- assertThat(changeIndexedCounter.getCount(changeInfo)).isEqualTo(1);
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ String changeId = createChange().getChangeId();
+ ChangeInfo changeInfo = info(changeId);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/heads/master").group(REGISTERED_USERS))
+ .update();
+ IndexChanges.Input in = new IndexChanges.Input();
+ changeIndexedCounter.clear();
+ in.changes = ImmutableSet.of(changeId);
+ adminRestSession.post("/config/server/index.changes", in).assertOK();
+ assertThat(changeIndexedCounter.getCount(changeInfo)).isEqualTo(1);
+ }
}
@Test
public void indexMultipleChanges() throws Exception {
- ImmutableSet.Builder<String> changeIds = ImmutableSet.builder();
- for (int i = 0; i < 10; i++) {
- changeIds.add(createChange().getChangeId());
- }
- IndexChanges.Input in = new IndexChanges.Input();
- in.changes = changeIds.build();
- changeIndexedCounter.clear();
- adminRestSession.post("/config/server/index.changes", in).assertOK();
- for (String changeId : in.changes) {
- assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1);
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ ImmutableSet.Builder<String> changeIds = ImmutableSet.builder();
+ for (int i = 0; i < 10; i++) {
+ changeIds.add(createChange().getChangeId());
+ }
+ IndexChanges.Input in = new IndexChanges.Input();
+ in.changes = changeIds.build();
+ changeIndexedCounter.clear();
+ adminRestSession.post("/config/server/index.changes", in).assertOK();
+ for (String changeId : in.changes) {
+ assertThat(changeIndexedCounter.getCount(info(changeId))).isEqualTo(1);
+ }
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index bb043c2..c6c26c9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -22,6 +22,8 @@
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -40,8 +42,6 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.WebLinkInfo;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -72,9 +72,9 @@
private static final String LABEL_CODE_REVIEW = "Code-Review";
- @Inject private DynamicSet<FileHistoryWebLink> fileHistoryWebLinkDynamicSet;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
private Project.NameKey newProjectName;
@@ -89,39 +89,39 @@
assertThat(inheritedName).isEqualTo(AllProjectsNameProvider.DEFAULT);
}
+ private Registration newFileHistoryWebLink() {
+ FileHistoryWebLink weblink =
+ new FileHistoryWebLink() {
+ @Override
+ public WebLinkInfo getFileHistoryWebLink(
+ String projectName, String revision, String fileName) {
+ return new WebLinkInfo(
+ "name", "imageURL", "http://view/" + projectName + "/" + fileName);
+ }
+ };
+ return extensionRegistry.newRegistration().add(weblink);
+ }
+
@Test
public void webLink() throws Exception {
- RegistrationHandle handle =
- fileHistoryWebLinkDynamicSet.add(
- "gerrit",
- (projectName, revision, fileName) ->
- new WebLinkInfo("name", "imageURL", "http://view/" + projectName + "/" + fileName));
- try {
+ try (Registration registration = newFileHistoryWebLink()) {
ProjectAccessInfo info = pApi().access();
assertThat(info.configWebLinks).hasSize(1);
assertThat(info.configWebLinks.get(0).url)
.isEqualTo("http://view/" + newProjectName + "/project.config");
- } finally {
- handle.remove();
}
}
@Test
public void webLinkNoRefsMetaConfig() throws Exception {
- RegistrationHandle handle =
- fileHistoryWebLinkDynamicSet.add(
- "gerrit",
- (projectName, revision, fileName) ->
- new WebLinkInfo("name", "imageURL", "http://view/" + projectName + "/" + fileName));
- try (Repository repo = repoManager.openRepository(newProjectName)) {
+ try (Repository repo = repoManager.openRepository(newProjectName);
+ Registration registration = newFileHistoryWebLink()) {
RefUpdate u = repo.updateRef(RefNames.REFS_CONFIG);
u.setForceUpdate(true);
assertThat(u.delete()).isEqualTo(Result.FORCED);
// This should not crash.
pApi().access();
- } finally {
- handle.remove();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
index 46fc689..8469fff 100644
--- a/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/event/CommentAddedEventIT.java
@@ -22,6 +22,8 @@
import static com.google.gerrit.server.project.testing.TestLabels.value;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -31,18 +33,15 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.events.CommentAddedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.inject.Inject;
-import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class CommentAddedEventIT extends AbstractDaemonTest {
- @Inject private DynamicSet<CommentAddedListener> source;
@Inject private ProjectOperations projectOperations;
+ @Inject private ExtensionRegistry extensionRegistry;
private final LabelType label =
label("CustomLabel", value(1, "Positive"), value(0, "No score"), value(-1, "Negative"));
@@ -50,9 +49,6 @@
private final LabelType pLabel =
label("CustomLabel2", value(1, "Positive"), value(0, "No score"));
- private RegistrationHandle eventListenerRegistration;
- private CommentAddedListener.Event lastCommentAddedEvent;
-
@Before
public void setUp() throws Exception {
projectOperations
@@ -61,12 +57,6 @@
.add(allowLabel(label.getName()).ref("refs/heads/*").group(ANONYMOUS_USERS).range(-1, 1))
.add(allowLabel(pLabel.getName()).ref("refs/heads/*").group(ANONYMOUS_USERS).range(0, 1))
.update();
- eventListenerRegistration = source.add("gerrit", event -> lastCommentAddedEvent = event);
- }
-
- @After
- public void cleanup() {
- eventListenerRegistration.remove();
}
private void saveLabelConfig() throws Exception {
@@ -77,16 +67,30 @@
}
}
+ private static class TestListener implements CommentAddedListener {
+ private CommentAddedListener.Event lastCommentAddedEvent;
+
+ @Override
+ public void onCommentAdded(Event event) {
+ lastCommentAddedEvent = event;
+ }
+
+ public CommentAddedListener.Event getLastCommentAddedEvent() {
+ assertThat(lastCommentAddedEvent).isNotNull();
+ return lastCommentAddedEvent;
+ }
+ }
+
/* Need to lookup info for the label under test since there can be multiple
* labels defined. By default Gerrit already has a Code-Review label.
*/
- private ApprovalValues getApprovalValues(LabelType label) {
+ private ApprovalValues getApprovalValues(LabelType label, TestListener listener) {
ApprovalValues res = new ApprovalValues();
- ApprovalInfo info = lastCommentAddedEvent.getApprovals().get(label.getName());
+ ApprovalInfo info = listener.getLastCommentAddedEvent().getApprovals().get(label.getName());
if (info != null) {
res.value = info.value;
}
- info = lastCommentAddedEvent.getOldApprovals().get(label.getName());
+ info = listener.getLastCommentAddedEvent().getOldApprovals().get(label.getName());
if (info != null) {
res.oldValue = info.value;
}
@@ -97,15 +101,18 @@
public void newChangeWithVote() throws Exception {
saveLabelConfig();
- // push a new change with -1 vote
- PushOneCommit.Result r = createChange();
- ReviewInput reviewInput = new ReviewInput().label(label.getName(), (short) -1);
- revision(r).review(reviewInput);
- ApprovalValues attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(0);
- assertThat(attr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ // push a new change with -1 vote
+ PushOneCommit.Result r = createChange();
+ ReviewInput reviewInput = new ReviewInput().label(label.getName(), (short) -1);
+ revision(r).review(reviewInput);
+ ApprovalValues attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(0);
+ assertThat(attr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
+ }
}
@Test
@@ -116,17 +123,19 @@
PushOneCommit.Result r = createChange();
ReviewInput reviewInput = new ReviewInput().message(label.getName());
revision(r).review(reviewInput);
-
- // push a new revision with +1 vote
- ChangeInfo c = info(r.getChangeId());
- r = amendChange(c.changeId);
- reviewInput = new ReviewInput().label(label.getName(), (short) 1);
- revision(r).review(reviewInput);
- ApprovalValues attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(0);
- assertThat(attr.value).isEqualTo(1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 2: %s+1", label.getName()));
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ // push a new revision with +1 vote
+ ChangeInfo c = info(r.getChangeId());
+ r = amendChange(c.changeId);
+ reviewInput = new ReviewInput().label(label.getName(), (short) 1);
+ revision(r).review(reviewInput);
+ ApprovalValues attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(0);
+ assertThat(attr.value).isEqualTo(1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 2: %s+1", label.getName()));
+ }
}
@Test
@@ -136,114 +145,120 @@
// push a change
PushOneCommit.Result r = createChange();
- // review with message only, do not apply votes
- ReviewInput reviewInput = new ReviewInput().message(label.getName());
- revision(r).review(reviewInput);
- // reply message only so vote is shown as 0
- ApprovalValues attr = getApprovalValues(label);
- assertThat(attr.oldValue).isNull();
- assertThat(attr.value).isEqualTo(0);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ // review with message only, do not apply votes
+ ReviewInput reviewInput = new ReviewInput().message(label.getName());
+ revision(r).review(reviewInput);
+ // reply message only so vote is shown as 0
+ ApprovalValues attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isNull();
+ assertThat(attr.value).isEqualTo(0);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
- // transition from un-voted to -1 vote
- reviewInput = new ReviewInput().label(label.getName(), -1);
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(0);
- assertThat(attr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
+ // transition from un-voted to -1 vote
+ reviewInput = new ReviewInput().label(label.getName(), -1);
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(0);
+ assertThat(attr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
- // transition vote from -1 to 0
- reviewInput = new ReviewInput().label(label.getName(), 0);
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(-1);
- assertThat(attr.value).isEqualTo(0);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: -%s", label.getName()));
+ // transition vote from -1 to 0
+ reviewInput = new ReviewInput().label(label.getName(), 0);
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(-1);
+ assertThat(attr.value).isEqualTo(0);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: -%s", label.getName()));
- // transition vote from 0 to 1
- reviewInput = new ReviewInput().label(label.getName(), 1);
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(0);
- assertThat(attr.value).isEqualTo(1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s+1", label.getName()));
+ // transition vote from 0 to 1
+ reviewInput = new ReviewInput().label(label.getName(), 1);
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(0);
+ assertThat(attr.value).isEqualTo(1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s+1", label.getName()));
- // transition vote from 1 to -1
- reviewInput = new ReviewInput().label(label.getName(), -1);
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isEqualTo(1);
- assertThat(attr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
+ // transition vote from 1 to -1
+ reviewInput = new ReviewInput().label(label.getName(), -1);
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isEqualTo(1);
+ assertThat(attr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s-1", label.getName()));
- // review with message only, do not apply votes
- reviewInput = new ReviewInput().message(label.getName());
- revision(r).review(reviewInput);
- attr = getApprovalValues(label);
- assertThat(attr.oldValue).isNull(); // no vote change so not included
- assertThat(attr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
+ // review with message only, do not apply votes
+ reviewInput = new ReviewInput().message(label.getName());
+ revision(r).review(reviewInput);
+ attr = getApprovalValues(label, listener);
+ assertThat(attr.oldValue).isNull(); // no vote change so not included
+ assertThat(attr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1:\n\n%s", label.getName()));
+ }
}
@Test
public void reviewChange_MultipleVotes() throws Exception {
- saveLabelConfig();
- PushOneCommit.Result r = createChange();
- ReviewInput reviewInput = new ReviewInput().label(label.getName(), -1);
- reviewInput.message = label.getName();
- revision(r).review(reviewInput);
+ TestListener listener = new TestListener();
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ saveLabelConfig();
+ PushOneCommit.Result r = createChange();
+ ReviewInput reviewInput = new ReviewInput().label(label.getName(), -1);
+ reviewInput.message = label.getName();
+ revision(r).review(reviewInput);
- ChangeInfo c = get(r.getChangeId(), DETAILED_LABELS);
- LabelInfo q = c.labels.get(label.getName());
- assertThat(q.all).hasSize(1);
- ApprovalValues labelAttr = getApprovalValues(label);
- assertThat(labelAttr.oldValue).isEqualTo(0);
- assertThat(labelAttr.value).isEqualTo(-1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s-1\n\n%s", label.getName(), label.getName()));
+ ChangeInfo c = get(r.getChangeId(), DETAILED_LABELS);
+ LabelInfo q = c.labels.get(label.getName());
+ assertThat(q.all).hasSize(1);
+ ApprovalValues labelAttr = getApprovalValues(label, listener);
+ assertThat(labelAttr.oldValue).isEqualTo(0);
+ assertThat(labelAttr.value).isEqualTo(-1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s-1\n\n%s", label.getName(), label.getName()));
- // there should be 3 approval labels (label, pLabel, and CRVV)
- assertThat(lastCommentAddedEvent.getApprovals()).hasSize(3);
+ // there should be 3 approval labels (label, pLabel, and CRVV)
+ assertThat(listener.getLastCommentAddedEvent().getApprovals()).hasSize(3);
- // check the approvals that were not voted on
- ApprovalValues pLabelAttr = getApprovalValues(pLabel);
- assertThat(pLabelAttr.oldValue).isNull();
- assertThat(pLabelAttr.value).isEqualTo(0);
+ // check the approvals that were not voted on
+ ApprovalValues pLabelAttr = getApprovalValues(pLabel, listener);
+ assertThat(pLabelAttr.oldValue).isNull();
+ assertThat(pLabelAttr.value).isEqualTo(0);
- LabelType crLabel = LabelType.withDefaultValues("Code-Review");
- ApprovalValues crlAttr = getApprovalValues(crLabel);
- assertThat(crlAttr.oldValue).isNull();
- assertThat(crlAttr.value).isEqualTo(0);
+ LabelType crLabel = LabelType.withDefaultValues("Code-Review");
+ ApprovalValues crlAttr = getApprovalValues(crLabel, listener);
+ assertThat(crlAttr.oldValue).isNull();
+ assertThat(crlAttr.value).isEqualTo(0);
- // update pLabel approval
- reviewInput = new ReviewInput().label(pLabel.getName(), 1);
- reviewInput.message = pLabel.getName();
- revision(r).review(reviewInput);
+ // update pLabel approval
+ reviewInput = new ReviewInput().label(pLabel.getName(), 1);
+ reviewInput.message = pLabel.getName();
+ revision(r).review(reviewInput);
- c = get(r.getChangeId(), DETAILED_LABELS);
- q = c.labels.get(label.getName());
- assertThat(q.all).hasSize(1);
- pLabelAttr = getApprovalValues(pLabel);
- assertThat(pLabelAttr.oldValue).isEqualTo(0);
- assertThat(pLabelAttr.value).isEqualTo(1);
- assertThat(lastCommentAddedEvent.getComment())
- .isEqualTo(String.format("Patch Set 1: %s+1\n\n%s", pLabel.getName(), pLabel.getName()));
+ c = get(r.getChangeId(), DETAILED_LABELS);
+ q = c.labels.get(label.getName());
+ assertThat(q.all).hasSize(1);
+ pLabelAttr = getApprovalValues(pLabel, listener);
+ assertThat(pLabelAttr.oldValue).isEqualTo(0);
+ assertThat(pLabelAttr.value).isEqualTo(1);
+ assertThat(listener.getLastCommentAddedEvent().getComment())
+ .isEqualTo(String.format("Patch Set 1: %s+1\n\n%s", pLabel.getName(), pLabel.getName()));
- // check the approvals that were not voted on
- labelAttr = getApprovalValues(label);
- assertThat(labelAttr.oldValue).isNull();
- assertThat(labelAttr.value).isEqualTo(-1);
+ // check the approvals that were not voted on
+ labelAttr = getApprovalValues(label, listener);
+ assertThat(labelAttr.oldValue).isNull();
+ assertThat(labelAttr.value).isEqualTo(-1);
- crlAttr = getApprovalValues(crLabel);
- assertThat(crlAttr.oldValue).isNull();
- assertThat(crlAttr.value).isEqualTo(0);
+ crlAttr = getApprovalValues(crLabel, listener);
+ assertThat(crlAttr.oldValue).isNull();
+ assertThat(crlAttr.value).isEqualTo(0);
+ }
}
private static class ApprovalValues {
diff --git a/javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java b/javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java
index 5ff1c32..8744cfad 100644
--- a/javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java
@@ -17,49 +17,48 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.inject.Inject;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
@NoHttpd
public class EventPayloadIT extends AbstractDaemonTest {
- @Inject private DynamicSet<RevisionCreatedListener> revisionCreatedListeners;
-
- private RegistrationHandle eventListenerRegistration;
- private RevisionCreatedListener.Event lastEvent;
-
- @Before
- public void setUp() throws Exception {
- eventListenerRegistration = revisionCreatedListeners.add("gerrit", event -> lastEvent = event);
- }
-
- @After
- public void cleanup() {
- eventListenerRegistration.remove();
- }
+ @Inject private ExtensionRegistry extensionRegistry;
@Test
public void defaultOptions() throws Exception {
- createChange();
-
- assertThat(lastEvent.getChange().submittable).isNotNull();
- assertThat(lastEvent.getRevision().files).isNotEmpty();
+ RevisionCreatedListener listener =
+ new RevisionCreatedListener() {
+ @Override
+ public void onRevisionCreated(Event event) {
+ assertThat(event.getChange().submittable).isNotNull();
+ assertThat(event.getRevision().files).isNotEmpty();
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ createChange();
+ }
}
@Test
@GerritConfig(name = "event.payload.listChangeOptions", value = "SKIP_MERGEABLE")
public void configuredOptions() throws Exception {
- createChange();
-
- assertThat(lastEvent.getChange().submittable).isNull();
- assertThat(lastEvent.getChange().mergeable).isNull();
- assertThat(lastEvent.getRevision().files).isNull();
- assertThat(lastEvent.getChange().subject).isNotEmpty();
+ RevisionCreatedListener listener =
+ new RevisionCreatedListener() {
+ @Override
+ public void onRevisionCreated(Event event) {
+ assertThat(event.getChange().submittable).isNull();
+ assertThat(event.getChange().mergeable).isNull();
+ assertThat(event.getRevision().files).isNull();
+ assertThat(event.getChange().subject).isNotEmpty();
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+ createChange();
+ }
}
}
diff --git a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
index a03d7f3..c841559 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java
@@ -20,102 +20,91 @@
import com.google.common.base.Joiner;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ChangeIndexedCounter;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.events.ChangeIndexedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Injector;
import java.util.List;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
@NoHttpd
@UseSsh
public abstract class AbstractIndexTests extends AbstractDaemonTest {
- @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
-
- private ChangeIndexedCounter changeIndexedCounter;
- private RegistrationHandle changeIndexedCounterHandle;
+ @Inject private ExtensionRegistry extensionRegistry;
/** @param injector injector */
public void configureIndex(Injector injector) {}
- @Before
- public void addChangeIndexedCounter() {
- changeIndexedCounter = new ChangeIndexedCounter();
- changeIndexedCounterHandle = changeIndexedListeners.add("gerrit", changeIndexedCounter);
- }
-
- @After
- public void removeChangeIndexedCounter() {
- if (changeIndexedCounterHandle != null) {
- changeIndexedCounterHandle.remove();
- }
- }
-
@Test
@GerritConfig(name = "index.autoReindexIfStale", value = "false")
public void indexChange() throws Exception {
- configureIndex(server.getTestInjector());
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ configureIndex(server.getTestInjector());
- PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
- String changeId = change.getChangeId();
- String changeLegacyId = change.getChange().getId().toString();
- ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
+ String changeId = change.getChangeId();
+ String changeLegacyId = change.getChange().getId().toString();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
- disableChangeIndexWrites();
- amendChange(changeId, "second test", "test2.txt", "test2");
+ disableChangeIndexWrites();
+ amendChange(changeId, "second test", "test2.txt", "test2");
- assertChangeQuery("message:second", change.getChange(), false);
- enableChangeIndexWrites();
+ assertChangeQuery("message:second", change.getChange(), false);
+ enableChangeIndexWrites();
- changeIndexedCounter.clear();
- String cmd = Joiner.on(" ").join("gerrit", "index", "changes", changeLegacyId);
- adminSshSession.exec(cmd);
- adminSshSession.assertSuccess();
+ changeIndexedCounter.clear();
+ String cmd = Joiner.on(" ").join("gerrit", "index", "changes", changeLegacyId);
+ adminSshSession.exec(cmd);
+ adminSshSession.assertSuccess();
- changeIndexedCounter.assertReindexOf(changeInfo, 1);
+ changeIndexedCounter.assertReindexOf(changeInfo, 1);
- assertChangeQuery("message:second", change.getChange(), true);
+ assertChangeQuery("message:second", change.getChange(), true);
+ }
}
@Test
@GerritConfig(name = "index.autoReindexIfStale", value = "false")
public void indexProject() throws Exception {
- configureIndex(server.getTestInjector());
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ configureIndex(server.getTestInjector());
- PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
- String changeId = change.getChangeId();
- ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ PushOneCommit.Result change = createChange("first change", "test1.txt", "test1");
+ String changeId = change.getChangeId();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
- disableChangeIndexWrites();
- amendChange(changeId, "second test", "test2.txt", "test2");
+ disableChangeIndexWrites();
+ amendChange(changeId, "second test", "test2.txt", "test2");
- assertChangeQuery("message:second", change.getChange(), false);
- enableChangeIndexWrites();
+ assertChangeQuery("message:second", change.getChange(), false);
+ enableChangeIndexWrites();
- changeIndexedCounter.clear();
- String cmd = Joiner.on(" ").join("gerrit", "index", "changes-in-project", project.get());
- adminSshSession.exec(cmd);
- adminSshSession.assertSuccess();
-
- boolean indexing = true;
- while (indexing) {
- String out = adminSshSession.exec("gerrit show-queue --wide");
+ changeIndexedCounter.clear();
+ String cmd = Joiner.on(" ").join("gerrit", "index", "changes-in-project", project.get());
+ adminSshSession.exec(cmd);
adminSshSession.assertSuccess();
- indexing = out.contains("Index all changes of project " + project.get());
+
+ boolean indexing = true;
+ while (indexing) {
+ String out = adminSshSession.exec("gerrit show-queue --wide");
+ adminSshSession.assertSuccess();
+ indexing = out.contains("Index all changes of project " + project.get());
+ }
+
+ changeIndexedCounter.assertReindexOf(changeInfo, 1);
+
+ assertChangeQuery("message:second", change.getChange(), true);
}
-
- changeIndexedCounter.assertReindexOf(changeInfo, 1);
-
- assertChangeQuery("message:second", change.getChange(), true);
}
protected void assertChangeQuery(String q, ChangeData change, boolean assertTrue)
diff --git a/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java b/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
index 09e97b2..ae45d90 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/SshTraceIT.java
@@ -20,9 +20,9 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.ExtensionRegistry;
+import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.UseSsh;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.server.logging.LoggingContext;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.PerformanceLogger;
@@ -33,66 +33,58 @@
import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.List;
-import org.junit.After;
-import org.junit.Before;
import org.junit.Test;
@UseSsh
public class SshTraceIT extends AbstractDaemonTest {
- @Inject private DynamicSet<ProjectCreationValidationListener> projectCreationValidationListeners;
- @Inject private DynamicSet<PerformanceLogger> performanceLoggers;
-
- private TraceValidatingProjectCreationValidationListener projectCreationListener;
- private RegistrationHandle projectCreationListenerRegistrationHandle;
- private TestPerformanceLogger testPerformanceLogger;
- private RegistrationHandle performanceLoggerRegistrationHandle;
-
- @Before
- public void setup() {
- projectCreationListener = new TraceValidatingProjectCreationValidationListener();
- projectCreationListenerRegistrationHandle =
- projectCreationValidationListeners.add("gerrit", projectCreationListener);
- testPerformanceLogger = new TestPerformanceLogger();
- performanceLoggerRegistrationHandle = performanceLoggers.add("gerrit", testPerformanceLogger);
- }
-
- @After
- public void cleanup() {
- projectCreationListenerRegistrationHandle.remove();
- performanceLoggerRegistrationHandle.remove();
- }
+ @Inject private ExtensionRegistry extensionRegistry;
@Test
public void sshCallWithoutTrace() throws Exception {
- adminSshSession.exec("gerrit create-project new1");
- adminSshSession.assertSuccess();
- assertThat(projectCreationListener.traceId).isNull();
- assertThat(projectCreationListener.foundTraceId).isFalse();
- assertThat(projectCreationListener.isLoggingForced).isFalse();
+ TraceValidatingProjectCreationValidationListener projectCreationListener =
+ new TraceValidatingProjectCreationValidationListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ adminSshSession.exec("gerrit create-project new1");
+ adminSshSession.assertSuccess();
+ assertThat(projectCreationListener.traceId).isNull();
+ assertThat(projectCreationListener.foundTraceId).isFalse();
+ assertThat(projectCreationListener.isLoggingForced).isFalse();
+ }
}
@Test
public void sshCallWithTrace() throws Exception {
- adminSshSession.exec("gerrit create-project --trace new2");
+ TraceValidatingProjectCreationValidationListener projectCreationListener =
+ new TraceValidatingProjectCreationValidationListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ adminSshSession.exec("gerrit create-project --trace new2");
- // The trace ID is written to stderr.
- adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
+ // The trace ID is written to stderr.
+ adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
- assertThat(projectCreationListener.traceId).isNotNull();
- assertThat(projectCreationListener.foundTraceId).isTrue();
- assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.traceId).isNotNull();
+ assertThat(projectCreationListener.foundTraceId).isTrue();
+ assertThat(projectCreationListener.isLoggingForced).isTrue();
+ }
}
@Test
public void sshCallWithTraceAndProvidedTraceId() throws Exception {
- adminSshSession.exec("gerrit create-project --trace --trace-id issue/123 new3");
+ TraceValidatingProjectCreationValidationListener projectCreationListener =
+ new TraceValidatingProjectCreationValidationListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ adminSshSession.exec("gerrit create-project --trace --trace-id issue/123 new3");
- // The trace ID is written to stderr.
- adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
+ // The trace ID is written to stderr.
+ adminSshSession.assertFailure(RequestId.Type.TRACE_ID.name());
- assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
- assertThat(projectCreationListener.foundTraceId).isTrue();
- assertThat(projectCreationListener.isLoggingForced).isTrue();
+ assertThat(projectCreationListener.traceId).isEqualTo("issue/123");
+ assertThat(projectCreationListener.foundTraceId).isTrue();
+ assertThat(projectCreationListener.isLoggingForced).isTrue();
+ }
}
@Test
@@ -103,9 +95,13 @@
@Test
public void performanceLoggingForSshCall() throws Exception {
- adminSshSession.exec("gerrit create-project new5");
- adminSshSession.assertSuccess();
- assertThat(testPerformanceLogger.logEntries()).isNotEmpty();
+ TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(testPerformanceLogger)) {
+ adminSshSession.exec("gerrit create-project new5");
+ adminSshSession.assertSuccess();
+ assertThat(testPerformanceLogger.logEntries()).isNotEmpty();
+ }
}
private static class TraceValidatingProjectCreationValidationListener
diff --git a/javatests/com/google/gerrit/integration/git/BUILD b/javatests/com/google/gerrit/integration/git/BUILD
new file mode 100644
index 0000000..6a6f5ad
--- /dev/null
+++ b/javatests/com/google/gerrit/integration/git/BUILD
@@ -0,0 +1,7 @@
+load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+
+acceptance_tests(
+ srcs = glob(["*IT.java"]),
+ group = "git",
+ labels = ["git"],
+)
diff --git a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
new file mode 100644
index 0000000..ece4d4a
--- /dev/null
+++ b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
@@ -0,0 +1,236 @@
+// Copyright (C) 2019 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.integration.git;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.deny;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.AccountCreator;
+import com.google.gerrit.acceptance.GerritServer.TestSshServerAddress;
+import com.google.gerrit.acceptance.GitClientVersion;
+import com.google.gerrit.acceptance.StandaloneSiteTest;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.inject.Inject;
+import java.io.File;
+import java.net.InetSocketAddress;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
+import org.junit.Test;
+
+@UseSsh
+public class GitProtocolV2IT extends StandaloneSiteTest {
+ private final String[] SSH_KEYGEN_CMD =
+ new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"};
+ private final String[] GIT_LS_REMOTE =
+ new String[] {"git", "-c", "protocol.version=2", "ls-remote", "-o", "trace=12345"};
+ private final String GIT_SSH_COMMAND =
+ "ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i";
+
+ @Inject private GerritApi gApi;
+ @Inject private AccountCreator accountCreator;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private @TestSshServerAddress InetSocketAddress sshAddress;
+ @Inject private @GerritServerConfig Config config;
+
+ @Test
+ public void testGitWireProtocolV2WithSsh() throws Exception {
+ // Minimum required git-core version that supports wire protocol v2 is 2.18.0
+ GitClientVersion requiredGitVersion = new GitClientVersion(2, 18, 0);
+ GitClientVersion actualGitVersion =
+ new GitClientVersion(execute(ImmutableList.of("git", "version")));
+ // If not found, test succeeds with assumption violation
+ assume().that(actualGitVersion).isAtLeast(requiredGitVersion);
+
+ try (ServerContext ctx = startServer()) {
+ ctx.getInjector().injectMembers(this);
+
+ // Create project
+ Project.NameKey project = Project.nameKey("foo");
+ gApi.projects().create(project.get());
+
+ // Set up project permission
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(deny(Permission.READ).ref("refs/*").group(SystemGroupBackend.ANONYMOUS_USERS))
+ .add(
+ allow(Permission.READ)
+ .ref("refs/heads/master")
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+
+ // Set protocol.version=2 in target repository
+ execute(
+ ImmutableList.of("git", "config", "protocol.version", "2"),
+ sitePaths.site_path.resolve("git").resolve(project.get() + Constants.DOT_GIT).toFile());
+
+ // Retrieve HTTP url
+ String url = config.getString("gerrit", null, "canonicalweburl");
+ String urlDestinationTemplate =
+ url.substring(0, 7)
+ + "%s:secret@"
+ + url.substring(7, url.length())
+ + "/a/"
+ + project.get();
+
+ // Retrieve SSH host and port
+ String sshDestinationTemplate =
+ "ssh://%s@" + sshAddress.getHostName() + ":" + sshAddress.getPort() + "/" + project.get();
+
+ // Admin user was already created by the base class
+ setUpUserAuthentication(admin.username());
+
+ // Create non-admin user
+ TestAccount user = accountCreator.user();
+ setUpUserAuthentication(user.username());
+
+ // Prepare data for new change on master branch
+ ChangeInput in = new ChangeInput(project.get(), "master", "Test public change");
+ in.newBranch = true;
+
+ // Create new change and retrieve SHA1 for the created patch set
+ String commit =
+ gApi.changes()
+ .id(gApi.changes().create(in).info().changeId)
+ .current()
+ .commit(false)
+ .commit;
+
+ // Prepare new change on secret branch
+ in = new ChangeInput(project.get(), "secret", "Test secret change");
+ in.newBranch = true;
+
+ // Create new change and retrieve SHA1 for the created patch set
+ String secretCommit =
+ gApi.changes()
+ .id(gApi.changes().create(in).info().changeId)
+ .current()
+ .commit(false)
+ .commit;
+
+ // Read refs from target repository using git wire protocol v2 over HTTP for admin user
+ String out =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_LS_REMOTE)
+ .add(String.format(urlDestinationTemplate, admin.username()))
+ .build(),
+ ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+
+ assertGitProtocolV2Refs(commit, out);
+ assertThat(out).contains(secretCommit);
+
+ // Read refs from target repository using git wire protocol v2 over SSH for admin user
+ out =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_LS_REMOTE)
+ .add(String.format(sshDestinationTemplate, admin.username()))
+ .build(),
+ ImmutableMap.of(
+ "GIT_SSH_COMMAND",
+ GIT_SSH_COMMAND
+ + sitePaths.data_dir.resolve(String.format("id_rsa_%s", admin.username())),
+ "GIT_TRACE_PACKET",
+ "1"));
+
+ assertGitProtocolV2Refs(commit, out);
+ assertThat(out).contains(secretCommit);
+
+ // Read refs from target repository using git wire protocol v2 over HTTP for non-admin user
+ out =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_LS_REMOTE)
+ .add(String.format(urlDestinationTemplate, user.username()))
+ .build(),
+ ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+
+ assertGitProtocolV2Refs(commit, out);
+ assertThat(out).doesNotContain(secretCommit);
+
+ // Read refs from target repository using git wire protocol v2 over SSH for non-admin user
+ out =
+ execute(
+ ImmutableList.<String>builder()
+ .add(GIT_LS_REMOTE)
+ .add(String.format(sshDestinationTemplate, user.username()))
+ .build(),
+ ImmutableMap.of(
+ "GIT_SSH_COMMAND",
+ GIT_SSH_COMMAND
+ + sitePaths.data_dir.resolve(String.format("id_rsa_%s", user.username())),
+ "GIT_TRACE_PACKET",
+ "1"));
+
+ assertGitProtocolV2Refs(commit, out);
+ assertThat(out).doesNotContain(secretCommit);
+ }
+ }
+
+ private void setUpUserAuthentication(String username) throws Exception {
+ // Assign HTTP password to user
+ gApi.accounts().id(username).setHttpPassword("secret");
+
+ // Generate private/public key for user
+ execute(
+ ImmutableList.<String>builder()
+ .add(SSH_KEYGEN_CMD)
+ .add(String.format("id_rsa_%s", username))
+ .build());
+
+ // Read the content of generated public key and add it for the user in Gerrit
+ gApi.accounts()
+ .id(username)
+ .addSshKey(
+ new String(
+ java.nio.file.Files.readAllBytes(
+ sitePaths.data_dir.resolve(String.format("id_rsa_%s.pub", username))),
+ UTF_8));
+ }
+
+ private static void assertGitProtocolV2Refs(String commit, String out) {
+ assertThat(out).contains("git< version 2");
+ assertThat(out).contains("refs/changes/01/1/1");
+ assertThat(out).contains("refs/changes/01/1/meta");
+ assertThat(out).contains(commit);
+ }
+
+ private String execute(ImmutableList<String> cmd) throws Exception {
+ return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of());
+ }
+
+ private String execute(ImmutableList<String> cmd, ImmutableMap<String, String> env)
+ throws Exception {
+ return execute(cmd, sitePaths.data_dir.toFile(), env);
+ }
+
+ private static String execute(ImmutableList<String> cmd, File dir) throws Exception {
+ return execute(cmd, dir, ImmutableMap.of());
+ }
+}
diff --git a/javatests/com/google/gerrit/server/logging/MetadataTest.java b/javatests/com/google/gerrit/server/logging/MetadataTest.java
index 89e5690..f9ae2c1 100644
--- a/javatests/com/google/gerrit/server/logging/MetadataTest.java
+++ b/javatests/com/google/gerrit/server/logging/MetadataTest.java
@@ -23,7 +23,7 @@
@Test
public void stringForLoggingOmitsEmptyOptionalValuesAndReformatsOptionalValuesThatArePresent() {
Metadata metadata = Metadata.builder().accountId(1000001).branchName("refs/heads/foo").build();
- assertThat(metadata.toStringForLogging())
+ assertThat(metadata.toStringForLoggingLazy().evaluate())
.isEqualTo("Metadata{accountId=1000001, branchName=refs/heads/foo, pluginMetadata=[]}");
}
@@ -31,6 +31,7 @@
public void
stringForLoggingOmitsEmptyOptionalValuesAndReformatsOptionalValuesThatArePresentNoFieldsSet() {
Metadata metadata = Metadata.builder().build();
- assertThat(metadata.toStringForLogging()).isEqualTo("Metadata{pluginMetadata=[]}");
+ assertThat(metadata.toStringForLoggingLazy().evaluate())
+ .isEqualTo("Metadata{pluginMetadata=[]}");
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index e8297af..2994c8d 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -121,10 +121,6 @@
.changeMetadata {
border-right: 1px solid var(--border-color);
}
- /* Prevent plugin text from overflowing. */
- #change_plugins {
- word-break: break-word;
- }
.commitMessage {
font-family: var(--monospace-font-family);
margin-right: 1em;
@@ -438,10 +434,6 @@
parent-is-current="[[_parentIsCurrent]]"
on-show-reply-dialog="_handleShowReplyDialog">
</gr-change-metadata>
- <!-- Plugins insert content into following container.
- Stop-gap until PolyGerrit plugins interface is ready.
- This will not work with Shadow DOM. -->
- <div id="change_plugins"></div>
</div>
<div id="mainChangeInfo" class="changeInfo-column mainChangeInfo">
<div id="commitAndRelated" class="hideOnMobileOverlay">
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 9e6997c..180d1a3 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -303,11 +303,11 @@
},
_calculatePatchChange(files) {
- const filesNoCommitMsg = files.filter(files => {
- return files.__path !== '/COMMIT_MSG';
+ const magicFilesExcluded = files.filter(files => {
+ return files.__path !== '/COMMIT_MSG' && files.__path !== '/MERGE_LIST';
});
- return filesNoCommitMsg.reduce((acc, obj) => {
+ return magicFilesExcluded.reduce((acc, obj) => {
const inserted = obj.lines_inserted ? obj.lines_inserted : 0;
const deleted = obj.lines_deleted ? obj.lines_deleted : 0;
const total_size = (obj.size && obj.binary) ? obj.size : 0;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 3f95fd0..50e7701 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -167,6 +167,9 @@
'/COMMIT_MSG': {
lines_inserted: 9,
},
+ '/MERGE_LIST': {
+ lines_inserted: 9,
+ },
'file_added_in_rev2.txt': {
lines_inserted: 1,
lines_deleted: 1,
@@ -200,6 +203,9 @@
'/COMMIT_MSG': {
lines_inserted: 9,
},
+ '/MERGE_LIST': {
+ lines_inserted: 9,
+ },
'myfile.txt': {
lines_inserted: 1,
lines_deleted: 1,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index e3a4821..a18d2f4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -194,9 +194,13 @@
},
getTargetDiffElement() {
- // Find the parent diff element of the cursor row.
- for (let diff = this.diffRow; diff; diff = diff.parentElement) {
- if (diff.tagName === 'GR-DIFF') { return diff; }
+ if (!this.diffRow) return null;
+
+ const hostOwner = Polymer.dom(/** @type {Node} */ (this.diffRow))
+ .getOwnerRoot();
+ if (hostOwner && hostOwner.host &&
+ hostOwner.host.tagName === 'GR-DIFF') {
+ return hostOwner.host;
}
return null;
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
index 7280f2f..ffb5efa 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
@@ -249,6 +249,15 @@
diffElement._diffChanged(mockDiffResponse.diffResponse);
});
+ test('getTargetDiffElement', () => {
+ cursorElement.initialLineNumber = 1;
+ assert.isTrue(!!cursorElement.diffRow);
+ assert.equal(
+ cursorElement.getTargetDiffElement(),
+ diffElement
+ );
+ });
+
test('getAddress', () => {
// It should initialize to the first chunk: line 5 of the revision.
assert.deepEqual(cursorElement.getAddress(),
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
index 46877d7..1fa3db1 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html
@@ -77,30 +77,13 @@
on-blur="_onInputBlur"
autocomplete="off">
- <template is="dom-if" if="[[_isPolymer2()]]">
- <!-- Content uses vertical-align:baseline. If iron-icon is placed
- directly in the slot and is not visible, vertical-align doesn't
- work, because display:none convert it from inline-block element to
- block element. To fix this problem, iron-icon is wrapped in div
- which never changes display type.
- The problem doesn't exist in Polymer1 because DOM is different.
- Applying the same fix to Polymer1 breaks vertical-align.
- -->
- <div slot="prefix">
- <iron-icon
- icon="gr-icons:search"
- class$="searchIcon [[_computeShowSearchIconClass(showSearchIcon)]]">
- </iron-icon>
- </div>
- </template>
- <template is="dom-if" if="[[!_isPolymer2()]]">
+ <!-- prefix as attribute is required to for polymer 1 -->
+ <div slot="prefix" prefix>
<iron-icon
- prefix
icon="gr-icons:search"
class$="searchIcon [[_computeShowSearchIconClass(showSearchIcon)]]">
</iron-icon>
- </template>
-
+ </div>
</paper-input>
<gr-autocomplete-dropdown
vertical-align="top"
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index 745cff1..d422df8 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -196,7 +196,7 @@
},
focus() {
- this.$.input.focus();
+ this._nativeInput.focus();
},
selectAll() {
@@ -330,7 +330,7 @@
default:
// For any normal keypress, return focus to the input to allow for
// unbroken user input.
- this.$.input.inputElement.focus();
+ this.focus();
// Since this has been a normal keypress, the suggestions will have
// been based on a previous input. Clear them. This prevents an
@@ -430,9 +430,5 @@
_computeShowSearchIconClass(showSearchIcon) {
return showSearchIcon ? 'showSearchIcon' : '';
},
-
- _isPolymer2() {
- return window.POLYMER2;
- },
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.js
new file mode 100644
index 0000000..cf2376e
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils.js
@@ -0,0 +1,95 @@
+/**
+ * @license
+ * Copyright (C) 2019 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.
+ */
+
+(function(window) {
+ 'use strict';
+
+ const PRELOADED_PROTOCOL = 'preloaded:';
+
+ let _restAPI;
+
+ const getRestAPI = () => {
+ if (!_restAPI) {
+ _restAPI = document.createElement('gr-rest-api-interface');
+ }
+ return _restAPI;
+ };
+
+ function getPluginNameFromUrl(url) {
+ if (!(url instanceof URL)) {
+ try {
+ url = new URL(url);
+ } catch (e) {
+ console.warn(e);
+ return null;
+ }
+ }
+ if (url.protocol === PRELOADED_PROTOCOL) {
+ return url.pathname;
+ }
+ const base = Gerrit.BaseUrlBehavior.getBaseUrl();
+ const pathname = url.pathname.replace(base, '');
+ // Site theme is server from predefined path.
+ if (pathname === '/static/gerrit-theme.html') {
+ return 'gerrit-theme';
+ } else if (!pathname.startsWith('/plugins')) {
+ console.warn('Plugin not being loaded from /plugins base path:',
+ url.href, '— Unable to determine name.');
+ return null;
+ }
+ // Pathname should normally look like this:
+ // /plugins/PLUGINNAME/static/SCRIPTNAME.html
+ // Or, for app/samples:
+ // /plugins/PLUGINNAME.html
+ return pathname.split('/')[2].split('.')[0];
+ }
+
+ // TODO (viktard): deprecate in favor of GrPluginRestApi.
+ function send(method, url, opt_callback, opt_payload) {
+ return getRestAPI().send(method, url, opt_payload).then(response => {
+ if (response.status < 200 || response.status >= 300) {
+ return response.text().then(text => {
+ if (text) {
+ return Promise.reject(text);
+ } else {
+ return Promise.reject(response.status);
+ }
+ });
+ } else {
+ return getRestAPI().getResponseObject(response);
+ }
+ }).then(response => {
+ if (opt_callback) {
+ opt_callback(response);
+ }
+ return response;
+ });
+ }
+
+ function resetInternalState() {
+ _restAPI = null;
+ }
+
+ window._apiUtils = {
+ getPluginNameFromUrl,
+ send,
+ getRestAPI,
+
+ // TEST only methods
+ resetInternalState,
+ };
+})(window);
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils_test.html
new file mode 100644
index 0000000..c407aa8
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-api-utils_test.html
@@ -0,0 +1,78 @@
+<!DOCTYPE html>
+<!--
+@license
+Copyright (C) 2019 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-api-interface</title>
+<script src="/test/common-test-setup.js"></script>
+<script src="/bower_components/webcomponentsjs/custom-elements-es5-adapter.js"></script>
+
+<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
+<script src="/bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+<link rel="import" href="gr-js-api-interface.html">
+
+<script>void(0);</script>
+
+<script>
+
+ const PRELOADED_PROTOCOL = 'preloaded:';
+
+ suite('gr-api-utils tests', () => {
+ suite('test getPluginNameFromUrl', () => {
+ const {getPluginNameFromUrl} = window._apiUtils;
+
+ test('with empty string', () => {
+ assert.equal(getPluginNameFromUrl(''), null);
+ });
+
+ test('with invalid url', () => {
+ assert.equal(getPluginNameFromUrl('test'), null);
+ });
+
+ test('with random invalid url', () => {
+ assert.equal(getPluginNameFromUrl('http://example.com'), null);
+ assert.equal(
+ getPluginNameFromUrl('http://example.com/static/a.html'),
+ null
+ );
+ });
+
+ test('with valid urls', () => {
+ assert.equal(
+ getPluginNameFromUrl('http://example.com/plugins/a.html'),
+ 'a'
+ );
+ assert.equal(
+ getPluginNameFromUrl('http://example.com/plugins/a/static/t.html'),
+ 'a'
+ );
+ });
+
+ test('with preloaded urls', () => {
+ assert.equal(getPluginNameFromUrl(`${PRELOADED_PROTOCOL}a`), 'a');
+ });
+
+ test('with gerrit-theme override', () => {
+ assert.equal(
+ getPluginNameFromUrl('http://example.com/static/gerrit-theme.html'),
+ 'gerrit-theme'
+ );
+ });
+ });
+ });
+</script>
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.js
new file mode 100644
index 0000000..8cdbcb7
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.js
@@ -0,0 +1,280 @@
+/**
+ * @license
+ * Copyright (C) 2019 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.
+ */
+
+(function(window) {
+ 'use strict';
+
+ /**
+ * Hash of loaded and installed plugins, name to Plugin object.
+ */
+ const _plugins = {};
+
+ /**
+ * Array of plugin URLs to be loaded, name to url.
+ */
+ let _pluginsPending = {};
+
+ let _pluginsInstalled = [];
+
+ let _pluginsPendingCount = -1;
+
+ const UNKNOWN_PLUGIN = 'unknown';
+ const PRELOADED_PROTOCOL = 'preloaded:';
+
+ const PLUGIN_LOADING_TIMEOUT_MS = 10000;
+
+ let _reporting;
+ const getReporting = () => {
+ if (!_reporting) {
+ _reporting = document.createElement('gr-reporting');
+ }
+ return _reporting;
+ };
+
+ // Import utils methods
+ const {
+ getPluginNameFromUrl,
+ send,
+ getRestAPI,
+ resetInternalState,
+ } = window._apiUtils;
+
+ const API_VERSION = '0.1';
+
+ window.Gerrit = window.Gerrit || {};
+ const Gerrit = window.Gerrit;
+
+ let _resolveAllPluginsLoaded = null;
+ let _allPluginsPromise = null;
+
+ Gerrit._endpoints = new GrPluginEndpoints();
+
+ // Provide reset plugins function to clear installed plugins between tests.
+ const app = document.querySelector('#app');
+ if (!app) {
+ // No gr-app found (running tests)
+ Gerrit._installPreloadedPlugins = installPreloadedPlugins;
+ Gerrit._flushPreinstalls = flushPreinstalls;
+ Gerrit._resetPlugins = () => {
+ _allPluginsPromise = null;
+ _pluginsInstalled = [];
+ _pluginsPending = {};
+ _pluginsPendingCount = -1;
+ _reporting = null;
+ _resolveAllPluginsLoaded = null;
+ resetInternalState();
+ Gerrit._endpoints = new GrPluginEndpoints();
+ for (const k of Object.keys(_plugins)) {
+ delete _plugins[k];
+ }
+ };
+ }
+
+ /**
+ * @deprecated Use plugin.styles().css(rulesStr) instead. Please, consult
+ * the documentation how to replace it accordingly.
+ */
+ Gerrit.css = function(rulesStr) {
+ console.warn('Gerrit.css(rulesStr) is deprecated!',
+ 'Use plugin.styles().css(rulesStr)');
+ if (!Gerrit._customStyleSheet) {
+ const styleEl = document.createElement('style');
+ document.head.appendChild(styleEl);
+ Gerrit._customStyleSheet = styleEl.sheet;
+ }
+
+ const name = '__pg_js_api_class_' +
+ Gerrit._customStyleSheet.cssRules.length;
+ Gerrit._customStyleSheet.insertRule('.' + name + '{' + rulesStr + '}', 0);
+ return name;
+ };
+
+ Gerrit.install = function(callback, opt_version, opt_src) {
+ // HTML import polyfill adds __importElement pointing to the import tag.
+ const script = document.currentScript &&
+ (document.currentScript.__importElement || document.currentScript);
+
+ let src = opt_src || (script && script.src);
+ if (!src || src.startsWith('data:')) {
+ src = script && script.baseURI;
+ }
+ const name = getPluginNameFromUrl(src);
+
+ if (opt_version && opt_version !== API_VERSION) {
+ Gerrit._pluginInstallError(`Plugin ${name} install error: only version ` +
+ API_VERSION + ' is supported in PolyGerrit. ' + opt_version +
+ ' was given.');
+ return;
+ }
+
+ const existingPlugin = _plugins[name];
+ const plugin = existingPlugin || new Plugin(src);
+ try {
+ callback(plugin);
+ if (name) {
+ _plugins[name] = plugin;
+ }
+ if (!existingPlugin) {
+ Gerrit._pluginInstalled(src);
+ }
+ } catch (e) {
+ Gerrit._pluginInstallError(`${e.name}: ${e.message}`);
+ }
+ };
+
+ Gerrit.getLoggedIn = function() {
+ console.warn('Gerrit.getLoggedIn() is deprecated! ' +
+ 'Use plugin.restApi().getLoggedIn()');
+ return document.createElement('gr-rest-api-interface').getLoggedIn();
+ };
+
+ Gerrit.get = function(url, callback) {
+ console.warn('.get() is deprecated! Use plugin.restApi().get()');
+ send('GET', url, callback);
+ };
+
+ Gerrit.post = function(url, payload, callback) {
+ console.warn('.post() is deprecated! Use plugin.restApi().post()');
+ send('POST', url, callback, payload);
+ };
+
+ Gerrit.put = function(url, payload, callback) {
+ console.warn('.put() is deprecated! Use plugin.restApi().put()');
+ send('PUT', url, callback, payload);
+ };
+
+ Gerrit.delete = function(url, opt_callback) {
+ console.warn('.delete() is deprecated! Use plugin.restApi().delete()');
+ return getRestAPI().send('DELETE', url).then(response => {
+ if (response.status !== 204) {
+ return response.text().then(text => {
+ if (text) {
+ return Promise.reject(text);
+ } else {
+ return Promise.reject(response.status);
+ }
+ });
+ }
+ if (opt_callback) {
+ opt_callback(response);
+ }
+ return response;
+ });
+ };
+
+ Gerrit.awaitPluginsLoaded = function() {
+ if (!_allPluginsPromise) {
+ if (Gerrit._arePluginsLoaded()) {
+ _allPluginsPromise = Promise.resolve();
+ } else {
+ let timeoutId;
+ _allPluginsPromise =
+ Promise.race([
+ new Promise(resolve => _resolveAllPluginsLoaded = resolve),
+ new Promise(resolve => timeoutId = setTimeout(
+ Gerrit._pluginLoadingTimeout, PLUGIN_LOADING_TIMEOUT_MS)),
+ ]).then(() => clearTimeout(timeoutId));
+ }
+ }
+ return _allPluginsPromise;
+ };
+
+ Gerrit._pluginLoadingTimeout = function() {
+ console.error(`Failed to load plugins: ${Object.keys(_pluginsPending)}`);
+ Gerrit._setPluginsPending([]);
+ };
+
+ Gerrit._setPluginsPending = function(plugins) {
+ _pluginsPending = plugins.reduce((o, url) => {
+ // TODO(viktard): Remove guard (@see Issue 8962)
+ o[getPluginNameFromUrl(url) || UNKNOWN_PLUGIN] = url;
+ return o;
+ }, {});
+ Gerrit._setPluginsCount(Object.keys(_pluginsPending).length);
+ };
+
+ Gerrit._setPluginsCount = function(count) {
+ _pluginsPendingCount = count;
+ if (Gerrit._arePluginsLoaded()) {
+ getReporting().pluginsLoaded(_pluginsInstalled);
+ if (_resolveAllPluginsLoaded) {
+ _resolveAllPluginsLoaded();
+ }
+ }
+ };
+
+ Gerrit._pluginInstallError = function(message) {
+ document.dispatchEvent(new CustomEvent('show-alert', {
+ detail: {
+ message: `Plugin install error: ${message}`,
+ },
+ }));
+ console.info(`Plugin install error: ${message}`);
+ Gerrit._setPluginsCount(_pluginsPendingCount - 1);
+ };
+
+ Gerrit._pluginInstalled = function(url) {
+ const name = getPluginNameFromUrl(url) || UNKNOWN_PLUGIN;
+ if (!_pluginsPending[name]) {
+ console.warn(`Unexpected plugin ${name} installed from ${url}.`);
+ } else {
+ delete _pluginsPending[name];
+ _pluginsInstalled.push(name);
+ Gerrit._setPluginsCount(_pluginsPendingCount - 1);
+ getReporting().pluginLoaded(name);
+ console.log(`Plugin ${name} installed.`);
+ }
+ };
+
+ Gerrit._arePluginsLoaded = function() {
+ return _pluginsPendingCount === 0;
+ };
+
+ Gerrit._getPluginScreenName = function(pluginName, screenName) {
+ return `${pluginName}-screen-${screenName}`;
+ };
+
+ Gerrit._isPluginPreloaded = function(url) {
+ const name = getPluginNameFromUrl(url);
+ if (name && Gerrit._preloadedPlugins) {
+ return name in Gerrit._preloadedPlugins;
+ } else {
+ return false;
+ }
+ };
+
+ function flushPreinstalls() {
+ if (window.Gerrit.flushPreinstalls) {
+ window.Gerrit.flushPreinstalls();
+ }
+ }
+
+ function installPreloadedPlugins() {
+ if (!Gerrit._preloadedPlugins) { return; }
+ for (const name in Gerrit._preloadedPlugins) {
+ if (!Gerrit._preloadedPlugins.hasOwnProperty(name)) { continue; }
+ const callback = Gerrit._preloadedPlugins[name];
+ Gerrit.install(callback, API_VERSION, PRELOADED_PROTOCOL + name);
+ }
+ }
+
+ flushPreinstalls();
+
+ // Preloaded plugins should be installed after Gerrit.install() is set,
+ // since plugin preloader substitutes Gerrit.install() temporarily.
+ installPreloadedPlugins();
+})(window);
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.html
new file mode 100644
index 0000000..b3ec30f
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit_test.html
@@ -0,0 +1,188 @@
+<!DOCTYPE html>
+<!--
+@license
+Copyright (C) 2019 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-api-interface</title>
+<script src="/test/common-test-setup.js"></script>
+<script src="/bower_components/webcomponentsjs/custom-elements-es5-adapter.js"></script>
+
+<script src="/bower_components/webcomponentsjs/webcomponents-lite.js"></script>
+<script src="/bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+<link rel="import" href="gr-js-api-interface.html">
+
+<script>void(0);</script>
+
+<test-fixture id="basic">
+ <template>
+ <gr-js-api-interface></gr-js-api-interface>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-gerrit tests', () => {
+ let element;
+ let plugin;
+ let sandbox;
+ let sendStub;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ sendStub = sandbox.stub().returns(Promise.resolve({status: 200}));
+ stub('gr-rest-api-interface', {
+ getAccount() {
+ return Promise.resolve({name: 'Judy Hopps'});
+ },
+ send(...args) {
+ return sendStub(...args);
+ },
+ });
+ element = fixture('basic');
+ Gerrit.install(p => { plugin = p; }, '0.1',
+ 'http://test.com/plugins/testplugin/static/test.js');
+ Gerrit._setPluginsPending([]);
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ element._removeEventCallbacks();
+ plugin = null;
+ });
+
+ test('reuse plugin for install calls', () => {
+ let otherPlugin;
+ Gerrit.install(p => { otherPlugin = p; }, '0.1',
+ 'http://test.com/plugins/testplugin/static/test.js');
+ assert.strictEqual(plugin, otherPlugin);
+ });
+
+ test('flushes preinstalls if provided', () => {
+ assert.doesNotThrow(() => {
+ Gerrit._flushPreinstalls();
+ });
+ window.Gerrit.flushPreinstalls = sandbox.stub();
+ Gerrit._flushPreinstalls();
+ assert.isTrue(window.Gerrit.flushPreinstalls.calledOnce);
+ delete window.Gerrit.flushPreinstalls;
+ });
+
+ test('versioning', () => {
+ const callback = sandbox.spy();
+ Gerrit.install(callback, '0.0pre-alpha');
+ assert(callback.notCalled);
+ });
+
+ test('_setPluginsCount', done => {
+ stub('gr-reporting', {
+ pluginsLoaded() {
+ done();
+ },
+ });
+ Gerrit._setPluginsCount(0);
+ });
+
+ test('_arePluginsLoaded', () => {
+ assert.isTrue(Gerrit._arePluginsLoaded());
+ Gerrit._setPluginsCount(1);
+ assert.isFalse(Gerrit._arePluginsLoaded());
+ Gerrit._setPluginsCount(0);
+ assert.isTrue(Gerrit._arePluginsLoaded());
+ });
+
+ test('_pluginInstalled', () => {
+ const pluginsLoadedStub = sandbox.stub();
+ stub('gr-reporting', {
+ pluginsLoaded: (...args) => pluginsLoadedStub(...args),
+ });
+ const plugins = [
+ 'http://test.com/plugins/foo/static/test.js',
+ 'http://test.com/plugins/bar/static/test.js',
+ ];
+ Gerrit._setPluginsPending(plugins);
+ Gerrit._pluginInstalled(plugins[0]);
+ Gerrit._pluginInstalled(plugins[1]);
+ assert.isTrue(pluginsLoadedStub.calledWithExactly(['foo', 'bar']));
+ });
+
+ test('install calls _pluginInstalled', () => {
+ sandbox.stub(Gerrit, '_pluginInstalled');
+ Gerrit.install(p => { plugin = p; }, '0.1',
+ 'http://test.com/plugins/testplugin/static/test.js');
+
+ // testplugin has already been installed once (in setup).
+ assert.isFalse(Gerrit._pluginInstalled.called);
+
+ // testplugin2 plugin has not yet been installed.
+ Gerrit.install(p => { plugin = p; }, '0.1',
+ 'http://test.com/plugins/testplugin2/static/test.js');
+ assert.isTrue(Gerrit._pluginInstalled.calledOnce);
+ });
+
+ test('plugin install errors mark plugins as loaded', () => {
+ Gerrit._setPluginsCount(1);
+ Gerrit.install(() => {}, '0.0pre-alpha');
+ return Gerrit.awaitPluginsLoaded();
+ });
+
+ test('multiple ui plugins per java plugin', () => {
+ const file1 = 'http://test.com/plugins/qaz/static/foo.nocache.js';
+ const file2 = 'http://test.com/plugins/qaz/static/bar.js';
+ Gerrit._setPluginsPending([file1, file2]);
+ Gerrit.install(() => {}, '0.1', file1);
+ Gerrit.install(() => {}, '0.1', file2);
+ return Gerrit.awaitPluginsLoaded();
+ });
+
+ test('plugin install errors shows toasts', () => {
+ const alertStub = sandbox.stub();
+ document.addEventListener('show-alert', alertStub);
+ Gerrit._setPluginsCount(1);
+ Gerrit.install(() => {}, '0.0pre-alpha');
+ return Gerrit.awaitPluginsLoaded().then(() => {
+ assert.isTrue(alertStub.calledOnce);
+ });
+ });
+
+ test('Gerrit._isPluginPreloaded', () => {
+ Gerrit._preloadedPlugins = {baz: ()=>{}};
+ assert.isFalse(Gerrit._isPluginPreloaded('plugins/foo/bar'));
+ assert.isFalse(Gerrit._isPluginPreloaded('http://a.com/42'));
+ assert.isTrue(Gerrit._isPluginPreloaded('preloaded:baz'));
+ Gerrit._preloadedPlugins = null;
+ });
+
+ test('preloaded plugins are installed', () => {
+ const installStub = sandbox.stub();
+ Gerrit._preloadedPlugins = {foo: installStub};
+ Gerrit._installPreloadedPlugins();
+ assert.isTrue(installStub.called);
+ const pluginApi = installStub.lastCall.args[0];
+ assert.strictEqual(pluginApi.getPluginName(), 'foo');
+ });
+
+ test('installing preloaded plugin', () => {
+ let plugin;
+ window.ASSETS_PATH = 'http://blips.com/chitz';
+ Gerrit.install(p => { plugin = p; }, '0.1', 'preloaded:foo');
+ assert.strictEqual(plugin.getPluginName(), 'foo');
+ assert.strictEqual(plugin.url('/some/thing.html'),
+ 'http://blips.com/chitz/plugins/foo/some/thing.html');
+ delete window.ASSETS_PATH;
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.html
index e460660..7fa2250 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.html
@@ -31,6 +31,7 @@
<link rel="import" href="../gr-rest-api-interface/gr-rest-api-interface.html">
<dom-module id="gr-js-api-interface">
+ <script src="gr-api-utils.js"></script>
<script src="gr-annotation-actions-context.js"></script>
<script src="gr-annotation-actions-js-api.js"></script>
<script src="gr-change-actions-js-api.js"></script>
@@ -40,4 +41,5 @@
<script src="gr-plugin-action-context.js"></script>
<script src="gr-plugin-rest-api.js"></script>
<script src="gr-public-js-api.js"></script>
+ <script src="gr-gerrit.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
index c7e4f09..330310f 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
@@ -73,23 +73,6 @@
plugin = null;
});
- test('reuse plugin for install calls', () => {
- let otherPlugin;
- Gerrit.install(p => { otherPlugin = p; }, '0.1',
- 'http://test.com/plugins/testplugin/static/test.js');
- assert.strictEqual(plugin, otherPlugin);
- });
-
- test('flushes preinstalls if provided', () => {
- assert.doesNotThrow(() => {
- Gerrit._flushPreinstalls();
- });
- window.Gerrit.flushPreinstalls = sandbox.stub();
- Gerrit._flushPreinstalls();
- assert.isTrue(window.Gerrit.flushPreinstalls.calledOnce);
- delete window.Gerrit.flushPreinstalls;
- });
-
test('url', () => {
assert.equal(plugin.url(), 'http://test.com/plugins/testplugin/');
assert.equal(plugin.url('/static/test.js'),
@@ -313,12 +296,6 @@
element.handleEvent(element.EventType.HIGHLIGHTJS_LOADED, {hljs: testHljs});
});
- test('versioning', () => {
- const callback = sandbox.spy();
- Gerrit.install(callback, '0.0pre-alpha');
- assert(callback.notCalled);
- });
-
test('getAccount', done => {
plugin.restApi().getLoggedIn().then(loggedIn => {
assert.isTrue(loggedIn);
@@ -326,77 +303,6 @@
});
});
- test('_setPluginsCount', done => {
- stub('gr-reporting', {
- pluginsLoaded() {
- done();
- },
- });
- Gerrit._setPluginsCount(0);
- });
-
- test('_arePluginsLoaded', () => {
- assert.isTrue(Gerrit._arePluginsLoaded());
- Gerrit._setPluginsCount(1);
- assert.isFalse(Gerrit._arePluginsLoaded());
- Gerrit._setPluginsCount(0);
- assert.isTrue(Gerrit._arePluginsLoaded());
- });
-
- test('_pluginInstalled', () => {
- const pluginsLoadedStub = sandbox.stub();
- stub('gr-reporting', {
- pluginsLoaded: (...args) => pluginsLoadedStub(...args),
- });
- const plugins = [
- 'http://test.com/plugins/foo/static/test.js',
- 'http://test.com/plugins/bar/static/test.js',
- ];
- Gerrit._setPluginsPending(plugins);
- Gerrit._pluginInstalled(plugins[0]);
- Gerrit._pluginInstalled(plugins[1]);
- assert.isTrue(pluginsLoadedStub.calledWithExactly(['foo', 'bar']));
- });
-
- test('install calls _pluginInstalled', () => {
- sandbox.stub(Gerrit, '_pluginInstalled');
- Gerrit.install(p => { plugin = p; }, '0.1',
- 'http://test.com/plugins/testplugin/static/test.js');
-
- // testplugin has already been installed once (in setup).
- assert.isFalse(Gerrit._pluginInstalled.called);
-
- // testplugin2 plugin has not yet been installed.
- Gerrit.install(p => { plugin = p; }, '0.1',
- 'http://test.com/plugins/testplugin2/static/test.js');
- assert.isTrue(Gerrit._pluginInstalled.calledOnce);
- });
-
- test('plugin install errors mark plugins as loaded', () => {
- Gerrit._setPluginsCount(1);
- Gerrit.install(() => {}, '0.0pre-alpha');
- return Gerrit.awaitPluginsLoaded();
- });
-
- test('multiple ui plugins per java plugin', () => {
- const file1 = 'http://test.com/plugins/qaz/static/foo.nocache.js';
- const file2 = 'http://test.com/plugins/qaz/static/bar.js';
- Gerrit._setPluginsPending([file1, file2]);
- Gerrit.install(() => {}, '0.1', file1);
- Gerrit.install(() => {}, '0.1', file2);
- return Gerrit.awaitPluginsLoaded();
- });
-
- test('plugin install errors shows toasts', () => {
- const alertStub = sandbox.stub();
- document.addEventListener('show-alert', alertStub);
- Gerrit._setPluginsCount(1);
- Gerrit.install(() => {}, '0.0pre-alpha');
- return Gerrit.awaitPluginsLoaded().then(() => {
- assert.isTrue(alertStub.calledOnce);
- });
- });
-
test('attributeHelper', () => {
assert.isOk(plugin.attributeHelper());
});
@@ -422,33 +328,6 @@
element.EventType.ADMIN_MENU_LINKS);
});
- test('Gerrit._isPluginPreloaded', () => {
- Gerrit._preloadedPlugins = {baz: ()=>{}};
- assert.isFalse(Gerrit._isPluginPreloaded('plugins/foo/bar'));
- assert.isFalse(Gerrit._isPluginPreloaded('http://a.com/42'));
- assert.isTrue(Gerrit._isPluginPreloaded('preloaded:baz'));
- Gerrit._preloadedPlugins = null;
- });
-
- test('preloaded plugins are installed', () => {
- const installStub = sandbox.stub();
- Gerrit._preloadedPlugins = {foo: installStub};
- Gerrit._installPreloadedPlugins();
- assert.isTrue(installStub.called);
- const pluginApi = installStub.lastCall.args[0];
- assert.strictEqual(pluginApi.getPluginName(), 'foo');
- });
-
- test('installing preloaded plugin', () => {
- let plugin;
- window.ASSETS_PATH = 'http://blips.com/chitz';
- Gerrit.install(p => { plugin = p; }, '0.1', 'preloaded:foo');
- assert.strictEqual(plugin.getPluginName(), 'foo');
- assert.strictEqual(plugin.url('/some/thing.html'),
- 'http://blips.com/chitz/plugins/foo/some/thing.html');
- delete window.ASSETS_PATH;
- });
-
suite('test plugin with base url', () => {
let baseUrlPlugin;
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
index 3b7b80d..d44acea 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
@@ -17,71 +17,18 @@
(function(window) {
'use strict';
- /**
- * Hash of loaded and installed plugins, name to Plugin object.
- */
- const _plugins = {};
-
- /**
- * Array of plugin URLs to be loaded, name to url.
- */
- let _pluginsPending = {};
-
- let _pluginsInstalled = [];
-
- let _pluginsPendingCount = -1;
-
const PRELOADED_PROTOCOL = 'preloaded:';
- const UNKNOWN_PLUGIN = 'unknown';
-
const PANEL_ENDPOINTS_MAPPING = {
CHANGE_SCREEN_BELOW_COMMIT_INFO_BLOCK: 'change-view-integration',
CHANGE_SCREEN_BELOW_CHANGE_INFO_BLOCK: 'change-metadata-item',
};
- const PLUGIN_LOADING_TIMEOUT_MS = 10000;
-
- let _restAPI;
-
- const getRestAPI = () => {
- if (!_restAPI) {
- _restAPI = document.createElement('gr-rest-api-interface');
- }
- return _restAPI;
- };
-
- let _reporting;
- const getReporting = () => {
- if (!_reporting) {
- _reporting = document.createElement('gr-reporting');
- }
- return _reporting;
- };
-
- // TODO (viktard): deprecate in favor of GrPluginRestApi.
- function send(method, url, opt_callback, opt_payload) {
- return getRestAPI().send(method, url, opt_payload).then(response => {
- if (response.status < 200 || response.status >= 300) {
- return response.text().then(text => {
- if (text) {
- return Promise.reject(text);
- } else {
- return Promise.reject(response.status);
- }
- });
- } else {
- return getRestAPI().getResponseObject(response);
- }
- }).then(response => {
- if (opt_callback) {
- opt_callback(response);
- }
- return response;
- });
- }
-
- const API_VERSION = '0.1';
+ // Import utils methods
+ const {
+ getPluginNameFromUrl,
+ send,
+ } = window._apiUtils;
/**
* Plugin-provided custom components can affect content in extension
@@ -99,50 +46,6 @@
STYLE: 'style',
};
- function flushPreinstalls() {
- if (window.Gerrit.flushPreinstalls) {
- window.Gerrit.flushPreinstalls();
- }
- }
-
- function installPreloadedPlugins() {
- if (!Gerrit._preloadedPlugins) { return; }
- for (const name in Gerrit._preloadedPlugins) {
- if (!Gerrit._preloadedPlugins.hasOwnProperty(name)) { continue; }
- const callback = Gerrit._preloadedPlugins[name];
- Gerrit.install(callback, API_VERSION, PRELOADED_PROTOCOL + name);
- }
- }
-
- function getPluginNameFromUrl(url) {
- if (!(url instanceof URL)) {
- try {
- url = new URL(url);
- } catch (e) {
- console.warn(e);
- return null;
- }
- }
- if (url.protocol === PRELOADED_PROTOCOL) {
- return url.pathname;
- }
- const base = Gerrit.BaseUrlBehavior.getBaseUrl();
- const pathname = url.pathname.replace(base, '');
- // Site theme is server from predefined path.
- if (pathname === '/static/gerrit-theme.html') {
- return 'gerrit-theme';
- } else if (!pathname.startsWith('/plugins')) {
- console.warn('Plugin not being loaded from /plugins base path:',
- url.href, '— Unable to determine name.');
- return;
- }
- // Pathname should normally look like this:
- // /plugins/PLUGINNAME/static/SCRIPTNAME.html
- // Or, for app/samples:
- // /plugins/PLUGINNAME.html
- return pathname.split('/')[2].split('.')[0];
- }
-
function Plugin(opt_url) {
this._domHooks = new GrDomHooksManager(this);
@@ -475,211 +378,5 @@
},
};
- flushPreinstalls();
-
- window.Gerrit = window.Gerrit || {};
- const Gerrit = window.Gerrit;
-
- let _resolveAllPluginsLoaded = null;
- let _allPluginsPromise = null;
-
- Gerrit._endpoints = new GrPluginEndpoints();
-
- // Provide reset plugins function to clear installed plugins between tests.
- const app = document.querySelector('#app');
- if (!app) {
- // No gr-app found (running tests)
- Gerrit._installPreloadedPlugins = installPreloadedPlugins;
- Gerrit._flushPreinstalls = flushPreinstalls;
- Gerrit._resetPlugins = () => {
- _allPluginsPromise = null;
- _pluginsInstalled = [];
- _pluginsPending = {};
- _pluginsPendingCount = -1;
- _reporting = null;
- _resolveAllPluginsLoaded = null;
- _restAPI = null;
- Gerrit._endpoints = new GrPluginEndpoints();
- for (const k of Object.keys(_plugins)) {
- delete _plugins[k];
- }
- };
- }
-
- /**
- * @deprecated Use plugin.styles().css(rulesStr) instead. Please, consult
- * the documentation how to replace it accordingly.
- */
- Gerrit.css = function(rulesStr) {
- console.warn('Gerrit.css(rulesStr) is deprecated!',
- 'Use plugin.styles().css(rulesStr)');
- if (!Gerrit._customStyleSheet) {
- const styleEl = document.createElement('style');
- document.head.appendChild(styleEl);
- Gerrit._customStyleSheet = styleEl.sheet;
- }
-
- const name = '__pg_js_api_class_' +
- Gerrit._customStyleSheet.cssRules.length;
- Gerrit._customStyleSheet.insertRule('.' + name + '{' + rulesStr + '}', 0);
- return name;
- };
-
- Gerrit.install = function(callback, opt_version, opt_src) {
- // HTML import polyfill adds __importElement pointing to the import tag.
- const script = document.currentScript &&
- (document.currentScript.__importElement || document.currentScript);
-
- let src = opt_src || (script && script.src);
- if (!src || src.startsWith('data:')) {
- src = script && script.baseURI;
- }
- const name = getPluginNameFromUrl(src);
-
- if (opt_version && opt_version !== API_VERSION) {
- Gerrit._pluginInstallError(`Plugin ${name} install error: only version ` +
- API_VERSION + ' is supported in PolyGerrit. ' + opt_version +
- ' was given.');
- return;
- }
-
- const existingPlugin = _plugins[name];
- const plugin = existingPlugin || new Plugin(src);
- try {
- callback(plugin);
- if (name) {
- _plugins[name] = plugin;
- }
- if (!existingPlugin) {
- Gerrit._pluginInstalled(src);
- }
- } catch (e) {
- Gerrit._pluginInstallError(`${e.name}: ${e.message}`);
- }
- };
-
- Gerrit.getLoggedIn = function() {
- console.warn('Gerrit.getLoggedIn() is deprecated! ' +
- 'Use plugin.restApi().getLoggedIn()');
- return document.createElement('gr-rest-api-interface').getLoggedIn();
- };
-
- Gerrit.get = function(url, callback) {
- console.warn('.get() is deprecated! Use plugin.restApi().get()');
- send('GET', url, callback);
- };
-
- Gerrit.post = function(url, payload, callback) {
- console.warn('.post() is deprecated! Use plugin.restApi().post()');
- send('POST', url, callback, payload);
- };
-
- Gerrit.put = function(url, payload, callback) {
- console.warn('.put() is deprecated! Use plugin.restApi().put()');
- send('PUT', url, callback, payload);
- };
-
- Gerrit.delete = function(url, opt_callback) {
- console.warn('.delete() is deprecated! Use plugin.restApi().delete()');
- return getRestAPI().send('DELETE', url).then(response => {
- if (response.status !== 204) {
- return response.text().then(text => {
- if (text) {
- return Promise.reject(text);
- } else {
- return Promise.reject(response.status);
- }
- });
- }
- if (opt_callback) {
- opt_callback(response);
- }
- return response;
- });
- };
-
- Gerrit.awaitPluginsLoaded = function() {
- if (!_allPluginsPromise) {
- if (Gerrit._arePluginsLoaded()) {
- _allPluginsPromise = Promise.resolve();
- } else {
- let timeoutId;
- _allPluginsPromise =
- Promise.race([
- new Promise(resolve => _resolveAllPluginsLoaded = resolve),
- new Promise(resolve => timeoutId = setTimeout(
- Gerrit._pluginLoadingTimeout, PLUGIN_LOADING_TIMEOUT_MS)),
- ]).then(() => clearTimeout(timeoutId));
- }
- }
- return _allPluginsPromise;
- };
-
- Gerrit._pluginLoadingTimeout = function() {
- console.error(`Failed to load plugins: ${Object.keys(_pluginsPending)}`);
- Gerrit._setPluginsPending([]);
- };
-
- Gerrit._setPluginsPending = function(plugins) {
- _pluginsPending = plugins.reduce((o, url) => {
- // TODO(viktard): Remove guard (@see Issue 8962)
- o[getPluginNameFromUrl(url) || UNKNOWN_PLUGIN] = url;
- return o;
- }, {});
- Gerrit._setPluginsCount(Object.keys(_pluginsPending).length);
- };
-
- Gerrit._setPluginsCount = function(count) {
- _pluginsPendingCount = count;
- if (Gerrit._arePluginsLoaded()) {
- getReporting().pluginsLoaded(_pluginsInstalled);
- if (_resolveAllPluginsLoaded) {
- _resolveAllPluginsLoaded();
- }
- }
- };
-
- Gerrit._pluginInstallError = function(message) {
- document.dispatchEvent(new CustomEvent('show-alert', {
- detail: {
- message: `Plugin install error: ${message}`,
- },
- }));
- console.info(`Plugin install error: ${message}`);
- Gerrit._setPluginsCount(_pluginsPendingCount - 1);
- };
-
- Gerrit._pluginInstalled = function(url) {
- const name = getPluginNameFromUrl(url) || UNKNOWN_PLUGIN;
- if (!_pluginsPending[name]) {
- console.warn(`Unexpected plugin ${name} installed from ${url}.`);
- } else {
- delete _pluginsPending[name];
- _pluginsInstalled.push(name);
- Gerrit._setPluginsCount(_pluginsPendingCount - 1);
- getReporting().pluginLoaded(name);
- console.log(`Plugin ${name} installed.`);
- }
- };
-
- Gerrit._arePluginsLoaded = function() {
- return _pluginsPendingCount === 0;
- };
-
- Gerrit._getPluginScreenName = function(pluginName, screenName) {
- return `${pluginName}-screen-${screenName}`;
- };
-
- Gerrit._isPluginPreloaded = function(url) {
- const name = getPluginNameFromUrl(url);
- if (name && Gerrit._preloadedPlugins) {
- return name in Gerrit._preloadedPlugins;
- } else {
- return false;
- }
- };
-
- // Preloaded plugins should be installed after Gerrit.install() is set,
- // since plugin preloader substitutes Gerrit.install() temporarily.
- installPreloadedPlugins();
+ window.Plugin = Plugin;
})(window);
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 10d5aff..a0e85ef 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -188,7 +188,9 @@
'shared/gr-js-api-interface/gr-annotation-actions-js-api_test.html',
'shared/gr-js-api-interface/gr-change-actions-js-api_test.html',
'shared/gr-js-api-interface/gr-change-reply-js-api_test.html',
+ 'shared/gr-js-api-interface/gr-api-utils_test.html',
'shared/gr-js-api-interface/gr-js-api-interface_test.html',
+ 'shared/gr-js-api-interface/gr-gerrit_test.html',
// TODO: uncomment file & fix tests. The file was missed in this list for a long time.
// 'shared/gr-js-api-interface/gr-plugin-action-context_test.html',
'shared/gr-js-api-interface/gr-plugin-endpoints_test.html',
diff --git a/resources/com/google/gerrit/server/mime/mime-types.properties b/resources/com/google/gerrit/server/mime/mime-types.properties
index f12b39e..4229e4a 100644
--- a/resources/com/google/gerrit/server/mime/mime-types.properties
+++ b/resources/com/google/gerrit/server/mime/mime-types.properties
@@ -31,6 +31,7 @@
cr = text/x-crystal
cs = text/x-csharp
csharp = text/x-csharp
+csproj = application/xml
css = text/css
cpp = text/x-c++src
cql = text/x-cassandra
@@ -242,6 +243,7 @@
vtl = text/velocity
webidl = text/x-webidl
wsdl = application/xml
+xaml = application/xml
xhtml = text/html
xml = application/xml
xsd = application/xml
