MyFaces Core
  1. MyFaces Core
  2. MYFACES-2552

TagValueExpression.getType() returns null if the property in the managed bean is null and the expression points to a facelets composite component attribute

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.0
    • Fix Version/s: 2.0.10, 2.1.4
    • Component/s: JSR-314
    • Labels:
      None

      Description

      if you have a facelets composite component with an attribute "test" that points to a property in a managed bean (e.g. #

      {myBean.property}

      ) which is currently null and you refer to that attribute in the implementation via #

      {cc.attrs.test} you can get the current value (null) or set a new value but you cannot get the type of the property (e.g. String[]). However if the property in the managed bean is non-null you can get the type.

      For example:

      <cc:interface name="mycomponent">
      <cc:attribute name="test" required="true"/>
      </cc:interface>
      <cc:implementation>
      <h:selectManyListbox value="#{cc.attrs.test}

      ">
      <f:selectItems value="#

      {some select items}

      "/>
      </h:selectManyListbox>
      </cc:implementation>

      --> calling #

      {cc.attrs.test}.getType() will fail if #{cc.attrs.test}

      resolves to null, but will work if #

      {cc.attrs.test}

      resolves to some valid value.

      This currently results in a NullPointerException in _SharedRendererUtils.getConvertedUISelectManyValue().

      1. spec745mf.patch.txt
        4 kB
        Matt Benson
      2. MYFACES-2552-spec-proposal.patch
        2 kB
        Jakob Korherr
      3. MYFACES-2552-4.patch
        8 kB
        Leonardo Uribe

        Issue Links

          Activity

          Hide
          Jakob Korherr added a comment -

          I just found out why this happens: #

          {cc.attrs}

          resolves to a Map (CompositeComponentAttributesMapWrapper) and thus javax.el.MapELResolver is used to resolve the values. Here is the important part of the implementation of the getType() method from Tomcat:

          if (base instanceof Map<?,?>)

          { context.setPropertyResolved(true); Object obj = ((Map<?,?>) base).get(property); return (obj != null) ? obj.getClass() : null; }

          This explains the behavior. So we can only circumvent this by not using a Map, however I don't know if we should really change this...

          Show
          Jakob Korherr added a comment - I just found out why this happens: # {cc.attrs} resolves to a Map (CompositeComponentAttributesMapWrapper) and thus javax.el.MapELResolver is used to resolve the values. Here is the important part of the implementation of the getType() method from Tomcat: if (base instanceof Map<?,?>) { context.setPropertyResolved(true); Object obj = ((Map<?,?>) base).get(property); return (obj != null) ? obj.getClass() : null; } This explains the behavior. So we can only circumvent this by not using a Map, however I don't know if we should really change this...
          Hide
          Jakob Korherr added a comment -

          One solution for this would be to return a special type != Map when resolving #

          {cc.attrs}

          and providing a special ELResolver for this type. Then we could use the original ValueExpressions of the attributes from the composite component to determine the type. Of course this would totally break the spec!!!

          What are your opinions about that? Is this too unimportant to make such great changes or should we consult the EG and maybe change this behavior? Maybe in the next major release (2.1)?

          Show
          Jakob Korherr added a comment - One solution for this would be to return a special type != Map when resolving # {cc.attrs} and providing a special ELResolver for this type. Then we could use the original ValueExpressions of the attributes from the composite component to determine the type. Of course this would totally break the spec!!! What are your opinions about that? Is this too unimportant to make such great changes or should we consult the EG and maybe change this behavior? Maybe in the next major release (2.1)?
          Hide
          Jakob Korherr added a comment -

          For now I'll commit a very ugly temporal workaround for this. Note that mojarra currently does the same thing as this workaround for this special scenario.

          Show
          Jakob Korherr added a comment - For now I'll commit a very ugly temporal workaround for this. Note that mojarra currently does the same thing as this workaround for this special scenario.
          Hide
          Leonardo Uribe added a comment -

          I think the only thing we can do is assume this case return null and deal with it, retrieving the real value and check its type. It is possible to change the ELResolver (Flash object implements Map but FlashELResolver resolve its values, instead MapELResolver).

          In this example there is no way to check the type without retrieve the value, and note #

          {cc.attrs}

          is a Map<String,Object>. I remember variants of the same issue long time ago on myfaces and in that time the solution was the same.

          Show
          Leonardo Uribe added a comment - I think the only thing we can do is assume this case return null and deal with it, retrieving the real value and check its type. It is possible to change the ELResolver (Flash object implements Map but FlashELResolver resolve its values, instead MapELResolver). In this example there is no way to check the type without retrieve the value, and note # {cc.attrs} is a Map<String,Object>. I remember variants of the same issue long time ago on myfaces and in that time the solution was the same.
          Hide
          Jakob Korherr added a comment -

          With this patch the CompositeComponentELResolver is enabled to determine the type of attributes from CompositeComponentAttributesMapWrapper. This would make some scenarios related to composite components work better, although it completely breaks the spec.

          I will try to get this into the spec.

          Show
          Jakob Korherr added a comment - With this patch the CompositeComponentELResolver is enabled to determine the type of attributes from CompositeComponentAttributesMapWrapper. This would make some scenarios related to composite components work better, although it completely breaks the spec. I will try to get this into the spec.
          Show
          Jakob Korherr added a comment - Filed spec issue: https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=745
          Hide
          Matt Benson added a comment - - edited

          For future reference, this issue can be worked around in any JSF 2+ environment by simply adding a custom ELResolver with this method:

          @Override
          public Class<?> getType(ELContext context, Object base, Object property) {
          if (base instanceof CompositeComponentExpressionHolder && property instanceof String) {
          ValueExpression expr = ((CompositeComponentExpressionHolder) base).getExpression((String) property);
          if (expr != null)

          { return expr.getType(context); }

          }
          return null;
          }

          Remaining methods should return null/false i.e. do nothing.

          Show
          Matt Benson added a comment - - edited For future reference, this issue can be worked around in any JSF 2+ environment by simply adding a custom ELResolver with this method: @Override public Class<?> getType(ELContext context, Object base, Object property) { if (base instanceof CompositeComponentExpressionHolder && property instanceof String) { ValueExpression expr = ((CompositeComponentExpressionHolder) base).getExpression((String) property); if (expr != null) { return expr.getType(context); } } return null; } Remaining methods should return null/false i.e. do nothing.
          Hide
          Matt Benson added a comment -

          Also, spec issue #745 is considered resolved at this point. We just need to address it when we do the JSF 2.2 implementation in MyFaces.

          Show
          Matt Benson added a comment - Also, spec issue #745 is considered resolved at this point. We just need to address it when we do the JSF 2.2 implementation in MyFaces.
          Hide
          Michael Kurz added a comment -

          Continued discussion from MyFaces-3311. What is the status on this?

          For what I see, it will be fixed in JSF 2.2 (see [1]). But shouldn't this be fixed for older versions too?

          [1]: http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-745

          Show
          Michael Kurz added a comment - Continued discussion from MyFaces-3311. What is the status on this? For what I see, it will be fixed in JSF 2.2 (see [1] ). But shouldn't this be fixed for older versions too? [1] : http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-745
          Hide
          Matt Benson added a comment -

          It's not technically considered a bug, but a spec enhancement, so probably won't be fixed for older versions. Have you tried the workaround at https://issues.apache.org/jira/browse/MYFACES-2552?focusedCommentId=13071894&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13071894 ?

          Show
          Matt Benson added a comment - It's not technically considered a bug, but a spec enhancement, so probably won't be fixed for older versions. Have you tried the workaround at https://issues.apache.org/jira/browse/MYFACES-2552?focusedCommentId=13071894&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13071894 ?
          Hide
          Michael Kurz added a comment -

          It's no real issue for me at present and there are several ways to make it work (with detours). But I am convinced this should be fixed as IMO this is basic functionality.

          Show
          Michael Kurz added a comment - It's no real issue for me at present and there are several ways to make it work (with detours). But I am convinced this should be fixed as IMO this is basic functionality.
          Hide
          Matt Benson added a comment -

          Patch to bring MyFaces into compliance with spec v2.2 in this regard. Augments Jakob's original approach as described in the spec.

          Show
          Matt Benson added a comment - Patch to bring MyFaces into compliance with spec v2.2 in this regard. Augments Jakob's original approach as described in the spec.
          Hide
          Matt Benson added a comment -

          This latest patch only begins to address the idea of providing a context parameter with which this behavior may be disabled for MyFaces < v2.2.x . We should probably hammer that out on dev@myfaces. I am still looking into reworking my tests into MyFaces' test structure.

          Show
          Matt Benson added a comment - This latest patch only begins to address the idea of providing a context parameter with which this behavior may be disabled for MyFaces < v2.2.x . We should probably hammer that out on dev@myfaces. I am still looking into reworking my tests into MyFaces' test structure.

            People

            • Assignee:
              Leonardo Uribe
              Reporter:
              Jakob Korherr
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development