Skip to content

Commit

Permalink
fix GDay value serialization (#879)
Browse files Browse the repository at this point in the history
* fix value only serialization error with several DateTime types
* add GDay unit test
* improve logging
* update changelog

---------

Co-authored-by: Michael Jacoby <[email protected]>
  • Loading branch information
tbischoff2 and mjacoby authored Sep 13, 2024
1 parent 7f5bae7 commit 17cc024
Show file tree
Hide file tree
Showing 12 changed files with 82 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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();

}
Original file line number Diff line number Diff line change
Expand Up @@ -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<SubmodelElement, File> data = Map.of(ValueOnlyExamples.PROPERTY_STRING, ValueOnlyExamples.PROPERTY_STRING_FILE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"propGDay": "---15"
}
5 changes: 4 additions & 1 deletion docs/source/other/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;


/**
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;


/**
Expand All @@ -35,6 +37,8 @@
*/
public abstract class AbstractPostResponseWithLocationHeaderMapper<T extends AbstractResponseWithPayload, U extends Request<T>> extends ResponseWithPayloadResponseMapper<T, U> {

private static final Logger LOGGER = LoggerFactory.getLogger(AbstractPostResponseWithLocationHeaderMapper.class);

protected AbstractPostResponseWithLocationHeaderMapper(ServiceContext serviceContext) {
super(serviceContext);
}
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;


/**
Expand All @@ -33,6 +35,8 @@
*/
public class GenerateSerializationByIdsResponseMapper extends AbstractResponseMapper<GenerateSerializationByIdsResponse, GenerateSerializationByIdsRequest> {

private static final Logger LOGGER = LoggerFactory.getLogger(GenerateSerializationByIdsResponseMapper.class);

public GenerateSerializationByIdsResponseMapper(ServiceContext serviceContext) {
super(serviceContext);
}
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,17 @@
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;


/**
* HTTP response mapper for {@link GetOperationAsyncStatusResponse}.
*/
public class GetOperationAsyncStatusResponseMapper extends AbstractResponseMapper<GetOperationAsyncStatusResponse, GetOperationAsyncStatusRequest> {

private static final Logger LOGGER = LoggerFactory.getLogger(GetOperationAsyncStatusResponseMapper.class);

public GetOperationAsyncStatusResponseMapper(ServiceContext serviceContext) {
super(serviceContext);
}
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;


/**
Expand All @@ -36,6 +38,8 @@
*/
public class ResponseWithPayloadResponseMapper<T extends AbstractResponseWithPayload, U extends Request<T>> extends AbstractResponseMapper<T, U> {

private static final Logger LOGGER = LoggerFactory.getLogger(ResponseWithPayloadResponseMapper.class);

public ResponseWithPayloadResponseMapper(ServiceContext serviceContext) {
super(serviceContext);
}
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down

0 comments on commit 17cc024

Please sign in to comment.