-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add position information to validation items #180
Comments
Upgraded to Photo eclipse installation, default formatting changed. No actual source changes here, just reformatted all source files to avoid confusing future diffs
This demonstrates the API by which positional info will be presented by the validator, i.e. via ValidationItem#getPositionInfo() Only a single validation currently adheres to this interface, namely a check that the `type` property in a Schema is among the allowed types. Working with a model that includes, e.g., a schema with `type: xxx` should threfore produce a validation item with a non-null and correct PositionInfo value. PositionInfo really needs to include information about the source URL, but it currently does not. That will be fixed along the way.
@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 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. |
@ghillairet New staging repos. PositionInfo now provides the URL of the containing document |
@andylowry I think you forgot to add an accessor for the private field I made a local build to have those changes but position is always null. |
@ghillairet Sorry about the accessor... comes from using a test that only relies on 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:
Here's my
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? |
@ghillairet New KZOP staging repo has new accessor: https://oss.sonatype.org/content/repositories/comreprezenkaizen-1053/ |
@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); |
Add missing positions when doing URL validations
The Validation API has been significantly simplified by: * Dropping the distinction between interface-level validatiosn and implementation-level validations. This is not currently supportable without a great deal of extra effort if we are to attach position info to validation messages, since the means of obtaining that info is built into *our* implementation. Of course, this also puts the whole idea of users substituting their own implementations in jeopardy as well, but it's not clear that was ever something that would work well. Maybe something for future reconsideration. * Now that we've done the above, we can focus entirely on Overlay<?> values in the validation APIs. This makes things much easier, since all the non-API methods supported by overlay objects are easily available. * Got rid of the injection mechanism for validators. This hurts performance and is of questionable value with the tearing down of the wall between intf and impl validations * Altered the way validators get ahold of the values to be validated and the ValidationResults object to add items to. The former is now the sole argument to the 'validate' method on a validator, which then stashes it in a protected variable for all to see. The results are managed in a ThreadLocal that needs to be established by the code that invokes validation of the root object, and is then obtained as needed by validators. There's an AutoClosable to allocate this in a way that guarantees it'll be cleared afteward (try-resource statement) Validations for the generated types have not been updated. That's the next stage in this work. Old validation code is untouched, for now (purely for reference purposes) in 'old' packages (e.g. *.val => *.old.val, and similarly)
All validator classes now operate in the improved validation context, and there are no more "overlay" validators (the ones that formerly needed to dig below the surface of the generated API). A number of other improvements to ValidationBase were made. The validations themselves have chnaged very little, except that fields that formerly had no validation (e.g. optional, unconstrained strings) nre now validated so that the underlying JSON node type is checked for compatibility.
JsonOverlay now defines field name constants in impl files, e.g. "F_description" for a "description" field. These are now used rather than literal strings in all the validators, to prevent typos. Astonishingly, there wree none of these in the existing code, despite sparse test coverage! (Tests were the only other way to spot these.)
@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. |
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. |
[#180] Incorporate position information into validation items
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). |
This depends on completion of RepreZen/JsonOverlay#27
The text was updated successfully, but these errors were encountered: