Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.2
    • Component/s: Uncategorized
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      184

      Description

      From the reporter:

      The javadocs say UIComponentELTag.setBinding() throws an
      IllegalArgumentException, but it is just a plain setter and the signature throws
      JspException.
      Can we please change this to explicitly throw a JspException if the parameter is
      not a value reference, and a NPE if the parameter is null?

      Dennis Byrne
      ---------------------------------------------------------------------------
      This looks like it was carried over from the original UComponentTag.

      Choices:

      1. Leave JspException as part of the signature, but change the javadocs.

      2. Deprecate setBinding throws JspException and create new method.

      I vote for #1, but we would still have to add the logic to check if the
      arg is a valid ValueExpression. I'm not convinced we need to check for NPE,
      since the hasBinding method is available for developers to use.

      We also need to look at the core tag implementations for inconsistencies in this
      area.

        Activity

        Hide
        rogerk added a comment -

        Sadly, I'm noticing a lot of inconsistencies with the way the "setBinding" method is
        described in the javadocs, and the way it is implemented. I would like to
        consistently make
        the "setBinding" method throw a JspException if the "binding" argument is not a
        valid expression,
        and have it consistently described that way in the javadocs. Here is a summary
        of the
        inconsistencies of the "setBinding" method:

        javax.faces.webapp.UIComponentTag

        • javadoc says IllegalArgumentException;
        • implementation throws IllegalArgumentException;
        • method signature throws JspException
          javax.faces.webapp.UIComponentELTag
        • javadoc says IllegalArgumentException;
        • implementation does not throw any exception
        • method signature throws JspException
          com.sun.faces.taglib.jsf_core.ActionListenerTag
        • javadoc says JspException
        • implementation does not throw any exception
        • method signature does not throw any exception
          com.sun.faces.taglib.jsf_core.ConverterTag
        • javadoc does not specify any exception
        • implementation does not throw any exception
        • method signature does not throw any exception
          com.sun.faces.taglib.jsf_core.PhaseListenerTag
        • javadoc says JspException
        • implementation does not throw any exception
        • method signature does not throw any exception
          com.sun.faces.taglib.jsf_core.ValidatorTag
        • javadoc does not specify any exception
        • implementation does not throw any exception
        • method signature does not throw any exception
          com.sun.faces.taglib.jsf_core.ValueChangeListenerTag
        • javadoc says JspException
        • implementation does not throw any exception
        • method signature does not throw any exception
        Show
        rogerk added a comment - Sadly, I'm noticing a lot of inconsistencies with the way the "setBinding" method is described in the javadocs, and the way it is implemented. I would like to consistently make the "setBinding" method throw a JspException if the "binding" argument is not a valid expression, and have it consistently described that way in the javadocs. Here is a summary of the inconsistencies of the "setBinding" method: javax.faces.webapp.UIComponentTag javadoc says IllegalArgumentException; implementation throws IllegalArgumentException; method signature throws JspException javax.faces.webapp.UIComponentELTag javadoc says IllegalArgumentException; implementation does not throw any exception method signature throws JspException com.sun.faces.taglib.jsf_core.ActionListenerTag javadoc says JspException implementation does not throw any exception method signature does not throw any exception com.sun.faces.taglib.jsf_core.ConverterTag javadoc does not specify any exception implementation does not throw any exception method signature does not throw any exception com.sun.faces.taglib.jsf_core.PhaseListenerTag javadoc says JspException implementation does not throw any exception method signature does not throw any exception com.sun.faces.taglib.jsf_core.ValidatorTag javadoc does not specify any exception implementation does not throw any exception method signature does not throw any exception com.sun.faces.taglib.jsf_core.ValueChangeListenerTag javadoc says JspException implementation does not throw any exception method signature does not throw any exception
        Hide
        rogerk added a comment -

        Ok. Looking at the setBinding methods that take a ValueExpression arg.

        example: UIComponentELTag.setBinding

        public void setBinding(ValueExpression binding) throws JspException

        { this.binding = binding; }

        The binding arg coming into this method will always be a valid ValueExpression
        (it had to have been created using createValueExpression somewhere before
        calling this method). Therefore, throwing a JspException in this case, really
        makes no sense.

        So, ideally, I think this method should not throw any exception.
        However, if we remove the "throws JspException" from the method:

        public void setBinding(ValueExpression binding) throws JspException { this.binding = binding; }

        this will break any implementations that have extended UIComponentELTag and have
        overridden the setBinding method.

        So I think our options become:

        1. Remove JspException from method and document as known backwards compatability
        problem.
        2. Leave JspException in method signature, but change the javadocs to be
        consistent with method signature. This would apply to the other core tag
        classes below. So, for example, jsf-core.ActionListenerTag.setBinding
        has JspException in javadocs, but method signature has no exception - in
        this case we remove the JspException in javadocs.

        I say we go for #2.

        Show
        rogerk added a comment - Ok. Looking at the setBinding methods that take a ValueExpression arg. example: UIComponentELTag.setBinding public void setBinding(ValueExpression binding) throws JspException { this.binding = binding; } The binding arg coming into this method will always be a valid ValueExpression (it had to have been created using createValueExpression somewhere before calling this method). Therefore, throwing a JspException in this case, really makes no sense. So, ideally, I think this method should not throw any exception. However, if we remove the "throws JspException" from the method: public void setBinding(ValueExpression binding) throws JspException { this.binding = binding; } this will break any implementations that have extended UIComponentELTag and have overridden the setBinding method. So I think our options become: 1. Remove JspException from method and document as known backwards compatability problem. 2. Leave JspException in method signature, but change the javadocs to be consistent with method signature. This would apply to the other core tag classes below. So, for example, jsf-core.ActionListenerTag.setBinding has JspException in javadocs, but method signature has no exception - in this case we remove the JspException in javadocs. I say we go for #2.
        Hide
        rogerk added a comment -

        Javadoc changes to be consistent with method signatures for the setBinding method:

        SECTION: Modified Files
        ----------------------------
        M jsf-api/src/javax/faces/webapp/UIComponentELTag.java
        M jsf-ri/src/com/sun/faces/taglib/jsf_core/ActionListenerTag.java
        M jsf-ri/src/com/sun/faces/taglib/jsf_core/PhaseListenerTag.java
        M jsf-ri/src/com/sun/faces/taglib/jsf_core/ValueChangeListenerTag.java

        Show
        rogerk added a comment - Javadoc changes to be consistent with method signatures for the setBinding method: SECTION: Modified Files ---------------------------- M jsf-api/src/javax/faces/webapp/UIComponentELTag.java M jsf-ri/src/com/sun/faces/taglib/jsf_core/ActionListenerTag.java M jsf-ri/src/com/sun/faces/taglib/jsf_core/PhaseListenerTag.java M jsf-ri/src/com/sun/faces/taglib/jsf_core/ValueChangeListenerTag.java
        Hide
        rogerk added a comment -

        Created an attachment (id=108)
        Changes (Javadoc)

        Show
        rogerk added a comment - Created an attachment (id=108) Changes (Javadoc)
        Hide
        rogerk added a comment -

        checked in (javadoc changes only).

        Show
        rogerk added a comment - checked in (javadoc changes only).
        Hide
        Ed Burns added a comment -

        Prepare to delete "spec" subcomponent.

        Show
        Ed Burns added a comment - Prepare to delete "spec" subcomponent.
        Hide
        Ed Burns added a comment -

        Move all to 1.2

        Show
        Ed Burns added a comment - Move all to 1.2
        Hide
        Manfred Riem added a comment -

        Closing resolved issue out

        Show
        Manfred Riem added a comment - Closing resolved issue out

          People

          • Assignee:
            rogerk
            Reporter:
            rogerk
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: