From 17cc02437c1eb3d29e734fc732d661c5350fbeb4 Mon Sep 17 00:00:00 2001 From: tbischoff2 <42869259+tbischoff2@users.noreply.github.com> Date: Fri, 13 Sep 2024 09:49:19 +0200 Subject: [PATCH] fix GDay value serialization (#879) * fix value only serialization error with several DateTime types * add GDay unit test * improve logging * update changelog --------- Co-authored-by: Michael Jacoby --- .../json/ValueOnlyJsonSerializer.java | 3 ++ .../value/AbstractDateTimeValueMixIn.java | 28 +++++++++++++++++++ .../json/ValueOnlyJsonSerializerTest.java | 7 +++++ .../json/fixture/ValueOnlyExamples.java | 8 ++++++ .../resources/valueonly/property-gday.json | 3 ++ docs/source/other/release-notes.md | 5 +++- .../service/endpoint/http/RequestHandler.java | 8 ++++++ ...tPostResponseWithLocationHeaderMapper.java | 5 ++++ ...erateSerializationByIdsResponseMapper.java | 5 ++++ ...GetOperationAsyncStatusResponseMapper.java | 5 ++++ .../ResponseWithPayloadResponseMapper.java | 5 ++++ .../endpoint/http/util/HttpHelper.java | 1 + 12 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 dataformat/json/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/dataformat/json/mixins/value/AbstractDateTimeValueMixIn.java create mode 100644 dataformat/json/src/test/resources/valueonly/property-gday.json diff --git a/dataformat/json/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/dataformat/json/ValueOnlyJsonSerializer.java b/dataformat/json/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/dataformat/json/ValueOnlyJsonSerializer.java index 217ac5d70..915d8c5e3 100644 --- a/dataformat/json/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/dataformat/json/ValueOnlyJsonSerializer.java +++ b/dataformat/json/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/dataformat/json/ValueOnlyJsonSerializer.java @@ -31,6 +31,7 @@ import de.fraunhofer.iosb.ilt.faaast.service.dataformat.json.mixins.PageMixin; import de.fraunhofer.iosb.ilt.faaast.service.dataformat.json.mixins.ResponseMixin; import de.fraunhofer.iosb.ilt.faaast.service.dataformat.json.mixins.ResultMixin; +import de.fraunhofer.iosb.ilt.faaast.service.dataformat.json.mixins.value.AbstractDateTimeValueMixIn; import de.fraunhofer.iosb.ilt.faaast.service.dataformat.json.mixins.value.GetOperationAsyncResultResponseValueMixin; import de.fraunhofer.iosb.ilt.faaast.service.dataformat.json.mixins.value.InvokeOperationRequestValueMixin; import de.fraunhofer.iosb.ilt.faaast.service.dataformat.json.mixins.value.OperationResultValueMixin; @@ -71,6 +72,7 @@ import de.fraunhofer.iosb.ilt.faaast.service.model.value.SubmodelElementCollectionValue; import de.fraunhofer.iosb.ilt.faaast.service.model.value.SubmodelElementListValue; import de.fraunhofer.iosb.ilt.faaast.service.model.value.TypedValue; +import de.fraunhofer.iosb.ilt.faaast.service.model.value.primitive.AbstractDateTimeValue; import de.fraunhofer.iosb.ilt.faaast.service.util.ElementValueHelper; import de.fraunhofer.iosb.ilt.faaast.service.util.ReflectionHelper; import java.util.HashSet; @@ -204,6 +206,7 @@ protected JsonMapper modifyMapper(JsonMapper mapper) { mapper.addMixIn(SubmodelElementCollectionValue.class, SubmodelElementCollectionValueMixin.class); mapper.addMixIn(SubmodelElementListValue.class, SubmodelElementListValueMixin.class); mapper.addMixIn(TypedValue.class, TypedValueMixin.class); + mapper.addMixIn(AbstractDateTimeValue.class, AbstractDateTimeValueMixIn.class); mapper.addMixIn(ReferenceElementValue.class, ReferenceElementValueMixin.class); mapper.addMixIn(Page.class, PageMixin.class); mapper.addMixIn(AbstractRequestWithModifier.class, AbstractRequestWithModifierMixin.class); diff --git a/dataformat/json/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/dataformat/json/mixins/value/AbstractDateTimeValueMixIn.java b/dataformat/json/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/dataformat/json/mixins/value/AbstractDateTimeValueMixIn.java new file mode 100644 index 000000000..65714cf00 --- /dev/null +++ b/dataformat/json/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/dataformat/json/mixins/value/AbstractDateTimeValueMixIn.java @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2021 Fraunhofer IOSB, eine rechtlich nicht selbstaendige + * Einrichtung der Fraunhofer-Gesellschaft zur Foerderung der angewandten + * Forschung e.V. + * 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 de.fraunhofer.iosb.ilt.faaast.service.dataformat.json.mixins.value; + +import com.fasterxml.jackson.annotation.JsonValue; + + +/** + * MixIn for AbstractDateTimeValue. + */ +public abstract class AbstractDateTimeValueMixIn { + + @JsonValue + abstract String asString(); + +} diff --git a/dataformat/json/src/test/java/de/fraunhofer/iosb/ilt/faaast/service/serialization/json/ValueOnlyJsonSerializerTest.java b/dataformat/json/src/test/java/de/fraunhofer/iosb/ilt/faaast/service/serialization/json/ValueOnlyJsonSerializerTest.java index 13773f325..d08c2837e 100644 --- a/dataformat/json/src/test/java/de/fraunhofer/iosb/ilt/faaast/service/serialization/json/ValueOnlyJsonSerializerTest.java +++ b/dataformat/json/src/test/java/de/fraunhofer/iosb/ilt/faaast/service/serialization/json/ValueOnlyJsonSerializerTest.java @@ -120,6 +120,13 @@ public void testProperty() throws SerializationException, JSONException, IOExcep } + @Test + public void testPropertyGDay() throws SerializationException, JSONException, IOException, ValueMappingException { + assertEquals(ValueOnlyExamples.PROPERTY_GDAY_FILE, ValueOnlyExamples.PROPERTY_GDAY); + assertValue(ValueOnlyExamples.PROPERTY_GDAY_FILE, ValueOnlyExamples.PROPERTY_GDAY); + } + + @Test public void testList() throws SerializationException, JSONException, IOException, ValueMappingException { Map data = Map.of(ValueOnlyExamples.PROPERTY_STRING, ValueOnlyExamples.PROPERTY_STRING_FILE, diff --git a/dataformat/json/src/test/java/de/fraunhofer/iosb/ilt/faaast/service/serialization/json/fixture/ValueOnlyExamples.java b/dataformat/json/src/test/java/de/fraunhofer/iosb/ilt/faaast/service/serialization/json/fixture/ValueOnlyExamples.java index 1ac84e696..a61714676 100644 --- a/dataformat/json/src/test/java/de/fraunhofer/iosb/ilt/faaast/service/serialization/json/fixture/ValueOnlyExamples.java +++ b/dataformat/json/src/test/java/de/fraunhofer/iosb/ilt/faaast/service/serialization/json/fixture/ValueOnlyExamples.java @@ -86,6 +86,7 @@ public class ValueOnlyExamples { public static final File PROPERTY_INT_FILE = new File(RESOURCE_PATH + "/property-int.json"); public static final File PROPERTY_DOUBLE_FILE = new File(RESOURCE_PATH + "/property-double.json"); public static final File PROPERTY_STRING_FILE = new File(RESOURCE_PATH + "/property-string.json"); + public static final File PROPERTY_GDAY_FILE = new File(RESOURCE_PATH + "/property-gday.json"); public static final File PROPERTY_DATETIME_FILE = new File(RESOURCE_PATH + "/property-datetime.json"); public static final File INVOKE_OPERATION_REQUEST_FILE = new File(RESOURCE_PATH + "/invoke-operation-request.json"); public static final File GET_OPERATION_ASYNC_RESULT_RESPONSE_FILE = new File(RESOURCE_PATH + "/get-operation-async-result-response.json"); @@ -152,6 +153,13 @@ public class ValueOnlyExamples { .value("foo") .build(); + public static final Property PROPERTY_GDAY = new DefaultProperty.Builder() + .category("category") + .idShort("propGDay") + .value("---15") + .valueType(DataTypeDefXsd.GDAY) + .build(); + public static final Range RANGE_DOUBLE = new DefaultRange.Builder() .idShort("rangeDouble") .valueType(DataTypeDefXsd.DOUBLE) diff --git a/dataformat/json/src/test/resources/valueonly/property-gday.json b/dataformat/json/src/test/resources/valueonly/property-gday.json new file mode 100644 index 000000000..873088f78 --- /dev/null +++ b/dataformat/json/src/test/resources/valueonly/property-gday.json @@ -0,0 +1,3 @@ +{ + "propGDay": "---15" +} diff --git a/docs/source/other/release-notes.md b/docs/source/other/release-notes.md index e898115a4..0f73000bb 100644 --- a/docs/source/other/release-notes.md +++ b/docs/source/other/release-notes.md @@ -10,7 +10,10 @@ **Internal changes & bugfixes** - General - Fixed bug in JSON valueOnly deserialization that could occur with complex strcutures (e.g. SubmodelElementCollections within SubmodelElementLists) - - Fixed bug caused by null values in JSON payload when inserting data via HTTP - null values are now treated as empty/default values +- Endpoint + - HTTP + - Fixed bug caused by null values in JSON payload when inserting data via HTTP - null values are now treated as empty/default values + - Fixed "serialization failed" bug when retrieving a property of type `GDay` ## 1.1.0 diff --git a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/RequestHandler.java b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/RequestHandler.java index 8ec360315..1bbd569f5 100644 --- a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/RequestHandler.java +++ b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/RequestHandler.java @@ -49,6 +49,8 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.servlets.CrossOriginFilter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** @@ -57,6 +59,7 @@ */ public class RequestHandler extends AbstractHandler { + private static final Logger LOGGER = LoggerFactory.getLogger(RequestHandler.class); private static final int DEFAULT_PREFLIGHT_MAX_AGE = 1800; private final ServiceContext serviceContext; private final HttpEndpointConfig config; @@ -101,6 +104,7 @@ public void handle(String string, Request baseRequest, HttpServletRequest reques method = HttpMethod.valueOf(request.getMethod()); } catch (IllegalArgumentException e) { + LOGGER.debug("invalid method", e); HttpHelper.send( response, StatusCode.CLIENT_METHOD_NOT_ALLOWED, @@ -126,6 +130,7 @@ public void handle(String string, Request baseRequest, HttpServletRequest reques executeAndSend(response, requestMappingManager.map(httpRequest)); } catch (MethodNotAllowedException e) { + LOGGER.debug("invalid method", e); HttpHelper.send( response, StatusCode.CLIENT_METHOD_NOT_ALLOWED, @@ -134,6 +139,7 @@ public void handle(String string, Request baseRequest, HttpServletRequest reques .build()); } catch (InvalidRequestException | IllegalArgumentException e) { + LOGGER.debug("bad request", e); HttpHelper.send( response, StatusCode.CLIENT_ERROR_BAD_REQUEST, @@ -142,6 +148,7 @@ public void handle(String string, Request baseRequest, HttpServletRequest reques .build()); } catch (SerializationException | RuntimeException e) { + LOGGER.warn("error handling request", e); HttpHelper.send( response, StatusCode.SERVER_INTERNAL_ERROR, @@ -191,6 +198,7 @@ private void handlePreflightedCORSRequest(String url, HttpServletRequest request } } catch (RuntimeException e) { + LOGGER.warn("error handling request", e); HttpHelper.send( response, StatusCode.CLIENT_ERROR_BAD_REQUEST, diff --git a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/AbstractPostResponseWithLocationHeaderMapper.java b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/AbstractPostResponseWithLocationHeaderMapper.java index fea0f4359..d021de67c 100644 --- a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/AbstractPostResponseWithLocationHeaderMapper.java +++ b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/AbstractPostResponseWithLocationHeaderMapper.java @@ -25,6 +25,8 @@ import de.fraunhofer.iosb.ilt.faaast.service.model.api.request.AbstractRequestWithModifier; import de.fraunhofer.iosb.ilt.faaast.service.model.api.response.AbstractResponseWithPayload; import jakarta.servlet.http.HttpServletResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** @@ -35,6 +37,8 @@ */ public abstract class AbstractPostResponseWithLocationHeaderMapper> extends ResponseWithPayloadResponseMapper { + private static final Logger LOGGER = LoggerFactory.getLogger(AbstractPostResponseWithLocationHeaderMapper.class); + protected AbstractPostResponseWithLocationHeaderMapper(ServiceContext serviceContext) { super(serviceContext); } @@ -64,6 +68,7 @@ public void map(U apiRequest, T apiResponse, HttpServletResponse httpResponse) { : OutputModifier.DEFAULT)); } catch (Exception e) { + LOGGER.warn("error handling request", e); HttpHelper.send( httpResponse, StatusCode.SERVER_INTERNAL_ERROR, diff --git a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/GenerateSerializationByIdsResponseMapper.java b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/GenerateSerializationByIdsResponseMapper.java index 8535df78c..f3c988a24 100644 --- a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/GenerateSerializationByIdsResponseMapper.java +++ b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/GenerateSerializationByIdsResponseMapper.java @@ -25,6 +25,8 @@ import de.fraunhofer.iosb.ilt.faaast.service.model.api.response.aasserialization.GenerateSerializationByIdsResponse; import jakarta.servlet.http.HttpServletResponse; import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** @@ -33,6 +35,8 @@ */ public class GenerateSerializationByIdsResponseMapper extends AbstractResponseMapper { + private static final Logger LOGGER = LoggerFactory.getLogger(GenerateSerializationByIdsResponseMapper.class); + public GenerateSerializationByIdsResponseMapper(ServiceContext serviceContext) { super(serviceContext); } @@ -51,6 +55,7 @@ public void map(GenerateSerializationByIdsRequest apiRequest, GenerateSerializat apiResponse.getDataformat().getFileExtensions().get(0)))); } catch (SerializationException e) { + LOGGER.warn("error serializing response", e); HttpHelper.send( httpResponse, StatusCode.SERVER_INTERNAL_ERROR, diff --git a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/GetOperationAsyncStatusResponseMapper.java b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/GetOperationAsyncStatusResponseMapper.java index 6da933921..0877ad0e6 100644 --- a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/GetOperationAsyncStatusResponseMapper.java +++ b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/GetOperationAsyncStatusResponseMapper.java @@ -26,6 +26,8 @@ import de.fraunhofer.iosb.ilt.faaast.service.util.EncodingHelper; import jakarta.servlet.http.HttpServletResponse; import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** @@ -33,6 +35,8 @@ */ public class GetOperationAsyncStatusResponseMapper extends AbstractResponseMapper { + private static final Logger LOGGER = LoggerFactory.getLogger(GetOperationAsyncStatusResponseMapper.class); + public GetOperationAsyncStatusResponseMapper(ServiceContext serviceContext) { super(serviceContext); } @@ -66,6 +70,7 @@ public void map(GetOperationAsyncStatusRequest apiRequest, GetOperationAsyncStat } } catch (SerializationException e) { + LOGGER.warn("error serializing response", e); HttpHelper.send(httpResponse, StatusCode.SERVER_INTERNAL_ERROR, Result.builder() diff --git a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/ResponseWithPayloadResponseMapper.java b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/ResponseWithPayloadResponseMapper.java index 09a56b02c..bc2be1f27 100644 --- a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/ResponseWithPayloadResponseMapper.java +++ b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/response/mapper/ResponseWithPayloadResponseMapper.java @@ -26,6 +26,8 @@ import de.fraunhofer.iosb.ilt.faaast.service.model.api.request.AbstractRequestWithModifier; import de.fraunhofer.iosb.ilt.faaast.service.model.api.response.AbstractResponseWithPayload; import jakarta.servlet.http.HttpServletResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** @@ -36,6 +38,8 @@ */ public class ResponseWithPayloadResponseMapper> extends AbstractResponseMapper { + private static final Logger LOGGER = LoggerFactory.getLogger(ResponseWithPayloadResponseMapper.class); + public ResponseWithPayloadResponseMapper(ServiceContext serviceContext) { super(serviceContext); } @@ -53,6 +57,7 @@ public void map(U apiRequest, T apiResponse, HttpServletResponse httpResponse) { : OutputModifier.DEFAULT)); } catch (SerializationException e) { + LOGGER.warn("error serializing response", e); HttpHelper.send( httpResponse, StatusCode.SERVER_INTERNAL_ERROR, diff --git a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/util/HttpHelper.java b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/util/HttpHelper.java index 0f1f5d3d6..ee0ebc6ba 100644 --- a/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/util/HttpHelper.java +++ b/endpoint/http/src/main/java/de/fraunhofer/iosb/ilt/faaast/service/endpoint/http/util/HttpHelper.java @@ -147,6 +147,7 @@ public static void send(HttpServletResponse response, StatusCode statusCode, Res sendJson(response, statusCode, new HttpJsonApiSerializer().write(result)); } catch (SerializationException e) { + LOGGER.warn("error serializing response", e); sendContent(response, StatusCode.SERVER_INTERNAL_ERROR, null, null); } }