Skip to content

Commit

Permalink
[#9] User-friendly validation messages
Browse files Browse the repository at this point in the history
Updated some of the messages to be easier to understand by the user.
  • Loading branch information
ghillairet committed Oct 29, 2015
1 parent 8a31462 commit 9988e42
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -38,7 +39,7 @@ class ValidationMessageTest {
/p:
get:
#validation error marker
parameters: 2
parameters: 2
responses:
'200':
description: OK
Expand All @@ -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'
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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'
Expand All @@ -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)
}

}
7 changes: 7 additions & 0 deletions com.reprezen.swagedit/src/com/reprezen/swagedit/Messages.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
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;
import org.eclipse.jface.text.IDocument;
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 {

Expand All @@ -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) {
Expand All @@ -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) {

This comment has been minimized.

Copy link
@tfesenko

tfesenko Oct 30, 2015

Member

rewrite makes me think that it has something to do with persistence - saving to a file or database. What about rephrase?

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<String> 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;
Expand Down

0 comments on commit 9988e42

Please sign in to comment.