Skip to content

Commit

Permalink
[#32] Incorrent validation marker location
Browse files Browse the repository at this point in the history
  • Loading branch information
ghillairet committed Dec 3, 2015
1 parent 3805a32 commit 41f6801
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.eclipse.core.resources.IMarker
import org.junit.Test

import static org.junit.Assert.assertEquals
import static org.junit.Assert.assertTrue

class ValidatorTest {

Expand Down Expand Up @@ -149,4 +150,31 @@ class ValidatorTest {
assertEquals(5, error.getLine())
}

@Test
def void shouldReturnCorrectErrorPositionOnPathWithHierarchy() throws IOException {
// invalid property schema
val content = '''
swagger: '2.0'
info:
version: 0.0.0
title: Simple API
paths:
/foo/{bar}:
get:
responses:
'200':
description: OK
schema:
'''

document.set(content)
val errors = validator.validate(document)

assertEquals(5, errors.size())

errors.forEach[
assertTrue(it.line == 10 || it.line == 11)
assertEquals(IMarker.SEVERITY_ERROR, it.level)
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.dadacoalition.yedit.YEditLog;
import org.dadacoalition.yedit.editor.YEdit;
Expand Down Expand Up @@ -100,7 +101,7 @@ public void doSaveAs() {
protected void checkErrors() {
final IFile file = ((IFileEditorInput) getEditorInput()).getFile();
final IDocument document = getDocumentProvider().getDocument(getEditorInput());
final List<SwaggerError> errors = validator.validate((SwaggerDocument) document);
final Set<SwaggerError> errors = validator.validate((SwaggerDocument) document);

for (SwaggerError error : errors) {
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
package com.reprezen.swagedit.validation;

import java.util.Iterator;
import java.util.Objects;

import org.eclipse.core.resources.IMarker;
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;

Expand Down Expand Up @@ -38,8 +36,8 @@ public static SwaggerError create(JsonMappingException exception) {
return new SwaggerError(IMarker.SEVERITY_ERROR, exception.getMessage(), line);
}

public static SwaggerError create(ProcessingMessage error, int line) {
return new SwaggerError(getLevel(error), rewriteError(error.asJson()), line);
public static SwaggerError create(JsonNode error, int line) {
return new SwaggerError(getLevel(error), rewriteError(error), line);
}

public static SwaggerError create(ParserException e) {
Expand All @@ -64,8 +62,6 @@ private static String rewriteError(JsonNode error) {
return rewriteTypeError(error);
case "enum":
return rewriteEnumError(error);
case "oneOf":
return rewriteOneOfError(error);
case "additionalProperties":
return rewriteAdditionalProperties(error);
case "required":
Expand All @@ -87,26 +83,6 @@ private static String rewriteAdditionalProperties(JsonNode error) {
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");
Expand All @@ -129,18 +105,18 @@ private static String rewriteEnumError(JsonNode error) {
return String.format(Messages.error_notInEnum, value.asText(), enumString);
}

protected static int getLevel(ProcessingMessage message) {
if (message == null) {
protected static int getLevel(JsonNode message) {
if (message == null || !message.has("level")) {
return IMarker.SEVERITY_INFO;
}

final LogLevel level = message.getLogLevel();
final String level = message.get("level").asText();

switch (level) {
case ERROR:
case FATAL:
case "error":
case "fatal":
return IMarker.SEVERITY_ERROR;
case WARNING:
case "warning":
return IMarker.SEVERITY_WARNING;
default:
return IMarker.SEVERITY_INFO;
Expand All @@ -164,4 +140,19 @@ public String toString() {
return "{ (level=" + getLevel() + ") " + getMessage() + " at line " + getLine() + " }";
}

@Override
public boolean equals(Object obj) {
if (obj instanceof SwaggerError) {
return Objects.equals(level, ((SwaggerError) obj).level) &&
Objects.equals(line, ((SwaggerError) obj).line) &&
Objects.equals(message, ((SwaggerError) obj).message);
}

return super.equals(obj);
}

@Override
public int hashCode() {
return message.hashCode();
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package com.reprezen.swagedit.validation;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;

import org.dadacoalition.yedit.YEditLog;
import org.eclipse.core.resources.IMarker;
Expand All @@ -15,6 +16,7 @@
import org.yaml.snakeyaml.nodes.ScalarNode;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.dataformat.yaml.snakeyaml.parser.ParserException;
import com.github.fge.jsonschema.core.exceptions.ProcessingException;
import com.github.fge.jsonschema.core.report.ProcessingMessage;
Expand All @@ -33,17 +35,17 @@ public class Validator {
private static final SwaggerSchema schema = new SwaggerSchema();

/**
* Returns a list or errors if validation fails.
* Returns a list or errors if validation fails.
*
* This method accepts as input a swagger YAML document and
* validates it against the swagger JSON Schema.
* This method accepts as input a swagger YAML document and validates it
* against the swagger JSON Schema.
*
* @param content
* @return list or errors
* @throws IOException
* @throws ParserException
* @throws IOException
* @throws ParserException
*/
public List<SwaggerError> validate(SwaggerDocument document) {
public Set<SwaggerError> validate(SwaggerDocument document) {
JsonNode jsonContent = null;
try {
jsonContent = document.asJson();
Expand All @@ -52,64 +54,92 @@ public List<SwaggerError> validate(SwaggerDocument document) {
}

if (jsonContent == null) {
return Collections.singletonList(new SwaggerError(
IMarker.SEVERITY_ERROR,
"Unable to read content. It may be invalid YAML"));
return Collections.singleton(
new SwaggerError(IMarker.SEVERITY_ERROR, "Unable to read content. It may be invalid YAML"));
} else {
final Node yaml = document.getYaml();
ProcessingReport report = null;

try {
report = schema.getSchema().validate(jsonContent);
report = schema.getSchema().validate(jsonContent, true);
} catch (ProcessingException e) {
final ProcessingMessage pm = e.getProcessingMessage();
final int line = getLine(pm, yaml);
final int line = getLine(pm.asJson(), yaml);

return Collections.singletonList(SwaggerError.create(pm, line));
return Collections.singleton(SwaggerError.create(pm.asJson(), line));
}

return create(report, document.getYaml());
}
}

private List<SwaggerError> create(ProcessingReport report, Node yamlTree) {
final List<SwaggerError> errors = new ArrayList<>();
private Set<SwaggerError> create(ProcessingReport report, Node yamlTree) {
final Set<SwaggerError> errors = new HashSet<>();
if (report != null) {
for (Iterator<ProcessingMessage> it = report.iterator(); it.hasNext();) {
final ProcessingMessage next = it.next();
final int line = getLine(next, yamlTree);
final Set<JsonNode> reportNodes = collectReports(next.asJson());

errors.add(SwaggerError.create(next, line));
for (JsonNode reportNode : reportNodes) {
errors.add( SwaggerError.create(reportNode, getLine(reportNode, yamlTree)) );
}
}
}

return errors;
}

private Set<JsonNode> collectReports(JsonNode message) {
final Set<JsonNode> allReports = new HashSet<>();

if (message.has("reports")) {
final JsonNode reports = message.get("reports");
allReports.add(((ObjectNode) message).without("reports"));

for (Iterator<String> it = reports.fieldNames(); it.hasNext();) {
String key = it.next();
JsonNode value = reports.get(key);

if (value.isArray()) {
for (JsonNode current : value) {
allReports.addAll(collectReports(current));
allReports.add(((ObjectNode) current).without("reports"));
}
} else if (value.isObject()) {
allReports.addAll(collectReports(value));
allReports.add(((ObjectNode) value).without("reports"));
}
}
} else {
allReports.add(message);
}

return allReports;
}

/*
* Returns the line for which an error message has been produced.
*
* The error message is a JSON object that contains the path to
* the invalid node. The path is accessible via instance.pointer.
* The path is in the forms:
* - /{id}
* - /{id}/~{nb}
* The error message is a JSON object that contains the path to the invalid
* node. The path is accessible via instance.pointer. The path is in the
* forms:
* - /{id}
* - /{id}/~{nb}
* - /{id}/~{id2}
*
* The line number is computed by after parsing the yaml content with
* the yaml parser. This latter returns a tree of Node, each corresponding
* to a yaml construct and including a position.
* The line number is computed by after parsing the yaml content with the
* yaml parser. This latter returns a tree of Node, each corresponding to a
* yaml construct and including a position.
*
* The Node matching the path is found by the methods findNode().
* The Node matching the path is found by the methods findNode().
*/
private int getLine(ProcessingMessage message, Node yamlTree) {
final JsonNode m = message.asJson();
if (!m.has("instance") || !m.get("instance").has("pointer"))
private int getLine(JsonNode error, Node yamlTree) {
if (!error.has("instance") || !error.get("instance").has("pointer"))
return 1;

String path = m.get("instance").get("pointer").asText();
String path = error.get("instance").get("pointer").asText();

if (path == null || path.isEmpty())
if (path == null || path.isEmpty())
return 1;

path = path.substring(1, path.length());
Expand Down Expand Up @@ -145,12 +175,12 @@ private Node findNode(MappingNode root, List<String> paths) {
}

final List<String> next = paths.subList(1, paths.size());
// ~1 seems to be used to escape /
if (path.startsWith("~1")) {
path = "/" + path.substring(2, path.length());
// ~1 is use to escape /
if (path.contains("~1")) {
path = path.replaceAll("~1", "/");
}

for (NodeTuple child: root.getValue()) {
for (NodeTuple child : root.getValue()) {
if (child.getKeyNode() instanceof ScalarNode) {
ScalarNode scalar = (ScalarNode) child.getKeyNode();

Expand Down

0 comments on commit 41f6801

Please sign in to comment.