From 1b9f917df801e3327e5bf2adac29cb757c3fb998 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Mon, 29 Jun 2026 09:38:25 +0200 Subject: [PATCH 1/2] UNOMI-934/935: Map rule and segment validation errors to HTTP 400 (Phase 2 PR 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend master’s AbstractRestExceptionMapper pattern so IllegalArgumentException and BadSegmentConditionException from POST /cxs/rules and /cxs/segments return 400 badRequest instead of 500. Service-layer validation is unchanged; adds dedicated mappers, RuntimeExceptionMapper fallback for wrapped causes, and unit tests. --- .../AbstractRestExceptionMapper.java | 9 ++++ .../BadSegmentConditionExceptionMapper.java | 50 +++++++++++++++++++ .../IllegalArgumentExceptionMapper.java | 49 ++++++++++++++++++ .../exception/RuntimeExceptionMapper.java | 10 ++-- .../exception/RestExceptionMapperTest.java | 29 +++++++++++ 5 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 rest/src/main/java/org/apache/unomi/rest/exception/BadSegmentConditionExceptionMapper.java create mode 100644 rest/src/main/java/org/apache/unomi/rest/exception/IllegalArgumentExceptionMapper.java diff --git a/rest/src/main/java/org/apache/unomi/rest/exception/AbstractRestExceptionMapper.java b/rest/src/main/java/org/apache/unomi/rest/exception/AbstractRestExceptionMapper.java index 5f2f55045..73e316c7a 100644 --- a/rest/src/main/java/org/apache/unomi/rest/exception/AbstractRestExceptionMapper.java +++ b/rest/src/main/java/org/apache/unomi/rest/exception/AbstractRestExceptionMapper.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonMappingException; +import org.apache.unomi.api.exceptions.BadSegmentConditionException; import org.apache.cxf.jaxrs.utils.JAXRSUtils; import org.apache.cxf.message.Message; import org.slf4j.Logger; @@ -69,6 +70,14 @@ protected boolean isJsonDeserializationError(Throwable rootCause) { return rootCause instanceof JsonMappingException || rootCause instanceof JsonParseException; } + /** + * @return {@code true} when the throwable represents invalid client input rejected by domain + * validation (e.g. rule or segment condition checks), rather than a server fault. + */ + protected boolean isClientValidationError(Throwable throwable) { + return throwable instanceof IllegalArgumentException || throwable instanceof BadSegmentConditionException; + } + protected Throwable getRootCause(Throwable throwable) { if (throwable == null) { return null; diff --git a/rest/src/main/java/org/apache/unomi/rest/exception/BadSegmentConditionExceptionMapper.java b/rest/src/main/java/org/apache/unomi/rest/exception/BadSegmentConditionExceptionMapper.java new file mode 100644 index 000000000..a0c5783bb --- /dev/null +++ b/rest/src/main/java/org/apache/unomi/rest/exception/BadSegmentConditionExceptionMapper.java @@ -0,0 +1,50 @@ +/* + * 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.unomi.rest.exception; + +import org.apache.unomi.api.exceptions.BadSegmentConditionException; +import org.osgi.service.component.annotations.Component; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; + +/** + * Maps {@link BadSegmentConditionException} (invalid segment condition on {@code POST /cxs/segments}) + * to a {@code 400 Bad Request} instead of a {@code 500 Internal Server Error}. + */ +@Provider +@Component(service = ExceptionMapper.class) +public class BadSegmentConditionExceptionMapper extends AbstractRestExceptionMapper + implements ExceptionMapper { + + private static final Logger LOGGER = LoggerFactory.getLogger(BadSegmentConditionExceptionMapper.class.getName()); + + @Override + public Response toResponse(BadSegmentConditionException exception) { + String requestContext = buildRequestContext(); + String errorMessage = LogSanitizer.forLogging(messageOrType(exception)); + + LOGGER.warn("Bad request on {} - Invalid segment condition: {} (Set BadSegmentConditionExceptionMapper to debug to get the full stacktrace)", + requestContext, errorMessage); + LOGGER.debug("Full bad segment condition exception details for request: {}", requestContext, exception); + + return badRequestResponse(); + } +} diff --git a/rest/src/main/java/org/apache/unomi/rest/exception/IllegalArgumentExceptionMapper.java b/rest/src/main/java/org/apache/unomi/rest/exception/IllegalArgumentExceptionMapper.java new file mode 100644 index 000000000..b5cb3f2ba --- /dev/null +++ b/rest/src/main/java/org/apache/unomi/rest/exception/IllegalArgumentExceptionMapper.java @@ -0,0 +1,49 @@ +/* + * 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.unomi.rest.exception; + +import org.osgi.service.component.annotations.Component; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; + +/** + * Maps {@link IllegalArgumentException} (e.g. invalid rule condition on {@code POST /cxs/rules}) + * to a {@code 400 Bad Request} instead of a {@code 500 Internal Server Error}. + */ +@Provider +@Component(service = ExceptionMapper.class) +public class IllegalArgumentExceptionMapper extends AbstractRestExceptionMapper + implements ExceptionMapper { + + private static final Logger LOGGER = LoggerFactory.getLogger(IllegalArgumentExceptionMapper.class.getName()); + + @Override + public Response toResponse(IllegalArgumentException exception) { + String requestContext = buildRequestContext(); + String errorMessage = LogSanitizer.forLogging(messageOrType(exception)); + + LOGGER.warn("Bad request on {} - Invalid argument: {} (Set IllegalArgumentExceptionMapper to debug to get the full stacktrace)", + requestContext, errorMessage); + LOGGER.debug("Full illegal argument exception details for request: {}", requestContext, exception); + + return badRequestResponse(); + } +} diff --git a/rest/src/main/java/org/apache/unomi/rest/exception/RuntimeExceptionMapper.java b/rest/src/main/java/org/apache/unomi/rest/exception/RuntimeExceptionMapper.java index a87cfbcc8..4c0b0c26b 100644 --- a/rest/src/main/java/org/apache/unomi/rest/exception/RuntimeExceptionMapper.java +++ b/rest/src/main/java/org/apache/unomi/rest/exception/RuntimeExceptionMapper.java @@ -39,9 +39,11 @@ public Response toResponse(RuntimeException exception) { ? rootCause.getMessage() : (exception.getMessage() != null ? exception.getMessage() : "")); - // Client errors (deserialization) are logged at WARN; genuine server faults at ERROR. - boolean isDeserializationError = isJsonDeserializationError(rootCause); - if (isDeserializationError) { + // Client errors (deserialization or domain validation) are logged at WARN; genuine server faults at ERROR. + boolean isClientError = isJsonDeserializationError(rootCause) + || isClientValidationError(exception) + || isClientValidationError(rootCause); + if (isClientError) { LOGGER.warn( "Bad request on {} - Root cause: {} - {} (Set RuntimeExceptionMapper to debug to get the full stacktrace)", requestContext, rootCauseClassName, rootCauseMessage); @@ -52,6 +54,6 @@ public Response toResponse(RuntimeException exception) { } LOGGER.debug("Full exception details for request: {}", requestContext, exception); - return isDeserializationError ? badRequestResponse() : internalServerErrorResponse(); + return isClientError ? badRequestResponse() : internalServerErrorResponse(); } } diff --git a/rest/src/test/java/org/apache/unomi/rest/exception/RestExceptionMapperTest.java b/rest/src/test/java/org/apache/unomi/rest/exception/RestExceptionMapperTest.java index 4ca8ce885..9dd46c60b 100644 --- a/rest/src/test/java/org/apache/unomi/rest/exception/RestExceptionMapperTest.java +++ b/rest/src/test/java/org/apache/unomi/rest/exception/RestExceptionMapperTest.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.databind.JsonMappingException; +import org.apache.unomi.api.exceptions.BadSegmentConditionException; import org.junit.jupiter.api.Test; import javax.ws.rs.InternalServerErrorException; @@ -86,6 +87,34 @@ void internalServerError_withNonJsonCause_mapsToInternalServerError() { assertErrorResponse(response, 500, "internalServerError"); } + @Test + void illegalArgumentException_mapsToBadRequest() { + Response response = new IllegalArgumentExceptionMapper() + .toResponse(new IllegalArgumentException("Invalid rule condition:\n- missing type")); + assertErrorResponse(response, 400, "badRequest"); + } + + @Test + void badSegmentConditionException_mapsToBadRequest() { + Response response = new BadSegmentConditionExceptionMapper() + .toResponse(new BadSegmentConditionException("Invalid segment condition:\n- missing parameter")); + assertErrorResponse(response, 400, "badRequest"); + } + + @Test + void runtimeException_withIllegalArgumentCause_mapsToBadRequest() { + RuntimeException exception = new RuntimeException(new IllegalArgumentException("invalid rule")); + Response response = new RuntimeExceptionMapper().toResponse(exception); + assertErrorResponse(response, 400, "badRequest"); + } + + @Test + void runtimeException_withBadSegmentConditionCause_mapsToBadRequest() { + RuntimeException exception = new RuntimeException(new BadSegmentConditionException("invalid segment")); + Response response = new RuntimeExceptionMapper().toResponse(exception); + assertErrorResponse(response, 400, "badRequest"); + } + private static void assertErrorResponse(Response response, int expectedStatus, String expectedErrorMessage) { assertEquals(expectedStatus, response.getStatus()); Object entity = response.getEntity(); From c30346a4bb607eceb26179727fde1f13e1bc3943 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Mon, 29 Jun 2026 11:52:15 +0200 Subject: [PATCH 2/2] UNOMI-934/935: Map wrapped validation errors in InternalServerErrorExceptionMapper to 400 Extend the existing wrapped-cause handling so IllegalArgumentException and BadSegmentConditionException surfaced via InternalServerErrorException also return badRequest, matching RuntimeExceptionMapper behavior. --- .../InternalServerErrorExceptionMapper.java | 14 ++++++++------ .../rest/exception/RestExceptionMapperTest.java | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/rest/src/main/java/org/apache/unomi/rest/exception/InternalServerErrorExceptionMapper.java b/rest/src/main/java/org/apache/unomi/rest/exception/InternalServerErrorExceptionMapper.java index 42c4d437e..a500ab093 100644 --- a/rest/src/main/java/org/apache/unomi/rest/exception/InternalServerErrorExceptionMapper.java +++ b/rest/src/main/java/org/apache/unomi/rest/exception/InternalServerErrorExceptionMapper.java @@ -27,8 +27,9 @@ /** * Maps {@link InternalServerErrorException}. When the underlying cause is a Jackson deserialization - * failure (a wrapped client error) the response is downgraded to a {@code 400 Bad Request}; - * otherwise it remains a {@code 500 Internal Server Error} with detailed, sanitized logging. + * failure or a domain validation error (a wrapped client error) the response is downgraded to a + * {@code 400 Bad Request}; otherwise it remains a {@code 500 Internal Server Error} with detailed, + * sanitized logging. */ @Provider @Component(service = ExceptionMapper.class) @@ -42,12 +43,13 @@ public Response toResponse(InternalServerErrorException exception) { String requestContext = buildRequestContext(); Throwable rootCause = getRootCause(exception); - // A wrapped JSON deserialization failure is really a client error -> 400 Bad Request. - if (isJsonDeserializationError(rootCause)) { + // A wrapped JSON deserialization failure or domain validation error is really a client + // error -> 400 Bad Request. + if (isJsonDeserializationError(rootCause) || isClientValidationError(rootCause)) { String errorMessage = LogSanitizer.forLogging(messageOrType(rootCause)); - LOGGER.warn("Bad request on {} - JSON deserialization error: {} (Set InternalServerErrorExceptionMapper to debug to get the full stacktrace)", + LOGGER.warn("Bad request on {} - Root cause: {} (Set InternalServerErrorExceptionMapper to debug to get the full stacktrace)", requestContext, errorMessage); - LOGGER.debug("Full JSON mapping exception details for request: {}", requestContext, exception); + LOGGER.debug("Full exception details for request: {}", requestContext, exception); return badRequestResponse(); } diff --git a/rest/src/test/java/org/apache/unomi/rest/exception/RestExceptionMapperTest.java b/rest/src/test/java/org/apache/unomi/rest/exception/RestExceptionMapperTest.java index 9dd46c60b..1306f17e6 100644 --- a/rest/src/test/java/org/apache/unomi/rest/exception/RestExceptionMapperTest.java +++ b/rest/src/test/java/org/apache/unomi/rest/exception/RestExceptionMapperTest.java @@ -115,6 +115,22 @@ void runtimeException_withBadSegmentConditionCause_mapsToBadRequest() { assertErrorResponse(response, 400, "badRequest"); } + @Test + void internalServerError_withIllegalArgumentCause_mapsToBadRequest() { + InternalServerErrorException exception = new InternalServerErrorException("wrapped", + new IllegalArgumentException("invalid rule")); + Response response = new InternalServerErrorExceptionMapper().toResponse(exception); + assertErrorResponse(response, 400, "badRequest"); + } + + @Test + void internalServerError_withBadSegmentConditionCause_mapsToBadRequest() { + InternalServerErrorException exception = new InternalServerErrorException("wrapped", + new BadSegmentConditionException("invalid segment")); + Response response = new InternalServerErrorExceptionMapper().toResponse(exception); + assertErrorResponse(response, 400, "badRequest"); + } + private static void assertErrorResponse(Response response, int expectedStatus, String expectedErrorMessage) { assertEquals(expectedStatus, response.getStatus()); Object entity = response.getEntity();