Syndesis: StackOverflowError when trying to create custom API connector based on OpenHAB API

[tplevko]

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

Description

I was trying to create a custom API connector using the OpenHAB API specification, it however seems to be not possible.

The swagger.json example can be found here:
http://demo.openhab.org:8080/rest/swagger.json

Attached the server log.
server_log.txt

[heiko-braun]

@tadayosi Could this be an atlasmap issue?

[zregvart]

@tadayosi Could this be an atlasmap issue?

Doesn't look to me like one, most likely it's the reference resolving logic when parsing the OpenAPI specification and generating the data shapes.

[tadayosi]

Maybe you meant @igarashitm? :-)

[zregvart]

The issue here is that we cannot support cyclic references between object definitions.

In the example of OpenHab OpenAPI document:

    "Thing": {
      "properties": {
        "handler": {
          "$ref": "#/definitions/ThingHandler"
        },
      "type": "object"
    },
    "ThingHandler": {
      "properties": {
        "thing": {
          "$ref": "#/definitions/Thing"
        }
      },
      "type": "object"
    }

The definition of Thing points to the definition of ThingHandler which again points to Thing.

What we're trying to do is to generate a canonicalized JSON schema for data shape of a single API operation, and that includes de-referencing all referenced definitions and inclining them into a single canonical form.

While we might consider removing this canonicalization from Syndesis as Atlasmap has support for dereferencing in JSON Schema (https://github.com/atlasmap/atlasmap/commit/3a58ae738e9bfbb51a6908aa9791c42328e62962). I think it still might lead to an endless loop in Atlasmap caused by the circular references like this.

@igarashitm should I try to remove the generation of canonicalized JSON schema for data shapes and hope all goes well in Atlasmap or is this futile?

[igarashitm]

So this ends up with the problem of recursive objects in general, and we ATM just disable (gray-out in the UI) recursive objects for Java and XML
https://github.com/atlasmap/atlasmap/issues/255
https://github.com/atlasmap/atlasmap/pull/578

And I think we should do the same for JSON as well, IIRC we haven't done yet. If you could file an AtlasMap issue with example schema, that helps.

On the other hand, canonicalization at Syndesis side would help if that recursive object is something we really want to support in the UI, but it needs some way to limit the depth.

And the ultimate solution for this problem is to refactor inspection service to on-demand style
https://github.com/atlasmap/atlasmap/issues/579
Which requires an architectural restructure.

[heiko-braun]

sorry, @tadayosi  – yes I meant @igarashitm :)

[zregvart]

So the only thing we can realistically do for Sprint 42 is to catch that StackOverflowError and report an error in the UI.

The way we require the data shape specification to be done is that it encompasses all properties in a single schema without references to other schemas. Within OpenAPI the definitions property can have multiple schemas that can reference each other in a cyclic manner. It is impossible de-reference these in a single canonical schema. I think we can revisit this when the required support in Atlasmap is there.

[zregvart]

With the validator implemented in #4714 when trying to create an custom API connector or an API provider integration an error is reported.

For API provider:

For API client:

[tplevko]

@zregvart @TovaCohen shouldn't this be documented as a known limitation for customAPI provider?

[zregvart]

Perhaps this draft can help:

OpenAPI handling for custom API connectors and API provider integrations does not support cyclic schema references, i.e. JSON schema specifying request or response body is not permitted to reference itself as a whole or any part of itself in a cyclic manner through any number of intermediate schemas.

Hope that's not too off putting.

[tplevko]

@zregvart does a follow-up feature request for this make sense? I mean for the cyclic-reference handling in some of the future releases.

[TovaCohen]

Thanks @zregvart !
I would tweak what you wrote just a bit. How's this:

The OpenAPI document cannot have cyclic schema references. For example, a JSON schema that specifies a request or response body cannot reference itself as a whole nor reference any part of itself through any number of intermediate schemas.

If you expect to add support for cyclic schema references in a future release, I could start the paragraph with "In this release, " or I can add a sentence: "Support for cyclic schema references is expected to be added in a future release."

I would add this limitation to two places in the user doc:

• 7.1.1 Requirements for REST API client connectors

• A new section in the chapter about creating api provider integrations - "Requirements for API provider integrations".

WDYT?

[zregvart]

@tplevko sure, follow-up feature request makes a lot of sense (would be an epic though), not exactly sure how common is that though, and if that OpenHab specification makes sense, it looks to me like it was generated from code and the cyclic reference is just a by-product of that.

@TovaCohen sounds great, for release priorities can we get @gaughan to comment?

[gaughan]

Address with OpenAPI 3.0 or after?

[TovaCohen]

Should I add this limitation to the 7.3 release notes?

[zregvart]

Address with OpenAPI 3.0 or after?

The cyclic nature of the JSON schema won't go away with newer specification support. From what I understand non-standalone JSON/XML schemas are not supported in Atlasmap so it needs to be addressed there. Once that's addressed we no longer need to perform schema canonicalization and can provide Atlasmap with multiple schemas that can then reference each other in a cyclic manner.

[gaughan]

Thanks Zoran, got ya
@TovaCohen yes, we should add the limitation as a note

[tplevko]

@zregvart just created this issue as a followup: https://github.com/syndesisio/syndesis/issues/4826

Also one thing - just gave this a spin with API provider and in this case, the problem with StackOverflowError seems to be still present.

[zregvart]

Let me try to reproduce this, could be taking two different code paths. Though the validations should be the same.

[zregvart]

So what I see is that the error is shown, perhaps the issue here is that we allow Next even if the OpenAPI document contains errors.

[tplevko]

Works OK now, the followup feature request issue - https://github.com/syndesisio/syndesis/issues/4826 Closing this as verified.