From 8af6235b208e411850a2422ec04e8502260e546d Mon Sep 17 00:00:00 2001 From: Guillaume Hillairet Date: Fri, 9 Jun 2017 18:16:18 +0200 Subject: [PATCH 1/5] [#353] Validation of external $ref property values should show error on unexpected object type Validation of reference types has been moved to ReferenceValidator that previously only validate correctness of references pointer/URIs. It now checks the type of the reference if this one is valid, this allow to reuse the code for loading the JSON document which is referenced by it. This validation could be slow because it will parse the referenced documents into Models. --- .../core/json/references/JsonReference.java | 24 ++++--- .../references/JsonReferenceCollector.java | 10 +-- .../references/JsonReferenceValidator.java | 65 ++++++++++++++++++- .../reprezen/swagedit/core/model/Model.java | 9 +++ .../swagedit/core/validation/Validator.java | 28 -------- .../openapi3/validation/ValidatorTest.xtend | 18 +++-- 6 files changed, 101 insertions(+), 53 deletions(-) diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReference.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReference.java index d394f353..901f6411 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReference.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReference.java @@ -105,14 +105,7 @@ public boolean isMissing(JsonDocument document, URI baseURI) { */ public JsonNode resolve(JsonDocument document, URI baseURI) { if (resolved == null) { - - final JsonNode doc; - if (isLocal()) { - doc = document.asJson(); - } else { - doc = manager.getDocument(resolveURI(baseURI)); - } - + JsonNode doc = getDocument(document, baseURI); if (doc != null) { try { resolved = doc.at(pointer); @@ -223,4 +216,19 @@ public static JsonPointer getPointer(ObjectNode node) { } } + /** + * Returns the JSON document that contains the node referenced by this reference. + * + * @param document + * @param baseURI + * @return referenced node + */ + public JsonNode getDocument(JsonDocument document, URI baseURI) { + if (isLocal()) { + return document.asJson(); + } else { + return manager.getDocument(resolveURI(baseURI)); + } + } + } diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceCollector.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceCollector.java index 6a210822..71616aa8 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceCollector.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceCollector.java @@ -13,9 +13,9 @@ import static com.reprezen.swagedit.core.json.references.JsonReference.PROPERTY; import java.net.URI; -import java.util.Set; +import java.util.Map; -import com.google.common.collect.Sets; +import com.google.common.collect.Maps; import com.reprezen.swagedit.core.model.AbstractNode; import com.reprezen.swagedit.core.model.Model; @@ -40,8 +40,8 @@ public JsonReferenceCollector(JsonReferenceFactory factory) { * @param model * @return all reference nodes */ - public Iterable collect(URI baseURI, Model model) { - final Set references = Sets.newHashSet(); + public Map collect(URI baseURI, Model model) { + final Map references = Maps.newHashMap(); for (AbstractNode node : model.allNodes()) { if (JsonReference.isReference(node)) { @@ -50,7 +50,7 @@ public Iterable collect(URI baseURI, Model model) { reference = factory.create(node); } if (reference != null) { - references.add(reference); + references.put(node, reference); } } } diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceValidator.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceValidator.java index 3d74380d..9f31a299 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceValidator.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceValidator.java @@ -15,11 +15,18 @@ import java.net.URI; import java.util.Collection; +import java.util.Map; import java.util.Set; +import org.eclipse.core.resources.IMarker; + +import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.Sets; import com.reprezen.swagedit.core.editor.JsonDocument; import com.reprezen.swagedit.core.model.AbstractNode; +import com.reprezen.swagedit.core.model.Model; +import com.reprezen.swagedit.core.schema.TypeDefinition; +import com.reprezen.swagedit.core.validation.Messages; import com.reprezen.swagedit.core.validation.SwaggerError; /** @@ -45,9 +52,11 @@ public Collection validate(URI baseURI, JsonDocument doc } protected Collection doValidate(URI baseURI, JsonDocument doc, - Iterable references) { + Map references) { Set errors = Sets.newHashSet(); - for (JsonReference reference : references) { + for (AbstractNode node : references.keySet()) { + JsonReference reference = references.get(node); + if (reference instanceof JsonReference.SimpleReference) { errors.add(createReferenceError(SEVERITY_WARNING, Messages.warning_simple_reference, reference)); } else if (reference.isInvalid()) { @@ -56,11 +65,63 @@ protected Collection doValidate(URI baseURI, JsonDocumen errors.add(createReferenceError(SEVERITY_WARNING, Messages.error_missing_reference, reference)); } else if (reference.containsWarning()) { errors.add(createReferenceError(SEVERITY_WARNING, Messages.error_invalid_reference, reference)); + } else { + validateType(doc, baseURI, node, reference, errors); } } return errors; } + /** + * This method checks that referenced objects are of expected type as defined in the schema. + * + * @param doc + * current document + * @param node + * node holding the reference + * @param reference + * actual reference + * @param errors + * current set of errors + */ + private void validateType(JsonDocument doc, URI baseURI, AbstractNode node, JsonReference reference, + Set errors) { + if (node == null) { + return; + } + + Model model = doc.getModel(); + TypeDefinition type = node.getType(); + + AbstractNode nodeValue = node.get(JsonReference.PROPERTY); + String pointer = (String) nodeValue.asValue().getValue(); + AbstractNode valueNode = model.find(pointer); + + if (valueNode == null) { + // Try to load the referenced node from an external document + JsonNode externalDoc = reference.getDocument(doc, baseURI); + if (externalDoc != null) { + try { + Model externalModel = Model.parse(model.getSchema(), externalDoc); + pointer = pointer.substring(pointer.indexOf("#") + 1, pointer.length()); + valueNode = externalModel.find(pointer); + + if (valueNode == null) { + return; + } + } catch (Exception e) { + // fail to parse the model or the pointer + return; + } + } + } + + if (!type.validate(valueNode)) { + errors.add( + createReferenceError(IMarker.SEVERITY_WARNING, Messages.error_invalid_reference_type, reference)); + } + } + protected SwaggerError createReferenceError(int severity, String message, JsonReference reference) { Object source = reference.getSource(); int line; diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/model/Model.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/model/Model.java index 69af56b4..54ff0b9e 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/model/Model.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/model/Model.java @@ -394,4 +394,13 @@ public List findByType(JsonPointer typePointer) { return instances; } + /** + * Returns the schema associated to this model. + * + * @return schema + */ + public CompositeSchema getSchema() { + return schema; + } + } diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/validation/Validator.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/validation/Validator.java index b96441a2..76268b7f 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/validation/Validator.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/validation/Validator.java @@ -51,7 +51,6 @@ import com.reprezen.swagedit.core.model.Model; import com.reprezen.swagedit.core.model.ObjectNode; import com.reprezen.swagedit.core.model.ValueNode; -import com.reprezen.swagedit.core.schema.TypeDefinition; /** * This class contains methods for validating a Swagger YAML document. @@ -179,7 +178,6 @@ protected Set validateModel(Model model) { protected void executeModelValidation(Model model, AbstractNode node, Set errors) { checkArrayTypeDefinition(errors, node); checkObjectTypeDefinition(errors, node); - checkReferenceType(errors, node); } /** @@ -294,32 +292,6 @@ protected void checkMissingRequiredProperties(Set errors, Abstract } } - /** - * This method checks that referenced objects are of expected type as defined in the schema. - * - * @param errors - * @param node - */ - protected void checkReferenceType(Set errors, AbstractNode node) { - if (JsonReference.isReference(node)) { - Model model = node.getModel(); - TypeDefinition type = node.getType(); - - AbstractNode nodeValue = node.get(JsonReference.PROPERTY); - AbstractNode valueNode = model.find((String) nodeValue.asValue().getValue()); - - if (valueNode == null) { - // probably external node, - // do not validate for now. - return; - } - - if (!type.validate(valueNode)) { - errors.add(error(nodeValue, IMarker.SEVERITY_WARNING, Messages.error_invalid_reference_type)); - } - } - } - protected SwaggerError error(AbstractNode node, int level, String message) { return new SwaggerError(node.getStart().getLine() + 1, level, message); } diff --git a/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ValidatorTest.xtend b/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ValidatorTest.xtend index 7685105c..2c561a46 100644 --- a/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ValidatorTest.xtend +++ b/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ValidatorTest.xtend @@ -153,8 +153,7 @@ class ValidatorTest { assertTrue(errors.map[message].forall[it.equals(Messages.error_invalid_reference_type)]) } -// Disable until remote references are validated -// @Test + @Test def void testValidationShouldFail_ForInvalidPointers() { val content = ''' openapi: "3.0.0" @@ -174,11 +173,11 @@ class ValidatorTest { document.set(content) val errors = validator.validate(document, null as URI) - assertEquals(2, errors.size()) + assertEquals(1, errors.size()) + println(errors.map[message]) assertThat( errors, hasItems( - new SwaggerError(13, IMarker.SEVERITY_ERROR, Messages.error_invalid_reference_type), new SwaggerError(13, IMarker.SEVERITY_WARNING, Messages.error_missing_reference) ) ) @@ -205,17 +204,16 @@ class ValidatorTest { document.set(content) val errors = validator.validate(document, null as URI) // Update with #353 Validation of external $ref property values should show error on unexpected object type" -// assertEquals(2, errors.size()) + assertEquals(1, errors.size()) assertThat( errors, - hasItems( -// new SwaggerError(13, IMarker.SEVERITY_ERROR, Messages.error_invalid_reference_type), + hasItems( new SwaggerError(13, IMarker.SEVERITY_WARNING, Messages.error_missing_reference) ) ) } - @Test +// @Test def void testValidationShouldFail_pathInNotJson() { val content = ''' openapi: "3.0.0" @@ -236,11 +234,11 @@ class ValidatorTest { document.set(content) val errors = validator.validate(document, null as URI) // Update with #353 Validation of external $ref property values should show error on unexpected object type" -// assertEquals(2, errors.size()) + assertEquals(2, errors.size()) assertThat( errors, hasItems( -// new SwaggerError(13, IMarker.SEVERITY_ERROR, Messages.error_invalid_reference_type), + new SwaggerError(13, IMarker.SEVERITY_ERROR, Messages.error_invalid_reference_type), new SwaggerError(13, IMarker.SEVERITY_ERROR, Messages.error_invalid_reference) ) ) From b1bd92e1903356f9b8b7048b1ab92111e0407c1a Mon Sep 17 00:00:00 2001 From: Guillaume Hillairet Date: Fri, 22 Sep 2017 17:17:29 +0200 Subject: [PATCH 2/5] [#353] Validation of external $ref property values should show error on unexpected object type --- .../swagedit/core/json/references/JsonDocumentManager.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonDocumentManager.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonDocumentManager.java index c5d6a0d3..aa440c8c 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonDocumentManager.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonDocumentManager.java @@ -91,11 +91,6 @@ public JsonNode getDocument(URL url) { } public JsonNode getDocument(URI uri) { - final IFile file = getFile(uri); - if (file == null || !file.exists()) { - return null; - } - try { return getDocument(uri.toURL()); } catch (MalformedURLException e) { From 692270ea9824bebe13610a5751b543e10431f31d Mon Sep 17 00:00:00 2001 From: Guillaume Hillairet Date: Wed, 27 Sep 2017 11:23:50 +0200 Subject: [PATCH 3/5] [#353] Validation of external $ref property values should show error on unexpected object type --- .../references/JsonReferenceValidator.java | 9 +- .../swagedit/openapi3/utils/Mocks.java | 32 ++ .../validation/ReferenceValidatorTest.xtend | 393 ++++++++++++++++++ .../JsonReferenceProposalProviderTest.xtend | 7 +- .../validation/ReferenceValidatorTest.xtend | 92 +++- 5 files changed, 517 insertions(+), 16 deletions(-) create mode 100644 com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ReferenceValidatorTest.xtend diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceValidator.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceValidator.java index 9f31a299..c2ab9c63 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceValidator.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReferenceValidator.java @@ -103,8 +103,13 @@ private void validateType(JsonDocument doc, URI baseURI, AbstractNode node, Json if (externalDoc != null) { try { Model externalModel = Model.parse(model.getSchema(), externalDoc); - pointer = pointer.substring(pointer.indexOf("#") + 1, pointer.length()); - valueNode = externalModel.find(pointer); + + int pos = pointer.indexOf("#"); + if (pos == -1) { + valueNode = externalModel.getRoot(); + } else { + valueNode = externalModel.find(pointer.substring(pos, pointer.length())); + } if (valueNode == null) { return; diff --git a/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/utils/Mocks.java b/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/utils/Mocks.java index 80bab266..465c9db5 100644 --- a/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/utils/Mocks.java +++ b/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/utils/Mocks.java @@ -3,15 +3,47 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.net.URI; +import java.util.Map; + +import org.eclipse.core.resources.IFile; import org.eclipse.jface.text.ITextSelection; import org.eclipse.jface.text.ITextViewer; import org.eclipse.jface.viewers.ISelectionProvider; import org.eclipse.swt.graphics.Point; +import com.fasterxml.jackson.databind.JsonNode; +import com.reprezen.swagedit.core.json.references.JsonDocumentManager; +import com.reprezen.swagedit.core.json.references.JsonReference; +import com.reprezen.swagedit.core.json.references.JsonReferenceFactory; +import com.reprezen.swagedit.core.model.AbstractNode; import com.reprezen.swagedit.openapi3.editor.OpenApi3Document; public class Mocks { + public static JsonReferenceFactory mockJsonReferenceFactory(final Map entries) { + final IFile file = mock(IFile.class); + when(file.exists()).thenReturn(true); + + return new JsonReferenceFactory() { + public JsonReference create(AbstractNode node) { + JsonReference ref = super.create(node); + ref.setDocumentManager(new JsonDocumentManager() { + @Override + public IFile getFile(URI uri) { + return file; + } + + @Override + public JsonNode getDocument(URI uri) { + return entries.get(uri); + } + }); + return ref; + }; + }; + } + public static ITextViewer mockTextViewer(OpenApi3Document document, int offset) { ITextViewer viewer = mock(ITextViewer.class); ISelectionProvider selectionProvider = mockSelectionProvider(); diff --git a/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ReferenceValidatorTest.xtend b/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ReferenceValidatorTest.xtend new file mode 100644 index 00000000..15e6947f --- /dev/null +++ b/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ReferenceValidatorTest.xtend @@ -0,0 +1,393 @@ +package com.reprezen.swagedit.openapi3.validation + +import com.fasterxml.jackson.databind.JsonNode +import com.reprezen.swagedit.core.json.references.JsonReferenceValidator +import com.reprezen.swagedit.core.validation.Messages +import com.reprezen.swagedit.core.validation.SwaggerError +import com.reprezen.swagedit.openapi3.editor.OpenApi3Document +import com.reprezen.swagedit.openapi3.utils.Mocks +import java.net.URI +import java.util.Map +import org.eclipse.core.resources.IMarker +import org.junit.Test + +import static org.junit.Assert.* +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory +import com.fasterxml.jackson.databind.ObjectMapper +import com.reprezen.swagedit.openapi3.schema.OpenApi3Schema + +class ReferenceValidatorTest { + + val document = new OpenApi3Document(new OpenApi3Schema) + + def validator(Map entries) { + new JsonReferenceValidator(Mocks.mockJsonReferenceFactory(entries)) + } + + @Test + def void shouldValidateReference_To_ValidType() { + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: '#/components/parameters/Valid' + responses: + '200': + description: OK + components: + parameters: + Valid: + name: Valid + in: query + type: string + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val resolvedURI = new URI(null, null, "/components/parameters/Valid") + val errors = validator(#{resolvedURI -> document.asJson}).validate(baseURI, document) + + assertEquals(0, errors.size()) + } + + @Test + def void shouldValidateReference_To_InvalidCorrect() { + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: '#/components/schemas/Valid' + responses: + '200': + description: OK + components: + schemas: + Valid: + type: string + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val resolvedURI = new URI(null, null, "/components/schemas/Valid") + val errors = validator(#{resolvedURI -> document.asJson}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_invalid_reference_type) + )) + } + + @Test + def void shouldValidateReference_To_InvalidDefinition() { + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: '#/components/parameters/Invalid' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val resolvedURI = new URI(null, null, "/components/parameters/Invalid") + val errors = validator(#{resolvedURI -> document.asJson}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_missing_reference) + )) + } + + @Test + def void shouldValidateReference_To_ValidPath() { + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: '#/paths/~1foo~1%7Bbar%7D' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val resolvedURI = new URI(null, null, "/paths/~1foo~1{bar}") + val errors = validator(#{resolvedURI -> document.asJson}).validate(baseURI, document) + + assertEquals(0, errors.size()) + } + + @Test + def void shouldWarnOnInvalidCharacters() { + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: '#/paths/~1foo~1{bar}' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val resolvedURI = new URI(null, null, "/paths/~1foo~1{bar}") + val errors = validator(#{resolvedURI -> document.asJson}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains(new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_invalid_reference))) + } + + @Test + def void shouldValidateReference_To_InvalidPath() { + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: '#/paths/~1foo' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val resolvedURI = new URI(null, null, "/paths/~1foo") + val errors = validator(#{resolvedURI -> document.asJson}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_missing_reference) + )) + } + + @Test + def void shouldValidateReference_To_ExternalFile() { + val other = ''' + openapi: '3.0.0' + ''' + + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: 'other.yaml' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val otherURI = URI.create("other.yaml") + val errors = validator(#{otherURI -> other.asJson}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_invalid_reference_type) + )) + } + + @Test + def void should_Not_ValidateReference_To_Invalid_ExternalFile() { + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: 'other.yaml' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val otherURI = URI.create("other.yaml") + val errors = validator(#{otherURI -> null}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_missing_reference) + )) + } + + @Test + def void shouldValidateReference_To_ExternalFileWithValidType() { + val other = ''' + components: + parameters: + foo: + name: foo + in: query + type: string + ''' + + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: 'other.yaml#/components/parameters/foo' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val otherURI = URI.create("other.yaml#/components/parameters/foo") + val errors = validator(#{otherURI -> other.asJson}).validate(baseURI, document) + + errors.forEach[println(it.message)] + assertEquals(0, errors.size()) + } + + @Test + def void shouldValidateReference_To_ExternalFileFragmentWithInvalidType() { + val other = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + ''' + + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: 'other.yaml#/info/version' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val otherURI = URI.create("other.yaml#/info/version") + val errors = validator(#{otherURI -> other.asJson}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_invalid_reference_type) + )) + } + + @Test + def void should_Not_ValidateReference_To_ExternalFile_WithInvalidFragment() { + val other = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + ''' + + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: 'other.yaml#/info/foo' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val otherURI = URI.create("other.yaml#/info/foo") + val errors = validator(#{otherURI -> other.asJson}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_missing_reference) + )) + } + + @Test + def void should_ProduceError_If_URI_is_Invalid() { + val content = ''' + openapi: '3.0.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: 'contains space ref.yaml#/info/foo' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val errors = validator(#{}).validate(baseURI, document) + + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_missing_reference) + )) + } + + def asJson(String string) { + new ObjectMapper(new YAMLFactory).readTree(string) + } + +} diff --git a/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/assist/JsonReferenceProposalProviderTest.xtend b/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/assist/JsonReferenceProposalProviderTest.xtend index c67b85ea..513b5579 100644 --- a/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/assist/JsonReferenceProposalProviderTest.xtend +++ b/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/assist/JsonReferenceProposalProviderTest.xtend @@ -10,6 +10,9 @@ *******************************************************************************/ package com.reprezen.swagedit.assist +import com.reprezen.swagedit.core.assist.Proposal +import com.reprezen.swagedit.core.assist.contexts.RegexContextType +import com.reprezen.swagedit.core.utils.SwaggerFileFinder.Scope import com.reprezen.swagedit.editor.SwaggerDocument import com.reprezen.swagedit.mocks.Mocks import com.reprezen.swagedit.tests.utils.PointerHelpers @@ -18,10 +21,6 @@ import org.junit.Test import static org.hamcrest.core.IsCollectionContaining.* import static org.junit.Assert.* -import com.reprezen.swagedit.core.utils.SwaggerFileFinder.Scope -import com.reprezen.swagedit.core.assist.Proposal -import com.reprezen.swagedit.core.assist.contexts.ContextType -import com.reprezen.swagedit.core.assist.contexts.RegexContextType class JsonReferenceProposalProviderTest { diff --git a/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/validation/ReferenceValidatorTest.xtend b/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/validation/ReferenceValidatorTest.xtend index 7012f323..5d04b1a8 100644 --- a/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/validation/ReferenceValidatorTest.xtend +++ b/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/validation/ReferenceValidatorTest.xtend @@ -1,18 +1,18 @@ package com.reprezen.swagedit.validation import com.fasterxml.jackson.databind.JsonNode -import com.fasterxml.jackson.databind.ObjectMapper -import com.reprezen.swagedit.Messages -import com.reprezen.swagedit.editor.SwaggerDocument import com.reprezen.swagedit.core.json.references.JsonReferenceValidator +import com.reprezen.swagedit.core.validation.Messages +import com.reprezen.swagedit.core.validation.SwaggerError +import com.reprezen.swagedit.editor.SwaggerDocument import com.reprezen.swagedit.mocks.Mocks +import io.swagger.util.Yaml import java.net.URI import java.util.Map import org.eclipse.core.resources.IMarker import org.junit.Test import static org.junit.Assert.* -import com.reprezen.swagedit.core.validation.SwaggerError class ReferenceValidatorTest { @@ -23,7 +23,37 @@ class ReferenceValidatorTest { } @Test - def void shouldValidateReference_To_ValidDefinition() { + def void shouldValidateReference_To_ValidType() { + val content = ''' + swagger: '2.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: '#/parameters/Valid' + responses: + '200': + description: OK + parameters: + Valid: + name: Valid + in: query + type: string + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val resolvedURI = new URI(null, null, "/definitions/Valid") + val errors = validator(#{resolvedURI -> document.asJson}).validate(baseURI, document) + + assertEquals(0, errors.size()) + } + + @Test + def void shouldValidateReference_To_InvalidCorrect() { val content = ''' swagger: '2.0' info: @@ -47,7 +77,10 @@ class ReferenceValidatorTest { val resolvedURI = new URI(null, null, "/definitions/Valid") val errors = validator(#{resolvedURI -> document.asJson}).validate(baseURI, document) - assertEquals(0, errors.size()) + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_invalid_reference_type) + )) } @Test @@ -183,7 +216,10 @@ class ReferenceValidatorTest { val otherURI = URI.create("other.yaml") val errors = validator(#{otherURI -> other.asJson}).validate(baseURI, document) - assertEquals(0, errors.size()) + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_invalid_reference_type) + )) } @Test @@ -215,7 +251,40 @@ class ReferenceValidatorTest { } @Test - def void shouldValidateReference_To_ExternalFileFragment() { + def void shouldValidateReference_To_ExternalFileWithValidType() { + val other = ''' + parameters: + foo: + name: foo + in: query + type: string + ''' + + val content = ''' + swagger: '2.0' + info: + version: 0.0.0 + title: Simple API + paths: + /foo/{bar}: + get: + parameters: + - $ref: 'other.yaml#/parameters/foo' + responses: + '200': + description: OK + ''' + + document.set(content) + val baseURI = new URI(null, null, null) + val otherURI = URI.create("other.yaml#/parameters/foo") + val errors = validator(#{otherURI -> other.asJson}).validate(baseURI, document) + + assertEquals(0, errors.size()) + } + + @Test + def void shouldValidateReference_To_ExternalFileFragmentWithInvalidType() { val other = ''' swagger: '2.0' info: @@ -243,7 +312,10 @@ class ReferenceValidatorTest { val otherURI = URI.create("other.yaml#/info/version") val errors = validator(#{otherURI -> other.asJson}).validate(baseURI, document) - assertEquals(0, errors.size()) + assertEquals(1, errors.size()) + assertTrue(errors.contains( + new SwaggerError(9, IMarker.SEVERITY_WARNING, Messages.error_invalid_reference_type) + )) } @Test @@ -338,7 +410,7 @@ class ReferenceValidatorTest { } def asJson(String string) { - io.swagger.util.Yaml.mapper().readTree(string) + Yaml.mapper().readTree(string) } } From 4b65e77552a543061b3b61193559aa2ae7420092 Mon Sep 17 00:00:00 2001 From: Guillaume Hillairet Date: Wed, 4 Oct 2017 15:04:49 +0200 Subject: [PATCH 4/5] [#353] Validation of external $ref property values should show error on unexpected object type Fix cache of model documents in DocumentManager. Fix javadoc. --- .../core/hyperlinks/JsonFileHyperlink.java | 4 -- .../json/references/JsonDocumentManager.java | 71 +++++++++++-------- .../core/json/references/JsonReference.java | 5 +- 3 files changed, 44 insertions(+), 36 deletions(-) diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/hyperlinks/JsonFileHyperlink.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/hyperlinks/JsonFileHyperlink.java index af558a65..5a82b897 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/hyperlinks/JsonFileHyperlink.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/hyperlinks/JsonFileHyperlink.java @@ -74,10 +74,6 @@ private IRegion getTarget() throws CoreException { } catch (IOException e) { return null; } - if (doc == null) { - return null; - } - return doc.getRegion(pointer); } diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonDocumentManager.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonDocumentManager.java index aa440c8c..3f08a189 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonDocumentManager.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonDocumentManager.java @@ -10,7 +10,6 @@ *******************************************************************************/ package com.reprezen.swagedit.core.json.references; -import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; import java.net.URL; @@ -54,38 +53,15 @@ public JsonDocumentManager() { * @return JSON tree */ public JsonNode getDocument(URL url) { - if (documents.containsKey(url)) { - return documents.get(url); - } + URL normalized = normalize(url); - JsonNode document; - if (url.getFile().endsWith("json")) { - try { - document = mapper.readTree(url); - } catch (Exception e) { - document = null; - } - } else if (url.getFile().endsWith("yaml") || url.getFile().endsWith("yml")) { - try { - document = yamlMapper.readTree(url); - } catch (IOException e) { - document = null; - } - } else { - // cannot decide which format, so we try both parsers - try { - document = mapper.readTree(url); - } catch (Exception e) { - try { - document = yamlMapper.readTree(url); - } catch (IOException ee) { - document = null; - } - } + if (documents.containsKey(normalized)) { + return documents.get(normalized); } + JsonNode document = parse(normalized); if (document != null) { - documents.put(url, document); + documents.put(normalized, document); } return document; } @@ -93,7 +69,7 @@ public JsonNode getDocument(URL url) { public JsonNode getDocument(URI uri) { try { return getDocument(uri.toURL()); - } catch (MalformedURLException e) { + } catch (IllegalArgumentException | MalformedURLException e) { return null; } } @@ -109,4 +85,39 @@ public IFile getFile(URI uri) { return uri != null ? DocumentUtils.getWorkspaceFile(uri) : null; } + private JsonNode parse(URL url) { + if (url.getFile().endsWith("json")) { + try { + return mapper.readTree(url); + } catch (Exception e) { + return null; + } + } else if (url.getFile().endsWith("yaml") || url.getFile().endsWith("yml")) { + try { + return yamlMapper.readTree(url); + } catch (Exception e) { + return null; + } + } else { + // cannot decide which format, so we try both parsers + try { + return mapper.readTree(url); + } catch (Exception e) { + try { + return yamlMapper.readTree(url); + } catch (Exception ee) { + return null; + } + } + } + } + + private URL normalize(URL url) { + try { + return new URL(url.getProtocol(), url.getHost(), url.getFile()); + } catch (MalformedURLException e) { + return url; + } + } + } diff --git a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReference.java b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReference.java index 901f6411..ed805f08 100644 --- a/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReference.java +++ b/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/json/references/JsonReference.java @@ -79,9 +79,9 @@ public boolean isInvalid() { } /** - * Returns true if the reference points to an existing JSON document and the pointer to an existing node inside that - * document. + * Returns true if the reference cannot be resolved and the pointer points to an inexistent element. * + * @param document * @param baseURI * @return true if the reference can be resolved. */ @@ -100,6 +100,7 @@ public boolean isMissing(JsonDocument document, URI baseURI) { * If the resolution of the referenced node fails, this method returns null. If the pointer does not points to an * existing node, this method will return a missing node (see JsonNode.isMissingNode()). * + * @param document * @param baseURI * @return referenced node */ From 00d892b01b8226a230df00584b5cfaf938d298be Mon Sep 17 00:00:00 2001 From: Guillaume Hillairet Date: Thu, 5 Oct 2017 12:22:20 +0200 Subject: [PATCH 5/5] [#353] Validation of external $ref property values should show error on unexpected object type Fix tests --- .../swagedit/openapi3/validation/ValidatorTest.xtend | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ValidatorTest.xtend b/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ValidatorTest.xtend index 2eed81af..9880909f 100644 --- a/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ValidatorTest.xtend +++ b/com.reprezen.swagedit.openapi3.tests/src/com/reprezen/swagedit/openapi3/validation/ValidatorTest.xtend @@ -134,8 +134,8 @@ class ValidatorTest { paths: {} components: schemas: - MyType1: + type: object properties: property: $ref: "INVALID" @@ -143,8 +143,8 @@ class ValidatorTest { document.set(content) val errors = validator.validate(document, null as URI) + assertEquals(1, errors.size()) - println(errors.map[message]) assertThat( errors, hasItems( @@ -164,8 +164,8 @@ class ValidatorTest { paths: {} components: schemas: - MyType1: + type: object properties: property: $ref: "https://www.reprezen.com/#"