Syndesis: apicurio - warnings are not updated when changed via gui and saved

[mcada]

See also https://issues.jboss.org/browse/ENTESB-11443

This is a...

[ ] Feature request
[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Documentation issue or request

The problem

When adding custom API connector and editing it in apicurio, operation numbers are refreshed, but warnings are not.

Expected behavior

Warnings from apicur gui should be propagated out when saved

Screenshot

Tasks involved / Steps to Reproduce

1. From customization page update an OpenAPI file with warnings
2. Go next and click review/edit button to open apicurio gui
3. Solve at least one warning and save, you will be returned to overview page
4. Note that warnings number was not updated

[tplevko]

@mcada seems like this should be fixed by now in https://github.com/syndesisio/syndesis/pull/3483 could you please verify?

[mcada]

@elvisisking I tried it with syndesis master branch --local installation and it is not fixed.

[elvisisking]

Not sure what could be going on @mcada. I just got latest from master and did this:

1. From the Customization page click the Create API Connector button to start the upload wizard.
2. On the Upload OpenAPI Specification step of the wizard choose the beer-api_2.0.json file.
3. Click the Next button to advance to the Review Actions step.
4. On the Review Actions step notice there are 1 error and 8 warnings.
5. Click the Review/Edit button to open the ApiCurio editor.
6. In the ApiCurio editor, delete the Delete method at path /beers/{beerId}.
7. Click Save button to take you back to the Review Actions step.
8. Notice that the number of warnings has decreased by one and is now seven.

Maybe let me know what steps and file you are using and I will give that a try. Thanks!

[mcada]

I use this file in my test: https://github.com/syndesisio/syndesis-qe/blob/master/ui-tests/src/test/resources/swagger/connectors/invalid/kie-server-swagger.json

I do not delete whole operation, I fix one warning:

But I also tried to delete one operation with warnings and warnings still were not updated.
Syndesis version:

Syndesis:
1.5-SNAPSHOT
Build ID:
57e59879-f9cc-47b3-8fcf-cba3c42cefdb
Commit ID:
86000159849c72a7d1d07eca5cf8385045fac8f2

[elvisisking]

@mcada Can you try a file with a bit less warnings (like the beer or petstore spec)? Then try and delete one of the offending methods in editor. Hopefully you will see the update.

I am seeing the same results that you are fixing the warning you have in the video above. It looks like the warnings in the editor are not exactly the same as those in the Review Actions step. However when I delete the POST operation at /server/containers/{id}/cases/{caseDefId}/instances the number of warnings does decrease by one after saving the editor.

[tplevko]

@elvisisking shouldn't the behavior be consistent across all OpenApi specifications?

[elvisisking]

Of course @tplevko! My point to @mcada above was that I believe the review is being re-run but is hard to verify using that specific test spec. I believe this is due to different validation means being done by the ApiCurio editor and by Syndesis (which is done when the ApiCurio editor closes). I would need to confirm that though. When testing with a spec that has way fewer errors and warnings, it should be much more obvious if the errors and warnings were updated when the ApiCurio editor is saved and closed.

[mcada]

@elvisisking you did not share that beer-api and I don't know where it comes from, I found one in yaml so I converted it into json and the results are quite interesting. This is what I used.
It has 8 warnings and 1 error, but in apicurio it has nothing. When I changed summary of one method to be longer than 120 characters, one warning appeared in apicurio, warning number was not changed outside.
I think we should use the same analysis of errors/warnings both from syndesis and apicurio.

[elvisisking]

After doing some research I believe I've tracked down how validation is being done @mcada. The issue is that the validation of the OpenAPI spec is being done differently depending on where at in the OpenAPI upload wizard the user is.

Here are the two cases:

• When the _Apicurio_ editor is open, the _Apicurio_ component is doing the validation and is using the oai-ts-core TypeScript library. (@EricWittmann)
• When the _Apicurio_ editor is not open, _Syndesis_ is doing the validation and uses the SyndesisSwaggerValidationRules Java class. (@zregvart)

So there is a potential the user will see different errors/warnings when the _Apicurio_ editor is open and when it is not.

[EricWittmann]

This discrepancy was identified and discussed early on in the process of designing the integration between Syndesis and Apicurio. I've mentioned it a few times in various meetings/emails but I don't think it made the cut for the 7.1 release.

@elvisisking is absolutely right - Apicurio editor does its own validation using the oai-ts-core library. What we should do (IMO) is replace SyndesisSwaggerValidationRules with a oai-ts-core based validation so that the same rules are applied in both places. The current inconsistency is weird for users.

I've offered before and will do so again here - I'm happy to help port whatever custom rules might exist in SyndesisSwaggerValidationRules to Typescript in oai-ts-core so that they can be run in both Apicurio and Syndesis. Note that once we switch over to oai-ts-core, the validation rules can be run in the browser or on the server (e.g. using Nashorn). Some additional functionality is needed in oai-ts-core - namely the ability to customize the set of validation rules that get run and the severity assigned to problems generated for each validation rule. These changes are needed regardless of whether oai-ts-core is used in Syndesis, and have been on my roadmap for awhile.

[dongniwang]

I added the notif/uxd label so @syndesisio/uxd is being notified on the progress of this. Adding the tag doesn't mean that there's UX work involved for this issue.

[elvisisking]

Have you seen JEP 335: Deprecate the Nashorn JavaScript Engine @EricWittmann? I guess there might be other alternatives like GraalJS.

[EricWittmann]

Yeah that's a bummer. When it happens we'll have to switch to another engine... graal or rhino. I haven't explored the options yet.

I'm actually using java.script.ScriptEngine although asking for Nashorn specifically. But it's very isolated and won't be difficult to replace when the time comes.

And who knows, maybe some "credible developers" will pick up Nashorn and keep it going!

[mmajerni]

Hi, could you provide information about the solution? What is the solution? Maybe some PR/commit?

[elvisisking]

There is no solution on this yet @mmajerni. Though it seems like going forward we need to, in some way, use the validation framework being used by Apicurio in order to facilitate a good user experience.

[tplevko]

@elvisisking in that case, I'm moving this issue from "Done" to "Backlog"

[stale[bot]]

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

[stale[bot]]

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

[EricWittmann]

@elvisisking - some good news in this area is that I'm migrating the OpenAPI library I wrote for Apicurio from typescript to Java. The new project will take over for oai-ts-core and oai-ts-commands. It's java-first and also transpiled from Java into TypeScript. So we should be able to use it in any java project as well as node.js and browser apps.

What this also means is that validation of an OpenAPI doc can be done server-side in Syndesis without needing Nashorn or Graal.

[EricWittmann]

Should we re-open this issue?

[mcada]

Definitely, the issue is still present.

[dongniwang]

@elvisisking - just wanted to check in and see what's the latest on this issue. Is there anything that needs UX support?

[EricWittmann]

The library that performs validation is now completed and used in Apicurio Studio. However it is not yet included in the product as part of Apicurito. The next version of the product (after whatever version is currently being productized) will include the updated library, which can be found here:

https://github.com/Apicurio/apicurio-data-models

The point is that for the next version of the product, we should really consider addressing this issue IMO.

[elvisisking]

I don't have anything to add @dongniwang. Though I haven't looked at this for quite awhile I believe the issue remains the same: 2 different validations (Apicurio, server) being used that can generate different errors and warnings.

[heiko-braun]

Migrated to https://issues.jboss.org/browse/ENTESB-11443