syndesisio / syndesis-integration-runtime

Apache License 2.0
1 stars 6 forks source link

Filter step should not start a new level in the yaml spec #8

Closed rhuss closed 6 years ago

rhuss commented 6 years ago

When adding a filter a new level is started in syndesis.yml. This is unnecessary and complicates the document structure, especially when there should be multiple filters.

Instead, a filter should be treated like a normal step, without including steps itself.

This also affects the project-generator in syndesis-rest.

jimmidyson commented 6 years ago

This is due to the Camel structure of a filter step, which does include child steps in the route. This should be possible to work around though, I'm looking at it now.

Just to point out that for choice/when/otherwise, child steps will still be valid though and that is a model we need to support.

jimmidyson commented 6 years ago

So if we change this, we would be unable to support flows like:

---
flows:
- steps:
  - kind: "endpoint"
    uri: "direct:start"
  - kind: "filter"
    expression: "$.[?(@.name == 'Jimmi')]"
    steps:
    - kind: "endpoint"
      uri: "mock:matched"
  - kind: "endpoint"
    uri: "mock:allMessages"

This flow would only send matching exchanges to the mock:matched endpoint, but all messages to the mock:allMessages endpoint.

Is this something we want to support? If so, then we need to leave the child steps of filter step as-is.

rhuss commented 6 years ago

Tbh, for 'forking' the flow I would expect a more general element like in

flows:
- steps:
  - kind: "endpoint"
    uri: "direct:start"
  - kind: "fork"
     - steps:
        - kind: "filter"
          expression: "$.[?(@.name == 'Jimmi')]"
        - kind: "endpoint"
           uri: "mock:matched"
  - kind: "endpoint"
     uri: "mock:allMessages"

This would be useful for other steps like datamapping, logging, ...

Actually, I'd prefer to have dedicated steps for building advanced flows (like choice, fork / split), and don't do it implicitly of a specific workflow (do one thing and do it well)

jimmidyson commented 6 years ago

I guess my real concern is that we're diverging from Camel DSL, and this flow schema has effectively been trated as an alternative Camel DSL. I'm OK with diverging (don't feel it needs to necessarily be 1:1 with existing DSLs) , as long as we are aware of this divergence and are making the decision for the right reasons. In this case, I do agree that diverging would make the DSL more readable/writable/manageable, especially if the flow contains multiple filters which would currently result in multiple nested child steps - horrible.

rhuss commented 6 years ago

my suggestion is very likely due to my camel non-knowledge (or ignorance), but is it really like this that a filter component starts a fork ? May I ask how the flow above would look like as a camel Java DSL route ? (could look it up but all you camel aficionados already know that by heart ;)  

jimmidyson commented 6 years ago

Here's some examples from https://camel.apache.org/message-filter.html:

Java DSL:

// Straight filter to one endpoint
from("direct:a")
  .filter(header("foo").isEqualTo("bar"))
  .to("direct:b");

// Filter to one endpoint, all messages to a different endpoint
from("direct:start")
  .filter().method(MyBean.class, "isGoldCustomer")
    .to("mock:result")
  .end()
.to("mock:end");

XML DSL:

<!--This is similar to what we have in the existing flow DSL -->
<from uri="direct:a"/>
<filter>
  <xpath>$foo = 'bar'</xpath>
  <to uri="direct:b"/>
</filter>
rhuss commented 6 years ago

I see.

But if we translate the DSL literally, wouldn't we have to add an extra level after each step (as this correspond to the chaining calls in the DSL ? And only at an end() go back one level ?

So that when we translate it directly 1:1 we would end up with a syntax like:

---
flows:
- steps:
  - kind: "endpoint"
    uri: "direct:start"
    steps:
    - kind: "filter"
      expression: "$.[?(@.name == 'Jimmi')]"
      steps:
      - kind: "endpoint"
        uri: "mock:matched"
    - kind: "endpoint"
      uri: "mock:allMessages"

for

from("direct:start")
  .filter().simple("$.[?(@.name == 'Jimmi')]")
    .to("mock:matched")
  .end()
  .to("mock:allMessages")

wonder whether we should go for a close 1:1 mapping leading to an ugly YAML syntax, but easy to implement or think about own, more flat, yaml syntax which deviates significantly from the DSL and is harder to implement ?

btw, wouldn't generating the Java DSL directly help, too (just to play devil's advocate ;-) ?

jimmidyson commented 6 years ago

But if we translate the DSL literally, wouldn't we have to add an extra level after each step (as this correspond to the chaining calls in the DSL ? And only at an end() go back one level ?

No - look at the XML DSL example in https://github.com/syndesisio/syndesis-integration-runtime/issues/8#issuecomment-320900731

Which leads me to think of actually just using the XML DSL. Hmm. Going to have to go back to think about why the flow/YAML DSL was created in the first place and see if it still applies here. I'm a bit loathe to deviate now - what would be the benefit vs cost of changing this - rather than just tweaking what we have.

rhuss commented 6 years ago

Ok, XML looks a bit simpler (as there is no nesting within <from> but still within <filter>.

I guess the first motivation was to make the funktion.yml syntax more restricted and opinionated so that its easier to start with.

However as we tend now to grow we might end up in yet another DSL for a full Camel route.

But you are probably right, we should stick to our current implementation, but orient ourselves to the XML syntax structure-wise. Even when we have filter nesting (but at the end, you probably don't have two filters after each other but a single one with a complex expression).

So let's agree to mimic the XML syntax in YAML, but leave out anything advanced which we do not want to support.