dev@javaserverfaces.java.net

ActionListenerTag.BindingActionListener behaves inconsitend in respect to Client- vs Serverside statesaving

From: Norbert Truchsess <norbert.truchsess_at_t-online.de>
Date: Sun, 28 Oct 2007 01:45:26 +0200

in com.sun.faces.taglib.jsf_core.ActionListenerTag.BindingActionListener we have:

  private static class BindingActionListener
         implements ActionListener, Serializable {

        private transient ActionListener instance;

...

        public void processAction(ActionEvent event) throws
AbortProcessingException {

            if (instance == null) { // <------ depends on way of statesaving!
                instance = (ActionListener)
                     Util.getListenerInstance(type, binding);
            }
            if (instance != null) {
                instance.processAction(event);
            } else {
                if (logger.isLoggable(Level.WARNING)) {
                    logger.log(Level.WARNING,
                              
"jsf.core.taglib.action_or_valuechange_listener.null_type_binding",
                               new Object[] {
                                "ActionListener",
                               
event.getComponent().getClientId(FacesContext.getCurrentInstance())});
                }
            }
        }

Since 'instance' is marked as transient, it will not undergo serialization and
thus will be null on postback after state is restored in case of client-side
statesaving. If statesaving is set to server, no serialization takes place so
BindingActionListener would return the old reference to instance on postback.

This is inconsistend behaviour.

And if the Listener e.g. happens to be a request-scoped managed bean returning
the old reference instead of re-evaluating the expression on postback will
return the reference to the outdated instance (which is very bad!)

Same issue with 'transient XXXXXListener instance' is being found in both
com.sun.faces.taglib.jsf_core.PhaseListenerTag.BindingPhaseListener and
com.sun.faces.taglib.jsf_core.ValueChangeListenerTag.BindingValueChangeListener

I've filed Issue # 656 against the RI:
https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=656

On the other hand the spec says:

'If binding is set, create a ValueExpression by invoking
Application.createValueExpression() with binding as the expression argument,
and Object.class as the expectedType argument. Use the ValueExpression to obtain
a reference to the ActionListener instance. If there is no exception thrown, and
ValueExpression.getValue() returned a non-null object that implements
javax.faces.event.ActionListener, register it by calling addActionListener().
If there was an exception thrown, rethrow the exception as a JspException.'

According to the spec the instance returned by evaluation of the binding-expression would have to be used. It doesn't mention that it's legal to use a lazy-loading wrapper for optimization.
But if we wouldn't use a lazy-loading wrapper, it wouldn't be possible to reevaluate the binding-expression on postback at all. From my point of view this is also a spec-bug - the spec should either specify that it has to be exactly the single instance of Listener that is to be used, or the binding-expression would have to be reevaluated on every access (as this is done with deferred expressions in UIComponent-attributes getter-methods).

I've filed a spec bug for this as well:
https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=320


- Norbert Truchsess