fix for invalid json xml processing message, and doc output formatting

eviltester · Apr 9, 2024 · 8f71d4b · 8f71d4b
1 parent 64d933c
commit 8f71d4b
Show file tree

Hide file tree

Showing 12 changed files with 153 additions and 36 deletions.
diff --git a/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/ermodelconversion/JsonThing.java b/thingifier/src/main/java/uk/co/compendiumdev/thingifier/api/ermodelconversion/JsonThing.java
@@ -30,9 +30,28 @@ public JsonThing(final JsonOutputConfig apiConfig) {
      * @return
      */
     public String asJsonTypedArrayWithContentsUntyped(final List<EntityInstance> things, String typeName) {
+        return asJsonObjectTypedArrayWithContentsUntyped(things, typeName).toString();
+    }
+
+    /*
+    This is suitable for passing through GsonBuilderPretty Printing e.g. to get
+
+    {
+  "todos": [
+        {
+          "id": 40,
+          "title": "A title",
+          "doneStatus": false,
+          "description": "my description"
+        }
+      ]
+    }
+
+     */
+    public JsonObject asJsonObjectTypedArrayWithContentsUntyped(final List<EntityInstance> things, String typeName) {
         final JsonObject arrayObj = new JsonObject();
         arrayObj.add(typeName, asJsonArray(things));
-        return arrayObj.toString();
+        return arrayObj;
     }
 
     /**

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
@@ -38,7 +38,7 @@ public Map<String, String> getStringMap() {
     private Map<String, String> stringMap(final Map<String, Object> args) {
         // todo: configuration to reject if wrong types for field definitions
         // default should be to handle and convert
-        Map<String, String> stringsInMap = new HashMap();
+        Map<String, String> stringsInMap = new HashMap<>();
         for (String key : args.keySet()) {
             Object theValue = args.get(key);
 
@@ -61,8 +61,7 @@ private Map<String, String> stringMap(final Map<String, Object> args) {
     // 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<Map.Entry<String,String>> getFlattenedStringMap() {
-        List<Map.Entry<String,String>> stringsInMap = flattenToStringMap("", getMap());
-        return stringsInMap;
+        return flattenToStringMap("", getMap());
     }
 
 
@@ -71,20 +70,20 @@ private List<Map.Entry<String,String>> flattenToStringMap(final String prefixkey
         // default should be to handle and convert
         List<Map.Entry<String,String>> stringsInMap = new ArrayList<>();
         if (theValue instanceof String ) {
-            stringsInMap.add(new AbstractMap.SimpleEntry<String,String>(prefixkey, (String)theValue));
+            stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixkey, (String)theValue));
         }
         if(theValue instanceof Double){
-            stringsInMap.add(new AbstractMap.SimpleEntry<String,String>(prefixkey, String.valueOf(theValue)));
+            stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixkey, String.valueOf(theValue)));
         }
         if(theValue instanceof Boolean){
-            stringsInMap.add(new AbstractMap.SimpleEntry<String,String>(prefixkey, String.valueOf(theValue)));
+            stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixkey, String.valueOf(theValue)));
         }
         if(theValue instanceof Integer){
-            stringsInMap.add(new AbstractMap.SimpleEntry<String,String>(prefixkey, String.valueOf(theValue)));
+            stringsInMap.add(new AbstractMap.SimpleEntry<>(prefixkey, String.valueOf(theValue)));
         }
         // todo: what else can come in?
         String separator = "";
-        if(prefixkey!=null && prefixkey.length() > 0 && !prefixkey.endsWith(".")){
+        if(prefixkey!=null && !prefixkey.isEmpty() && !prefixkey.endsWith(".")){
             separator = ".";
         }
         if(theValue instanceof Map){
@@ -105,7 +104,7 @@ private List<Map.Entry<String,String>> flattenToStringMap(final String prefixkey
     }
 
     public List<String> getObjectNames(){
-        List<String> objectOrCollectionNames = new ArrayList();
+        List<String> objectOrCollectionNames = new ArrayList<>();
         for (String key : args.keySet()) {
             if (!(args.get(key) instanceof String || args.get(key) instanceof Double)) {
                 objectOrCollectionNames.add(key);
@@ -128,19 +127,24 @@ public String validBodyBasedOnContentType(){
         final ContentTypeHeaderParser contentTypeParser = new ContentTypeHeaderParser(request.getHeader("content-type"));
         if (contentTypeParser.isXML()) {
             String validateResultsErrorReport = this.xmlParser.validateXML();
-            if(validateResultsErrorReport.length()!=0){
-                return validateResultsErrorReport;
+            if(!validateResultsErrorReport.isEmpty()){
+                return "Invalid XML Payload: " + validateResultsErrorReport;
             }
+            return "";
         }
 
         if(contentTypeParser.isJSON()){
             try{
                 new Gson().fromJson(request.getBody(), Map.class);
+                return "";
             }catch(Exception e){
-                return e.getMessage();
+                // Gson does not give a sensible parse error so use a generic description
+                return "Invalid Json Payload: please check the syntax of the request body";
             }
         }
-        return "";
+
+
+        return "Unknown content Type: API cannot parse %s".formatted(request.getContentTypeHeader());
     }
 
     /**

diff --git a/...uk/co/compendiumdev/thingifier/api/http/headers/headerparser/ContentTypeHeaderParser.java b/...uk/co/compendiumdev/thingifier/api/http/headers/headerparser/ContentTypeHeaderParser.java
@@ -12,15 +12,16 @@ public ContentTypeHeaderParser(final String header) {
     }
 
     public boolean isXML() {
-        return header.contains("application/xml");
+        // text/xml in standard https://datatracker.ietf.org/doc/html/rfc3023
+        return header.contains("application/xml") || header.contains("text/xml");
     }
 
     public boolean isJSON() {
         return header.contains("application/json");
     }
 
     public boolean isMissing() {
-        return (header.length()==0);
+        return (header.isEmpty());
     }
 
     public boolean isText() {

diff --git a/...in/java/uk/co/compendiumdev/thingifier/htmlgui/htmlgen/RestApiDocumentationGenerator.java b/...in/java/uk/co/compendiumdev/thingifier/htmlgui/htmlgen/RestApiDocumentationGenerator.java
@@ -197,8 +197,14 @@ public String getApiDocumentation(final ApiRoutingDefinition routingDefinitions,
                     output.append("<p>Example JSON Output from API calls</p>\n");
                     output.append("<pre class='json'>\n");
                     output.append("<code class='json'>\n");
-                    output.append(new GsonBuilder().setPrettyPrinting()
-                            .create().toJson(jsonThing.asJsonObject(exampleThing.getInstance())));
+                    if(thingifier.apiConfig().willReturnSingleGetItemsAsCollection()){
+                        output.append(new GsonBuilder().setPrettyPrinting()
+                                .create().toJson(jsonThing.asJsonObjectTypedArrayWithContentsUntyped(
+                                        List.of(exampleThing.getInstance()),aThingDefinition.getPlural())));
+                    }else {
+                        output.append(new GsonBuilder().setPrettyPrinting()
+                                .create().toJson(jsonThing.asJsonObject(exampleThing.getInstance())));
+                    }
                     output.append("</code>\n");
                     output.append("</pre>\n");
                 }
@@ -207,7 +213,15 @@ public String getApiDocumentation(final ApiRoutingDefinition routingDefinitions,
                     output.append("<p>Example XML Output from API calls</p>\n");
                     output.append("<pre class='xml'>\n");
                     output.append("<code class='xml'>\n");
-                    output.append(this.XMLPrettyPrinter.prettyPrintHtml(xmlThing.getSingleObjectXml(exampleThing.getInstance())));
+                    if(thingifier.apiConfig().willReturnSingleGetItemsAsCollection()) {
+                        output.append(this.XMLPrettyPrinter.prettyPrintHtml(
+                                xmlThing.getCollectionOfThings(
+                                        List.of(exampleThing.getInstance()),
+                                        aThingDefinition)));
+                    }else{
+                        output.append(this.XMLPrettyPrinter.prettyPrintHtml(
+                                xmlThing.getSingleObjectXml(exampleThing.getInstance())));
+                    }
                     output.append("</code>\n");
                     output.append("</pre>\n");
                 }

diff --git a/...test/java/uk/co/compendiumdev/casestudy/todomanager/http_api/JsonRequestResponseTest.java b/...test/java/uk/co/compendiumdev/casestudy/todomanager/http_api/JsonRequestResponseTest.java
@@ -14,6 +14,8 @@
 import uk.co.compendiumdev.thingifier.api.response.ApiResponse;
 import uk.co.compendiumdev.thingifier.core.domain.instances.EntityInstance;
 
+import java.util.Map;
+
 public class JsonRequestResponseTest {
 
     private Thingifier todoManager;
@@ -189,6 +191,7 @@ public void canPostAndCreateAnItemWithJson() {
     public void canPostAndCreateAnItemWithJsonAndReceiveXml() {
 
         HttpApiRequest request = new HttpApiRequest("todos");
+        request.setHeaders(Map.of("content-type", "application/json"));
         request.getHeaders().putAll(HeadersSupport.acceptXml());
         request.getHeaders().putAll(HeadersSupport.containsJson());
 

diff --git a/...rc/test/java/uk/co/compendiumdev/casestudy/todomanager/http_api/RelationshipHttpTest.java b/...rc/test/java/uk/co/compendiumdev/casestudy/todomanager/http_api/RelationshipHttpTest.java
@@ -47,6 +47,7 @@ public void canCreateARelationshipBetweenProjectAndTodoViaTasks(){
 
         HttpApiRequest request = new HttpApiRequest("projects/" + aproject.getPrimaryKeyValue() + "/tasks");
         request.getHeaders().putAll(HeadersSupport.acceptJson());
+        request.getHeaders().putAll(HeadersSupport.containsJson());
 
         //{"guid":"%s"}
         String body = String.format("{\"guid\":\"%s\"}", atodo.getPrimaryKeyValue());
@@ -70,6 +71,7 @@ public void canCreateARelationshipBetweenProjectAndTodoViaTasksUsingID(){
 
         HttpApiRequest request = new HttpApiRequest("projects/" + aproject.getPrimaryKeyValue() + "/tasks");
         request.getHeaders().putAll(HeadersSupport.acceptJson());
+        request.getHeaders().putAll(HeadersSupport.containsJson());
 
         //{"guid":"%s"}
         String body = String.format("{\"id\":\"%s\"}", atodo.getFieldValue("id").asString());
@@ -95,6 +97,7 @@ public void canCreateARelationshipAndTodoBetweenProjectAndTodoViaTasks(){
 
         HttpApiRequest request = new HttpApiRequest("projects/" + aproject.getPrimaryKeyValue() + "/tasks");
         request.getHeaders().putAll(HeadersSupport.acceptJson());
+        request.getHeaders().putAll(HeadersSupport.containsJson());
 
         //{"title":"My New To do"}
         String body = "{\"title\":\"My New To do\"}";
@@ -125,6 +128,7 @@ public void cannotCreateARelationshipBetweenProjectAndCategoryViaTasks(){
 
         HttpApiRequest request = new HttpApiRequest("projects/" + aproject.getPrimaryKeyValue() + "/tasks");
         request.getHeaders().putAll(HeadersSupport.acceptJson());
+        request.getHeaders().putAll(HeadersSupport.containsJson());
 
         //{"guid":"%s"}
         String body = String.format("{\"guid\":\"%s\"}", acategory.getPrimaryKeyValue());
@@ -154,6 +158,7 @@ public void cannotCreateARelationshipWhenGivenGuidDoesNotExist(){
 
         HttpApiRequest request = new HttpApiRequest("projects/" + aproject.getPrimaryKeyValue() + "/tasks");
         request.getHeaders().putAll(HeadersSupport.acceptJson());
+        request.getHeaders().putAll(HeadersSupport.containsJson());
 
         //{"guid":"%s"}
         String body = String.format("{\"guid\":\"%s\"}", atodo.getPrimaryKeyValue() + "bob");
@@ -182,6 +187,7 @@ public void canCreateARelationshipBetweenCategoryAndTodoViaTodos(){
 
         HttpApiRequest request = new HttpApiRequest("categories/" + acategory.getPrimaryKeyValue() + "/todos");
         request.getHeaders().putAll(HeadersSupport.acceptJson());
+        request.getHeaders().putAll(HeadersSupport.containsJson());
 
         //{"guid":"%s"}
         String body = String.format("{\"guid\":\"%s\"}", atodo.getPrimaryKeyValue());
@@ -204,6 +210,8 @@ public void canCreateARelationshipAndTodoBetweenCategoryAndTodoViaTodos(){
 
         HttpApiRequest request = new HttpApiRequest("categories/" + acategory.getPrimaryKeyValue() + "/todos");
         request.getHeaders().putAll(HeadersSupport.acceptJson());
+        request.getHeaders().putAll(HeadersSupport.containsJson());
+
 
         //{"title":"My New To do"}
         String body = "{\"title\":\"My New To do\"}";
@@ -341,6 +349,7 @@ public void canCreateAnEstimateForTodoMandatoryRelationship(){
 
         HttpApiRequest request = new HttpApiRequest("todos/" + atodo.getPrimaryKeyValue() + "/estimates" );
         request.getHeaders().putAll(HeadersSupport.acceptJson());
+        request.getHeaders().putAll(HeadersSupport.containsJson());
 
         String body = "{\"duration\":\"3\"}";
         request.setBody(body);

diff --git a/...fier/src/test/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParserTest.java b/...fier/src/test/java/uk/co/compendiumdev/thingifier/api/http/bodyparser/BodyParserTest.java
@@ -25,7 +25,69 @@ public void simpleJsonParse(){
         Assertions.assertEquals("5", map.get("duration"));
     }
 
+    @Test
+    public void simpleJsonParseErrorMessage(){
+
+        HttpApiRequest request = new HttpApiRequest("/estimates");
+        request.setHeaders(Map.of("content-type", "application/json"));
+        request.setBody("{'duration':'5'");
+
+        List<String> names = new ArrayList<>();
+
+        names.add("estimate");
+
+        final String validated = new BodyParser(request, names).validBodyBasedOnContentType();
+
+        Assertions.assertEquals("Invalid Json Payload: please check the syntax of the request body",validated);
+    }
+
+    @Test
+    public void simpleXmlParseErrorMessage(){
+
+        HttpApiRequest request = new HttpApiRequest("/estimates");
+        request.setHeaders(Map.of("content-type", "application/xml"));
+        request.setBody("<estimate><duration>5</duration>");
+
+        List<String> names = new ArrayList<>();
+
+        names.add("estimate");
+
+        final String validated = new BodyParser(request, names).validBodyBasedOnContentType();
+
+        Assertions.assertEquals("Invalid XML Payload: Unclosed tag estimate at 32 [character 33 line 1]",validated);
+    }
 
+    @Test
+    public void simpleTextXmlParseErrorMessage(){
+
+        HttpApiRequest request = new HttpApiRequest("/estimates");
+        request.setHeaders(Map.of("content-type", "text/xml"));
+        request.setBody("<estimate><duration>5</duration>");
+
+        List<String> names = new ArrayList<>();
+
+        names.add("estimate");
+
+        final String validated = new BodyParser(request, names).validBodyBasedOnContentType();
+
+        Assertions.assertEquals("Invalid XML Payload: Unclosed tag estimate at 32 [character 33 line 1]",validated);
+    }
+
+    @Test
+    public void simpleUnknownContentParseErrorMessage(){
+
+        HttpApiRequest request = new HttpApiRequest("/estimates");
+        request.setHeaders(Map.of("content-type", "application/csv"));
+        request.setBody("duration,5");
+
+        List<String> names = new ArrayList<>();
+
+        names.add("estimate");
+
+        final String validated = new BodyParser(request, names).validBodyBasedOnContentType();
+
+        Assertions.assertEquals("Unknown content Type: API cannot parse application/csv",validated);
+    }
 
     @Test
     public void embeddedObjectParseIgnoredOnStringMap(){

diff --git a/...k/co/compendiumdev/thingifier/api/http/requests/ThingifierHttpApiRequestHandlingTest.java b/...k/co/compendiumdev/thingifier/api/http/requests/ThingifierHttpApiRequestHandlingTest.java
@@ -102,6 +102,7 @@ public void aPostRequestWillCreateNewSessionWithDatabase(){
 
         final Map<String,String> headers = new HashMap<>();
         headers.put(ThingifierHttpApi.HTTP_SESSION_HEADER_NAME, "other_things");
+        headers.put("content-type", "application/json");
 
         final HttpApiResponse response = api.post(new HttpApiRequest("/things")
                                             .setHeaders(headers)

diff --git a/...ifier/src/test/java/uk/co/compendiumdev/thingifier/api/non_http/NestedObjectsApiTest.java b/...ifier/src/test/java/uk/co/compendiumdev/thingifier/api/non_http/NestedObjectsApiTest.java
@@ -16,6 +16,8 @@
 import uk.co.compendiumdev.thingifier.core.domain.instances.EntityInstance;
 import uk.co.compendiumdev.thingifier.core.domain.instances.InstanceFields;
 
+import java.util.Map;
+
 
 public class NestedObjectsApiTest {
 
@@ -61,6 +63,7 @@ void canAmendConnie(){
 
         final HttpApiRequest amendConnieRequest = new HttpApiRequest("/things/" + instance.getPrimaryKeyValue());
         amendConnieRequest.setVerb(HttpApiRequest.VERB.POST);
+        amendConnieRequest.setHeaders(Map.of("content-type", "application/json"));
         //{"person" : {"firstname": "bob"}}
         amendConnieRequest.setBody("{\"person\" : {\"firstname\": \"bob\"}}");
 
@@ -80,6 +83,7 @@ void canCreateBob(){
         Assertions.assertEquals(0,thing.countInstances());
 
         final HttpApiRequest createBobRequest = new HttpApiRequest("/things");
+        createBobRequest.setHeaders(Map.of("content-type", "application/json"));
         createBobRequest.setVerb(HttpApiRequest.VERB.POST);
         //{"person" : {"firstname": "bob", "surname" : "dobbs"}}
         createBobRequest.setBody("{\"person\" : {\"firstname\": \"bob\", \"surname\" : \"dobbs\"}}");
@@ -114,6 +118,7 @@ public void failValidationAtObjectFieldLevel() {
 
         final HttpApiRequest failToCreateBobRequest = new HttpApiRequest("/things");
         failToCreateBobRequest.setVerb(HttpApiRequest.VERB.POST);
+        failToCreateBobRequest.setHeaders(Map.of("content-type", "application/json"));
         //{"person" : {"firstname": "bob"}}
         failToCreateBobRequest.setBody("{\"person\" : {\"firstname\": \"bob\"}}");
 

diff --git a/...ava/uk/co/compendiumdev/thingifier/tactical/postmanreplication/CreateALotOfTodosTest.java b/...ava/uk/co/compendiumdev/thingifier/tactical/postmanreplication/CreateALotOfTodosTest.java
@@ -49,12 +49,12 @@ public void canCreateAndAmendSequence(){
 
         // CREATE todos WITH POST
 
-        final HashMap<String, String> givenBody = new HashMap<String, String>();
+        final HashMap<String, String> givenBody = new HashMap<>();
 
         for(int i=0; i<100; i++){
             givenBody.put("title", "a title " + i);
 
-            given().body(givenBody).
+            given().body(givenBody).contentType("application/json").
                     when().post("/todos").
                     then().
                     statusCode(201).