Feature request: Add method getMethodReference for MethodExpression parameter #279

Closed
dmaidaniuk opened this Issue Jun 30, 2016 · 12 comments

Projects

None yet

2 participants

@dmaidaniuk
dmaidaniuk commented Jun 30, 2016 edited

In our project we extended OmniFaces ExpressionInspector by one more additional method MethodReference getMethodReference(ELContext elContext, MethodExpression methodExpression). Method is pretty obvious as ongoing for existing similar method for ValueExpression. And it could be useful for other OmniFaces users.
That's how it looks like on OmniFaces v1.8.3 code base:

package org.omnifaces.el.experimental;

import javax.el.ELContext;
import javax.el.ELException;
import javax.el.ExpressionFactory;
import javax.el.MethodExpression;
import javax.el.PropertyNotFoundException;
import javax.el.ValueExpression;
import javax.el.ValueReference;
import javax.faces.view.facelets.FaceletContext;
import org.omnifaces.el.ExpressionInspector;
import static org.omnifaces.el.ExpressionInspector.findMethod;
import static org.omnifaces.el.ExpressionInspector.getValueReference;
import org.omnifaces.el.MethodExpressionValueExpressionAdapter;
import org.omnifaces.el.MethodReference;
import org.omnifaces.util.Faces;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class ExtendedExpressionInspector extends ExpressionInspector {

    private static final Logger log = LoggerFactory.getLogger(ExtendedExpressionInspector.class);

    private ExtendedExpressionInspector() {
    }

    /**
     * Gets a MethodReference from a MethodExpression.
     * <p>
     * Note that the method reference contains <em>a</em> method with the name the expression refers to. Overloads are
     * not supported.
     * <p>
     * Reused <code>ExpressionInspector#getValueReference(...)</code> method to apply advanced OmniFaces ELContext wrapper.
     * Standard <code>com.sun.faces.el.ELContextImpl</code> has problems with resolving parameterized methods.
     * <p>
     * @param elContext the context of this evaluation
     * @param methodExpression the method expression being evaluated
     * @return a MethodReference holding the final base and Method where the value expression evaluated to or NULL.
     */
    public static MethodReference getMethodReference(ELContext elContext, MethodExpression methodExpression) {

        MethodReference methodReference = null;
        if (methodExpression == null) {
            return methodReference;
        }

        String expr = methodExpression.getExpressionString();
        try {

            ExpressionFactory factory = Faces.getApplication().getExpressionFactory();
            ValueExpression valueExpression = factory.createValueExpression(elContext, expr, Object.class);

            methodReference = getSafelyMethodReference(elContext, valueExpression);
            if (methodReference == null) {                 
                methodReference = getMethodReferenceInFaceletContext(factory, expr);
            }
        }
        catch (ELException e) {
            if (log.isTraceEnabled()) {
                log.trace("Error during evaluation: " + e.getMessage());
            }
        }
        return methodReference;
    }

    /**
     * Protected from NullPointerException implementation of <code>ExpressionInspector.getMethodReference(...)</code> method.
     * Also caught <code>PropertyNotFoundException</code> in case if base class redefined by <code>ui:param</code>.
     * <p>
     * 
     * @param elContext
     * @param valueExpression
     * @return MethodReference of given valueExpression or NULL
     */
    public static MethodReference getSafelyMethodReference(ELContext elContext, ValueExpression valueExpression) {
        try {
            ValueReference valueReference = getValueReference(elContext, valueExpression);
            Object base = valueReference.getBase();
            Object methodName = valueReference.getProperty();
            if (base != null && methodName != null) {
                return new MethodReference(base, findMethod(base, methodName.toString()));
            }
        }
        catch (PropertyNotFoundException ex) {
            if (log.isTraceEnabled()) {
                log.trace("Error during evaluation: " + ex.getMessage());
            }
        }
        return null;
    }

    private static MethodReference getMethodReferenceInFaceletContext(ExpressionFactory factory, String expr) {
        // UI Param case first
        FaceletContext faceletElContext = Faces.getFaceletContext();
        ValueExpression exp = factory.createValueExpression(faceletElContext, expr, Object.class);
        MethodReference methodReference = getSafelyMethodReference(faceletElContext, exp);
        if (methodReference == null) { // OmniFaces MethodParam case next
            Object faceletValue = exp.getValue(faceletElContext);
            if (faceletValue instanceof MethodExpressionValueExpressionAdapter) {
                ValueExpression originExpression = ((MethodExpressionValueExpressionAdapter)faceletValue).getValueExpression();
                methodReference = getSafelyMethodReference(faceletElContext, originExpression);
            }
        }
        return methodReference;
    }

}

I'm not sure that given implementation is optimal. However it works quite good for us.
Also if such method will be included into OmniFaces it will be easier for us to migrate to latest OmniFaces 2.x version, where ExpressionInspector was significantly changed.

Thanks,
Dmytro

@BalusC
Member
BalusC commented Jul 5, 2016

We will look at it.

I'm however curious to your real world use cases for this.

@dmaidaniuk
dmaidaniuk commented Jul 6, 2016 edited

Good. Thanks!

About real world use case: we use this code to check if method has custom security annotation, similar to standard Apache Shiro's RequiresPermissions. If such annotation present in JSF bean for target method behind MethodExpression and user don't have required permissions, than we set rendered attribute of related commandButton/commandLink to false in VisitCallback. As a result user will not see actions which aren't allowed for him.
Unfortunately standard shiro-web extension and even shiro-faces project don't have such functionality out of the box.

Regards,
Dmytro

@BalusC
Member
BalusC commented Jul 6, 2016

Makes sense. Thank you.

@BalusC BalusC closed this in 414f8ce Jul 6, 2016
@BalusC
Member
BalusC commented Jul 6, 2016

It's available in today's latest 2.5-SNAPSHOT. Please let me know if everything works for you.

@dmaidaniuk

Cool! This was quick! I will test this snapshot ASAP.

@dmaidaniuk

Hi @BalusC,

I verified your implementation and found one issue in it. Method expression can be attached to UI param and passed to some reusable template. In template it wrapped to o:methodParam and attached to action attribute:
template.xhtml

<ui:composition xmlns="http://www.w3.org/1999/xhtml"
                xmlns:f="http://java.sun.com/jsf/core"
                xmlns:h="http://java.sun.com/jsf/html"
                xmlns:ui="http://java.sun.com/jsf/facelets"
                xmlns:p="http://primefaces.org/ui"
                xmlns:o="http://omnifaces.org/ui">

  <o:methodParam name="remoteUpdateAction" value="#{updateAction}"/>

  <!-- some truncated markup -->

  <p:commandLink value="Update" action="#{remoteUpdateAction}" />

  <ui:insert name="content" />

</ui:composition>

usage.xhtml

<ui:composition template="/templates/template.xhtml"
                xmlns="http://www.w3.org/1999/xhtml"
                xmlns:f="http://java.sun.com/jsf/core"
                xmlns:ui="http://java.sun.com/jsf/facelets"
                xmlns:p="http://primefaces.org/ui"
                xmlns:h="http://java.sun.com/jsf/html">

   <ui:param name="updateAction" value="#{bean.updateSomething}"/>

   <ui:define name="content">
   <!-- some truncated markup -->
   </ui:define>

</ui:composition>

In given scenario MethodReference getMethodReference(ELContext context, MethodExpression methodExpression) will fail with NullPointerException in line

String property = inspectorElContext.getProperty().toString();

because inspectorElContext.getProperty() will be NULL here. That's why in my initial example we added method getSafelyMethodReference(...) where catch possible nullable situations.

In rest cases method works good.

Thanks,
Dmytro

@BalusC BalusC reopened this Jul 8, 2016
@BalusC
Member
BalusC commented Jul 8, 2016 edited

I considered o:methodParam and ui:param cases individually. But I indeed didn't consider the case where they are combined. Thank you for reporting back, I'll look at it.

@BalusC BalusC closed this in d583e8c Jul 9, 2016
@BalusC
Member
BalusC commented Jul 9, 2016

I have further improved getMethodReference(). It's available in today's latest 2.5-SNAPSHOT. Let me know if that does it for you.

@dmaidaniuk

This build gives me next error on methods with parameters:

javax.el.MethodNotFoundException: /app/admin/index.xhtml @102,133 action="#{setupModel.triggerJob(job)}": JBWEB006019: Method not found: foo.bar.SetupModel@4223cbb8.triggerJob()
at com.sun.faces.facelets.el.TagMethodExpression.getMethodInfo(TagMethodExpression.java:97) [jsf-impl-2.2.8.jar:2.2.8]
at org.omnifaces.el.ExpressionInspector.getMethodReference(ExpressionInspector.java:139) [omnifaces-2.5-SNAPSHOT.jar:2.5-SNAPSHOT]

As you can see somehow it is searching method without parameters when actually requested parametrized one.
I only found one possible explanation in Seam project documentation here (Section 35.1.2. Limitations and Hints):

Calling a MethodExpression from Java code — Normally, when a MethodExpression is created, the parameter types are passed in by JSF. In the case of a method binding, JSF assumes that there are no parameters to pass. With this extension, we can't know the parameter types until after the expression has been evaluated. This has two minor consequences:

When you invoke a MethodExpression in Java code, parameters you pass may be ignored. Parameters defined in the expression will take precedence.

Ordinarily, it is safe to call methodExpression.getMethodInfo().getParamTypes() at any time. For an expression with parameters, you must first invoke the MethodExpression before calling getParamTypes().

I don't know if it is true for EL 2.2 also.

@BalusC
Member
BalusC commented Jul 14, 2016

Thank you for the feedback. I have yet further improved getMethodReference(). It's available in today's latest 2.5-SNAPSHOT. Let me know if that works out better.

@dmaidaniuk

Seems now all works fine.
I have a problem with optional action attribute in facelets custom tag, where NullPointerException can be thrown. But it is a different story.

Thanks, @BalusC !

@BalusC
Member
BalusC commented Jul 20, 2016

Great, thank you for the feedback and helping with improving OmniFaces!

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