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 
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

Back to the top