From 1311061c82666f2a2213b52f637580c77b82c017 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 19 Nov 2020 14:30:39 +0100 Subject: [PATCH] Pass transaction id from request to mercurial hooks --- pom.xml | 2 +- scm-core/pom.xml | 6 ++ .../main/java/sonia/scm/TransactionId.java | 71 +++++++++++++++++++ .../java/sonia/scm/TransactionIdTest.java | 42 +++++++++++ scm-plugins/scm-hg-plugin/pom.xml | 14 ++++ .../DefaultHgEnvironmentBuilder.java | 4 ++ .../repository/hooks/DefaultHookHandler.java | 4 ++ .../resources/sonia/scm/python/scmhooks.py | 3 +- .../DefaultHgEnvironmentBuilderTest.java | 9 +++ .../hooks/DefaultHookHandlerTest.java | 20 +++++- .../main/java/sonia/scm/filter/MDCFilter.java | 10 ++- .../java/sonia/scm/filter/MDCFilterTest.java | 37 +++++----- 12 files changed, 195 insertions(+), 27 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/TransactionId.java create mode 100644 scm-core/src/test/java/sonia/scm/TransactionIdTest.java diff --git a/pom.xml b/pom.xml index b8192de3a8..46c0b2db13 100644 --- a/pom.xml +++ b/pom.xml @@ -506,7 +506,7 @@ - jakarta.xml.bind + jakarta.xml.bind jakarta.xml.bind-api ${jaxb.version} diff --git a/scm-core/pom.xml b/scm-core/pom.xml index 85a539c471..6ff4d7a7b0 100644 --- a/scm-core/pom.xml +++ b/scm-core/pom.xml @@ -250,6 +250,12 @@ test + + ch.qos.logback + logback-classic + test + + diff --git a/scm-core/src/main/java/sonia/scm/TransactionId.java b/scm-core/src/main/java/sonia/scm/TransactionId.java new file mode 100644 index 0000000000..68169ba7d4 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/TransactionId.java @@ -0,0 +1,71 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm; + +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.MDC; + +import java.util.Optional; + +/** + * Id of the current transaction. + * The transaction id is mainly used for logging and debugging. + * + * @since 2.10.0 + */ +public final class TransactionId { + + @VisibleForTesting + public static final String KEY = "transaction_id"; + + private TransactionId() { + } + + /** + * Binds the given transaction id to the current thread. + * + * @param transactionId transaction id + */ + public static void set(String transactionId) { + MDC.put(KEY, transactionId); + } + + /** + * Returns an optional transaction id. + * If there is no transaction id bound to the thread, the method will return an empty optional. + * + * @return optional transaction id + */ + public static Optional get() { + return Optional.ofNullable(MDC.get(KEY)); + } + + /** + * Removes a bound transaction id from the current thread. + */ + public static void clear() { + MDC.remove(KEY); + } +} diff --git a/scm-core/src/test/java/sonia/scm/TransactionIdTest.java b/scm-core/src/test/java/sonia/scm/TransactionIdTest.java new file mode 100644 index 0000000000..053f2ce86c --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/TransactionIdTest.java @@ -0,0 +1,42 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class TransactionIdTest { + + @Test + void shouldSetGetAndClear() { + TransactionId.set("42"); + + assertThat(TransactionId.get()).contains("42"); + TransactionId.clear(); + assertThat(TransactionId.get()).isEmpty(); + } + +} diff --git a/scm-plugins/scm-hg-plugin/pom.xml b/scm-plugins/scm-hg-plugin/pom.xml index a6f125a6f9..3b280b4519 100644 --- a/scm-plugins/scm-hg-plugin/pom.xml +++ b/scm-plugins/scm-hg-plugin/pom.xml @@ -50,9 +50,23 @@ com.google.guava guava + + org.slf4j + slf4j-simple + + + org.slf4j + slf4j-nop + + + ch.qos.logback + logback-classic + test + + diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/DefaultHgEnvironmentBuilder.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/DefaultHgEnvironmentBuilder.java index 597a56af15..1888f0aeae 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/DefaultHgEnvironmentBuilder.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/DefaultHgEnvironmentBuilder.java @@ -27,6 +27,7 @@ package sonia.scm.repository; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; +import sonia.scm.TransactionId; import sonia.scm.repository.hooks.HookEnvironment; import sonia.scm.repository.hooks.HookServer; import sonia.scm.security.AccessToken; @@ -60,6 +61,8 @@ public class DefaultHgEnvironmentBuilder implements HgEnvironmentBuilder { static final String ENV_REPOSITORY_ID = "SCM_REPOSITORY_ID"; @VisibleForTesting static final String ENV_HTTP_POST_ARGS = "SCM_HTTP_POST_ARGS"; + @VisibleForTesting + static final String ENV_TRANSACTION_ID = "SCM_TRANSACTION_ID"; private final AccessTokenBuilderFactory accessTokenBuilderFactory; private final HgRepositoryHandler repositoryHandler; @@ -114,6 +117,7 @@ public class DefaultHgEnvironmentBuilder implements HgEnvironmentBuilder { env.put(ENV_HOOK_PORT, String.valueOf(getHookPort())); env.put(ENV_BEARER_TOKEN, accessToken()); env.put(ENV_CHALLENGE, hookEnvironment.getChallenge()); + TransactionId.get().ifPresent(transactionId -> env.put(ENV_TRANSACTION_ID, transactionId)); } private String accessToken() { diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/hooks/DefaultHookHandler.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/hooks/DefaultHookHandler.java index 2d035bf32f..9ec95a5dcc 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/hooks/DefaultHookHandler.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/hooks/DefaultHookHandler.java @@ -34,6 +34,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.ExceptionWithContext; import sonia.scm.NotFoundException; +import sonia.scm.TransactionId; import sonia.scm.repository.RepositoryHookType; import sonia.scm.repository.api.HgHookMessage; import sonia.scm.repository.spi.HgHookContextProvider; @@ -90,6 +91,7 @@ class DefaultHookHandler implements HookHandler { private Response handleHookRequest(Request request) { LOG.trace("process {} hook for node {}", request.getType(), request.getNode()); + TransactionId.set(request.getTransactionId()); HgHookContextProvider context = hookContextProviderFactory.create(request.getRepositoryId(), request.getNode()); @@ -119,6 +121,7 @@ class DefaultHookHandler implements HookHandler { return error(context, ex); } finally { environment.clearPendingState(); + TransactionId.clear(); } } @@ -161,6 +164,7 @@ class DefaultHookHandler implements HookHandler { public static class Request { private String token; private RepositoryHookType type; + private String transactionId; private String repositoryId; private String challenge; private String node; diff --git a/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/python/scmhooks.py b/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/python/scmhooks.py index 72c5f7b528..1fdbc07b15 100644 --- a/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/python/scmhooks.py +++ b/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/python/scmhooks.py @@ -36,6 +36,7 @@ port = os.environ['SCM_HOOK_PORT'] challenge = os.environ['SCM_CHALLENGE'] token = os.environ['SCM_BEARER_TOKEN'] repositoryId = os.environ['SCM_REPOSITORY_ID'] +transactionId = os.environ['SCM_TRANSACTION_ID'] def print_messages(ui, messages): for message in messages: @@ -50,7 +51,7 @@ def fire_hook(ui, repo, hooktype, node): ui.debug( b"send scm-hook for " + node + b"\n" ) connection = socket.socket(socket.AF_INET, socket.SOCK_STREAM) try: - values = {'token': token, 'type': hooktype, 'repositoryId': repositoryId, 'challenge': challenge, 'node': node.decode('utf8') } + values = {'token': token, 'type': hooktype, 'repositoryId': repositoryId, 'transactionId': transactionId, 'challenge': challenge, 'node': node.decode('utf8') } connection.connect(("127.0.0.1", int(port))) connection.send(json.dumps(values).encode('utf-8')) diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/DefaultHgEnvironmentBuilderTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/DefaultHgEnvironmentBuilderTest.java index c28bb21e78..7bf3a3c5bf 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/DefaultHgEnvironmentBuilderTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/DefaultHgEnvironmentBuilderTest.java @@ -33,6 +33,7 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.SCMContext; +import sonia.scm.TransactionId; import sonia.scm.repository.hooks.HookEnvironment; import sonia.scm.repository.hooks.HookServer; import sonia.scm.security.AccessToken; @@ -102,6 +103,14 @@ class DefaultHgEnvironmentBuilderTest { .containsEntry(ENV_HOOK_PORT, "2042"); } + @Test + void shouldSetTransactionId() throws IOException { + TransactionId.set("ti42"); + Repository heartOfGold = prepareForWrite("/opt/python", "21"); + Map env = builder.write(heartOfGold); + assertThat(env).containsEntry(ENV_TRANSACTION_ID, "ti42"); + } + @Test void shouldThrowIllegalStateIfServerCouldNotBeStarted() throws IOException { when(server.start()).thenThrow(new IOException("failed to start")); diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/hooks/DefaultHookHandlerTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/hooks/DefaultHookHandlerTest.java index fcdbc4792c..541e0620ef 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/hooks/DefaultHookHandlerTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/hooks/DefaultHookHandlerTest.java @@ -36,6 +36,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.ExceptionWithContext; import sonia.scm.NotFoundException; +import sonia.scm.TransactionId; import sonia.scm.repository.RepositoryHookType; import sonia.scm.repository.api.HgHookMessage; import sonia.scm.repository.api.HgHookMessageProvider; @@ -188,6 +189,23 @@ class DefaultHookHandlerTest { assertThat(received).containsExactly("Some note", "Some error", "Abort it"); } + @Test + void shouldSetAndClearTransactionId() throws IOException { + mockMessageProvider(); + + AtomicReference ref = new AtomicReference<>(); + doAnswer(ic -> { + TransactionId.get().ifPresent(ref::set); + return null; + }).when(hookEventFacade).handle("42"); + + DefaultHookHandler.Request request = createRequest(RepositoryHookType.POST_RECEIVE); + send(request); + + assertThat(ref).hasValue("ti21"); + assertThat(TransactionId.get()).isEmpty(); + } + @Test void shouldHandleAuthenticationFailure() throws IOException { doThrow(AuthenticationException.class) @@ -244,7 +262,7 @@ class DefaultHookHandlerTest { private DefaultHookHandler.Request createRequest(RepositoryHookType type, String challenge) { String secret = CipherUtil.getInstance().encode("secret"); return new DefaultHookHandler.Request( - secret, type, "42", challenge, "abc" + secret, type, "ti21", "42", challenge, "abc" ); } diff --git a/scm-webapp/src/main/java/sonia/scm/filter/MDCFilter.java b/scm-webapp/src/main/java/sonia/scm/filter/MDCFilter.java index 6272e04fd8..d254546ff0 100644 --- a/scm-webapp/src/main/java/sonia/scm/filter/MDCFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/filter/MDCFilter.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.filter; //~--- non-JDK imports -------------------------------------------------------- @@ -34,6 +34,7 @@ import org.apache.shiro.subject.Subject; import org.slf4j.MDC; import sonia.scm.SCMContext; +import sonia.scm.TransactionId; import sonia.scm.security.DefaultKeyGenerator; import sonia.scm.web.filter.HttpFilter; @@ -72,9 +73,6 @@ public class MDCFilter extends HttpFilter @VisibleForTesting static final String MDC_USERNAME = "username"; - @VisibleForTesting - static final String MDC_TRANSACTION_ID = "transaction_id"; - //~--- methods -------------------------------------------------------------- /** @@ -98,7 +96,7 @@ public class MDCFilter extends HttpFilter MDC.put(MDC_CLIENT_HOST, request.getRemoteHost()); MDC.put(MDC_REQUEST_METHOD, request.getMethod()); MDC.put(MDC_REQUEST_URI, request.getRequestURI()); - MDC.put(MDC_TRANSACTION_ID, TRANSACTION_KEY_GENERATOR.createKey()); + TransactionId.set(TRANSACTION_KEY_GENERATOR.createKey()); try { @@ -111,7 +109,7 @@ public class MDCFilter extends HttpFilter MDC.remove(MDC_CLIENT_HOST); MDC.remove(MDC_REQUEST_METHOD); MDC.remove(MDC_REQUEST_URI); - MDC.remove(MDC_TRANSACTION_ID); + TransactionId.clear(); } } diff --git a/scm-webapp/src/test/java/sonia/scm/filter/MDCFilterTest.java b/scm-webapp/src/test/java/sonia/scm/filter/MDCFilterTest.java index db12e17c1d..935285dbcf 100644 --- a/scm-webapp/src/test/java/sonia/scm/filter/MDCFilterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/filter/MDCFilterTest.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.filter; import com.github.sdorra.shiro.ShiroRule; @@ -34,6 +34,7 @@ import org.mockito.junit.MockitoJUnitRunner; import org.slf4j.MDC; import sonia.scm.AbstractTestBase; import sonia.scm.SCMContext; +import sonia.scm.TransactionId; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -50,28 +51,28 @@ import static org.mockito.Mockito.when; /** * Unit tests for {@link MDCFilter}. - * + * * @author Sebastian Sdorra */ @RunWith(MockitoJUnitRunner.Silent.class) public class MDCFilterTest extends AbstractTestBase { - + @Rule public ShiroRule shiro = new ShiroRule(); - + @Mock private HttpServletRequest request; - + @Mock private HttpServletResponse response; - + private final MDCFilter filter = new MDCFilter(); /** * Tests {@link MDCFilter#doFilter(HttpServletRequest, HttpServletResponse, FilterChain)}. - * + * * @throws IOException - * @throws ServletException + * @throws ServletException */ @Test @SubjectAware( @@ -85,44 +86,44 @@ public class MDCFilterTest extends AbstractTestBase { when(request.getRemoteAddr()).thenReturn("127.0.0.1"); when(request.getRemoteHost()).thenReturn("localhost"); when(request.getMethod()).thenReturn("GET"); - + MDCCapturingFilterChain chain = new MDCCapturingFilterChain(); filter.doFilter(request, response, chain); - + assertNotNull(chain.ctx); assertEquals("trillian", chain.ctx.get(MDCFilter.MDC_USERNAME)); assertEquals("api/v1/repositories", chain.ctx.get(MDCFilter.MDC_REQUEST_URI)); assertEquals("127.0.0.1", chain.ctx.get(MDCFilter.MDC_CLIENT_IP)); assertEquals("localhost", chain.ctx.get(MDCFilter.MDC_CLIENT_HOST)); assertEquals("GET", chain.ctx.get(MDCFilter.MDC_REQUEST_METHOD)); - assertNotNull(chain.ctx.get(MDCFilter.MDC_TRANSACTION_ID)); + assertNotNull(chain.ctx.get(TransactionId.KEY)); } - + /** * Tests {@link MDCFilter#doFilter(HttpServletRequest, HttpServletResponse, FilterChain)} as anonymous user. - * + * * @throws IOException - * @throws ServletException + * @throws ServletException */ @Test @SubjectAware public void testDoFilterAsAnonymous() throws IOException, ServletException { MDCCapturingFilterChain chain = new MDCCapturingFilterChain(); filter.doFilter(request, response, chain); - + assertNotNull(chain.ctx); assertEquals(SCMContext.USER_ANONYMOUS, chain.ctx.get(MDCFilter.MDC_USERNAME)); } - + private static class MDCCapturingFilterChain implements FilterChain { private Map ctx; - + @Override public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { this.ctx = MDC.getCopyOfContextMap(); } - + } }