RepreZen / KaiZen-OpenApi-Parser

High-performance Parser, Validator, and Java Object Model for OpenAPI 3.x
130 stars 31 forks source link

Add position information to validation items #180

Open andylowry opened 6 years ago

andylowry commented 6 years ago

This depends on completion of https://github.com/RepreZen/JsonOverlay/issues/27

andylowry commented 6 years ago

@ghillairet I've still got a fair bit of work to do, but you can exercise KZOP with position-aware validation items using two OSS staging repositories. the URLs are:

When looking at a validation item following a parse, you can use ValidationItem#getPositionInfo() to get position information. There currently isn't a document URL in the position info, which there needs to be. I'll add that shortly. But you do get character offset, line #, and column # for both the beginning and end of the region occupied by the identified JSON value. And you also get the JsonPointer that addresses the element in its file.

Let me know if there's more you need in order to try to integrate this into KZOE. I'll try to be as responsive as possible, given that I'm vacationing with family this week.

Currently this should only be tested with YAML files, which of course makes sense with KZOP. I haven't tested position capture for JSON files, and it's probably buggy.

andylowry commented 6 years ago

@ghillairet New staging repos. PositionInfo now provides the URL of the containing document

ghillairet commented 6 years ago

@andylowry I think you forgot to add an accessor for the private field pos here https://github.com/RepreZen/KaiZen-OpenApi-Parser/blob/task/180/kaizen-openapi-parser/src/main/java/com/reprezen/kaizen/oasparser/val/ValidationResults.java#L140. Also it looks like the build for JsonOverlay here https://oss.sonatype.org/content/repositories/comreprezenjsonoverlay-1030/ does not contain changes to add position infos.

I made a local build to have those changes but position is always null.

andylowry commented 6 years ago

@ghillairet Sorry about the accessor... comes from using a test that only relies on toString. I'll push changes to that shortly.

But I'm not seeing your issue with the position info always coming out null.

For example, the following program shows position info both in the context of a validation item and when accessed directly from the parsed model:

package test;

import com.reprezen.jsonoverlay.Overlay;
import com.reprezen.kaizen.oasparser.OpenApi3Parser;
import com.reprezen.kaizen.oasparser.model3.OpenApi3;
import com.reprezen.kaizen.oasparser.val.ValidationResults.ValidationItem;

public class Test {

    public static void main(String[] args) throws Exception {
        OpenApi3 model = new OpenApi3Parser().parse(Test.class.getResource("test.yaml"), true);
        for (ValidationItem item : model.getValidationItems()) {
            System.out.println(item);
        }
        System.out.println(Overlay.of(model).getPositionInfo().get().toString(false));
    }
}

Here's my test.yaml file:

---
openapi: 3.0.0
info:
  version: "1.0"
  title: Foo

paths:
  /foo:
    get:
      responses:
        200:
          description: "OK"
          content:
            default:
              schema:
                type: xxx

Can you check your "Maven Dependencies" in Eclipse project view to see that you actually are getting v3.0.2.201808131204 from the staging repo?

andylowry commented 6 years ago

@ghillairet New KZOP staging repo has new accessor:

https://oss.sonatype.org/content/repositories/comreprezenkaizen-1053/

ghillairet commented 6 years ago

@andylowry Ok thanks. I'm getting the positions now. It's because I was using:

new OpenApi3Parser().parse(tree, url, true);

and not

new OpenApi3Parser().parse(url, true);
andylowry commented 6 years ago

@ghillairet Latest staging repos:

This includes a significantly simplified validation framework, and all validation items should now come with position info.

@tfesenko 's updates for v3.0.1 of the spec are not included... I'll manually merge them next. (automated merge is certain to fail miserably because the signatures of all the methods in the validation framework have changed quite a bit).

Still, I think this will give you a pretty good basis for more broadly testing integration in KZOE.

andylowry commented 6 years ago

@ghillairet @tedepstein

It turns out that the Jackson JSON parser provides extremely broken token location information, and has done so for many versions. It makes extracting usable location information pretty much impossible, IMO.

If I use the YAML parser to parse JSON, I get far better results. As of v1.2, YAML is (or claims to be) a true superset of JSON,. (There's a caveat that JSON doesn't explicitly prohibit duplicate keys in an object, while YAML does. But JSON also doesn't say what a parser should do with duplicate keys, so the argument is that a YAML parser's behavior in this situation - flagging as an error - is in any case preferable to a JSON parser that probably silently accepts it and chooses one of the values in some implentation-dependent fashion.)

My feeling is that we should just parse JSON and YAML using the Jackson YAML parser, rather than either of the other options:

For now I'll go with my recommended approach, but if that sounds problematic we can discuss other options.

pjroth commented 4 years ago

This would have helped me out today, I got a ValidationItem that only showed in toString representation, "List may not be empty" (I had a missing "responses" section on one of my endpoints).