Camel
  1. Camel
  2. CAMEL-4809

interceptSendToEndpoint with predicate and skip

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.9.0
    • Fix Version/s: 2.10.0
    • Component/s: camel-core, camel-spring
    • Labels:
      None
    • Patch Info:
      Patch Available
    • Estimated Complexity:
      Unknown

      Description

      The <when> clause in the interceptors behave like filters. This is okay with intercept and interceptFrom, where the only possible route manipulation is <stop />, which is expressed inside the routing block.

      However, with interceptSendToEndpoint there is one edge case that could cause ambiguity. When skipSendToEndpoint=true, along with a <when> clause, the user might expect that the skipping will only occur if the condition is met. However, it occurs always. The <when /> only determines whether the routing logic contained inside the body of the intercept block will occur or not.

      I propose to add a new attribute onlySkipWhenConditionMet, so that when this is true, the <when> clause will be evaluated to determine if the originally intended endpoint is actually skipped or not.

      1. interceptSendToEndpointConditionalSkip_v2.diff
        19 kB
        Raúl Kripalani
      2. interceptSendToEndpointConditionalSkip.diff
        18 kB
        Raúl Kripalani
      3. test-interceptor.xml
        1 kB
        Raúl Kripalani

        Activity

        Raúl Kripalani created issue -
        Raúl Kripalani made changes -
        Field Original Value New Value
        Attachment test-interceptor.xml [ 12508285 ]
        Hide
        Raúl Kripalani added a comment -

        I've attached a Camel route in Spring DSL that reproduces the behaviour.

        Show
        Raúl Kripalani added a comment - I've attached a Camel route in Spring DSL that reproduces the behaviour.
        Hide
        Claus Ibsen added a comment -

        Ah yeah skipSendToEndpoint should only trigger if when is true. So its better to fix that logic part if possible. And not introduce a new option, that just confuses people.

        Show
        Claus Ibsen added a comment - Ah yeah skipSendToEndpoint should only trigger if when is true. So its better to fix that logic part if possible. And not introduce a new option, that just confuses people.
        Hide
        Raúl Kripalani added a comment -

        Patch attached, implementing the following behaviour.

        In Java DSL:

        • If the when() clause is attached to the interceptSendToEndpoint, the condition is satisfied and skip is enabled via interceptSendToEndpoint().when()..., the original endpoint will be skipped. If the condition is not met, it will not be skipped.
        • The conditional skip only applies to when() clauses attached directly to the interceptSendToEndpoint. That is, if a new choice block is opened in the following way: interceptSendToEndpoint().choice().when()... and skip is enabled, the evaluation of the conditions will not affect the skipping outcome, i.e. the endpoint will always be skipped. This is because the choice() block pertains to the body of the interception rather than the interception condition.
        • Since the interceptSendToEndpoint().when() gives way to a ChoiceDefinition fluent builder, which means that the user can define further conditions and even an otherwise. If any of them is satisfied, the skipping will occur.

        In Spring DSL, the <when /> immediately after the <interceptSendToEndpoint /> creates a FilterDefinition. Hence only one condition exists and things become simpler.

        Show
        Raúl Kripalani added a comment - Patch attached, implementing the following behaviour. In Java DSL: If the when() clause is attached to the interceptSendToEndpoint, the condition is satisfied and skip is enabled via interceptSendToEndpoint().when()..., the original endpoint will be skipped. If the condition is not met, it will not be skipped. The conditional skip only applies to when() clauses attached directly to the interceptSendToEndpoint. That is, if a new choice block is opened in the following way: interceptSendToEndpoint().choice().when()... and skip is enabled, the evaluation of the conditions will not affect the skipping outcome, i.e. the endpoint will always be skipped. This is because the choice() block pertains to the body of the interception rather than the interception condition. Since the interceptSendToEndpoint().when() gives way to a ChoiceDefinition fluent builder, which means that the user can define further conditions and even an otherwise. If any of them is satisfied, the skipping will occur. In Spring DSL, the <when /> immediately after the <interceptSendToEndpoint /> creates a FilterDefinition. Hence only one condition exists and things become simpler.
        Raúl Kripalani made changes -
        Raúl Kripalani made changes -
        Patch Info Patch Available [ 10042 ]
        Component/s camel-core [ 12313938 ]
        Component/s camel-spring [ 12313939 ]
        Hide
        Raúl Kripalani added a comment - - edited

        In order to detect if the ChoiceProcessor matched any of its clauses without having to evaluate all predicates again, I created a new property Exchange.LAST_MATCHING_CLAUSE. The ChoiceProcessor will set the value of this property in the Exchange depending on the outcome of the evaluation:

        • If a when is matched, its index will be stored
        • If the otherwise is executed, then the string 'OTHERWISE' will be stored
        • If no when is matched and no otherwise exists, the property will not exist

        I guess this feature will be useful in scenarios other than this particular one, even for the end user. So it could get documented in the choice DSL page too.

        Show
        Raúl Kripalani added a comment - - edited In order to detect if the ChoiceProcessor matched any of its clauses without having to evaluate all predicates again, I created a new property Exchange.LAST_MATCHING_CLAUSE . The ChoiceProcessor will set the value of this property in the Exchange depending on the outcome of the evaluation: If a when is matched, its index will be stored If the otherwise is executed, then the string 'OTHERWISE' will be stored If no when is matched and no otherwise exists, the property will not exist I guess this feature will be useful in scenarios other than this particular one, even for the end user. So it could get documented in the choice DSL page too.
        Hide
        Claus Ibsen added a comment -

        I think we should fix this without the need for adding properties to the exchange from the CBR.

        I would rather fixup the Java and XML DSL to be similar in their model/runtime of the interceptSendToEndpoint with the predicate.

        The unit tests can of course be used as to test if the fix works.

        Show
        Claus Ibsen added a comment - I think we should fix this without the need for adding properties to the exchange from the CBR. I would rather fixup the Java and XML DSL to be similar in their model/runtime of the interceptSendToEndpoint with the predicate. The unit tests can of course be used as to test if the fix works.
        Claus Ibsen made changes -
        Assignee Claus Ibsen [ davsclaus ]
        Hide
        Claus Ibsen added a comment -

        I have fixed the DSL to be consistent using WhenDefinction

        Show
        Claus Ibsen added a comment - I have fixed the DSL to be consistent using WhenDefinction
        Hide
        Raúl Kripalani added a comment -

        Thanks Claus. I didn't want to take the step of changing the fluent builder just in case users were relying on the returned ChoiceDefinition. But it makes more sense this way, definitely.
        I've attached a new version of the patch. This time with checkstyle passed

        Show
        Raúl Kripalani added a comment - Thanks Claus. I didn't want to take the step of changing the fluent builder just in case users were relying on the returned ChoiceDefinition. But it makes more sense this way, definitely. I've attached a new version of the patch. This time with checkstyle passed
        Raúl Kripalani made changes -
        Hide
        Claus Ibsen added a comment -

        Thanks Raul for the 2nd patch.

        I have thought whether it would be worthwhile to create a WhenInterceptSendToEndpointDefinition which extends WhenDefinition, and then delegate the filter predicate, to have it copy the FILTER_MATCHED result to another property. Kinda like what you do. But without adding that extra set property definition. Then we avoid adding that extra step in the route model, which gets visible for tooling and whatnot, at runtime.

        Show
        Claus Ibsen added a comment - Thanks Raul for the 2nd patch. I have thought whether it would be worthwhile to create a WhenInterceptSendToEndpointDefinition which extends WhenDefinition, and then delegate the filter predicate, to have it copy the FILTER_MATCHED result to another property. Kinda like what you do. But without adding that extra set property definition. Then we avoid adding that extra step in the route model, which gets visible for tooling and whatnot, at runtime.
        Hide
        Claus Ibsen added a comment -

        I have applied a fix to trunk with part of your work as well. I introduced a WhenSkipSendToEndpointDefinition which handles the predicate stuff.

        Show
        Claus Ibsen added a comment - I have applied a fix to trunk with part of your work as well. I introduced a WhenSkipSendToEndpointDefinition which handles the predicate stuff.
        Claus Ibsen made changes -
        Fix Version/s 2.10 [ 12317612 ]
        Affects Version/s 2.9.0 [ 12316374 ]
        Claus Ibsen made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Raúl Kripalani added a comment -

        Documentation added on the InterceptSendToEndpoint section of the Intercept page.

        Show
        Raúl Kripalani added a comment - Documentation added on the InterceptSendToEndpoint section of the Intercept page.
        Hide
        Raúl Kripalani added a comment -

        Also warned about the change of behaviour under "Important changes to consider when upgrading" on the Camel 2.10 release notes.

        Show
        Raúl Kripalani added a comment - Also warned about the change of behaviour under "Important changes to consider when upgrading" on the Camel 2.10 release notes.
        Raúl Kripalani made changes -
        Comment [ Also warned about the change of behaviour under "Important changes to consider when upgrading" on the Camel 2.10 release notes. ]

          People

          • Assignee:
            Claus Ibsen
            Reporter:
            Raúl Kripalani
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development