From 65c0a278c0e9c824cf25a2c54fef5d92f32386f3 Mon Sep 17 00:00:00 2001 From: Alan Richardson Date: Fri, 31 Jul 2026 09:07:12 +0100 Subject: [PATCH] Add strict declared type validation --- .../EntityPatchDocumentMapper.java | 41 +- .../apihandlers/ThingBodyCommandMapper.java | 9 + .../api/http/bodyparser/ApiBodyFields.java | 73 ++-- .../api/http/bodyparser/BodyParser.java | 120 ++---- .../bodyparser/JsonBodyValueConverter.java | 70 ++++ .../application/WriteValidationPolicy.java | 39 +- .../application/command/BodyFieldValue.java | 3 + .../api/http/bodyparser/BodyParserTest.java | 84 +++++ .../ThingifierHttpApiRequestHandlingTest.java | 131 +++++++ .../application/ThingCommandServiceTest.java | 351 +++++++++++++++++- 10 files changed, 772 insertions(+), 149 deletions(-) create mode 100644 thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/JsonBodyValueConverter.java diff --git a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/EntityPatchDocumentMapper.java b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/EntityPatchDocumentMapper.java index c701e6e2..e2f01a03 100644 --- a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/EntityPatchDocumentMapper.java +++ b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/EntityPatchDocumentMapper.java @@ -2,7 +2,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; import com.fasterxml.jackson.databind.node.ObjectNode; import com.flipkart.zjsonpatch.JsonPatch; import java.util.Map; @@ -11,14 +11,13 @@ import uk.co.compendiumdev.thingifier.api.ermodelconversion.JsonThing; import uk.co.compendiumdev.thingifier.api.http.ThingifierRequestContext; import uk.co.compendiumdev.thingifier.api.http.bodyparser.ApiBodyFields; +import uk.co.compendiumdev.thingifier.api.http.bodyparser.JsonBodyValueConverter; import uk.co.compendiumdev.thingifier.apiconfig.EntityPatchUpdateStyle; import uk.co.compendiumdev.thingifier.core.domain.definitions.EntityDefinition; import uk.co.compendiumdev.thingifier.core.domain.instances.EntityInstance; public final class EntityPatchDocumentMapper { - private static final ObjectMapper JSON = new ObjectMapper(); - private final ThingifierApiRuntime runtime; private final ThingWriteRequestMapper writeMapper; @@ -71,7 +70,7 @@ private ThingWriteRequestMapping mapJsonMergePatch( JsonNode patchDocument; try { - patchDocument = JSON.readTree(rawBody); + patchDocument = JsonBodyValueConverter.readTree(rawBody); } catch (JsonProcessingException e) { return malformedPatch("Malformed JSON Merge Patch document"); } @@ -95,7 +94,7 @@ private ThingWriteRequestMapping mapJsonPatch( JsonNode patchDocument; try { - patchDocument = JSON.readTree(rawBody); + patchDocument = JsonBodyValueConverter.readTree(rawBody); } catch (JsonProcessingException e) { return malformedPatch("Malformed JSON Patch document"); } @@ -122,19 +121,16 @@ private JsonNode applyMergePatch(final JsonNode target, final JsonNode patch) { ObjectNode result = target != null && target.isObject() ? ((ObjectNode) target).deepCopy() - : JSON.createObjectNode(); - patch.fields() - .forEachRemaining( - entry -> { - if (entry.getValue().isNull()) { - result.remove(entry.getKey()); - } else { - result.set( - entry.getKey(), - applyMergePatch( - result.get(entry.getKey()), entry.getValue())); - } - }); + : JsonNodeFactory.instance.objectNode(); + for (Map.Entry entry : patch.properties()) { + if (entry.getValue().isNull()) { + result.remove(entry.getKey()); + } else { + result.set( + entry.getKey(), + applyMergePatch(result.get(entry.getKey()), entry.getValue())); + } + } return result; } @@ -145,12 +141,13 @@ private ThingWriteRequestMapping mapReplacement( } return writeMapper.mapPatchReplacingFields( - route, ApiBodyFields.fromMap(JSON.convertValue(patchedDocument, Map.class))); + route, + ApiBodyFields.fromMap(JsonBodyValueConverter.objectNodeAsMap(patchedDocument))); } private JsonNode jsonFor(final EntityInstance instance) { try { - return JSON.readTree( + return JsonBodyValueConverter.readTree( new JsonThing(runtime.apiConfig().jsonOutput()) .asJsonObject(instance) .toString()); @@ -190,7 +187,7 @@ private ParseResult parseJsonObject(final String rawBody, final boolean allowEmp JsonNode document; try { - document = JSON.readTree(body); + document = JsonBodyValueConverter.readTree(body); } catch (JsonProcessingException e) { return ParseResult.error(malformedPatch("Malformed JSON document")); } @@ -201,7 +198,7 @@ private ParseResult parseJsonObject(final String rawBody, final boolean allowEmp } return ParseResult.bodyFields( - ApiBodyFields.fromMap(JSON.convertValue(document, Map.class))); + ApiBodyFields.fromMap(JsonBodyValueConverter.objectNodeAsMap(document))); } private ThingWriteRequestMapping malformedPatch(final String message) { diff --git a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/ThingBodyCommandMapper.java b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/ThingBodyCommandMapper.java index d78abab7..c4b8cb5a 100644 --- a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/ThingBodyCommandMapper.java +++ b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/adapter/http/apihandlers/ThingBodyCommandMapper.java @@ -182,6 +182,15 @@ private BodyFieldValue.SourceType sourceTypeFor(final String sourceType) { if ("NUMERIC".equals(sourceType)) { return BodyFieldValue.SourceType.NUMERIC; } + if ("OBJECT".equals(sourceType)) { + return BodyFieldValue.SourceType.OBJECT; + } + if ("ARRAY".equals(sourceType)) { + return BodyFieldValue.SourceType.ARRAY; + } + if ("NULL".equals(sourceType)) { + return BodyFieldValue.SourceType.NULL; + } return BodyFieldValue.SourceType.SOMETHING_ELSE; } } diff --git a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/ApiBodyFields.java b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/ApiBodyFields.java index 407077fd..d00a7399 100644 --- a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/ApiBodyFields.java +++ b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/ApiBodyFields.java @@ -1,5 +1,7 @@ package uk.co.compendiumdev.thingifier.api.http.bodyparser; +import java.math.BigDecimal; +import java.math.BigInteger; import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collections; @@ -35,16 +37,8 @@ public Map asStringMap() { for (String key : fields.keySet()) { Object value = fields.get(key); - if (value instanceof Boolean) { - stringsInMap.put(key, String.valueOf(value)); - } - - if (value instanceof String) { - stringsInMap.put(key, (String) value); - } - - if (value instanceof Double) { - stringsInMap.put(key, String.valueOf(value)); + if (isScalarValue(value)) { + stringsInMap.put(key, stringValue(value)); } } return stringsInMap; @@ -77,17 +71,8 @@ public List topLevelFields() { private List> flattenToStringMap( final String prefixKey, final Object value) { List> stringsInMap = new ArrayList<>(); - if (value instanceof String) { - stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixKey, (String) value)); - } - if (value instanceof Double) { - stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixKey, String.valueOf(value))); - } - if (value instanceof Boolean) { - stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixKey, String.valueOf(value))); - } - if (value instanceof Integer) { - stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixKey, String.valueOf(value))); + if (isScalarValue(value)) { + stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixKey, stringValue(value))); } String separator = ""; @@ -102,8 +87,8 @@ private List> flattenToStringMap( stringsInMap.addAll(nestedValues); } } - if (value instanceof ArrayList) { - for (Object nestedValue : (ArrayList) value) { + if (value instanceof List) { + for (Object nestedValue : (List) value) { List> nestedValues = flattenToStringMap(prefixKey + separator, nestedValue); stringsInMap.addAll(nestedValues); @@ -112,29 +97,61 @@ private List> flattenToStringMap( return stringsInMap; } - private String stringValue(final Object value) { + private static boolean isScalarValue(final Object value) { + return value instanceof String || value instanceof Boolean || value instanceof Number; + } + + private static String stringValue(final Object value) { if (value instanceof String) { return (String) value; } - if (value instanceof Boolean || value instanceof Double || value instanceof Integer) { + if (value instanceof BigDecimal) { + return ((BigDecimal) value).toPlainString(); + } + if (value instanceof Boolean || value instanceof Number) { return String.valueOf(value); } return ""; } - private String sourceTypeName(final Object value) { + public static String sourceTypeNameFor(final Object value) { + if (value == null) { + return "NULL"; + } if (value instanceof String) { return "STRING"; } if (value instanceof Boolean) { return "BOOLEAN"; } - if (value instanceof Integer) { + if (isIntegralNumber(value)) { return "INTEGER"; } - if (value instanceof Float || value instanceof Double) { + if (isDecimalNumber(value)) { return "NUMERIC"; } + if (value instanceof Map) { + return "OBJECT"; + } + if (value instanceof List) { + return "ARRAY"; + } return "Something Else"; } + + private static boolean isIntegralNumber(final Object value) { + return value instanceof Byte + || value instanceof Short + || value instanceof Integer + || value instanceof Long + || value instanceof BigInteger; + } + + private static boolean isDecimalNumber(final Object value) { + return value instanceof Float || value instanceof Double || value instanceof BigDecimal; + } + + private String sourceTypeName(final Object value) { + return sourceTypeNameFor(value); + } } diff --git a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParser.java b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParser.java index ec57a205..866448dc 100644 --- a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParser.java +++ b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParser.java @@ -1,6 +1,6 @@ package uk.co.compendiumdev.thingifier.api.http.bodyparser; -import com.google.gson.Gson; +import com.fasterxml.jackson.core.JsonProcessingException; import java.util.*; import uk.co.compendiumdev.thingifier.api.http.HttpApiRequest; import uk.co.compendiumdev.thingifier.api.http.bodyparser.xml.XMLParserAbstraction; @@ -33,77 +33,21 @@ public Map getStringMap() { private Map stringMap(final Map args) { // todo: configuration to reject if wrong types for field definitions // default should be to handle and convert - Map stringsInMap = new HashMap<>(); - for (String key : args.keySet()) { - Object theValue = args.get(key); - - if (theValue instanceof Boolean) { - stringsInMap.put(key, String.valueOf(theValue)); - } - - if (theValue instanceof String) { - stringsInMap.put(key, (String) theValue); - } - - if (theValue instanceof Double) { - stringsInMap.put(key, String.valueOf(theValue)); - } - } - return stringsInMap; + return ApiBodyFields.fromMap(args).asStringMap(); } // since complex keys can be duplicated, // we can't use a hashmap, so we are using a list of map entries // the map entries could be a custom Key Value Pair implementation if we wanted public List> getFlattenedStringMap() { - return flattenToStringMap("", getMap()); - } - - private List> flattenToStringMap( - final String prefixkey, final Object theValue) { - // todo: configuration to reject if wrong types for field definitions - // default should be to handle and convert - List> stringsInMap = new ArrayList<>(); - if (theValue instanceof String) { - stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixkey, (String) theValue)); - } - if (theValue instanceof Double) { - stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixkey, String.valueOf(theValue))); - } - if (theValue instanceof Boolean) { - stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixkey, String.valueOf(theValue))); - } - if (theValue instanceof Integer) { - stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixkey, String.valueOf(theValue))); - } - // todo: what else can come in? - String separator = ""; - if (prefixkey != null && !prefixkey.isEmpty() && !prefixkey.endsWith(".")) { - separator = "."; - } - if (theValue instanceof Map) { - for (Map.Entry entry : ((Map) theValue).entrySet()) { - String key = entry.getKey(); - Object aValue = entry.getValue(); - List> nestedValues = - flattenToStringMap(prefixkey + separator + key, aValue); - stringsInMap.addAll(nestedValues); - } - } - if (theValue instanceof ArrayList) { - for (Object aValue : (ArrayList) theValue) { - List> nestedValues = - flattenToStringMap(prefixkey + separator, aValue); - stringsInMap.addAll(nestedValues); - } - } - return stringsInMap; + return ApiBodyFields.fromMap(getMap()).asFlattenedStringMap(); } public List getObjectNames() { + parseMap(); List objectOrCollectionNames = new ArrayList<>(); for (String key : args.keySet()) { - if (!(args.get(key) instanceof String || args.get(key) instanceof Double)) { + if (!isScalarValue(args.get(key))) { objectOrCollectionNames.add(key); } } @@ -141,7 +85,7 @@ public String validBodyBasedOnContentType() { if (contentTypeParser.isJSON()) { try { - new Gson().fromJson(request.getBody(), Map.class); + JsonBodyValueConverter.readTree(request.getBody()); return ""; } catch (Exception e) { // Gson does not give a sensible parse error so use a generic description @@ -160,7 +104,8 @@ public void parseMap() { return; } - if (request.getBody().trim().isEmpty()) { + String body = request.getBody() == null ? "" : request.getBody(); + if (body.trim().isEmpty()) { args = new HashMap<>(); return; } @@ -182,7 +127,11 @@ public void parseMap() { args = this.xmlParser.xmlAsMap(); } else { // assume it is json - args = new Gson().fromJson(request.getBody(), Map.class); + try { + args = JsonBodyValueConverter.jsonObjectAsMap(request.getBody()); + } catch (JsonProcessingException e) { + throw new IllegalArgumentException("Invalid JSON Payload", e); + } } if (args == null) { @@ -211,22 +160,7 @@ public ValidationReport validateAgainstTypeIgnoring( } Object theValue = arg.getValue(); - String isInstanceType = "Something Else"; - if (theValue instanceof String) { - isInstanceType = "STRING"; - } - if (theValue instanceof Boolean) { - isInstanceType = "BOOLEAN"; - } - if (theValue instanceof Integer) { - isInstanceType = "INTEGER"; - } - if (theValue instanceof Float) { - isInstanceType = "NUMERIC"; - } - if (theValue instanceof Double) { - isInstanceType = "NUMERIC"; - } + String isInstanceType = ApiBodyFields.sourceTypeNameFor(theValue); // TODO: add " but was %s" e.g. should be BOOLEAN but was STRING - remember to change in // challenges checking @@ -236,29 +170,41 @@ public ValidationReport validateAgainstTypeIgnoring( field.getName(), field.getType(), isInstanceType); if (field.getType() == FieldType.BOOLEAN) { - if (!(theValue instanceof Boolean)) { + if (!isInstanceType.equals("BOOLEAN")) { report.setValid(false); report.addErrorMessage(errorMessage); } } if (field.getType() == FieldType.INTEGER || field.getType() == FieldType.AUTO_INCREMENT) { - if (!(theValue instanceof Double)) { + if (!isInstanceType.equals("INTEGER")) { report.setValid(false); report.addErrorMessage(errorMessage); - } else { - // enforce an int - arg.setValue(((Double) theValue).intValue()); } } if (field.getType() == FieldType.FLOAT) { - if (!(theValue instanceof Double)) { + if (!(isInstanceType.equals("INTEGER") || isInstanceType.equals("NUMERIC"))) { report.setValid(false); report.addErrorMessage(errorMessage); } } - // everything else goes + if ((field.getType() == FieldType.STRING + || field.getType() == FieldType.ENUM + || field.getType() == FieldType.DATE + || field.getType() == FieldType.AUTO_GUID) + && !isInstanceType.equals("STRING")) { + report.setValid(false); + report.addErrorMessage(errorMessage); + } + if (field.getType() == FieldType.OBJECT && !isInstanceType.equals("OBJECT")) { + report.setValid(false); + report.addErrorMessage(errorMessage); + } } return report; } + + private boolean isScalarValue(final Object value) { + return value instanceof String || value instanceof Boolean || value instanceof Number; + } } diff --git a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/JsonBodyValueConverter.java b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/JsonBodyValueConverter.java new file mode 100644 index 00000000..98b05fac --- /dev/null +++ b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/JsonBodyValueConverter.java @@ -0,0 +1,70 @@ +package uk.co.compendiumdev.thingifier.api.http.bodyparser; + +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.json.JsonReadFeature; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +public final class JsonBodyValueConverter { + + private static final ObjectMapper JSON = + new ObjectMapper( + JsonFactory.builder().enable(JsonReadFeature.ALLOW_SINGLE_QUOTES).build()); + + private JsonBodyValueConverter() {} + + public static JsonNode readTree(final String body) throws JsonProcessingException { + return JSON.readTree(body); + } + + public static Map jsonObjectAsMap(final String body) + throws JsonProcessingException { + return objectNodeAsMap(readTree(body)); + } + + public static Map objectNodeAsMap(final JsonNode node) { + if (node == null || !node.isObject()) { + throw new IllegalArgumentException("JSON document must be an object"); + } + + Map values = new LinkedHashMap<>(); + for (Map.Entry field : node.properties()) { + values.put(field.getKey(), valueFrom(field.getValue())); + } + return values; + } + + private static Object valueFrom(final JsonNode node) { + if (node == null || node.isNull()) { + return null; + } + if (node.isTextual()) { + return node.asText(); + } + if (node.isBoolean()) { + return Boolean.valueOf(node.asBoolean()); + } + if (node.isIntegralNumber()) { + return node.bigIntegerValue(); + } + if (node.isNumber()) { + return node.decimalValue(); + } + if (node.isObject()) { + return objectNodeAsMap(node); + } + if (node.isArray()) { + List values = new ArrayList<>(); + for (JsonNode item : node) { + values.add(valueFrom(item)); + } + return values; + } + return node.asText(); + } +} diff --git a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/application/WriteValidationPolicy.java b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/application/WriteValidationPolicy.java index dd5bbbdd..e27d282d 100644 --- a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/application/WriteValidationPolicy.java +++ b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/application/WriteValidationPolicy.java @@ -139,17 +139,7 @@ private ThingCommandResult validateDeclaredFieldTypesIgnoring( "%s should be %s but was %s", field.getName(), field.getType(), fieldValue.sourceTypeDisplayName()); - if (field.getType() == FieldType.BOOLEAN - && fieldValue.getSourceType() != BodyFieldValue.SourceType.BOOLEAN) { - errors.add(errorMessage); - } - if ((field.getType() == FieldType.INTEGER - || field.getType() == FieldType.AUTO_INCREMENT) - && fieldValue.getSourceType() != BodyFieldValue.SourceType.NUMERIC) { - errors.add(errorMessage); - } - if (field.getType() == FieldType.FLOAT - && fieldValue.getSourceType() != BodyFieldValue.SourceType.NUMERIC) { + if (!sourceTypeAllowedFor(field.getType(), fieldValue.getSourceType())) { errors.add(errorMessage); } } @@ -160,6 +150,33 @@ private ThingCommandResult validateDeclaredFieldTypesIgnoring( return ThingCommandResult.error(String.join(", ", errors)); } + private boolean sourceTypeAllowedFor( + final FieldType fieldType, final BodyFieldValue.SourceType sourceType) { + if (sourceType == null) { + return false; + } + + switch (fieldType) { + case STRING: + case ENUM: + case DATE: + case AUTO_GUID: + return sourceType == BodyFieldValue.SourceType.STRING; + case BOOLEAN: + return sourceType == BodyFieldValue.SourceType.BOOLEAN; + case INTEGER: + case AUTO_INCREMENT: + return sourceType == BodyFieldValue.SourceType.INTEGER; + case FLOAT: + return sourceType == BodyFieldValue.SourceType.INTEGER + || sourceType == BodyFieldValue.SourceType.NUMERIC; + case OBJECT: + return sourceType == BodyFieldValue.SourceType.OBJECT; + default: + return false; + } + } + private ThingCommandResult duplicateProtectedFieldError( final EntityDefinition entity, final List fieldValues, diff --git a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/application/command/BodyFieldValue.java b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/application/command/BodyFieldValue.java index cb9c7844..1023afa4 100644 --- a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/application/command/BodyFieldValue.java +++ b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/application/command/BodyFieldValue.java @@ -12,6 +12,9 @@ public enum SourceType { BOOLEAN("BOOLEAN"), INTEGER("INTEGER"), NUMERIC("NUMERIC"), + OBJECT("OBJECT"), + ARRAY("ARRAY"), + NULL("NULL"), SOMETHING_ELSE("Something Else"); private final String displayName; diff --git a/thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParserTest.java b/thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParserTest.java index 4bef351c..1f748bd4 100644 --- a/thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParserTest.java +++ b/thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParserTest.java @@ -24,6 +24,72 @@ public void simpleJsonParse() { Assertions.assertEquals("5", map.get("duration")); } + @Test + public void jsonBodyFieldsPreserveTopLevelSourceTypes() { + HttpApiRequest request = new HttpApiRequest("/items"); + request.setBody( + "{" + + "\"text\":\"hello\"," + + "\"flag\":true," + + "\"whole\":2," + + "\"decimal\":2.0," + + "\"object\":{\"child\":\"value\"}," + + "\"array\":[1,\"two\"]," + + "\"nothing\":null" + + "}"); + + ApiBodyFields fields = new BodyParser(request, List.of("item", "items")).bodyFields(); + + Assertions.assertEquals("STRING", sourceType(fields, "text")); + Assertions.assertEquals("BOOLEAN", sourceType(fields, "flag")); + Assertions.assertEquals("INTEGER", sourceType(fields, "whole")); + Assertions.assertEquals("NUMERIC", sourceType(fields, "decimal")); + Assertions.assertEquals("OBJECT", sourceType(fields, "object")); + Assertions.assertEquals("ARRAY", sourceType(fields, "array")); + Assertions.assertEquals("NULL", sourceType(fields, "nothing")); + Assertions.assertEquals("2", value(fields, "whole")); + Assertions.assertEquals("2.0", value(fields, "decimal")); + } + + @Test + public void nestedJsonValuesStillFlattenToStrings() { + HttpApiRequest request = new HttpApiRequest("/items"); + request.setBody( + "{" + + "\"relationships\":{\"project\":{\"id\":2,\"weight\":2.5,\"guid\":\"p1\"}}," + + "\"metadata\":{\"source\":\"api\"}" + + "}"); + + ApiBodyFields fields = new BodyParser(request, List.of("item", "items")).bodyFields(); + List> flattened = fields.asFlattenedStringMap(); + + Assertions.assertEquals("OBJECT", sourceType(fields, "relationships")); + Assertions.assertTrue( + flattened.stream() + .anyMatch( + entry -> + "relationships.project.id".equals(entry.getKey()) + && "2".equals(entry.getValue()))); + Assertions.assertTrue( + flattened.stream() + .anyMatch( + entry -> + "relationships.project.weight".equals(entry.getKey()) + && "2.5".equals(entry.getValue()))); + Assertions.assertTrue( + flattened.stream() + .anyMatch( + entry -> + "relationships.project.guid".equals(entry.getKey()) + && "p1".equals(entry.getValue()))); + Assertions.assertTrue( + flattened.stream() + .anyMatch( + entry -> + "metadata.source".equals(entry.getKey()) + && "api".equals(entry.getValue()))); + } + @Test public void simpleJsonParseErrorMessage() { @@ -274,4 +340,22 @@ public void embeddedCollectionOfObjectsFromXML() { // estimate is a LinkedTreeMap of LinkedTreeMap "to do" of ArrayList of LinkedTreeMap } + + private String sourceType(final ApiBodyFields fields, final String name) { + for (ApiBodyField field : fields.topLevelFields()) { + if (field.name().equals(name)) { + return field.sourceType(); + } + } + return ""; + } + + private String value(final ApiBodyFields fields, final String name) { + for (ApiBodyField field : fields.topLevelFields()) { + if (field.name().equals(name)) { + return field.value(); + } + } + return ""; + } } diff --git a/thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/http/requests/ThingifierHttpApiRequestHandlingTest.java b/thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/http/requests/ThingifierHttpApiRequestHandlingTest.java index 003ee0d2..687b4868 100644 --- a/thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/http/requests/ThingifierHttpApiRequestHandlingTest.java +++ b/thingifier/src/test/java/uk/co/compendiumdev/thingifier/api/http/requests/ThingifierHttpApiRequestHandlingTest.java @@ -8,6 +8,7 @@ import uk.co.compendiumdev.thingifier.api.http.HttpApiRequest; import uk.co.compendiumdev.thingifier.api.http.HttpApiResponse; import uk.co.compendiumdev.thingifier.api.http.ThingifierHttpApi; +import uk.co.compendiumdev.thingifier.apiconfig.EntityPatchUpdateStyle; import uk.co.compendiumdev.thingifier.core.EntityRelModel; import uk.co.compendiumdev.thingifier.core.domain.datapopulator.RepositoryDataPopulator; import uk.co.compendiumdev.thingifier.core.domain.definitions.EntityDefinition; @@ -140,6 +141,111 @@ public void aPostRequestWillCreateNewSessionWithDatabase() { .getDefinitionWithSingularOrPluralNamed("thing"))); } + @Test + public void strictPostRejectsNumericValueForStringField() { + Thingifier thingifier = strictTypedThingifier(); + ThingifierHttpApi api = new ThingifierHttpApi(thingifier, null, null); + + HttpApiResponse response = api.post(jsonRequest("/things", "{\"title\":2}")); + + Assertions.assertEquals(422, response.getStatusCode()); + Assertions.assertTrue( + response.getBody().contains("title should be STRING but was INTEGER")); + Assertions.assertEquals( + 0, + thingifier + .getStore(EntityRelModel.DEFAULT_DATABASE_NAME) + .entityQueries() + .count(thingifier.getDefinitionNamed("thing"))); + } + + @Test + public void strictPostRejectsDecimalForIntegerAndAcceptsIntegerForFloat() { + Thingifier thingifier = strictTypedThingifier(); + ThingifierHttpApi api = new ThingifierHttpApi(thingifier, null, null); + + HttpApiResponse decimalInteger = api.post(jsonRequest("/things", "{\"priority\":2.0}")); + HttpApiResponse integerFloat = api.post(jsonRequest("/things", "{\"amount\":2}")); + + Assertions.assertEquals(422, decimalInteger.getStatusCode()); + Assertions.assertTrue( + decimalInteger.getBody().contains("priority should be INTEGER but was NUMERIC")); + Assertions.assertEquals(201, integerFloat.getStatusCode()); + Assertions.assertEquals( + 1, + thingifier + .getStore(EntityRelModel.DEFAULT_DATABASE_NAME) + .entityQueries() + .count(thingifier.getDefinitionNamed("thing"))); + } + + @Test + public void lenientPostStillAcceptsNumericValueForStringField() { + Thingifier thingifier = strictTypedThingifier(); + thingifier.apiConfig().setApiToEnforceDeclaredTypesInInput(false); + ThingifierHttpApi api = new ThingifierHttpApi(thingifier, null, null); + + HttpApiResponse response = api.post(jsonRequest("/things", "{\"title\":2}")); + + Assertions.assertEquals(201, response.getStatusCode()); + Assertions.assertEquals( + "2", + response.apiResponse().getReturnedInstance().getFieldValue("title").asString()); + } + + @Test + public void strictPutRejectsNumericValueForStringField() { + Thingifier thingifier = strictTypedThingifier(); + ThingifierHttpApi api = new ThingifierHttpApi(thingifier, null, null); + EntityInstance existing = createThing(thingifier, "Original"); + + HttpApiResponse response = + api.put(jsonRequest("/things/" + existing.getPrimaryKeyValue(), "{\"title\":2}")); + + Assertions.assertEquals(422, response.getStatusCode()); + Assertions.assertTrue( + response.getBody().contains("title should be STRING but was INTEGER")); + Assertions.assertEquals( + "Original", + thingifier + .getStore(EntityRelModel.DEFAULT_DATABASE_NAME) + .entityQueries() + .findByQueryIdentifier( + thingifier.getDefinitionNamed("thing"), + existing.getPrimaryKeyValue()) + .getFieldValue("title") + .asString()); + } + + @Test + public void strictPatchRejectsNumericValueForStringField() { + Thingifier thingifier = strictTypedThingifier(); + thingifier + .apiConfig() + .writeMethods() + .entities() + .patchCan(EntityPatchUpdateStyle.PARTIAL_JSON_UPDATE); + ThingifierHttpApi api = new ThingifierHttpApi(thingifier, null, null); + EntityInstance existing = createThing(thingifier, "Original"); + + HttpApiResponse response = + api.patch(jsonRequest("/things/" + existing.getPrimaryKeyValue(), "{\"title\":2}")); + + Assertions.assertEquals(422, response.getStatusCode()); + Assertions.assertTrue( + response.getBody().contains("title should be STRING but was INTEGER")); + Assertions.assertEquals( + "Original", + thingifier + .getStore(EntityRelModel.DEFAULT_DATABASE_NAME) + .entityQueries() + .findByQueryIdentifier( + thingifier.getDefinitionNamed("thing"), + existing.getPrimaryKeyValue()) + .getFieldValue("title") + .asString()); + } + @Test public void aDeleteRequestWillCreateNewSessionWithDatabase() { @@ -191,4 +297,29 @@ public void aDeleteRequestWillCreateNewSessionWithDatabase() { // OPTIONS // etc. + + private Thingifier strictTypedThingifier() { + Thingifier thingifier = new Thingifier(); + thingifier.apiConfig().setApiToEnforceAcceptHeaderForResponses(false); + EntityDefinition defn = thingifier.getERmodel().createEntityDefinition("thing", "things"); + defn.addAsPrimaryKeyField(Field.is("guid", FieldType.AUTO_GUID)); + defn.addField(Field.is("title", FieldType.STRING)); + defn.addField(Field.is("priority", FieldType.INTEGER)); + defn.addField(Field.is("amount", FieldType.FLOAT)); + return thingifier; + } + + private EntityInstance createThing(final Thingifier thingifier, final String title) { + EntityDefinition thing = thingifier.getDefinitionNamed("thing"); + return thingifier + .getStore(EntityRelModel.DEFAULT_DATABASE_NAME) + .entities() + .create(EntityInstanceDraft.forEntity(thing).withField("title", title)); + } + + private HttpApiRequest jsonRequest(final String path, final String body) { + return new HttpApiRequest(path) + .setHeaders(Map.of("content-type", "application/json")) + .setBody(body); + } } diff --git a/thingifier/src/test/java/uk/co/compendiumdev/thingifier/application/ThingCommandServiceTest.java b/thingifier/src/test/java/uk/co/compendiumdev/thingifier/application/ThingCommandServiceTest.java index cbf23ce4..71461117 100644 --- a/thingifier/src/test/java/uk/co/compendiumdev/thingifier/application/ThingCommandServiceTest.java +++ b/thingifier/src/test/java/uk/co/compendiumdev/thingifier/application/ThingCommandServiceTest.java @@ -265,6 +265,305 @@ public void createCommandValidatesDeclaredBodyTypesInApplication() { 0, store.entityQueries().count(thingifier.getDefinitionNamed("todo"))); } + @Test + public void createCommandRejectsNonStringSourceTypesForStringFieldsWhenStrict() { + Thingifier thingifier = declaredTypesThingifier(); + ThingStore store = storeFor(thingifier); + + ThingCommandResult integerResult = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("text", "2"), + List.of( + bodyField( + "text", + "2", + BodyFieldValue.SourceType.INTEGER)), + List.of(), + true)); + ThingCommandResult numericResult = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("text", "2.5"), + List.of( + bodyField( + "text", + "2.5", + BodyFieldValue.SourceType.NUMERIC)), + List.of(), + true)); + ThingCommandResult booleanResult = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("text", "true"), + List.of( + bodyField( + "text", + "true", + BodyFieldValue.SourceType.BOOLEAN)), + List.of(), + true)); + ThingCommandResult nullResult = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + List.of(), + List.of( + bodyField( + "text", + "", + BodyFieldValue.SourceType.NULL)), + List.of(), + true)); + + Assertions.assertEquals( + List.of("text should be STRING but was INTEGER"), integerResult.getErrorMessages()); + Assertions.assertEquals( + List.of("text should be STRING but was NUMERIC"), numericResult.getErrorMessages()); + Assertions.assertEquals( + List.of("text should be STRING but was BOOLEAN"), booleanResult.getErrorMessages()); + Assertions.assertEquals( + List.of("text should be STRING but was NULL"), nullResult.getErrorMessages()); + Assertions.assertEquals( + 0, store.entityQueries().count(thingifier.getDefinitionNamed("sample"))); + } + + @Test + public void createCommandDistinguishesIntegerAndNumericSourcesWhenStrict() { + Thingifier thingifier = declaredTypesThingifier(); + ThingStore store = storeFor(thingifier); + + ThingCommandResult integerResult = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("count", "2"), + List.of( + bodyField( + "count", + "2", + BodyFieldValue.SourceType.INTEGER)), + List.of(), + true)); + ThingCommandResult decimalIntegerResult = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("count", "2.0"), + List.of( + bodyField( + "count", + "2.0", + BodyFieldValue.SourceType.NUMERIC)), + List.of(), + true)); + ThingCommandResult integerFloatResult = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("amount", "2"), + List.of( + bodyField( + "amount", + "2", + BodyFieldValue.SourceType.INTEGER)), + List.of(), + true)); + ThingCommandResult numericFloatResult = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("amount", "2.5"), + List.of( + bodyField( + "amount", + "2.5", + BodyFieldValue.SourceType.NUMERIC)), + List.of(), + true)); + + Assertions.assertTrue(integerResult.isSuccessful()); + Assertions.assertEquals("2", integerResult.getInstance().getFieldValue("count").asString()); + Assertions.assertEquals( + List.of("count should be INTEGER but was NUMERIC"), + decimalIntegerResult.getErrorMessages()); + Assertions.assertTrue(integerFloatResult.isSuccessful()); + Assertions.assertEquals( + "2.0", integerFloatResult.getInstance().getFieldValue("amount").asString()); + Assertions.assertTrue(numericFloatResult.isSuccessful()); + Assertions.assertEquals( + "2.5", numericFloatResult.getInstance().getFieldValue("amount").asString()); + } + + @Test + public void createCommandStrictlyValidatesBooleanEnumDateAndObjectSources() { + Thingifier thingifier = declaredTypesThingifier(); + ThingStore store = storeFor(thingifier); + + ThingCommandResult booleanString = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("flag", "true"), + List.of( + bodyField( + "flag", + "true", + BodyFieldValue.SourceType.STRING)), + List.of(), + true)); + ThingCommandResult booleanValue = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("flag", "true"), + List.of( + bodyField( + "flag", + "true", + BodyFieldValue.SourceType.BOOLEAN)), + List.of(), + true)); + ThingCommandResult stringBackedFields = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + List.of( + new NamedValue("status", "NEW"), + new NamedValue("date", "2026-07-31")), + List.of( + bodyField( + "status", + "NEW", + BodyFieldValue.SourceType.STRING), + bodyField( + "date", + "2026-07-31", + BodyFieldValue.SourceType.STRING)), + List.of(), + true)); + ThingCommandResult enumNumeric = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("status", "2"), + List.of( + bodyField( + "status", + "2", + BodyFieldValue.SourceType.INTEGER)), + List.of(), + true)); + ThingCommandResult objectValue = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + List.of(), + List.of( + bodyField( + "metadata", + "", + BodyFieldValue.SourceType.OBJECT)), + List.of(), + true)); + ThingCommandResult objectString = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("metadata", "not an object"), + List.of( + bodyField( + "metadata", + "not an object", + BodyFieldValue.SourceType.STRING)), + List.of(), + true)); + + Assertions.assertEquals( + List.of("flag should be BOOLEAN but was STRING"), booleanString.getErrorMessages()); + Assertions.assertTrue(booleanValue.isSuccessful()); + Assertions.assertTrue(stringBackedFields.isSuccessful()); + Assertions.assertEquals( + List.of("status should be ENUM but was INTEGER"), enumNumeric.getErrorMessages()); + Assertions.assertTrue(objectValue.isSuccessful()); + Assertions.assertEquals( + List.of("metadata should be OBJECT but was STRING"), + objectString.getErrorMessages()); + } + + @Test + public void createCommandStrictlyValidatesAutoGuidSourceTypes() { + Thingifier thingifier = autoGuidPrimaryKeyThingifier(); + ThingStore store = storeFor(thingifier); + String guid = "11111111-1111-1111-1111-111111111111"; + + ThingCommandResult stringGuid = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + guid, + List.of(), + List.of( + bodyField( + "guid", + guid, + BodyFieldValue.SourceType.STRING)), + List.of(), + true)); + ThingCommandResult numericGuid = + serviceFor(thingifier, store, true) + .execute( + new CreateThingCommand( + "sample", + "", + fields("guid", "2"), + List.of( + bodyField( + "guid", + "2", + BodyFieldValue.SourceType.INTEGER)), + List.of(), + true)); + + Assertions.assertTrue(stringGuid.isSuccessful()); + Assertions.assertEquals(guid, stringGuid.getInstance().getPrimaryKeyValue()); + Assertions.assertEquals( + List.of("guid should be AUTO_GUID but was INTEGER"), + numericGuid.getErrorMessages()); + } + @Test public void createCommandMissingMandatoryFieldReturnsValidationCategory() { Thingifier thingifier = mandatoryTitleThingifier(); @@ -306,7 +605,7 @@ public void createCommandMaxInstanceLimitReturnsConflictCategory() { } @Test - public void createCommandNormalizesNumericIntegerBodyValuesInApplication() { + public void createCommandRejectsNumericIntegerBodyValuesWhenStrict() { Thingifier thingifier = typedTodoThingifier(); ThingStore store = storeFor(thingifier); @@ -325,6 +624,33 @@ public void createCommandNormalizesNumericIntegerBodyValuesInApplication() { List.of(), true)); + Assertions.assertTrue(result.isError()); + Assertions.assertEquals( + List.of("priority should be INTEGER but was NUMERIC"), result.getErrorMessages()); + Assertions.assertEquals( + 0, store.entityQueries().count(thingifier.getDefinitionNamed("todo"))); + } + + @Test + public void createCommandNormalizesNumericIntegerBodyValuesWhenLenient() { + Thingifier thingifier = typedTodoThingifier(); + ThingStore store = storeFor(thingifier); + + ThingCommandResult result = + serviceFor(thingifier, store, false) + .execute( + new CreateThingCommand( + "todo", + "", + fields("priority", "2.0"), + List.of( + bodyField( + "priority", + "2.0", + BodyFieldValue.SourceType.NUMERIC)), + List.of(), + true)); + Assertions.assertTrue(result.isSuccessful()); Assertions.assertEquals("2", result.getInstance().getFieldValue("priority").asString()); } @@ -797,6 +1123,29 @@ private Thingifier typedTodoThingifier() { return thingifier; } + private Thingifier declaredTypesThingifier() { + Thingifier thingifier = new Thingifier(); + EntityDefinition sample = thingifier.defineThing("sample", "samples"); + sample.addField(Field.is("text", FieldType.STRING)); + sample.addField(Field.is("flag", FieldType.BOOLEAN)); + sample.addField(Field.is("count", FieldType.INTEGER)); + sample.addField(Field.is("amount", FieldType.FLOAT)); + sample.addField(Field.is("status", FieldType.ENUM).withExample("NEW")); + sample.addField(Field.is("date", FieldType.DATE)); + sample.addField( + Field.is("metadata", FieldType.OBJECT) + .withField(Field.is("source", FieldType.STRING))); + return thingifier; + } + + private Thingifier autoGuidPrimaryKeyThingifier() { + Thingifier thingifier = new Thingifier(); + EntityDefinition sample = thingifier.defineThing("sample", "samples"); + sample.addAsPrimaryKeyField(Field.is("guid", FieldType.AUTO_GUID)); + sample.addField(Field.is("text", FieldType.STRING)); + return thingifier; + } + private Thingifier mandatoryTitleThingifier() { Thingifier thingifier = new Thingifier(); EntityDefinition todo = thingifier.defineThing("todo", "todos");