From 80f8d6102406a3382659f11953a450213ddef4ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Tue, 8 Apr 2025 14:33:39 +0200 Subject: [PATCH] Only log the exception if is it resolved This commit improves AbstractEndpointExceptionResolver to only invoke the logException method for an exception that has been resolved. To help with chaining them, a CompositeEndpointExceptionResolver has been introduced. If no resolvers were able to resolve the exception, the last instance will then log the exception. Closes gh-736 --- .../AbstractEndpointExceptionResolver.java | 8 +- .../CompositeEndpointExceptionResolver.java | 52 +++++++++++ ...ompositeEndpointExceptionResolverTest.java | 87 +++++++++++++++++++ 3 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 spring-ws-core/src/main/java/org/springframework/ws/server/endpoint/CompositeEndpointExceptionResolver.java create mode 100644 spring-ws-core/src/test/java/org/springframework/ws/server/endpoint/CompositeEndpointExceptionResolverTest.java diff --git a/spring-ws-core/src/main/java/org/springframework/ws/server/endpoint/AbstractEndpointExceptionResolver.java b/spring-ws-core/src/main/java/org/springframework/ws/server/endpoint/AbstractEndpointExceptionResolver.java index 60c82f59..13a197a2 100644 --- a/spring-ws-core/src/main/java/org/springframework/ws/server/endpoint/AbstractEndpointExceptionResolver.java +++ b/spring-ws-core/src/main/java/org/springframework/ws/server/endpoint/AbstractEndpointExceptionResolver.java @@ -100,12 +100,14 @@ public abstract class AbstractEndpointExceptionResolver implements EndpointExcep if (this.mappedEndpoints != null && !this.mappedEndpoints.contains(mappedEndpoint)) { return false; } - // Log exception, both at debug log level and at warn level, if desired. if (this.logger.isDebugEnabled()) { this.logger.debug("Resolving exception from endpoint [" + endpoint + "]: " + ex); } - logException(ex, messageContext); - return resolveExceptionInternal(messageContext, endpoint, ex); + boolean resolved = resolveExceptionInternal(messageContext, endpoint, ex); + if (resolved) { + logException(ex, messageContext); + } + return resolved; } /** diff --git a/spring-ws-core/src/main/java/org/springframework/ws/server/endpoint/CompositeEndpointExceptionResolver.java b/spring-ws-core/src/main/java/org/springframework/ws/server/endpoint/CompositeEndpointExceptionResolver.java new file mode 100644 index 00000000..61048874 --- /dev/null +++ b/spring-ws-core/src/main/java/org/springframework/ws/server/endpoint/CompositeEndpointExceptionResolver.java @@ -0,0 +1,52 @@ +/* + * Copyright 2005-2025 the original author or authors. + * + * 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 + * + * https://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.springframework.ws.server.endpoint; + +import org.springframework.ws.context.MessageContext; +import org.springframework.ws.server.EndpointExceptionResolver; + +/** + * A composite of {@linkplain AbstractEndpointExceptionResolver endpoint exception + * resolvers}. + * + * @author Stephane Nicoll + * @since 4.1.0 + */ +public class CompositeEndpointExceptionResolver implements EndpointExceptionResolver { + + private final Iterable resolvers; + + public CompositeEndpointExceptionResolver(Iterable resolvers) { + this.resolvers = resolvers; + } + + @Override + public final boolean resolveException(MessageContext messageContext, Object endpoint, Exception ex) { + AbstractEndpointExceptionResolver currentResolver = null; + for (AbstractEndpointExceptionResolver resolver : this.resolvers) { + currentResolver = resolver; + if (currentResolver.resolveException(messageContext, endpoint, ex)) { + return true; + } + } + if (currentResolver != null) { + currentResolver.logException(ex, messageContext); + } + return false; + } + +} diff --git a/spring-ws-core/src/test/java/org/springframework/ws/server/endpoint/CompositeEndpointExceptionResolverTest.java b/spring-ws-core/src/test/java/org/springframework/ws/server/endpoint/CompositeEndpointExceptionResolverTest.java new file mode 100644 index 00000000..be2d45c0 --- /dev/null +++ b/spring-ws-core/src/test/java/org/springframework/ws/server/endpoint/CompositeEndpointExceptionResolverTest.java @@ -0,0 +1,87 @@ +/* + * Copyright 2005-2025 the original author or authors. + * + * 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 + * + * https://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.springframework.ws.server.endpoint; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import org.springframework.ws.context.MessageContext; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link CompositeEndpointExceptionResolver}. + * + * @author Stephane Nicoll + */ +class CompositeEndpointExceptionResolverTest { + + @Test + void logExceptionIsInvokedOnlyOnResolvedInstance() { + TestExceptionResolver first = new TestExceptionResolver(false); + TestExceptionResolver second = new TestExceptionResolver(true); + TestExceptionResolver third = new TestExceptionResolver(false); + CompositeEndpointExceptionResolver resolver = new CompositeEndpointExceptionResolver( + List.of(first, second, third)); + boolean resolved = resolver.resolveException(null, null, new RuntimeException("test")); + assertThat(resolved).isTrue(); + assertThat(first.logException).isFalse(); + assertThat(second.logException).isTrue(); + assertThat(third.logException).isFalse(); + } + + @Test + void logExceptionIsInvokedOnLastResolvedInstanceIfNecessary() { + TestExceptionResolver first = new TestExceptionResolver(false); + TestExceptionResolver second = new TestExceptionResolver(false); + TestExceptionResolver third = new TestExceptionResolver(false); + CompositeEndpointExceptionResolver resolver = new CompositeEndpointExceptionResolver( + List.of(first, second, third)); + boolean resolved = resolver.resolveException(null, null, new RuntimeException("test")); + assertThat(resolved).isFalse(); + assertThat(first.logException).isFalse(); + assertThat(second.logException).isFalse(); + assertThat(third.logException).isTrue(); + } + + public static class TestExceptionResolver extends AbstractEndpointExceptionResolver { + + private final boolean resolve; + + private boolean logException; + + public TestExceptionResolver(boolean resolve) { + this.resolve = resolve; + } + + @Override + protected boolean resolveExceptionInternal(MessageContext messageContext, Object endpoint, Exception ex) { + return this.resolve; + } + + @Override + protected void logException(Exception ex, MessageContext messageContext) { + if (this.logException) { + throw new IllegalStateException("Log exception already called"); + } + this.logException = true; + } + + } + +} \ No newline at end of file