Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [faces-dev] Outstanding enhancement request for Jakarta EL

Hi Mark,

sorry for being not clear enough and going around in circles.

i think the example you provided is really complex and not something i would like to have.


| - Add a new implicit variable to faces: "event"

This is actually something very similar i was thinking about.
But instead of just push one single AjaxBehavior to "event", i would like to push all MethodExpression#invoke arguments to "arguments".

This is how a ELResolver for this would look like (untested):
https://gist.github.com/tandraschko/1dc77ef27093a239c3319019193d3c62

so before we call the listener (MethodExpression#invoke(elContext, new Object[] { event }), we could do:
ArgumentsELResolver.ARGUMENTS.set(new Object[] { event })

this would allow the user, to access the event via: arguments[0]
For example:
#{bean.doSomething(arguments[0], 'aaa','bbb','ccc')}

Hope its more clear now.
This solution is really really generic and would be possible to include it directly to EL.

Best regards,
Thomas


Am Mi., 17. März 2021 um 11:25 Uhr schrieb Mark Thomas <markt@xxxxxxxxxx>:
Thomas,

We seem to be going around in circles.

Your proposal below is incomplete. You have proposed a change to the
implementation of MethodExpressionImpl.invoke but it is not clear what
the addition of putTempVar() and removeTempVar() is meant to achieve or
how clients of the EL API are meant to use it.

The only way I can see to fill in the gaps is as follows:

*** Example usage start ***

// Define the EL _expression_
String _expression_ = "#{bean.doSomething('aaa','bbb','ccc')}"

// Plumbing to create a MethodExpression instance
ExpressionFactory factory = ELManager.getExpressionFactory();
ELManager manager = new ELManager();
ELContext context = manager.getELContext();
MyBean myBean = new MyBean();
ValueExpression ve =
         factory.createValueExpression(myBean, MyBean.class);
context.getVariableMapper().setVariable("bean", ve);
MethodExpression me =
         factory.createMethodExpression(context, _expression_, null, null);

// Want to call
// MyBean.doSomething(Object extra, Object a, Object b, Object c)
// i.e. call a different method on the MyBean instance and
// provide the additional parameter(s) required for that method.
// Something like:
Object[] originalParams = me.getOriginalParameters()
Object[] newParams = new Object[4]
newParams[0] = new MyInsertedObject();
newParams[1] = oldParams[0];
// etc. Can re-order parameters, replace parameters etc as required

// Need a new parameter here to indicate provided params override any
// parsed parameters
me.invoke(context,newParams, true);

*** Example usage end ***


This would require two changes to the EL API:
- A new method MethodExpression.getOriginalParameters()
- A new invoke method where provided parameters took precedence over
   parameters parsed from the _expression_

However there are some drawbacks with this solution.

1. This is a fairly narrow solution. It assumes that the method name is
the same for the "old" method and the "new" method.

2. The handling of EvaluationListener instances would need to change.
Clearly calling with the original _expression_ would be misleading.
Calling with an amended version of the _expression_ would add a reasonable
amount of complexity.

3. This assumes that the parameter manipulation will always occur for
given method expressions. As soon as there is a requirement for the
parameter manipulation to be optional (i.e. under the control of the
developer using faces) then the solutions get more complex.


All of the above is a lot more complex than the alternative solution
which is available with the current EL API:

- Add a new implicit variable to faces: "event"
- developers using faces add that parameter to the _expression_ when they
   want it to be used
- provide a new (or extend an existing) ELResolver to resolve "event" to
   the right object

The advantages of the this solution are:
- it is available with the current EL API
- current EvaluationListener handling can remain unchanged
- simple for developers using faces to control whether or not they want
   to call a method with or without "event"
- no requirement for with and without "event" method names to be the
   same
- not limited to simple method expressions
- from what I have seen of the faces code so far, this would require
   less changes in faces


If you have a different solution in mind that is simpler then please
provide the equivalent of the sample usage I provided above so I can
evaluate what changes would be required in the EL API.

Mark


On 17/03/2021 08:04, Thomas Andraschko wrote:
> Hi Mark,
>
> why isnt something like this:
>
> https://github.com/apache/tomcat/blob/master/java/org/apache/el/MethodExpressionImpl.java#L267
> <https://github.com/apache/tomcat/blob/master/java/org/apache/el/MethodExpressionImpl.java#L267>
>
> try {
>       putTempVar("arguments", params);
>       Object result = this.getNode().invoke(ctx, this.paramTypes, params);
> }
> finally {
>       removeTempVar("arguments");
> }
>
>
> not a good change for the EL API?
>
> I mean, there is no way of the EL API to reference a "implement" param
> passed to MethodExpression#invoke.
> Of course we could workaround this by adding a
> "ImplementArgumentELResolver" in JSF but as i said, its not only a
> problem for JSF in theory.
>
> Best Regards,
> Thomas
>
> Am Di., 9. März 2021 um 18:38 Uhr schrieb Mark Thomas <markt@xxxxxxxxxx
> <mailto:markt@xxxxxxxxxx>>:
>
>     On 25/02/2021 17:22, Thomas Andraschko wrote:
>      > Hi Mark,
>      >
>      > in general there are 2 ways where the parameters of the method
>     comes from:
>      > 1) params passed to MethodExpression#invoke (#{myBean.myMethod},
>     in our
>      > case AjaxBehaviorEvent)
>      > 2) params defined as EL-ref by the user _expression_, which will be
>      > resolved by ELResolvers (#{myBean.myMethod(someOtherELVar)}
>      >
>      > With the approach suggested by my last mail,  you can do
>      > #{myBean.myMethod(arguments[0], someOtherELVar)}.
>      > IMO its much cleaner as magically try to resolve the method with
>     params
>      > passed to MethodExpression#invoke and provided in the EL-String.
>
>     How is this different to using #{myBean.myMethod(event,
>     someOtherELVar)}
>     where event is resolved by a JSF provided resolver?
>
>     Anything where JSF is able to do something like:
>
>     - if this _expression_ contains a method call
>         - identify the method
>         - obtain the parameter types and values
>         - look for a different method on the same object with a different
>           signature
>         - switch the _expression_ to use the new method and a newly
>     provided set
>           of parameters (which may include some, all or non of the original
>           ones)
>         - call the new method
>
>     is going to require massive change to the EL API. The biggest issue is
>     that the EL API does not expose the structure of the _expression_. The
>     magnitude of change required to do this is not justified given simpler
>     solutions are available with the current API.
>
>     Given this, I intend to close issue#6 with a recommendation that an
>     implicit variable is used along the lines of:
>     #{myBean.myMethod(event, someOtherELVar)}
>
>     Kind regards,
>
>     Mark
>
>      >
>      > Best regards,
>      > Thomas
>      >
>      > Am Do., 25. Feb. 2021 um 17:46 Uhr schrieb Mark Thomas
>     <markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
>      > <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>>:
>      >
>      >     Thomas,
>      >
>      >     That would likely be part of an implementation but I really
>     need to
>      >     understand how you would see this working as a user of the EL
>     API. What
>      >     would the EL _expression_ look like? What API would you call to
>     manipulate
>      >     the parameters? I'm struggling to come up with anything that
>     isn't
>      >     significantly messier than using implicit parameters that are
>     then
>      >     resolved by the ELContext.
>      >
>      >     Mark
>      >
>      >
>      >     On 24/02/2021 10:45, Thomas Andraschko wrote:
>      >      > Hi Mark,
>      >      >
>      >      > i think about something like this:
>      >      >
>      >
>     https://github.com/apache/tomcat/blob/master/java/org/apache/el/MethodExpressionImpl.java#L267
>     <https://github.com/apache/tomcat/blob/master/java/org/apache/el/MethodExpressionImpl.java#L267>
>      >   
>       <https://github.com/apache/tomcat/blob/master/java/org/apache/el/MethodExpressionImpl.java#L267 <https://github.com/apache/tomcat/blob/master/java/org/apache/el/MethodExpressionImpl.java#L267>>
>      >      >
>      >      > try {
>      >      >      putTempVar("arguments", params);
>      >      >      Object result = this.getNode().invoke(ctx,
>     this.paramTypes,
>      >     params);
>      >      > }
>      >      > finally {
>      >      >      removeTempVar("arguments");
>      >      > }
>      >      >
>      >      > Do you know what i mean?
>      >      > Maybe you have a good idea to implement this easily :) I did a
>      >     short try
>      >      > it but quite hard to open Tomcat with its Ant build and i dont
>      >     know the
>      >      > EL internals that good like you.
>      >      >
>      >      > Best regards,
>      >      > Thomas
>      >      >
>      >      >
>      >      > Am Mi., 24. Feb. 2021 um 10:22 Uhr schrieb Mark Thomas
>      >     <markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
>     <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>
>      >      > <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
>     <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>>>:
>      >      >
>      >      >     Hi Thomas,
>      >      >
>      >      >     Can you provide an example of how you would like this
>     to work
>      >     and the
>      >      >     specific changes you are asking for in the EL API as there
>      >     are lots of
>      >      >     ways this could be done.
>      >      >
>      >      >     Thanks,
>      >      >
>      >      >     Mark
>      >      >
>      >      >
>      >      >     On 24/02/2021 08:53, Thomas Andraschko wrote:
>      >      >     > Hi Mark,
>      >      >     >
>      >      >     > you may be right that this could be done in JSF only by
>      >     manual push
>      >      >     > "event" and the obj instance to the ELContext.
>      >      >     >
>      >      >     > But i still think this could be a good change for the EL
>      >     specs to
>      >      >     > introduce the "arguments" array as there is no way to
>      >     reference the
>      >      >     > parameters passed to MethodExpression#invoke.
>      >      >     > I like it more to enhance the EL specs instead of
>     changing a
>      >      >     > "implementation detail" in JSF (same as e.g.
>     RequestScoped
>      >     should be
>      >      >     > implemented in Servlet, not CDI).
>      >      >     > It just makes EL a bit more flexible.
>      >      >     > I think there might be other usescases as well and the
>      >      >     implementation is
>      >      >     > quite easy at EL side.
>      >      >     >
>      >      >     > Best regards,
>      >      >     > Thomas
>      >      >     >
>      >      >     >
>      >      >     > Am Di., 23. Feb. 2021 um 15:49 Uhr schrieb Mark Thomas
>      >      >     <markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
>     <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>
>      >     <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
>     <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>>
>      >      >     > <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
>     <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>
>      >     <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>
>     <mailto:markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>>>>>:
>      >      >     >
>      >      >     >     On 22/02/2021 13:40, Thomas Andraschko wrote:
>      >      >     >     > any feedback, Mark? :)
>      >      >     >
>      >      >     >     Hi,
>      >      >     >
>      >      >     >     Sorry I have taken a while to reply.
>      >      >     >
>      >      >     >     I agree with you that there are two solutions as you
>      >     set out
>      >      >     below. More
>      >      >     >     commentary in-line.
>      >      >     >
>      >      >     >     > Am Fr., 5. Feb. 2021 um 12:19 Uhr schrieb Thomas
>      >     Andraschko
>      >      >     >     > <tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>
>      >     <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>> <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>
>      >     <mailto:tandraschko@xxxxxxxxxx <mailto:tandraschko@xxxxxxxxxx>>>
>      >      >     <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>
>      >     <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>> <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>
>      >     <mailto:tandraschko@xxxxxxxxxx <mailto:tandraschko@xxxxxxxxxx>>>>
>      >      >     >     <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>
>      >     <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>> <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>
>      >     <mailto:tandraschko@xxxxxxxxxx <mailto:tandraschko@xxxxxxxxxx>>>
>      >      >     <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>
>      >     <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>> <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>
>      >     <mailto:tandraschko@xxxxxxxxxx
>     <mailto:tandraschko@xxxxxxxxxx>>>>>>:
>      >      >     >     >
>      >      >     >     >     Hi Mark,
>      >      >     >     >
>      >      >     >     >     just to be clear, this is what JSF always does
>      >     and i dont
>      >      >     >     think that
>      >      >     >     >     this should be changed, everything should
>     be in EL.
>      >      >     >     >
>      >      >     >     >     MethodExpression me =
>      >      >     >     >   
>       ExpressionFactory#createMethodExpression(context,
>      >      >     >     expressionString,
>      >      >     >     >     new Class[] { AjaxBehaviorEvent.class })
>      >      >     >     >     me.invoke(context, new Object[] {
>      >      >     ajaxBehaviorEventInstance });
>      >      >     >     >
>      >      >     >     >
>      >      >     >     >     I think there are 2 solutions for the EL
>     specs /
>      >     impl:
>      >      >     >     >
>      >      >     >     >     1) add a syntax to reference a "implicit"
>     param,
>      >     maybe
>      >      >     similar
>      >      >     >     to JS:
>      >      >     >     >         #{bean.method(arguments[0],  someVar)}
>      >      >     >
>      >      >     >     This solution does not require any changes to the EL
>      >      >     specification. It
>      >      >     >     does require (relatively simple) changes to the
>     JSF. In
>      >     summary:
>      >      >     >
>      >      >     >     - define "event" as referring to the appropriate
>      >     AjaxBehaviorEvent
>      >      >     >
>      >      >     >     - users would then us #{bean.method(event, someVar)}
>      >      >     >
>      >      >     >     - JSF would evaluate this in an ELContext that
>     resolved
>      >      >     "event" to the
>      >      >     >       appropriate (creating if necessary)
>     AjaxBehaviorEvent
>      >     instance
>      >      >     >
>      >      >     >
>      >      >     >     I also looked into several variations of this
>     but for a
>      >      >     solution where
>      >      >     >     the user indicates (e.g. via an additional
>     parameter in the
>      >      >     method) they
>      >      >     >     want to receive the AjaxBehaviorEvent the approach
>      >     above is by
>      >      >     far the
>      >      >     >     simplest to implement - both for EL and JSF.
>      >      >     >
>      >      >     >
>      >      >     >     >     2) add some auto-lookup-magic into EL,
>     like you
>      >      >     mentioned in the
>      >      >     >     >     last mail
>      >      >     >
>      >      >     >     This is more complex that option 1. The search order
>      >     needs to
>      >      >     be well
>      >      >     >     defined. If user specifies
>     #{bean.method(someVar)} then, if
>      >      >     both exist,
>      >      >     >     which is used in preference method(event,
>     someVar) or
>      >      >     method(someVar)?
>      >      >     >     If there is any requirement to make that preference
>      >      >     configurable you are
>      >      >     >     - essentially - back at option 1 and a simpler
>     solution.
>      >      >     >
>      >      >     >     Assuming a preference order can be defined,
>     there needs
>      >     to be
>      >      >     sufficient
>      >      >     >     API exposed to:
>      >      >     >
>      >      >     >     - determine if a method with the additional
>     parameter
>      >     exists
>      >      >     >     - execute that method with the additional parameter
>      >     provided
>      >      >     by the
>      >      >     >       caller
>      >      >     >
>      >      >     >     How much of this is bundled up into a single API
>     call
>      >     to EL vs
>      >      >     EL being
>      >      >     >     extended with a handful of new generic methods
>     to allow
>      >     a range of
>      >      >     >     manipulations on MethodExpressions is TBD. The
>     generic
>      >      >     approach requires
>      >      >     >     more work in both EL and JSF but provides for
>     greater
>      >     flexibility.
>      >      >     >
>      >      >     >     >     I >personally< like 1) more as it forces
>     the user
>      >     to think
>      >      >     >     about the
>      >      >     >     >     method-mapping and is more flexible.
>      >      >     >
>      >      >     >     +1 I think this is the much better solution for this
>      >      >     particular problem.
>      >      >     >
>      >      >     >     >     2) is of course also nice but we need a lot of
>      >      >     fallbacks. Not sure
>      >      >     >     >     about the performance here, but probably
>     the resolved
>      >      >     method is
>      >      >     >     >     cached in the "MethodExpression" instance?
>      >      >     >
>      >      >     >     Maybe. That would be an issue for the EL
>     implementations
>      >      >     rather than the
>      >      >     >     API.
>      >      >     >
>      >      >     >
>      >      >     >     So, in summary, I think the best way forward is
>     option one
>      >      >     which should
>      >      >     >     be implementable in JSF without any changes to
>     the EL API.
>      >      >     >
>      >      >     >     Kind regards,
>      >      >     >
>      >      >     >     Mark
>      >      >     >     _______________________________________________
>      >      >     >     faces-dev mailing list
>      >      >     > faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>
>      >     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>>
>      >      >     <mailto:faces-dev@xxxxxxxxxxx
>     <mailto:faces-dev@xxxxxxxxxxx> <mailto:faces-dev@xxxxxxxxxxx
>     <mailto:faces-dev@xxxxxxxxxxx>>
>      >     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>>>
>      >      >     >     To unsubscribe from this list, visit
>      >      >     > https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>
>      >     <https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>>
>      >      >     >
>      >      >     >
>      >      >     > _______________________________________________
>      >      >     > faces-dev mailing list
>      >      >     > faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>
>      >     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>>
>      >      >     > To unsubscribe from this list, visit
>      >      > https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>
>      >     <https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>>
>      >      >     >
>      >      >
>      >      >     _______________________________________________
>      >      >     faces-dev mailing list
>      >      > faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>
>      >     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>>
>      >      >     To unsubscribe from this list, visit
>      >      > https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>
>      >     <https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>>
>      >      >
>      >      >
>      >      > _______________________________________________
>      >      > faces-dev mailing list
>      >      > faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>
>      >      > To unsubscribe from this list, visit
>      > https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>
>      >     <https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>>
>      >      >
>      >
>      >     _______________________________________________
>      >     faces-dev mailing list
>      > faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     <mailto:faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>>
>      >     To unsubscribe from this list, visit
>      > https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>
>      >     <https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>>
>      >
>      >
>      > _______________________________________________
>      > faces-dev mailing list
>      > faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>      > To unsubscribe from this list, visit
>     https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>
>      >
>
>     _______________________________________________
>     faces-dev mailing list
>     faces-dev@xxxxxxxxxxx <mailto:faces-dev@xxxxxxxxxxx>
>     To unsubscribe from this list, visit
>     https://www.eclipse.org/mailman/listinfo/faces-dev
>     <https://www.eclipse.org/mailman/listinfo/faces-dev>
>
>
> _______________________________________________
> faces-dev mailing list
> faces-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/faces-dev
>

_______________________________________________
faces-dev mailing list
faces-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/faces-dev

Back to the top