This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit f08f82f9c3b7f30c4df824ba3b7be46fbeb9c5ae Author: Benoit Tellier <[email protected]> AuthorDate: Thu Mar 26 12:25:29 2020 +0700 JAMES-3078 MDC hierarchical MDC context for reactor --- .../james/imap/processor/UnselectProcessor.java | 3 +- .../java/org/apache/james/util/MDCBuilder.java | 5 ++ .../java/org/apache/james/util/ReactorUtils.java | 50 +++++++++++++++++++ .../org/apache/james/util/ReactorUtilsTest.java | 50 +++++++++++++++++++ .../draft/methods/GetVacationResponseMethod.java | 5 +- .../draft/methods/SetVacationResponseMethod.java | 8 ++- .../james/jmap/http/AuthenticationRoutes.java | 57 ++++++++++++++-------- .../org/apache/james/jmap/http/DownloadRoutes.java | 22 +++++++-- .../org/apache/james/jmap/http/JMAPApiRoutes.java | 16 +++--- .../org/apache/james/jmap/http/LoggingHelper.java | 46 +++++++++++++++++ .../org/apache/james/jmap/http/UploadRoutes.java | 12 ++++- .../apache/james/jmap/http/JMAPApiRoutesTest.java | 9 ++-- .../java/org/apache/james/jmap/JMAPRoutes.java | 1 - 13 files changed, 243 insertions(+), 41 deletions(-) diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/UnselectProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/UnselectProcessor.java index 64176c1..f6fa95a 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/UnselectProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/UnselectProcessor.java @@ -66,8 +66,7 @@ public class UnselectProcessor extends AbstractMailboxProcessor<UnselectRequest> @Override protected Closeable addContextToMDC(UnselectRequest request) { - return MDCBuilder.create() - .addContext(MDCBuilder.ACTION, "UNSELECT") + return MDCBuilder.of(MDCBuilder.ACTION, "UNSELECT") .build(); } } diff --git a/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java b/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java index bac3b4b..9bf6c5b 100644 --- a/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java +++ b/server/container/util/src/main/java/org/apache/james/util/MDCBuilder.java @@ -98,6 +98,11 @@ public class MDCBuilder { return new MDCBuilder(); } + public static MDCBuilder of(String key, Object value) { + return create() + .addContext(key, value); + } + private final ImmutableMap.Builder<String, String> contextMap = ImmutableMap.builder(); private final ImmutableList.Builder<MDCBuilder> nestedBuilder = ImmutableList.builder(); diff --git a/server/container/util/src/main/java/org/apache/james/util/ReactorUtils.java b/server/container/util/src/main/java/org/apache/james/util/ReactorUtils.java index a87ab17..df9c937 100644 --- a/server/container/util/src/main/java/org/apache/james/util/ReactorUtils.java +++ b/server/container/util/src/main/java/org/apache/james/util/ReactorUtils.java @@ -18,16 +18,23 @@ ****************************************************************/ package org.apache.james.util; +import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; import java.util.Iterator; import java.util.Optional; +import java.util.function.Consumer; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.core.publisher.Signal; +import reactor.util.context.Context; public class ReactorUtils { + + public static final String MDC_KEY_PREFIX = "MDC-"; + public static <T> Mono<T> executeAndEmpty(Runnable runnable) { return Mono.fromRunnable(runnable).then(Mono.empty()); } @@ -100,4 +107,47 @@ public class ReactorUtils { private static int byteToInt(ByteBuffer buffer) { return buffer.get() & 0xff; } + + public static Consumer<Signal<?>> logOnError(Consumer<Throwable> errorLogStatement) { + return signal -> { + if (!signal.isOnError()) { + return; + } + try { + try (Closeable mdc = retrieveMDCBuilder(signal).build()) { + errorLogStatement.accept(signal.getThrowable()); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + }; + } + + public static Consumer<Signal<?>> log(Runnable logStatement) { + return signal -> { + try (Closeable mdc = retrieveMDCBuilder(signal).build()) { + logStatement.run(); + } catch (IOException e) { + throw new RuntimeException(e); + } + }; + } + + public static Context context(String keySuffix, MDCBuilder mdcBuilder) { + return Context.of(mdcKey(keySuffix), mdcBuilder); + } + + private static String mdcKey(String value) { + return MDC_KEY_PREFIX + value; + } + + private static MDCBuilder retrieveMDCBuilder(Signal<?> signal) { + return signal.getContext().stream() + .filter(entry -> entry.getKey() instanceof String) + .filter(entry -> entry.getValue() instanceof MDCBuilder) + .filter(entry -> ((String) entry.getKey()).startsWith(MDC_KEY_PREFIX)) + .map(entry -> (MDCBuilder) entry.getValue()) + .reduce(MDCBuilder.create(), MDCBuilder::addContext); + } + } diff --git a/server/container/util/src/test/java/org/apache/james/util/ReactorUtilsTest.java b/server/container/util/src/test/java/org/apache/james/util/ReactorUtilsTest.java index 06edbc5..1ef5184 100644 --- a/server/container/util/src/test/java/org/apache/james/util/ReactorUtilsTest.java +++ b/server/container/util/src/test/java/org/apache/james/util/ReactorUtilsTest.java @@ -27,11 +27,13 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.IntStream; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.slf4j.MDC; import com.google.common.collect.ImmutableList; import com.google.common.primitives.Bytes; @@ -280,4 +282,52 @@ class ReactorUtilsTest { assertThat(chunks).isEqualTo(expected); } } + + @Nested + class MDCTest { + @Test + void contextShouldEnhanceMDC() { + String value = "value"; + String key = "key"; + + Flux.just(1) + .doOnEach(ReactorUtils.log(() -> { + assertThat(MDC.get(key)).isEqualTo(value); + })) + .subscriberContext(ReactorUtils.context("test", MDCBuilder.of(key, value))) + .blockLast(); + } + + @Test + void contextShouldNotOverwritePreviousKeys() { + String value1 = "value1"; + String value2 = "value2"; + String key = "key"; + + Flux.just(1) + .doOnEach(ReactorUtils.log(() -> { + assertThat(MDC.get(key)).isEqualTo(value1); + })) + .subscriberContext(ReactorUtils.context("test", MDCBuilder.of(key, value1))) + .subscriberContext(ReactorUtils.context("test", MDCBuilder.of(key, value2))) + .blockLast(); + } + + @Test + void contextShouldCombineMDCs() { + String value1 = "value1"; + String value2 = "value2"; + String key1 = "key1"; + String key2 = "key2"; + + Flux.just(1) + .doOnEach(ReactorUtils.log(() -> { + assertThat(MDC.get(key1)).isEqualTo(value1); + assertThat(MDC.get(key2)).isEqualTo(value2); + })) + .subscriberContext(ReactorUtils.context("test1", MDCBuilder.of(key1, value1))) + .subscriberContext(ReactorUtils.context("test2", MDCBuilder.of(key2, value2))) + .blockLast(); + } + } } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetVacationResponseMethod.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetVacationResponseMethod.java index 1b11bd3..da38ebd 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetVacationResponseMethod.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetVacationResponseMethod.java @@ -19,6 +19,8 @@ package org.apache.james.jmap.draft.methods; +import static org.apache.james.jmap.http.LoggingHelper.jmapAction; + import javax.inject.Inject; import org.apache.james.jmap.api.vacation.AccountId; @@ -76,7 +78,8 @@ public class GetVacationResponseMethod implements Method { .methodCallId(methodCallId) .responseName(RESPONSE_NAME) .response(response) - .build())))); + .build())))) + .subscriberContext(jmapAction("VACATION")); } private Mono<GetVacationResponse> process(MailboxSession mailboxSession) { diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetVacationResponseMethod.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetVacationResponseMethod.java index d8952b8..4da3a22 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetVacationResponseMethod.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/SetVacationResponseMethod.java @@ -19,6 +19,9 @@ package org.apache.james.jmap.draft.methods; +import static org.apache.james.jmap.http.LoggingHelper.jmapAction; +import static org.apache.james.util.ReactorUtils.context; + import javax.inject.Inject; import org.apache.james.jmap.api.vacation.AccountId; @@ -32,6 +35,7 @@ import org.apache.james.jmap.draft.model.SetVacationResponse; import org.apache.james.jmap.draft.model.VacationResponse; import org.apache.james.mailbox.MailboxSession; import org.apache.james.metrics.api.MetricFactory; +import org.apache.james.util.MDCBuilder; import com.google.common.base.Preconditions; @@ -77,7 +81,9 @@ public class SetVacationResponseMethod implements Method { SetVacationRequest setVacationRequest = (SetVacationRequest) request; return metricFactory.runPublishingTimerMetricLogP99(JMAP_PREFIX + METHOD_NAME.getName(), - () -> process(methodCallId, mailboxSession, setVacationRequest)); + () -> process(methodCallId, mailboxSession, setVacationRequest) + .subscriberContext(jmapAction("SET_VACATION")) + .subscriberContext(context("set-vacation", MDCBuilder.of("update", setVacationRequest.getUpdate())))); } private Flux<JmapResponse> process(MethodCallId methodCallId, MailboxSession mailboxSession, SetVacationRequest setVacationRequest) { diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java index 5470969..db645dd 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/AuthenticationRoutes.java @@ -29,6 +29,11 @@ import static io.netty.handler.codec.http.HttpResponseStatus.UNAUTHORIZED; import static org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE; import static org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE_UTF8; import static org.apache.james.jmap.http.JMAPUrls.AUTHENTICATION; +import static org.apache.james.jmap.http.LoggingHelper.jmapAction; +import static org.apache.james.jmap.http.LoggingHelper.jmapAuthContext; +import static org.apache.james.jmap.http.LoggingHelper.jmapContext; +import static org.apache.james.util.ReactorUtils.log; +import static org.apache.james.util.ReactorUtils.logOnError; import java.io.IOException; import java.util.Objects; @@ -118,29 +123,40 @@ public class AuthenticationRoutes implements JMAPRoutes { } else { throw new RuntimeException(objectRequest.getClass() + " " + objectRequest); } - }) - .onErrorResume(BadRequestException.class, e -> handleBadRequest(response, e)) - .onErrorResume(e -> handleInternalError(response, e)))) + }))) + .onErrorResume(BadRequestException.class, e -> handleBadRequest(response, e)) + .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e))) + .onErrorResume(e -> handleInternalError(response, e)) + .subscriberContext(jmapContext(request)) + .subscriberContext(jmapAction("auth-post")) .subscribeOn(Schedulers.elastic()); } private Mono<Void> returnEndPointsResponse(HttpServerRequest req, HttpServerResponse resp) { - try { return authenticator.authenticate(req) - .then(resp.status(OK) - .header(CONTENT_TYPE, JSON_CONTENT_TYPE_UTF8) - .sendString(Mono.just(mapper.writeValueAsString(EndPointsResponse - .builder() - .api(JMAPUrls.JMAP) - .eventSource(JMAPUrls.NOT_IMPLEMENTED) - .upload(JMAPUrls.UPLOAD) - .download(JMAPUrls.DOWNLOAD) - .build()))) - .then()) + .flatMap(session -> returnEndPointsResponse(resp) + .subscriberContext(jmapAuthContext(session))) .onErrorResume(BadRequestException.class, e -> handleBadRequest(resp, e)) + .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e))) .onErrorResume(InternalErrorException.class, e -> handleInternalError(resp, e)) .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(resp, e)) + .subscriberContext(jmapContext(req)) + .subscriberContext(jmapAction("returnEndPoints")) .subscribeOn(Schedulers.elastic()); + } + + private Mono<Void> returnEndPointsResponse(HttpServerResponse resp) { + try { + return resp.status(OK) + .header(CONTENT_TYPE, JSON_CONTENT_TYPE_UTF8) + .sendString(Mono.just(mapper.writeValueAsString(EndPointsResponse + .builder() + .api(JMAPUrls.JMAP) + .eventSource(JMAPUrls.NOT_IMPLEMENTED) + .upload(JMAPUrls.UPLOAD) + .download(JMAPUrls.DOWNLOAD) + .build()))) + .then(); } catch (JsonProcessingException e) { throw new InternalErrorException("Error serializing endpoint response", e); } @@ -151,8 +167,11 @@ public class AuthenticationRoutes implements JMAPRoutes { return authenticator.authenticate(req) .flatMap(session -> Mono.from(accessTokenManager.revoke(AccessToken.fromString(authorizationHeader))) - .then(resp.status(NO_CONTENT).send().then())) + .then(resp.status(NO_CONTENT).send().then()) + .subscriberContext(jmapAuthContext(session))) .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(resp, e)) + .subscriberContext(jmapContext(req)) + .subscriberContext(jmapAction("auth-delete")) .subscribeOn(Schedulers.elastic()); } @@ -204,8 +223,8 @@ public class AuthenticationRoutes implements JMAPRoutes { case EXPIRED: return returnForbiddenAuthentication(resp); case INVALID: - LOGGER.warn("Use of an invalid ContinuationToken : {}", request.getToken().serialize()); - return returnUnauthorizedResponse(resp); + return returnUnauthorizedResponse(resp) + .doOnEach(log(() -> LOGGER.warn("Use of an invalid ContinuationToken : {}", request.getToken().serialize()))); case OK: return manageAuthenticationResponse(request, resp); default: @@ -221,8 +240,8 @@ public class AuthenticationRoutes implements JMAPRoutes { if (success) { return returnAccessTokenResponse(resp, username); } else { - LOGGER.info("Authentication failure for {}", username); - return returnUnauthorizedResponse(resp); + return returnUnauthorizedResponse(resp) + .doOnEach(log(() -> LOGGER.info("Authentication failure for {}", username))); } }); } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java index d639d16..57b99bd 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/DownloadRoutes.java @@ -23,6 +23,10 @@ import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND; import static io.netty.handler.codec.http.HttpResponseStatus.OK; import static org.apache.james.jmap.HttpConstants.TEXT_PLAIN_CONTENT_TYPE; import static org.apache.james.jmap.http.JMAPUrls.DOWNLOAD; +import static org.apache.james.jmap.http.LoggingHelper.jmapAction; +import static org.apache.james.jmap.http.LoggingHelper.jmapAuthContext; +import static org.apache.james.jmap.http.LoggingHelper.jmapContext; +import static org.apache.james.util.ReactorUtils.logOnError; import java.io.InputStream; import java.io.UnsupportedEncodingException; @@ -116,9 +120,13 @@ public class DownloadRoutes implements JMAPRoutes { private Mono<Void> post(HttpServerRequest request, HttpServerResponse response, DownloadPath downloadPath) { return authenticator.authenticate(request) .flatMap(session -> Mono.from(metricFactory.runPublishingTimerMetric("JMAP-download-post", - respondAttachmentAccessToken(session, downloadPath, response))) - .onErrorResume(InternalErrorException.class, e -> handleInternalError(response, e)) - .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e))) + respondAttachmentAccessToken(session, downloadPath, response))) + .subscriberContext(jmapAuthContext(session))) + .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e)) + .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e))) + .onErrorResume(e -> handleInternalError(response, e)) + .subscriberContext(jmapContext(request)) + .subscriberContext(jmapAction("download-post")) .subscribeOn(Schedulers.elastic()); } @@ -142,9 +150,13 @@ public class DownloadRoutes implements JMAPRoutes { private Mono<Void> get(HttpServerRequest request, HttpServerResponse response, DownloadPath downloadPath) { return authenticator.authenticate(request) .flatMap(session -> Mono.from(metricFactory.runPublishingTimerMetric("JMAP-download-get", - download(session, downloadPath, response))) - .onErrorResume(InternalErrorException.class, e -> handleInternalError(response, e))) + download(session, downloadPath, response))) + .subscriberContext(jmapAuthContext(session))) .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e)) + .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e))) + .onErrorResume(e -> handleInternalError(response, e)) + .subscriberContext(jmapContext(request)) + .subscriberContext(jmapAction("download-get")) .subscribeOn(Schedulers.elastic()); } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java index 9973616..9a483e2 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/JMAPApiRoutes.java @@ -22,6 +22,9 @@ import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE; import static io.netty.handler.codec.http.HttpResponseStatus.OK; import static org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE; import static org.apache.james.jmap.http.JMAPUrls.JMAP; +import static org.apache.james.jmap.http.LoggingHelper.jmapAuthContext; +import static org.apache.james.jmap.http.LoggingHelper.jmapContext; +import static org.apache.james.util.ReactorUtils.logOnError; import java.io.IOException; @@ -87,15 +90,16 @@ public class JMAPApiRoutes implements JMAPRoutes { private Mono<Void> post(HttpServerRequest request, HttpServerResponse response) { return authenticator.authenticate(request) .flatMap(session -> Flux.merge( - userProvisioner.provisionUser(session), - defaultMailboxesProvisioner.createMailboxesIfNeeded(session)) - .then() - .thenReturn(session)) - .flatMap(session -> Mono.from(metricFactory.runPublishingTimerMetric("JMAP-request", - post(request, response, session)))) + userProvisioner.provisionUser(session), + defaultMailboxesProvisioner.createMailboxesIfNeeded(session)) + .then(Mono.from(metricFactory.runPublishingTimerMetric("JMAP-request", + post(request, response, session)))) + .subscriberContext(jmapAuthContext(session))) .onErrorResume(BadRequestException.class, e -> handleBadRequest(response, e)) .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e)) + .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e))) .onErrorResume(e -> handleInternalError(response, e)) + .subscriberContext(jmapContext(request)) .subscribeOn(Schedulers.elastic()); } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/LoggingHelper.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/LoggingHelper.java new file mode 100644 index 0000000..af504f63 --- /dev/null +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/LoggingHelper.java @@ -0,0 +1,46 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you 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 org.apache.james.jmap.http; + +import static org.apache.james.util.ReactorUtils.context; + +import org.apache.james.mailbox.MailboxSession; +import org.apache.james.util.MDCBuilder; + +import reactor.netty.http.server.HttpServerRequest; +import reactor.util.context.Context; + +public interface LoggingHelper { + static Context jmapAuthContext(MailboxSession session) { + return context("JMAP_AUTH", + MDCBuilder.of(MDCBuilder.USER, session.getUser().asString())); + } + + static Context jmapContext(HttpServerRequest req) { + return context("JMAP", MDCBuilder.create() + .addContext(MDCBuilder.PROTOCOL, "JMAP") + .addContext(MDCBuilder.IP, req.hostAddress().getHostString())); + } + + static Context jmapAction(String action) { + return context("JMAP_ACTION", + MDCBuilder.of(MDCBuilder.ACTION, action)); + } +} diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java index f29ae56..ff0f306 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/http/UploadRoutes.java @@ -23,6 +23,10 @@ import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST; import static io.netty.handler.codec.http.HttpResponseStatus.CREATED; import static org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE_UTF8; import static org.apache.james.jmap.http.JMAPUrls.UPLOAD; +import static org.apache.james.jmap.http.LoggingHelper.jmapAction; +import static org.apache.james.jmap.http.LoggingHelper.jmapAuthContext; +import static org.apache.james.jmap.http.LoggingHelper.jmapContext; +import static org.apache.james.util.ReactorUtils.logOnError; import java.io.EOFException; import java.io.IOException; @@ -91,11 +95,15 @@ public class UploadRoutes implements JMAPRoutes { return response.status(BAD_REQUEST).send(); } else { return authenticator.authenticate(request) - .flatMap(session -> post(request, response, contentType, session)) + .flatMap(session -> post(request, response, contentType, session) + .subscriberContext(jmapAuthContext(session))) .onErrorResume(CancelledUploadException.class, e -> handleCanceledUpload(response, e)) .onErrorResume(BadRequestException.class, e -> handleBadRequest(response, e)) .onErrorResume(UnauthorizedException.class, e -> handleAuthenticationFailure(response, e)) - .onErrorResume(InternalErrorException.class, e -> handleInternalError(response, e)) + .doOnEach(logOnError(e -> LOGGER.error("Unexpected error", e))) + .onErrorResume(e -> handleInternalError(response, e)) + .subscriberContext(jmapContext(request)) + .subscriberContext(jmapAction("upload-get")) .subscribeOn(Schedulers.elastic()); } } diff --git a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JMAPApiRoutesTest.java b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JMAPApiRoutesTest.java index e8cafca..5cb214f 100644 --- a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JMAPApiRoutesTest.java +++ b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/http/JMAPApiRoutesTest.java @@ -24,18 +24,19 @@ import static io.restassured.config.RestAssuredConfig.newConfig; import static org.apache.james.jmap.http.JMAPUrls.JMAP; import static org.hamcrest.Matchers.equalTo; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.nio.charset.StandardCharsets; -import java.util.stream.Stream; +import org.apache.james.core.Username; import org.apache.james.jmap.draft.methods.ErrorResponse; import org.apache.james.jmap.draft.methods.Method; import org.apache.james.jmap.draft.methods.RequestHandler; import org.apache.james.jmap.draft.model.InvocationResponse; import org.apache.james.jmap.draft.model.MethodCallId; -import org.apache.james.mailbox.MailboxSession; +import org.apache.james.mailbox.MailboxSessionUtil; import org.apache.james.metrics.tests.RecordingMetricFactory; import org.junit.After; import org.junit.Before; @@ -85,8 +86,8 @@ public class JMAPApiRoutesTest { .setBasePath(JMAP) .build(); - when(mockedAuthFilter.authenticate(any())) - .thenReturn(Mono.just(mock(MailboxSession.class))); + doReturn(Mono.just(MailboxSessionUtil.create(Username.of("bob")))) + .when(mockedAuthFilter).authenticate(any()); when(mockedUserProvisionner.provisionUser(any())) .thenReturn(Mono.empty()); when(mockedMailboxesProvisionner.createMailboxesIfNeeded(any())) diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutes.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutes.java index 565c9c1..c42085d 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutes.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/JMAPRoutes.java @@ -48,7 +48,6 @@ public interface JMAPRoutes { Logger logger(); default Mono<Void> handleInternalError(HttpServerResponse response, Throwable e) { - logger().error("Internal error", e); return response.status(INTERNAL_SERVER_ERROR).send(); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
