From 6bcaa0c1b71889c6c7d491014f03071490864c22 Mon Sep 17 00:00:00 2001 From: Monika Machunik | hybris Date: Mon, 30 Mar 2015 16:33:52 +0200 Subject: [PATCH 1/4] fixed handling merging default media type keys with existing media type keys --- .../parser/visitor/MediaTypeResolver.java | 73 +++++++++++++++---- .../builder/DefaultMediaTypeTestCase.java | 16 ++++ src/test/resources/org/raml/media-type.yaml | 35 +++++++++ 3 files changed, 108 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java b/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java index 83ac59bf..06fb3938 100644 --- a/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java +++ b/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java @@ -109,26 +109,67 @@ private boolean isValidMediaType(String value) public List resolve(MappingNode bodyNode) { List validationResults = new ArrayList(); - if (mediaType == null) - { - return validationResults; + + NodeTuple mediaTypeNodeTuple = null; + + if (mediaType!=null){ + + for (NodeTuple tuple : bodyNode.getValue()) + { + if (mediaType.equals(((ScalarNode) tuple.getKeyNode()).getValue())){ + mediaTypeNodeTuple = tuple; + break; + } + } + + if (mediaTypeNodeTuple==null){ + Node keyNode = new ScalarNode(Tag.STR, mediaType, null, null, null); + Node valueNode = new MappingNode(Tag.MAP, new ArrayList(), false); + NodeTuple newTuple = new NodeTuple(keyNode, valueNode); + bodyNode.getValue().add(newTuple); + mediaTypeNodeTuple= newTuple; + } } - for (NodeTuple tuple : bodyNode.getValue()) - { - if (!MEDIA_TYPE_KEYS.contains(((ScalarNode) tuple.getKeyNode()).getValue())) - { - return validationResults; - } - } - List copy = new ArrayList(bodyNode.getValue()); - Node keyNode = new ScalarNode(Tag.STR, mediaType, null, null, null); - Node valueNode = new MappingNode(Tag.MAP, copy, false); - bodyNode.getValue().clear(); - bodyNode.getValue().add(new NodeTuple(keyNode, valueNode)); + + mergeMediaLevelKeys(mediaTypeNodeTuple, bodyNode); + return validationResults; } - /** + private void mergeMediaLevelKeys(NodeTuple targetTouple, MappingNode sourceNode) { + + List tuplesToRemove = new ArrayList(); + + for (NodeTuple sourceTuple : sourceNode.getValue()) + { + String mediaTypeKey = ((ScalarNode) sourceTuple.getKeyNode()).getValue(); + + if (MEDIA_TYPE_KEYS.contains(mediaTypeKey)) + { + tuplesToRemove.add(sourceTuple); + + if (targetTouple!=null){ + List targetSubTuples = ((MappingNode)targetTouple.getValueNode()).getValue(); + boolean containsSuchKey = false; + for (NodeTuple targetSubTuple: targetSubTuples){ + if(mediaTypeKey.equals(((ScalarNode)targetSubTuple.getKeyNode()).getValue())){ + containsSuchKey = true; + break; + } + } + if (!containsSuchKey){ + targetSubTuples.add(sourceTuple); + } + } + } + } + + for (NodeTuple toRemove: tuplesToRemove){ + sourceNode.getValue().remove(toRemove); + } + } + + /** * if no explicit media type is defined in either the request or * response body, the default one is applied * diff --git a/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java b/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java index 14c66938..f244fd54 100644 --- a/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java +++ b/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java @@ -17,6 +17,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.raml.model.ActionType.GET; @@ -123,4 +124,19 @@ public void emitter() assertThat(raml2.getResources().get("/simple").getActions().size(), is(raml1.getResources().get("/simple").getActions().size())); } + + @Test + public void merged() + { + Resource resource = raml.getResource("/merged"); + Map getResponseBody = resource.getAction(ActionType.GET).getResponses().get("200").getBody(); + assertThat(getResponseBody.size(), is(2)); + assertThat(getResponseBody.containsKey("application/json"), is(true)); + assertThat(getResponseBody.get("application/json").getSchema(), notNullValue()); + assertThat(getResponseBody.get("application/json").getExample(), notNullValue()); + assertThat(getResponseBody.containsKey("text/html"), is(true)); + assertThat(getResponseBody.get("text/html").getSchema(), nullValue()); + assertThat(getResponseBody.get("text/html").getExample(), notNullValue()); + } + } diff --git a/src/test/resources/org/raml/media-type.yaml b/src/test/resources/org/raml/media-type.yaml index 47910493..1d73cb9f 100644 --- a/src/test/resources/org/raml/media-type.yaml +++ b/src/test/resources/org/raml/media-type.yaml @@ -32,6 +32,26 @@ resourceTypes: type: typeParent put: is: [traitOne] + - dummy: + get: + responses: + 200: + body: + example: | + { + "id": "foo" + } + application/json: + schema: | + { "$schema": "http://json-schema.org/draft-03/schema", + "type": "object", + "description": "A dummy schema", + "properties": { + "id": { "type": "string" } + } + } + text/html: + example: dummy resource example /simple: type: typeChild @@ -57,3 +77,18 @@ resourceTypes: get: responses: 200: + +/merged: + type: dummy + get: + responses: + 200: + body: + schema: | + { "$schema": "http://json-schema.org/draft-03/schema", + "type": "object", + "description": "A merged schema", + "properties": { + "id": { "type": "string" } + } + } \ No newline at end of file From 403d5093d313dd441e219df67f3a717703cb8299 Mon Sep 17 00:00:00 2001 From: Monika Machunik | hybris Date: Mon, 30 Mar 2015 17:45:33 +0200 Subject: [PATCH 2/4] fixed the order of applying media type keys, extended the test --- .../raml/parser/visitor/MediaTypeResolver.java | 12 +++++++----- .../builder/DefaultMediaTypeTestCase.java | 17 ++++++++++++----- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java b/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java index 06fb3938..b66f62db 100644 --- a/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java +++ b/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java @@ -150,16 +150,18 @@ private void mergeMediaLevelKeys(NodeTuple targetTouple, MappingNode sourceNode) if (targetTouple!=null){ List targetSubTuples = ((MappingNode)targetTouple.getValueNode()).getValue(); - boolean containsSuchKey = false; + + NodeTuple existingTuple = null; for (NodeTuple targetSubTuple: targetSubTuples){ if(mediaTypeKey.equals(((ScalarNode)targetSubTuple.getKeyNode()).getValue())){ - containsSuchKey = true; - break; + existingTuple = targetSubTuple; + break; } } - if (!containsSuchKey){ - targetSubTuples.add(sourceTuple); + if (existingTuple!=null){ + targetSubTuples.remove(existingTuple); } + targetSubTuples.add(sourceTuple); } } } diff --git a/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java b/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java index f244fd54..7503acf9 100644 --- a/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java +++ b/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java @@ -125,18 +125,25 @@ public void emitter() is(raml1.getResources().get("/simple").getActions().size())); } - @Test + @Test public void merged() { Resource resource = raml.getResource("/merged"); Map getResponseBody = resource.getAction(ActionType.GET).getResponses().get("200").getBody(); assertThat(getResponseBody.size(), is(2)); + assertThat(getResponseBody.containsKey("application/json"), is(true)); - assertThat(getResponseBody.get("application/json").getSchema(), notNullValue()); - assertThat(getResponseBody.get("application/json").getExample(), notNullValue()); + MimeType applicationJson = getResponseBody.get("application/json"); + assertThat(applicationJson.getSchema(), notNullValue()); + assertThat(applicationJson.getSchema().contains("merged"), is(true)); + assertThat(applicationJson.getExample(), notNullValue()); + assertThat(applicationJson.getExample().contains("foo"), is(true)); + assertThat(getResponseBody.containsKey("text/html"), is(true)); - assertThat(getResponseBody.get("text/html").getSchema(), nullValue()); - assertThat(getResponseBody.get("text/html").getExample(), notNullValue()); + MimeType textHtml = getResponseBody.get("text/html"); + assertThat(textHtml.getSchema(), nullValue()); + assertThat(textHtml.getExample(), notNullValue()); + assertThat(textHtml.getExample().contains("dummy resource example"), is(true)); } } From e7e6f3ca8f49ac88c53f58d7605661bcbd780dd6 Mon Sep 17 00:00:00 2001 From: Monika Machunik | hybris Date: Thu, 2 Apr 2015 16:42:46 +0200 Subject: [PATCH 3/4] added some corner case test and improved the code --- .../parser/visitor/MediaTypeResolver.java | 77 +++++++++++-------- .../builder/DefaultMediaTypeTestCase.java | 37 +++++++++ src/test/resources/org/raml/media-type.yaml | 18 ++++- 3 files changed, 97 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java b/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java index b66f62db..2efeb38d 100644 --- a/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java +++ b/src/main/java/org/raml/parser/visitor/MediaTypeResolver.java @@ -110,33 +110,39 @@ public List resolve(MappingNode bodyNode) { List validationResults = new ArrayList(); - NodeTuple mediaTypeNodeTuple = null; - if (mediaType!=null){ - for (NodeTuple tuple : bodyNode.getValue()) - { - if (mediaType.equals(((ScalarNode) tuple.getKeyNode()).getValue())){ - mediaTypeNodeTuple = tuple; - break; - } - } - - if (mediaTypeNodeTuple==null){ + NodeTuple mediaTypeNodeTuple = findNodeByKey(bodyNode, mediaType); + + //if mediaTypeNode is null create it, but do not add it to the parent node yet + if (mediaTypeNodeTuple == null){ Node keyNode = new ScalarNode(Tag.STR, mediaType, null, null, null); Node valueNode = new MappingNode(Tag.MAP, new ArrayList(), false); NodeTuple newTuple = new NodeTuple(keyNode, valueNode); - bodyNode.getValue().add(newTuple); - mediaTypeNodeTuple= newTuple; - } + mediaTypeNodeTuple = newTuple; + } + + moveOrphanedNodes(mediaTypeNodeTuple, bodyNode); + + // in case the mediaTypeNodeTuple was not yet appended to the parent node, + // append it but only if it is not empty + if (!bodyNode.getValue().contains(mediaTypeNodeTuple) && + !((MappingNode)mediaTypeNodeTuple.getValueNode()).getValue().isEmpty()){ + bodyNode.getValue().add(mediaTypeNodeTuple); + } } - mergeMediaLevelKeys(mediaTypeNodeTuple, bodyNode); - return validationResults; } - private void mergeMediaLevelKeys(NodeTuple targetTouple, MappingNode sourceNode) { + /** + * It is moving all orphaned schema/example/formParameters nodes from the source node under the + * targetTouple node. + * + * @param targetTouple the target node which should become new parent of orphaned nodes + * @param sourceNode the current parent node of the possibly orphaned nodes + */ + private void moveOrphanedNodes(NodeTuple targetTouple, MappingNode sourceNode) { List tuplesToRemove = new ArrayList(); @@ -144,33 +150,36 @@ private void mergeMediaLevelKeys(NodeTuple targetTouple, MappingNode sourceNode) { String mediaTypeKey = ((ScalarNode) sourceTuple.getKeyNode()).getValue(); + // if it is an orphaned schema/example/formParameters if (MEDIA_TYPE_KEYS.contains(mediaTypeKey)) { - tuplesToRemove.add(sourceTuple); - - if (targetTouple!=null){ - List targetSubTuples = ((MappingNode)targetTouple.getValueNode()).getValue(); - - NodeTuple existingTuple = null; - for (NodeTuple targetSubTuple: targetSubTuples){ - if(mediaTypeKey.equals(((ScalarNode)targetSubTuple.getKeyNode()).getValue())){ - existingTuple = targetSubTuple; - break; - } - } - if (existingTuple!=null){ - targetSubTuples.remove(existingTuple); - } - targetSubTuples.add(sourceTuple); - } + // move it to the targetTouple + tuplesToRemove.add(sourceTuple); + MappingNode targetToupleNode = (MappingNode)targetTouple.getValueNode(); + NodeTuple existingTuple = findNodeByKey(targetToupleNode, mediaTypeKey); + if (existingTuple!=null){ + targetToupleNode.getValue().remove(existingTuple); + } + targetToupleNode.getValue().add(sourceTuple); } } + // remove the orphaned schema/example/formParameters that were marked for removal earlier for (NodeTuple toRemove: tuplesToRemove){ sourceNode.getValue().remove(toRemove); } } + private NodeTuple findNodeByKey(MappingNode parentNode, String key) { + for (NodeTuple tuple : parentNode.getValue()) + { + if (key.equals(((ScalarNode) tuple.getKeyNode()).getValue())){ + return tuple; + } + } + return null; + } + /** * if no explicit media type is defined in either the request or * response body, the default one is applied diff --git a/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java b/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java index 7503acf9..324e0fa4 100644 --- a/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java +++ b/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java @@ -29,6 +29,7 @@ import org.apache.commons.io.IOUtils; import org.junit.BeforeClass; import org.junit.Test; +import org.mockito.internal.matchers.NotNull; import org.raml.model.ActionType; import org.raml.model.MimeType; import org.raml.model.Raml; @@ -94,6 +95,42 @@ public void emptyBody() assertThat(resource.getAction(PUT).getBody().size(), is(1)); assertThat(resource.getAction(PUT).getResponses().get("200").getBody().size(), is(1)); } + + @Test + public void mergedWithType() + { + Resource resource = raml.getResource("/mergedWithType"); + Map getResponseBody = resource.getAction(ActionType.GET).getResponses().get("200").getBody(); + assertThat(getResponseBody.size(), is(2)); + + assertThat(getResponseBody.containsKey("application/json"), is(true)); + MimeType applicationJson = getResponseBody.get("application/json"); + assertThat(applicationJson.getSchema(), notNullValue()); + assertThat(applicationJson.getSchema().contains("merged"), is(true)); + assertThat(applicationJson.getExample(), notNullValue()); + assertThat(applicationJson.getExample().contains("foo"), is(true)); + + assertThat(getResponseBody.containsKey("text/html"), is(true)); + MimeType textHtml = getResponseBody.get("text/html"); + assertThat(textHtml.getSchema(), nullValue()); + assertThat(textHtml.getExample(), notNullValue()); + assertThat(textHtml.getExample().contains("dummy resource example"), is(true)); + } + + @Test + public void merged() + { + Resource resource = raml.getResource("/merged"); + Map getResponseBody = resource.getAction(ActionType.GET).getResponses().get("200").getBody(); + assertThat(getResponseBody.size(), is(1)); + + assertThat(getResponseBody.containsKey("application/json"), is(false)); + assertThat(getResponseBody.containsKey("text/html"), is(true)); + + MimeType textHtml = getResponseBody.get("text/html"); + assertThat(textHtml.getSchema().contains("merged"), is(true)); + assertThat(textHtml.getExample(), nullValue()); + } @Test public void noBody() diff --git a/src/test/resources/org/raml/media-type.yaml b/src/test/resources/org/raml/media-type.yaml index 1d73cb9f..9988bd2f 100644 --- a/src/test/resources/org/raml/media-type.yaml +++ b/src/test/resources/org/raml/media-type.yaml @@ -79,6 +79,21 @@ resourceTypes: 200: /merged: + get: + responses: + 200: + body: + text/html: + schema: | + { "$schema": "http://json-schema.org/draft-03/schema", + "type": "object", + "description": "A merged schema", + "properties": { + "id": { "type": "string" } + } + } + +/mergedWithType: type: dummy get: responses: @@ -91,4 +106,5 @@ resourceTypes: "properties": { "id": { "type": "string" } } - } \ No newline at end of file + } + From 27065bdf28342262443dfb268b0f6b751bfe2849 Mon Sep 17 00:00:00 2001 From: Monika Machunik | hybris Date: Thu, 2 Apr 2015 16:47:34 +0200 Subject: [PATCH 4/4] wrong merge --- .../builder/DefaultMediaTypeTestCase.java | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java b/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java index 324e0fa4..a6b92d3c 100644 --- a/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java +++ b/src/test/java/org/raml/parser/builder/DefaultMediaTypeTestCase.java @@ -161,26 +161,5 @@ public void emitter() assertThat(raml2.getResources().get("/simple").getActions().size(), is(raml1.getResources().get("/simple").getActions().size())); } - - @Test - public void merged() - { - Resource resource = raml.getResource("/merged"); - Map getResponseBody = resource.getAction(ActionType.GET).getResponses().get("200").getBody(); - assertThat(getResponseBody.size(), is(2)); - - assertThat(getResponseBody.containsKey("application/json"), is(true)); - MimeType applicationJson = getResponseBody.get("application/json"); - assertThat(applicationJson.getSchema(), notNullValue()); - assertThat(applicationJson.getSchema().contains("merged"), is(true)); - assertThat(applicationJson.getExample(), notNullValue()); - assertThat(applicationJson.getExample().contains("foo"), is(true)); - - assertThat(getResponseBody.containsKey("text/html"), is(true)); - MimeType textHtml = getResponseBody.get("text/html"); - assertThat(textHtml.getSchema(), nullValue()); - assertThat(textHtml.getExample(), notNullValue()); - assertThat(textHtml.getExample().contains("dummy resource example"), is(true)); - } - + }