fix: api provider bugs

[riccardo-forina]

Partially fixes syndesisio/syndesis#5697
Partially fixes syndesisio/syndesis#5600
Fixes syndesisio/syndesis#5675
Fixes #397
Fixes syndesisio/syndesis#5665

Since the spec is returned by an API, every time you go to the API definition you get the one relative to the last saved version of the integration.
So if edit the definition, change your mind and try to edit it again, you'll lose the previous changes.

Also, this will completely delete whatever was present on the integration before the change in the spec. @zregvart can you confirm this is right?

[riccardo-forina]

PR Storybook available here

[riccardo-forina]

PR Storybook available here

[gashcrumb]

👍

[pure-bot]

Pull request approved by @gashcrumb - applying approved label

[zregvart]

So if edit the definition, change your mind and try to edit it again, you'll lose the previous changes.

I think there's an explicit Save button, so the change your mind bit should be clicking on Cancel, when put this way I think this makes sense.

Also, this will completely delete whatever was present on the integration before the change in the spec. @zregvart can you confirm this is right?

I think in the Angular UI we saved the integration before editing the API definition in Apicurio in that case changes to the flows will be preserved.

[riccardo-forina]

This is an issue, the react ui doesn’t force a save ever, so we can’t guarantee this flow. What would fix this is a version of the api that takes an integration object instead of an integration id, so that we could forward the integration in whatever state it is while the user is working.
I guess in this case we would need to save the reference spec somewhere, like in the metadata? This would be to open apicurio directly without asking the be for a spec.

Does this make sense?

[zregvart]

We could do a multipart PUT /api/v1/integrations/{id}/specification, one part Integration JSON another part the new OpenAPI JSON. That should fix it, right?

[riccardo-forina]

Think so but why the id? This could happen for new integrations yet to be saved.

[zregvart]

Think so but why the id? This could happen for new integrations yet to be saved.

That's because we start with the OpenAPI document and create an Integration from it via POST /api/v1/apis/generator that returns the generated Integration JSON. You can't start from both Integration and OpenAPI document...

[riccardo-forina]

Right but again, the integration could not be saved. Anyway, if we store the spec in the metadata we don’t need the specification endpoint anymore, don’t we? We just need the generator one, and that’s the one that should accept both an integration and the spec in the body

[zregvart]

Sure we can do that, there's one caveat POST /api/v1/apis/generator is not without any side effects, we're storing the submitted OpenAPI document, the idea is that we serve from the running integration the same OpenAPI document the client provided.

I'm a bit hesitant to store the OpenAPI specification in metadata as it will be persisted alongside the Integration, and that will be (in general) a large JSON in JSON document which will lead to performance issues.

(we're kinda using the word specification wrong here, it means the OpenAPI document)

I'm thinking we probably need the specification endpoint, but we can create one without side effects. To refactor this fully we would:

1. rename POST /api/v1/apis/generator to POST /api/v1/integrations/specification to generate integration from specification (receives multipart specification)
2. rename POST /api/v1/apis/info to POST /api/v1/integrations/specification/info to generate integration summary from specification (receives multipart specification)
3. add PUT /api/v1/integrations/specification to update the integration from specification (receives multipart specification and integration)
4. add POST /api/v1/integrations to create integration and specification (receives multipart specification and integration)
5. remove PUT /api/v1/integrations/{id}/specification

Does that make sense?

[riccardo-forina]

Not sure about 4, doesn't that exist already? It's also a bit problematic to keep hold of the specification for us if it's not stored in the integration itself, the whole editor app is stateless - data gets passed from route to route - and we would have to add the specification string to this passing. It's definitely not ideal.

[zregvart]

I think this discussion just uncovered a bunch of stuff we need discuss and design better, perhaps best done interactively.

I suggest that for the time being we can do the simplest thing, does just adding one endpoint, something like PUT /api/v1/apis/generator that takes both the integration and the OpenAPI document and returns the updated integration based on the OpenAPI document help? It can be invoked as soon as the user saves in Apicurio and the resulting integration can replace the one in the UI?

[riccardo-forina]

I think that would patch this, yep!

[zregvart]

I've created syndesisio/syndesis#5719 and I'll work on it right away.