From 6e06662e82e0d20921ea06b2cf9ae3cb9996405c Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 9 Oct 2019 08:32:19 +0200 Subject: [PATCH] Add helper class to register extensions in acceptance tests Registering an extension in tests comes with quite some boilerplate code. E.g. ChangeIT does: @Inject private DynamicSet changeIndexedListeners; 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(); } } This registers the ChangeIndexedCounter for all tests in ChangeIT although only one test is using it. We could register the extension only for that one test, then we would have: @Inject private DynamicSet changeIndexedListeners; @Test public void starUnstar() throws Exception { ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter(); RegistrationHandle changeIndexedCounterHandle = changeIndexedListeners.add("gerrit", changeIndexedCounter); try { ... } finally { changeIndexedCounterHandle.remove(); } } With the new ExtensionRegistry class we can write this as: @Inject private ExtensionRegistry extensionRegistry; @Test public void starUnstar() throws Exception { ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter(); try (Registration registration = extensionRegistry.newRegistration().add(changeIndexedCounter)) { ... } It's also possible to chain add calls to add multiple extensions with one registration: try (Registration registration = extensionRegistry.newRegistration().add(extension1).add(extension2)) { ... } For now only TraceIT is adapted to use the ExtensionRegistry helper class. TraceIT now registers extensions always in the methods where they are used. This should improve the readability of the tests. Due to erasure of generic types at runtime we need one add method per extension point in ExtensionRegistry.Registration. Change-Id: I027b4161e3c5365f479586871fa79e2cb1447f82 Signed-off-by: Edwin Kempin --- java/com/google/gerrit/acceptance/BUILD | 1 + .../gerrit/acceptance/ExtensionRegistry.java | 95 +++ .../gerrit/acceptance/rest/TraceIT.java | 735 ++++++++++-------- 3 files changed, 521 insertions(+), 310 deletions(-) create mode 100644 java/com/google/gerrit/acceptance/ExtensionRegistry.java diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD index d645828363..9b57282025 100644 --- a/java/com/google/gerrit/acceptance/BUILD +++ b/java/com/google/gerrit/acceptance/BUILD @@ -124,6 +124,7 @@ java_library2( "//java/com/google/gerrit/server", "//java/com/google/gerrit/server/audit", "//java/com/google/gerrit/server/git/receive", + "//java/com/google/gerrit/server/logging", "//java/com/google/gerrit/server/restapi", "//java/com/google/gerrit/server/schema", "//java/com/google/gerrit/server/util/git", diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java new file mode 100644 index 0000000000..7d53f9b50a --- /dev/null +++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java @@ -0,0 +1,95 @@ +// 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 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.ExceptionHook; +import com.google.gerrit.server.git.validators.CommitValidationListener; +import com.google.gerrit.server.logging.PerformanceLogger; +import com.google.gerrit.server.rules.SubmitRule; +import com.google.gerrit.server.validators.ProjectCreationValidationListener; +import com.google.inject.Inject; +import java.util.ArrayList; +import java.util.List; + +public class ExtensionRegistry { + private final DynamicSet changeIndexedListeners; + private final DynamicSet commitValidationListeners; + private final DynamicSet exceptionHooks; + private final DynamicSet performanceLoggers; + private final DynamicSet projectCreationValidationListeners; + private final DynamicSet submitRules; + + @Inject + ExtensionRegistry( + DynamicSet changeIndexedListeners, + DynamicSet commitValidationListeners, + DynamicSet exceptionHooks, + DynamicSet performanceLoggers, + DynamicSet projectCreationValidationListeners, + DynamicSet submitRules) { + this.changeIndexedListeners = changeIndexedListeners; + this.commitValidationListeners = commitValidationListeners; + this.exceptionHooks = exceptionHooks; + this.performanceLoggers = performanceLoggers; + this.projectCreationValidationListeners = projectCreationValidationListeners; + this.submitRules = submitRules; + } + + public Registration newRegistration() { + return new Registration(); + } + + public class Registration implements AutoCloseable { + private final List registrationHandles = new ArrayList<>(); + + public Registration add(ChangeIndexedListener changeIndexedListener) { + return add(changeIndexedListeners, changeIndexedListener); + } + + public Registration add(CommitValidationListener commitValidationListener) { + return add(commitValidationListeners, commitValidationListener); + } + + public Registration add(ExceptionHook exceptionHook) { + return add(exceptionHooks, exceptionHook); + } + + public Registration add(PerformanceLogger performanceLogger) { + return add(performanceLoggers, performanceLogger); + } + + public Registration add(ProjectCreationValidationListener projectCreationListener) { + return add(projectCreationValidationListeners, projectCreationListener); + } + + public Registration add(SubmitRule submitRule) { + return add(submitRules, submitRule); + } + + private Registration add(DynamicSet dynamicSet, T extension) { + RegistrationHandle registrationHandle = dynamicSet.add("gerrit", extension); + registrationHandles.add(registrationHandle); + return this; + } + + @Override + public void close() { + registrationHandles.forEach(h -> h.remove()); + } + } +} diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java index 0e5120830b..52de5adf43 100644 --- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java @@ -26,14 +26,14 @@ import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Iterables; import com.google.common.truth.Expect; 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.RestResponse; import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.extensions.api.changes.ReviewInput; 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.httpd.restapi.ParameterParser; import com.google.gerrit.httpd.restapi.RestApiServlet; import com.google.gerrit.server.ExceptionHook; @@ -58,8 +58,6 @@ import java.util.Optional; import java.util.SortedMap; import java.util.SortedSet; import org.apache.http.message.BasicHeader; -import org.junit.After; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -81,50 +79,24 @@ import org.junit.Test; public class TraceIT extends AbstractDaemonTest { @Rule public final Expect expect = Expect.create(); - @Inject private DynamicSet projectCreationValidationListeners; - @Inject private DynamicSet commitValidationListeners; - @Inject private DynamicSet changeIndexedListeners; - @Inject private DynamicSet performanceLoggers; - @Inject private DynamicSet submitRules; - @Inject private DynamicSet exceptionHooks; + @Inject private ExtensionRegistry extensionRegistry; @Inject private WorkQueue workQueue; - private TraceValidatingProjectCreationValidationListener projectCreationListener; - private RegistrationHandle projectCreationListenerRegistrationHandle; - private TraceValidatingCommitValidationListener commitValidationListener; - private RegistrationHandle commitValidationRegistrationHandle; - private TestPerformanceLogger testPerformanceLogger; - private RegistrationHandle performanceLoggerRegistrationHandle; - - @Before - public void setup() { - projectCreationListener = new TraceValidatingProjectCreationValidationListener(); - projectCreationListenerRegistrationHandle = - projectCreationValidationListeners.add("gerrit", projectCreationListener); - commitValidationListener = new TraceValidatingCommitValidationListener(); - commitValidationRegistrationHandle = - commitValidationListeners.add("gerrit", commitValidationListener); - testPerformanceLogger = new TestPerformanceLogger(); - performanceLoggerRegistrationHandle = performanceLoggers.add("gerrit", testPerformanceLogger); - } - - @After - public void cleanup() { - projectCreationListenerRegistrationHandle.remove(); - commitValidationRegistrationHandle.remove(); - performanceLoggerRegistrationHandle.remove(); - } - @Test public void restCallWithoutTrace() throws Exception { - RestResponse response = adminRestSession.put("/projects/new1"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new1"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new1"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new1"); + } } @Test @@ -132,9 +104,8 @@ public class TraceIT extends AbstractDaemonTest { String changeId = createChange().getChangeId(); TraceChangeIndexedListener changeIndexedListener = new TraceChangeIndexedListener(); - RegistrationHandle registrationHandle = - changeIndexedListeners.add("gerrit", changeIndexedListener); - try { + try (Registration registration = + extensionRegistry.newRegistration().add(changeIndexedListener)) { RestResponse response = adminRestSession.post( "/changes/" + changeId + "/revisions/current/review", ReviewInput.approve()); @@ -142,169 +113,223 @@ public class TraceIT extends AbstractDaemonTest { // The logging tag with the project name is also set if tracing is off. assertThat(changeIndexedListener.tags.get("project")).containsExactly(project.get()); - } finally { - registrationHandle.remove(); } } @Test public void restCallWithTraceRequestParam() throws Exception { - RestResponse response = - adminRestSession.put("/projects/new2?" + ParameterParser.TRACE_PARAMETER); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNotNull(); - assertThat(projectCreationListener.traceId).isNotNull(); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new2"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = + adminRestSession.put("/projects/new2?" + ParameterParser.TRACE_PARAMETER); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNotNull(); + assertThat(projectCreationListener.traceId).isNotNull(); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new2"); + } } @Test public void restCallWithTraceRequestParamAndProvidedTraceId() throws Exception { - RestResponse response = - adminRestSession.put("/projects/new3?" + ParameterParser.TRACE_PARAMETER + "=issue/123"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); - assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new3"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = + adminRestSession.put("/projects/new3?" + ParameterParser.TRACE_PARAMETER + "=issue/123"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); + assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new3"); + } } @Test public void restCallWithTraceHeader() throws Exception { - RestResponse response = - adminRestSession.putWithHeader( - "/projects/new4", new BasicHeader(RestApiServlet.X_GERRIT_TRACE, null)); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNotNull(); - assertThat(projectCreationListener.traceId).isNotNull(); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new4"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = + adminRestSession.putWithHeader( + "/projects/new4", new BasicHeader(RestApiServlet.X_GERRIT_TRACE, null)); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNotNull(); + assertThat(projectCreationListener.traceId).isNotNull(); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new4"); + } } @Test public void restCallWithTraceHeaderAndProvidedTraceId() throws Exception { - RestResponse response = - adminRestSession.putWithHeader( - "/projects/new5", new BasicHeader(RestApiServlet.X_GERRIT_TRACE, "issue/123")); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); - assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new5"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = + adminRestSession.putWithHeader( + "/projects/new5", new BasicHeader(RestApiServlet.X_GERRIT_TRACE, "issue/123")); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); + assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new5"); + } } @Test public void restCallWithTraceRequestParamAndTraceHeader() throws Exception { - // trace ID only specified by trace header - RestResponse response = - adminRestSession.putWithHeader( - "/projects/new6?trace", new BasicHeader(RestApiServlet.X_GERRIT_TRACE, "issue/123")); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); - assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new6"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + // trace ID only specified by trace header + RestResponse response = + adminRestSession.putWithHeader( + "/projects/new6?trace", new BasicHeader(RestApiServlet.X_GERRIT_TRACE, "issue/123")); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); + assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new6"); - // trace ID only specified by trace request parameter - response = - adminRestSession.putWithHeader( - "/projects/new7?trace=issue/123", new BasicHeader(RestApiServlet.X_GERRIT_TRACE, null)); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); - assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new7"); + // trace ID only specified by trace request parameter + response = + adminRestSession.putWithHeader( + "/projects/new7?trace=issue/123", + new BasicHeader(RestApiServlet.X_GERRIT_TRACE, null)); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); + assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new7"); - // same trace ID specified by trace header and trace request parameter - response = - adminRestSession.putWithHeader( - "/projects/new8?trace=issue/123", - new BasicHeader(RestApiServlet.X_GERRIT_TRACE, "issue/123")); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); - assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new8"); + // same trace ID specified by trace header and trace request parameter + response = + adminRestSession.putWithHeader( + "/projects/new8?trace=issue/123", + new BasicHeader(RestApiServlet.X_GERRIT_TRACE, "issue/123")); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isEqualTo("issue/123"); + assertThat(projectCreationListener.traceId).isEqualTo("issue/123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new8"); - // different trace IDs specified by trace header and trace request parameter - response = - adminRestSession.putWithHeader( - "/projects/new9?trace=issue/123", - new BasicHeader(RestApiServlet.X_GERRIT_TRACE, "issue/456")); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeaders(RestApiServlet.X_GERRIT_TRACE)) - .containsExactly("issue/123", "issue/456"); - assertThat(projectCreationListener.traceIds).containsExactly("issue/123", "issue/456"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new9"); + // different trace IDs specified by trace header and trace request parameter + response = + adminRestSession.putWithHeader( + "/projects/new9?trace=issue/123", + new BasicHeader(RestApiServlet.X_GERRIT_TRACE, "issue/456")); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeaders(RestApiServlet.X_GERRIT_TRACE)) + .containsExactly("issue/123", "issue/456"); + assertThat(projectCreationListener.traceIds).containsExactly("issue/123", "issue/456"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new9"); + } } @Test public void pushWithoutTrace() throws Exception { - PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); - PushOneCommit.Result r = push.to("refs/heads/master"); - r.assertOkStatus(); - assertThat(commitValidationListener.traceId).isNull(); - assertThat(commitValidationListener.isLoggingForced).isFalse(); + TraceValidatingCommitValidationListener commitValidationListener = + new TraceValidatingCommitValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(commitValidationListener)) { + PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); + PushOneCommit.Result r = push.to("refs/heads/master"); + r.assertOkStatus(); + assertThat(commitValidationListener.traceId).isNull(); + assertThat(commitValidationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + // The logging tag with the project name is also set if tracing is off. + assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + } } @Test public void pushWithTrace() throws Exception { - PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); - push.setPushOptions(ImmutableList.of("trace")); - PushOneCommit.Result r = push.to("refs/heads/master"); - r.assertOkStatus(); - assertThat(commitValidationListener.traceId).isNotNull(); - assertThat(commitValidationListener.isLoggingForced).isTrue(); - assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + TraceValidatingCommitValidationListener commitValidationListener = + new TraceValidatingCommitValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(commitValidationListener)) { + PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); + push.setPushOptions(ImmutableList.of("trace")); + PushOneCommit.Result r = push.to("refs/heads/master"); + r.assertOkStatus(); + assertThat(commitValidationListener.traceId).isNotNull(); + assertThat(commitValidationListener.isLoggingForced).isTrue(); + assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + } } @Test public void pushWithTraceAndProvidedTraceId() throws Exception { - PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); - push.setPushOptions(ImmutableList.of("trace=issue/123")); - PushOneCommit.Result r = push.to("refs/heads/master"); - r.assertOkStatus(); - assertThat(commitValidationListener.traceId).isEqualTo("issue/123"); - assertThat(commitValidationListener.isLoggingForced).isTrue(); - assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + TraceValidatingCommitValidationListener commitValidationListener = + new TraceValidatingCommitValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(commitValidationListener)) { + PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); + push.setPushOptions(ImmutableList.of("trace=issue/123")); + PushOneCommit.Result r = push.to("refs/heads/master"); + r.assertOkStatus(); + assertThat(commitValidationListener.traceId).isEqualTo("issue/123"); + assertThat(commitValidationListener.isLoggingForced).isTrue(); + assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + } } @Test public void pushForReviewWithoutTrace() throws Exception { - PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); - PushOneCommit.Result r = push.to("refs/for/master"); - r.assertOkStatus(); - assertThat(commitValidationListener.traceId).isNull(); - assertThat(commitValidationListener.isLoggingForced).isFalse(); + TraceValidatingCommitValidationListener commitValidationListener = + new TraceValidatingCommitValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(commitValidationListener)) { + PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); + PushOneCommit.Result r = push.to("refs/for/master"); + r.assertOkStatus(); + assertThat(commitValidationListener.traceId).isNull(); + assertThat(commitValidationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + // The logging tag with the project name is also set if tracing is off. + assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + } } @Test public void pushForReviewWithTrace() throws Exception { - PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); - push.setPushOptions(ImmutableList.of("trace")); - PushOneCommit.Result r = push.to("refs/for/master"); - r.assertOkStatus(); - assertThat(commitValidationListener.traceId).isNotNull(); - assertThat(commitValidationListener.isLoggingForced).isTrue(); - assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + TraceValidatingCommitValidationListener commitValidationListener = + new TraceValidatingCommitValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(commitValidationListener)) { + PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); + push.setPushOptions(ImmutableList.of("trace")); + PushOneCommit.Result r = push.to("refs/for/master"); + r.assertOkStatus(); + assertThat(commitValidationListener.traceId).isNotNull(); + assertThat(commitValidationListener.isLoggingForced).isTrue(); + assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + } } @Test public void pushForReviewWithTraceAndProvidedTraceId() throws Exception { - PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); - push.setPushOptions(ImmutableList.of("trace=issue/123")); - PushOneCommit.Result r = push.to("refs/for/master"); - r.assertOkStatus(); - assertThat(commitValidationListener.traceId).isEqualTo("issue/123"); - assertThat(commitValidationListener.isLoggingForced).isTrue(); - assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + TraceValidatingCommitValidationListener commitValidationListener = + new TraceValidatingCommitValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(commitValidationListener)) { + PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); + push.setPushOptions(ImmutableList.of("trace=issue/123")); + PushOneCommit.Result r = push.to("refs/for/master"); + r.assertOkStatus(); + assertThat(commitValidationListener.traceId).isEqualTo("issue/123"); + assertThat(commitValidationListener.isLoggingForced).isTrue(); + assertThat(commitValidationListener.tags.get("project")).containsExactly(project.get()); + } } @Test @@ -345,246 +370,345 @@ public class TraceIT extends AbstractDaemonTest { @Test public void performanceLoggingForRestCall() throws Exception { - RestResponse response = adminRestSession.put("/projects/new10"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger(); + try (Registration registration = + extensionRegistry.newRegistration().add(testPerformanceLogger)) { + RestResponse response = adminRestSession.put("/projects/new10"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - // This assertion assumes that the server invokes the PerformanceLogger plugins before it sends - // the response to the client. If this assertion gets flaky it's likely that this got changed on - // server-side. - assertThat(testPerformanceLogger.logEntries()).isNotEmpty(); + // This assertion assumes that the server invokes the PerformanceLogger plugins before it + // sends + // the response to the client. If this assertion gets flaky it's likely that this got changed + // on + // server-side. + assertThat(testPerformanceLogger.logEntries()).isNotEmpty(); + } } @Test public void performanceLoggingForPush() throws Exception { - PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); - PushOneCommit.Result r = push.to("refs/heads/master"); - r.assertOkStatus(); - assertThat(testPerformanceLogger.logEntries()).isNotEmpty(); + TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger(); + try (Registration registration = + extensionRegistry.newRegistration().add(testPerformanceLogger)) { + PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); + PushOneCommit.Result r = push.to("refs/heads/master"); + r.assertOkStatus(); + assertThat(testPerformanceLogger.logEntries()).isNotEmpty(); + } } @Test @GerritConfig(name = "tracing.performanceLogging", value = "false") public void noPerformanceLoggingIfDisabled() throws Exception { - RestResponse response = adminRestSession.put("/projects/new11"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + TestPerformanceLogger testPerformanceLogger = new TestPerformanceLogger(); + try (Registration registration = + extensionRegistry.newRegistration().add(testPerformanceLogger)) { + RestResponse response = adminRestSession.put("/projects/new11"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); - PushOneCommit.Result r = push.to("refs/heads/master"); - r.assertOkStatus(); + PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo); + PushOneCommit.Result r = push.to("refs/heads/master"); + r.assertOkStatus(); - assertThat(testPerformanceLogger.logEntries()).isEmpty(); + assertThat(testPerformanceLogger.logEntries()).isEmpty(); + } } @Test @GerritConfig(name = "tracing.issue123.projectPattern", value = "new12") public void traceProject() throws Exception { - RestResponse response = adminRestSession.put("/projects/new12"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isEqualTo("issue123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new12"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new12"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isEqualTo("issue123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new12"); + } } @Test @GerritConfig(name = "tracing.issue123.projectPattern", value = "new.*") public void traceProjectMatchRegEx() throws Exception { - RestResponse response = adminRestSession.put("/projects/new13"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isEqualTo("issue123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new13"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new13"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isEqualTo("issue123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new13"); + } } @Test @GerritConfig(name = "tracing.issue123.projectPattern", value = "foo.*") public void traceProjectNoMatch() throws Exception { - RestResponse response = adminRestSession.put("/projects/new13"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new13"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new13"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new13"); + } } @Test @GerritConfig(name = "tracing.issue123.projectPattern", value = "][") public void traceProjectInvalidRegEx() throws Exception { - RestResponse response = adminRestSession.put("/projects/new14"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new14"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new14"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new14"); + } } @Test @GerritConfig(name = "tracing.issue123.account", value = "1000000") public void traceAccount() throws Exception { - RestResponse response = adminRestSession.put("/projects/new15"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isEqualTo("issue123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new15"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new15"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isEqualTo("issue123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new15"); + } } @Test @GerritConfig(name = "tracing.issue123.account", value = "1000001") public void traceAccountNoMatch() throws Exception { - RestResponse response = adminRestSession.put("/projects/new16"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new16"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new16"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new16"); + } } @Test @GerritConfig(name = "tracing.issue123.account", value = "999") public void traceAccountNotFound() throws Exception { - RestResponse response = adminRestSession.put("/projects/new17"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new17"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new17"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new17"); + } } @Test @GerritConfig(name = "tracing.issue123.account", value = "invalid") public void traceAccountInvalidId() throws Exception { - RestResponse response = adminRestSession.put("/projects/new18"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new18"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new18"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new18"); + } } @Test @GerritConfig(name = "tracing.issue123.requestType", value = "REST") public void traceRequestType() throws Exception { - RestResponse response = adminRestSession.put("/projects/new19"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isEqualTo("issue123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new19"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new19"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isEqualTo("issue123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new19"); + } } @Test @GerritConfig(name = "tracing.issue123.requestType", value = "SSH") public void traceRequestTypeNoMatch() throws Exception { - RestResponse response = adminRestSession.put("/projects/new20"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new20"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new20"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new20"); + } } @Test @GerritConfig(name = "tracing.issue123.requestType", value = "FOO") public void traceProjectInvalidRequestType() throws Exception { - RestResponse response = adminRestSession.put("/projects/new21"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new21"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new21"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new21"); + } } @Test @GerritConfig(name = "tracing.issue123.account", value = "1000000") @GerritConfig(name = "tracing.issue123.projectPattern", value = "new.*") public void traceProjectForAccount() throws Exception { - RestResponse response = adminRestSession.put("/projects/new22"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isEqualTo("issue123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new22"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new22"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isEqualTo("issue123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new22"); + } } @Test @GerritConfig(name = "tracing.issue123.account", value = "1000000") @GerritConfig(name = "tracing.issue123.projectPattern", value = "foo.*") public void traceProjectForAccountNoProjectMatch() throws Exception { - RestResponse response = adminRestSession.put("/projects/new23"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new23"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new23"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new23"); + } } @Test @GerritConfig(name = "tracing.issue123.account", value = "1000001") @GerritConfig(name = "tracing.issue123.projectPattern", value = "new.*") public void traceProjectForAccountNoAccountMatch() throws Exception { - RestResponse response = adminRestSession.put("/projects/new24"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new24"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new24"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new24"); + } } @Test @GerritConfig(name = "tracing.issue123.requestUriPattern", value = "/projects/.*") public void traceRequestUri() throws Exception { - RestResponse response = adminRestSession.put("/projects/new23"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isEqualTo("issue123"); - assertThat(projectCreationListener.isLoggingForced).isTrue(); - assertThat(projectCreationListener.tags.get("project")).containsExactly("new23"); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new23"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isEqualTo("issue123"); + assertThat(projectCreationListener.isLoggingForced).isTrue(); + assertThat(projectCreationListener.tags.get("project")).containsExactly("new23"); + } } @Test @GerritConfig(name = "tracing.issue123.requestUriPattern", value = "/projects/.*/foo") public void traceRequestUriNoMatch() throws Exception { - RestResponse response = adminRestSession.put("/projects/new23"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new23"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new23"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new23"); + } } @Test @GerritConfig(name = "tracing.issue123.requestUriPattern", value = "][") public void traceRequestUriInvalidRegEx() throws Exception { - RestResponse response = adminRestSession.put("/projects/new24"); - assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); - assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); - assertThat(projectCreationListener.traceId).isNull(); - assertThat(projectCreationListener.isLoggingForced).isFalse(); + TraceValidatingProjectCreationValidationListener projectCreationListener = + new TraceValidatingProjectCreationValidationListener(); + try (Registration registration = + extensionRegistry.newRegistration().add(projectCreationListener)) { + RestResponse response = adminRestSession.put("/projects/new24"); + assertThat(response.getStatusCode()).isEqualTo(SC_CREATED); + assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); + assertThat(projectCreationListener.traceId).isNull(); + assertThat(projectCreationListener.isLoggingForced).isFalse(); - // The logging tag with the project name is also set if tracing is off. - assertThat(projectCreationListener.tags.get("project")).containsExactly("new24"); + // The logging tag with the project name is also set if tracing is off. + assertThat(projectCreationListener.tags.get("project")).containsExactly("new24"); + } } @Test @@ -595,15 +719,12 @@ public class TraceIT extends AbstractDaemonTest { TraceSubmitRule traceSubmitRule = new TraceSubmitRule(); traceSubmitRule.failAlways = true; - RegistrationHandle submitRuleRegistrationHandle = submitRules.add("gerrit", traceSubmitRule); - try { + try (Registration registration = extensionRegistry.newRegistration().add(traceSubmitRule)) { RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit"); assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR); assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).startsWith("retry-on-failure-"); assertThat(traceSubmitRule.traceId).startsWith("retry-on-failure-"); assertThat(traceSubmitRule.isLoggingForced).isTrue(); - } finally { - submitRuleRegistrationHandle.remove(); } } @@ -615,25 +736,22 @@ public class TraceIT extends AbstractDaemonTest { TraceSubmitRule traceSubmitRule = new TraceSubmitRule(); traceSubmitRule.failAlways = true; - RegistrationHandle submitRuleRegistrationHandle = submitRules.add("gerrit", traceSubmitRule); - RegistrationHandle exceptionHookRegistrationHandle = - exceptionHooks.add( - "gerrit", - new ExceptionHook() { - @Override - public boolean shouldRetry(Throwable t) { - return true; - } - }); - try { + try (Registration registration = + extensionRegistry + .newRegistration() + .add(traceSubmitRule) + .add( + new ExceptionHook() { + @Override + public boolean shouldRetry(Throwable t) { + return true; + } + })) { RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit"); assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR); assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); assertThat(traceSubmitRule.traceId).isNull(); assertThat(traceSubmitRule.isLoggingForced).isFalse(); - } finally { - submitRuleRegistrationHandle.remove(); - exceptionHookRegistrationHandle.remove(); } } @@ -644,15 +762,12 @@ public class TraceIT extends AbstractDaemonTest { TraceSubmitRule traceSubmitRule = new TraceSubmitRule(); traceSubmitRule.failOnce = true; - RegistrationHandle submitRuleRegistrationHandle = submitRules.add("gerrit", traceSubmitRule); - try { + try (Registration registration = extensionRegistry.newRegistration().add(traceSubmitRule)) { RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit"); assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR); assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull(); assertThat(traceSubmitRule.traceId).isNull(); assertThat(traceSubmitRule.isLoggingForced).isFalse(); - } finally { - submitRuleRegistrationHandle.remove(); } }