From 9988e4235418b62b5dcfe0cca028b65122884cb7 Mon Sep 17 00:00:00 2001 From: ghillairet Date: Thu, 29 Oct 2015 14:29:29 +0100 Subject: [PATCH] [#9] User-friendly validation messages Updated some of the messages to be easier to understand by the user. --- .../tests/ValidationMessageTest.xtend | 44 +++++++-- .../src/com/reprezen/swagedit/Messages.java | 7 ++ .../com/reprezen/swagedit/messages.properties | 8 +- .../swagedit/validation/SwaggerError.java | 89 +++++++++++++++++-- 4 files changed, 133 insertions(+), 15 deletions(-) diff --git a/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/tests/ValidationMessageTest.xtend b/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/tests/ValidationMessageTest.xtend index 46c0f229..ae808d5e 100644 --- a/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/tests/ValidationMessageTest.xtend +++ b/com.reprezen.swagedit.tests/src/com/reprezen/swagedit/tests/ValidationMessageTest.xtend @@ -24,10 +24,11 @@ class ValidationMessageTest { val error = errors.get(0) assertEquals(expectedMessage, error.message) } - + @Test def testMessage_additionalItems_notAllowed() { - var expected = 'instance type (integer) does not match any allowed primitive type (allowed: ["array"])' + // previous message 'instance type (integer) does not match any allowed primitive type (allowed: ["array"])' + var expected = 'value of type integer is not allowed, value should be of type array' // parameters should contain an array of object val content = ''' swagger: '2.0' @@ -38,7 +39,7 @@ class ValidationMessageTest { /p: get: #validation error marker - parameters: 2 + parameters: 2 responses: '200': description: OK @@ -49,7 +50,8 @@ class ValidationMessageTest { @Test def testMessage_typeNoMatch() { - var expected = 'instance type (integer) does not match any allowed primitive type (allowed: ["object"])' + // previous message 'instance type (integer) does not match any allowed primitive type (allowed: ["object"])' + var expected = 'value of type integer is not allowed, value should be of type object' // responses should contain an object val content = ''' swagger: '2.0' @@ -68,7 +70,8 @@ class ValidationMessageTest { @Test def testMessage_notInEnum() { - val expected = 'instance value ("foo") not found in enum (possible values: ["http","https","ws","wss"])' + // previous message 'instance value ("foo") not found in enum (possible values: ["http","https","ws","wss"])' + val expected = 'value foo is not allowed, value should be one of "http", "https", "ws", "wss"' val content = ''' swagger: '2.0' info: @@ -91,7 +94,14 @@ class ValidationMessageTest { @Test def testMessage_oneOf_fail() { - val expected = 'instance failed to match exactly one schema (matched 0 out of 2)' + // previous message 'instance failed to match exactly one schema (matched 0 out of 2)' + val expected = + ''' + value of type integer is not allowed, value should be of type string + object has properties "description" which are not allowed + object has missing required properties "$ref" + ''' + val content = ''' swagger: '2.0' info: @@ -111,7 +121,9 @@ class ValidationMessageTest { @Test def testMessage_additionalProperties_notAllowed() { - val expected = 'object instance has properties which are not allowed by the schema: ["description"]' + // previous message 'object instance has properties which are not allowed by the schema: ["description"]' + val expected = 'object has properties "description" which are not allowed' + // description should be 2 spaces forward val content = ''' swagger: '2.0' @@ -130,4 +142,22 @@ class ValidationMessageTest { assertModelHasValidationError(expected, content) } + @Test + def testMessage_object_missingMembers() { + val expected = 'object has missing required properties "title"' + val content = ''' + swagger: '2.0' + info: + version: 0.0.0 + paths: + /: + get: + responses: + '200': + description: OK + ''' + + assertModelHasValidationError(expected, content) + } + } \ No newline at end of file diff --git a/com.reprezen.swagedit/src/com/reprezen/swagedit/Messages.java b/com.reprezen.swagedit/src/com/reprezen/swagedit/Messages.java index 59ce584a..86a80cb1 100644 --- a/com.reprezen.swagedit/src/com/reprezen/swagedit/Messages.java +++ b/com.reprezen.swagedit/src/com/reprezen/swagedit/Messages.java @@ -6,9 +6,16 @@ public class Messages extends NLS { private static final String BUNDLE_NAME = "com.reprezen.swagedit.messages"; + // UI public static String swagger; public static String wizard_description; + // errors + public static String error_typeNoMatch; + public static String error_notInEnum; + public static String error_additional_properties_not_allowed; + public static String error_required_properties; + static { NLS.initializeMessages(BUNDLE_NAME, Messages.class); } diff --git a/com.reprezen.swagedit/src/com/reprezen/swagedit/messages.properties b/com.reprezen.swagedit/src/com/reprezen/swagedit/messages.properties index d42a2b9b..0b596d2f 100644 --- a/com.reprezen.swagedit/src/com/reprezen/swagedit/messages.properties +++ b/com.reprezen.swagedit/src/com/reprezen/swagedit/messages.properties @@ -3,4 +3,10 @@ swagger = Swagger # wizard -wizard_description = This wizard creates a new file with *.yaml extension that can be opened by the swagger editor. \ No newline at end of file +wizard_description = This wizard creates a new file with *.yaml extension that can be opened by the swagger editor. + +# errors +error_typeNoMatch = value of type %s is not allowed, value should be of type %s +error_notInEnum = value %s is not allowed, value should be one of %s +error_additional_properties_not_allowed = object has properties %s which are not allowed +error_required_properties = object has missing required properties %s \ No newline at end of file diff --git a/com.reprezen.swagedit/src/com/reprezen/swagedit/validation/SwaggerError.java b/com.reprezen.swagedit/src/com/reprezen/swagedit/validation/SwaggerError.java index d5f03644..564690c8 100644 --- a/com.reprezen.swagedit/src/com/reprezen/swagedit/validation/SwaggerError.java +++ b/com.reprezen.swagedit/src/com/reprezen/swagedit/validation/SwaggerError.java @@ -1,5 +1,7 @@ package com.reprezen.swagedit.validation; +import java.util.Iterator; + import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IMarker; import org.eclipse.core.runtime.CoreException; @@ -7,9 +9,12 @@ import org.yaml.snakeyaml.parser.ParserException; import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.dataformat.yaml.snakeyaml.scanner.ScannerException; import com.github.fge.jsonschema.core.report.LogLevel; import com.github.fge.jsonschema.core.report.ProcessingMessage; +import com.google.common.base.Joiner; +import com.reprezen.swagedit.Messages; public class SwaggerError { @@ -36,8 +41,8 @@ public static SwaggerError create(JsonMappingException exception) { return new SwaggerError(IMarker.SEVERITY_ERROR, exception.getMessage(), line); } - public static SwaggerError create(ProcessingMessage message, int line) { - return new SwaggerError(getLevel(message), message.getMessage(), line); + public static SwaggerError create(ProcessingMessage error, int line) { + return new SwaggerError(getLevel(error), rewriteError(error.asJson()), line); } public static SwaggerError create(ParserException e) { @@ -52,11 +57,81 @@ public static SwaggerError create(ScannerException e) { return new SwaggerError(IMarker.SEVERITY_ERROR, e.getMessage(), e.getProblemMark().getLine() + 1); } -// private static String newMessage(ProcessingMessage message) { -// JsonNode json = message.asJson(); -// -// } - + private static String rewriteError(JsonNode error) { + if (error == null || !error.has("keyword")) { + return ""; + } + + switch (error.get("keyword").asText()) { + case "type": + return rewriteTypeError(error); + case "enum": + return rewriteEnumError(error); + case "oneOf": + return rewriteOneOfError(error); + case "additionalProperties": + return rewriteAdditionalProperties(error); + case "required": + return rewriteRequiredProperties(error); + default: + return error.get("message").asText(); + } + } + + private static String rewriteRequiredProperties(JsonNode error) { + JsonNode missing = error.get("missing"); + + return String.format(Messages.error_required_properties, Joiner.on(", ").join(missing)); + } + + private static String rewriteAdditionalProperties(JsonNode error) { + final JsonNode unwanted = error.get("unwanted"); + + return String.format(Messages.error_additional_properties_not_allowed, Joiner.on(", ").join(unwanted)); + } + + private static String rewriteOneOfError(JsonNode node) { + final JsonNode reports = node.get("reports"); + String result = ""; + + for (Iterator it = reports.fieldNames(); it.hasNext();) { + final String key = it.next(); + final JsonNode value = reports.get(key); + + if (value.isObject()) { + result += rewriteError(value) + "\n"; + } else if (value.isArray()) { + for (JsonNode one: value) { + result += rewriteError(one) + "\n"; + } + } + } + + return result; + } + + private static String rewriteTypeError(JsonNode error) { + final JsonNode found = error.get("found"); + final JsonNode expected = error.get("expected"); + + String expect; + if (expected.isArray()) { + expect = expected.get(0).asText(); + } else { + expect = expected.asText(); + } + + return String.format(Messages.error_typeNoMatch, found.asText(), expect); + } + + private static String rewriteEnumError(JsonNode error) { + final JsonNode value = error.get("value"); + final JsonNode enums = error.get("enum"); + final String enumString = Joiner.on(", ").join(enums); + + return String.format(Messages.error_notInEnum, value.asText(), enumString); + } + protected static int getLevel(ProcessingMessage message) { if (message == null) { return IMarker.SEVERITY_INFO;