INT-1639: Add SpEL-Function support #827

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@artembilan
Spring member
  • Introduce <spel-function>
  • Introduce integrationEvaluationContextAware infrastructure
  • Some refactoring according new infrastructure
  • Some tests

JIRA: https://jira.springsource.org/browse/INT-1639, https://jira.springsource.org/browse/INT-2602

@artembilan
Spring member

It's just an initial commit. For review to be sure in right way.
There should be JavaDocs, tests, docs.
And also there are many places in the project, which should be refactored according new infrastructure.
In addition: my idea to ensure BeanFactoryResolver to the SpEL to the entire project.

BTW, a question:
I understand the difference between StandardTypeConverter and BeanFactoryTypeConverter, but why ExpressionUtils doesn't use the last one?

@garyrussell garyrussell commented on an outdated diff Jul 1, 2013
...ration/endpoint/ExpressionMessageProducerSupport.java
@@ -40,10 +45,24 @@ public void setPayloadExpression(String payloadExpression) {
}
}
+ @Override
+ public void setIntegrationEvaluationContext(StandardEvaluationContext evaluationContext) {
@garyrussell
garyrussell Jul 1, 2013

Standard

Argument can be an EvaluationContext.

@garyrussell
Spring member

Artem,

This looks good.

I think the refactoring of other classes to use this mechanism should be done in a separate PR (or at least not squashed).

Regarding your question, the ExpressionUtils code was moved from http; The original code in http used the StandardTypeConverter. bff8c9a That code was subsequently refactored to make it thread-safe, and finally ended up in ExpressionUtils.

Reading the comment in https://jira.springsource.org/browse/INT-1281 leads me to believe the BFTC effectively makes the SpEL TypeConverter fall back to Spring PropertyEditors when the conversion service can't convert (emulating similar behavior for the spring beans TypeConverter).

I suppose it's possible that this fallback behavior might be needed when evaluating SpEL against messages so we could change it. On the other hand, the BFTC has caused some unexpected issues recently because it didn't have enough test cases. So, unless you can come up with a specific use case where the STC wouldn't work, maybe we should leave alone for now?

@artembilan artembilan INT-1639: Add SpEL-Function support
* Introduce `<spel-function>`
* Introduce `integrationEvaluationContextAware` infrastructure
* Some refactoring according new infrastructure
* Some tests

JIRA: https://jira.springsource.org/browse/INT-1639, https://jira.springsource.org/browse/INT-2602
1c71083
@artembilan
Spring member

Squashed initial commit to address current PR's comments.
My general concern is about naming convention. Is it OK? Or should it be ExpressionFunction?
I want to know it before I start to java-doc new features

@garyrussell garyrussell commented on the diff Jul 9, 2013
...ork/integration/config/xml/spring-integration-3.0.xsd
@@ -3846,6 +3846,15 @@ The list of component name patterns you want to track (e.g., tracked-components
</xsd:complexType>
</xsd:element>
+ <xsd:element name="spel-function">
+ <xsd:complexType>
+ <xsd:attribute name="id" type="xsd:string" use="required"/>
+ <xsd:attribute name="class" type="xsd:string" use="required"/>
@garyrussell
garyrussell Jul 9, 2013
                    <tool:annotation kind="direct">
                        <tool:expected-type type="java.lang.Class" />
                    </tool:annotation>

??

@artembilan
artembilan Jul 10, 2013

Yes, thanks.
But it was just a prototype, to be sure in right solution. BTW, my IntelliJ IDEA suggests classes anyway 😄
I'll fix it and add a description to this new element.

@garyrussell
Spring member

I see what you mean; the SpelFunction class really has nothing to do with SpEL directly, or even expressions - it is simply a "named" method. We may (probably) never use it outside of SpEL, but perhaps it would be better to give it a generic name; maybe just Function ? (<int:function/>)

@artembilan
Spring member

We may (probably) never use it outside of SpEL

Interest point to discuss. If end-user will decide to use some MethodBean on his own, outside of SpEL we should provide for him some option to make it available within SpEL or not. And again it whould look like <spel-function>.
and again we have to have some separate class for this one:

Collection<SpelFunction> getSpelFunctions(BeanFactory beanFactory)

to register them within EC.
Or we should introduce some SpELFunctionRegistry, to have an ability to filter MethodBean as spel-functions.
I haven't looked at this as a generic solution, but now my opinion is:

  • rename SpelFunction to some generic one
  • leave <spel-function> and do not hint, that it can be used somewhere else
  • Introduce SpELFunctionRegistry and make a tandem with SpelFunctionParser

In this case any generic bean of renamed SpelFunction won't be determined as spel-function.
There is a window to think how to make it achievable within Java Config.

Any thoughts - welcome!

@artembilan
Spring member

Another thought: eliminate SpelFunction at all and have one bean SpelFunctionRegistry with Map<String, Method> property. And SpelFunctionParser will register its entry in that registry.
So, in this case we can leave <spel-function> tag and will avoid any inconsistency around 'named method'.
There will be need to think how to register xpath, json, requestScope etc.. out-of-the-box functions...
But they are separate JIRAs

@garyrussell
Spring member

Hi Artem

I had a discussion with @markfisher earlier today about a related subject (adding custom PropertyAccessors to SI SpEL evaluation contexts).

Maybe a simple approach to cover both use cases would be to provide your IntegrationEvaluationContextFactoryBean and INTEGRATION_EVALUATION_CONTEXT_BEAN_NAME and simply allow users to contribute additional PropertyAccessors and/or functions to it by overriding the default FB with their own definition.

WDYT?

@artembilan
Spring member

Hello!

I'm not sure yet what you mean about custom PropertyAccessors, but we should understad where and how we may allow end-users to support their own implementations. My new IntegrationEvaluationContextFactoryBean is some global factory, who produces identical EvaluationContext instances, when we call it (or inject) from context as a bean. So, will it be appropriate, if we allow end-user to inject his own PropertyAccessor globally, for all SpELs within SI-application?
From other side, if it's OK to do it globally, why don't introduce high-level customization:

<spel-property-accessor ref="">
    <beans:bean/>
</spel-property-accessor>

Regarding getting rid of <spel-function> not agree. The main reason of this new feature to have #xpath('/foo/@bar', payload.xmlValue) SpEL ability. This is about #jsonPath() too, when we'll implement int-json module. I need some time to find other usefull out of the box functions, e.g. something from Spring Batch step scope... But you already can see what it will give to end-users.
I say it one more time: I'm going to remove ambiguous SpelFunction class and introduce SpelFunctionRegistry.
And more: <spel-function> might not be enough. Guys might want to inject to their SpELs global variables too.
E.g. requestScope isn't e function it is just an accessor to the RequestContextHolder.getRequestAttributes() at a glance...
WDYT now ?

@garyrussell
Spring member

Hi Artem; we ( @markfisher and I ) decided to break this into two parts.

  1. Externalize the evaluation context creation and allow the property accessors/functions to be modified
  2. First class support for SpEL functions.

    This was a great start for 1. - I basically took what you already had and tweaked it a little.

garyrussell@c4e6db5

It still needs more tests but it seems to work well - probably the quickest place to start to take a look is this test garyrussell@c4e6db5#diff-11 where I add a custom property accessor that can handle the test class that has methods obtainBar and updateBar on Foo, allowing expression="payload.bar" on a message with payload type Foo. See FooAccessor in garyrussell@c4e6db5#diff-12

@artembilan
Spring member

So, looks like we should open new PR and this one close for now, because it will depend.
And new JIRA, of course!

@garyrussell
Spring member

OK; I am polishing to make the build run and will issue a new PR in a while, closing this one for now.

If you have any better ideas please share - consider a custom payload (not a POJO) and we want to use SpEL expressions on it in transformers, filters etc. throughout the application, without having to configure every endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment