-
Notifications
You must be signed in to change notification settings - Fork 13
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
Adapt Editor to use KaiZen OpenApi Parser for parsing, validation, and content-assist support #451
Comments
Slack conversation with some relevant discussion: Andy Lowry [6:21 PM] Ted Epstein [6:20 PM] Andy Lowry [6:21 PM]
Ted Epstein [6:22 PM] Andy Lowry [6:22 PM] Ted Epstein [6:22 PM] Andy Lowry [6:23 PM] Ted Epstein [6:24 PM] Andy Lowry [6:25 PM] Ted Epstein [6:26 PM] Andy Lowry [6:28 PM] Ted Epstein [6:32 PM] Andy Lowry [6:36 PM]
Ted Epstein [6:38 PM] Andy Lowry [6:40 PM] Ted Epstein [6:41 PM] Andy Lowry [6:41 PM] Ted Epstein [6:44 PM] Andy Lowry [6:49 PM] |
@andylowry , your last comment made me think there may be some misunderstanding about the problem with elided properties. Let's say the editor is opened with the insertion point here: The user invokes content assist, and KZOE has to populate the suggestion list with property names that are not already present. In this (obviously contrived) case, But IIUC, the presence of Maybe this problem isn't big enough to worry about. Worst case is that I just wanted to make sure we had a shared understanding of the basic problem. |
@tedepstein My point is that, in iterating over the top-level properties, you're going to see all their parent paths. You'll find some that are simple, like You'll also, at top level, find a field named It's not the easiest thing in the world, but let's assume you can figure out where the nearest point in your editor is that's an ancestor of your current editing point and corresponds to a JO object. You can then look at that JO object's It's not the most straightforward problem, but it seems reasonably tractable. |
@andylowry , OK, but in the example I gave, would the |
KZOE is the editor. It can see what's in the JSON. If it sees a |
@tedepstein Actually, in re-reading my prior comment, I see I wasn't thinking of an algorithmic solution to this, but rather something where KZOE would know, by special-case code, how to deal with |
@tedepstein References will be another potentially tricky area. There's lots of support for getting information about references from KZOP, but I imagine KZOE will mostly be looking at the YAML to detect references. The other side is offering references as recommendations. There's no way, currently, to identify where references are allowed within a model. That would need to be added to the types file. Currently, the parser just accepts them anywhere. |
It seems like the JO object graph replaces the current "model" that Guillaume has built to support context-sensitive code assist and validation. But some algorithms need information that isn't available directly through the current or proposed JO API. KZOE needs to go directly to the YAML (SnakeYAML) or JSON (Jackson) layer in these cases. I don't know to what extent this is currently problematic, or suboptimal in terms of code simplicity, memory and processing efficiency. And I don't know whether this gets better or worse with the introduction of KZOP/JO. @ghillairet , maybe you can offer some analysis here. Are there any cases where it would be helpful for |
@tedepstein The problem with providing direct access to the original Internally, I keep a registry that links every JsonNode to its JO object. The registry is used to ensure that multiple references that resolve to the same JsonNode end up being represented by the same JO object. Exposing that registry in some fashion might be helpful, but only if it's guaranteed to continue to be accurate, and that's not the case with mutation API. (The registry is currently used only during parse, so no problem there). |
@tedepstein @andylowry I started experimenting with KZOP in branch https://github.com/RepreZen/KaiZen-OpenAPI-Editor/tree/task/kzop-parser-wip You can see a simplistic content assist being done in https://github.com/RepreZen/KaiZen-OpenAPI-Editor/blob/task/kzop-parser-wip/com.reprezen.swagedit.core/src/com/reprezen/swagedit/core/assist/OpenApiProposalProvider.java It shows that content assist could be doable that way, but I will need to wait for new version of KZOP for that. |
@ghillairet Re the validation issues, that's a known deficiency that will be addressed. In general, there's a need to go carefully through the validation logic to make sure it reflects every requirement appearing in the specification. I think the "extra properties" check is the primary (and perhaps only) general hole in the current validation; mostly the rest will be mistakes in the current validation logic, or specific requirements that are not checked. |
Also related, I found this bug in KZOP RepreZen/KaiZen-OpenApi-Parser#168 that prevents finding some elements during content assist. |
@ghillairet Right, thanks for reporting that. I'll check if it's still a problem after the refactoring is complete, and fix it if so. |
Expected benefits include:
Currently, KZOP does not understand Swagger-OpenAPI 2.0, so schema-based approach will need to be retained until that can be added to KZOP. It may make sense, even then, to leave the existing implementation as a configurable option.
The text was updated successfully, but these errors were encountered: