Conditional flow fails with collections in API Provider

[mmuzikar]

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

Conditional flows cannot be used with collections in Syndesis, but API Provider makes a collection around its request body, even when it is specified in the API specification to contain only one object of specified type. Example: conditional-provider.zip In this integration you can see that in the specification in the POST method it expects request body of Task. I'd expect that to be one Task. So if I create a condition ${body.completed} it should work, or at least ${body["completed"]}. I tried also splitting it, but split doesn't work well on these types of collections (see #4862). Also I tried ${body[0]["completed"]} but that also didn't work for some reason. The log reported something along the lines of cannot call method [0]["completed"] on ArrayList.

[christophd]

I think the correct expression would be ${body.body.completed}

This is because the 1st body references the Camel exchange body and the 2nd body the body of the unified JSON payload that is used in API provider. I think we would have the same expression when using an advanced filter step in API provider.

I think it is a bit uncomfortable and unclear to the user but that's another part of the story.

@zregvart Am I right with this regarding unified JSON?

[christophd]

@mmuzikar I think this has nothing to do with collections on API provider. The error message was misleading I guess. can you please verify using the expression ${body.body.completed}?

[mmuzikar]

@christophd Yes you are right, that works. It is certainly not the most obvious solution though. If this is the intended solution, it should be at least documented somewhere (@TovaCohen can you elaborate if it is already documented, or add a mention of that in the documentation?). But this is a thing you don't really expect that you have to read documentation for, maybe a little mention in the UI where there is the "Provide a condition that you want to evaluate (for example, ${in.header.type} == 'note' or ${in.body.title} contains 'Important')." Or in a perfect world if the datatype is a JSON schema there could be a note added or something. (cc @syndesisio/uxd )

[zregvart]

@zregvart Am I right with this regarding unified JSON?

Yup, and that's consistent with the data shape.

[TovaCohen]

In the user guide, in the doc for adding conditional flows, a prerequisite for adding a conditional flow step is:

Input to a Conditional Flows step must be an individual message. In the integration visualization, if the previous step’s Data Type shows (Collection), add a Split step.

Does that cover what we need to tell the user? Do you want to add anything?

[mmuzikar]

The problem here is the data shape is not collection, it can seem like it is from the logs, but adding a split step in front of conditional step will only break the integration. @zregvart and @christophd feel free to correct me, but if a step defines it's own datashape with JSON (those are API Providers, API connectors, webhooks, templates etc.), the exchange does contain another body field. So with "normal" connections/steps you use ${body.<field>} but due to using custom data shapes, you need to use ${body.body.<field>}. To put it into simple I made a quick illustration of my mental understanding of what the difference is. It's really simplified, I think the inner body is also enveloped in a collection, but I think it shows the difference fairly well EDIT: changed the image based on Zoran's explanation, which also helped reasoning behind why the datashapes are like this

[dongniwang]

cc @Essjaysee since we're conducting a usability study for conditional flow and may encounter the situation described in this comment.

[zregvart]

With most HTTP-related functionality (custom API connectors, API provider, webhook) we use something that we call "unified" data shape, this is the shape that we try to standardize on so we can easily both use it in the mapping step and have a common data format when using different HTTP-related functionality in the same flow/integration. The origins of this shape are from the inability to specify multiple data shapes for one step, in this case we need two one for HTTP headers (parameters) and one for the HTTP body, so therefore we create a new data shape that contains both.

This shape has a specific JSON schema defining it which is auto-generated (custom API connectors, API provider) or provided by the user (webhook).

The fact that there's a body property in the body of the Camel message is because of this. This also reflects in the use of mapper, filter steps or here in the conditional flows.

We have made provisions in split/aggregate for this specific data shape in order for the experience to be better for the end user. I think there are some improvements for conditional flows planned for next release.

In the long term I would look to support defining the data shape(s) differently, this is a limitation we have with the mapper and the way it can only express the shape of the Camel message body and needs to be addressed there first.

[paoloantinori]

Hi guys, what's been the decision about this? is the bug been confirmed or turned in an enhancement?

[zregvart]

To me this feels like an improvement, it's just the way things are at the moment and it's expected/designed that way albeit a bit unexpected for the user.

[TovaCohen]

It sounds like I need to add something to the 7.4 user doc, but I don't know what to add or where it belongs. Can someone suggest what's best to do for the doc? Or let me know that nothing is required?

[heiko-braun]

@christophd Can you work with @TovaCohen to identify the parts that should be documented?

[christophd]

@TovaCohen let me share my thoughts on what should be documented here:

As @zregvart mentioned most HTTP-related functionality (custom API connectors, API provider, webhook) use a unified Json Data Shape that represents the request body and headers.

So when writing condition expressions in advanced filter or conditional flows the user has to be aware of that unified Json being used. As unified Json data shapes hold the actual message body content in a property called body the expression has to use something special in order to access the message content.

A condition expression (using simple language) evaluated on a unified Json data shape needs to look like this: ${body.body.foo}

Where foo is the actual message content property to access. The 1st body in that expression path represents the exchange body and the 2nd the special unified Json data shape body property.

A condition expression (using simple language) evaluated on a non-unified Json data shape can look like this: ${body.foo}

I think we already have some sections in the docs describing the unified Json data shape and where these are typically used. So I think this belongs to these sections.

[TovaCohen]

Thanks @christophd ! That helps a lot. We don't have any doc that specifically addresses the unified json data shape. Nor does our doc refer to a Camel exchange, except in the doc for developing extensions. Please take a look at this gdoc:

https://docs.google.com/document/d/1bXQDOp_z4H-v4JW-dO9dMLQ1DuRX_lEq5Qmu7Ty5tJQ/edit?usp=sharing

In that doc, I am proposing an addition to the doc for adding advanced filter steps, and an update to the doc for specifying conditions in a conditional flows step. Please review that and let me know what needs to change.

Also in that doc, I copied the content that we provide about webhook data shapes. As far as I can tell, that doc does not make it clear that a specification of body.body is required in advanced filter steps that are in simple integrations that start with a webhook. Please let me know how we can make that doc more useful. Or, if I am misunderstanding something - please set me straight! Thanks!

[TovaCohen]

After talking with Martin, I updated the doc. And I pushed and merged the update: https://github.com/syndesisio/syndesis/pull/6312 Let me know if anything needs to change. And this is the downstream merge request: https://gitlab.cee.redhat.com/jboss-fuse-documentation/fuse7/merge_requests/500