MyFaces Core
  1. MyFaces Core
  2. MYFACES-1577

PropertyResolver should throw PropertyNotFoundException

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.2.0
    • Component/s: JSR-252
    • Labels:
      None

      Description

      According to the spec several methods in PropertyResolver should throw PropertyNotFoundException in the following circumstances:

      getValue(Object base, int index)
      PropertyNotFoundException - if the index is out of bounds or if base is null

      setValue(Object base, int index, Object value)
      PropertyNotFoundException - if the index is out of bounds or if base is null

      setValue(Object base, Object property, Object value)
      PropertyNotFoundException - if the specified bean base object property does not exist or if base or property is null

      BTW, MYFACES-1576 already addressed these two cases:
      getType(Object base, int index)
      PropertyNotFoundException - if the index is out of bounds or if base is null
      getType(Object base, Object property)
      PropertyNotFoundException - if the specified bean base object property does not exist or if base or property is null

      1. MYFACES-1577.patch
        2 kB
        Paul McMahan
      2. MYFACES-1577-2.patch
        1.0 kB
        Paul McMahan

        Activity

        Paul McMahan created issue -
        Hide
        Paul McMahan added a comment -

        patch checks arguments passed to PropertyResolverImpl.getValue() and PropertyResolverImpl.setValue() according to the spec

        Show
        Paul McMahan added a comment - patch checks arguments passed to PropertyResolverImpl.getValue() and PropertyResolverImpl.setValue() according to the spec
        Paul McMahan made changes -
        Field Original Value New Value
        Attachment MYFACES-1577.patch [ 12354379 ]
        Paul McMahan made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Dennis Byrne added a comment -

        rev 523150

        Thanks again to Paul McMahan

        Show
        Dennis Byrne added a comment - rev 523150 Thanks again to Paul McMahan
        Dennis Byrne made changes -
        Assignee Dennis Byrne [ dennisbyrne ]
        Resolution Fixed [ 1 ]
        Fix Version/s 1.2.0-SNAPSHOT [ 12311938 ]
        Status Patch Available [ 10002 ] Closed [ 6 ]
        Hide
        Mathias Broekelmann added a comment -

        unfortunately the tck fails now.

        The TCK expects it this way:
        getValue(Object base, int index) should throw NPE or return a null value if base is null
        getValue(Object base, int index) should throw IndexOutOfBoundsException if Index is out of bounds or return a null value

        Show
        Mathias Broekelmann added a comment - unfortunately the tck fails now. The TCK expects it this way: getValue(Object base, int index) should throw NPE or return a null value if base is null getValue(Object base, int index) should throw IndexOutOfBoundsException if Index is out of bounds or return a null value
        Mathias Broekelmann made changes -
        Resolution Fixed [ 1 ]
        Status Closed [ 6 ] Reopened [ 4 ]
        Hide
        Martin Marinschek added a comment -

        Please revert this anyone - we have to accept a base of null so that:

        #

        {person.address.street}

        doesn't fail, even if address is null.

        I wanted to change this once as well, but it's not spec conform.

        regards,

        Martin

        Show
        Martin Marinschek added a comment - Please revert this anyone - we have to accept a base of null so that: # {person.address.street} doesn't fail, even if address is null. I wanted to change this once as well, but it's not spec conform. regards, Martin
        Hide
        Paul McMahan added a comment -

        It was actually the setValue() errors that prompted me to provide the patch. But while working on the patch I also noticed the discrepancy between getValue() and the spec/javadoc so I adjusted its behavior as well. As they say, "no good deed goes unpunished"

        I believe that the current behavior of getValue() is consistent with the spec/javadoc and that the test cases are in error. Last week I created GERONIMOTCK-22 and GERONIMOTCK-23 to track challenges for those test cases. But while that gets sorted out I agree that getValue() should be reverted back to its original form which as I recall threw the exceptions like you described.

        Show
        Paul McMahan added a comment - It was actually the setValue() errors that prompted me to provide the patch. But while working on the patch I also noticed the discrepancy between getValue() and the spec/javadoc so I adjusted its behavior as well. As they say, "no good deed goes unpunished" I believe that the current behavior of getValue() is consistent with the spec/javadoc and that the test cases are in error. Last week I created GERONIMOTCK-22 and GERONIMOTCK-23 to track challenges for those test cases. But while that gets sorted out I agree that getValue() should be reverted back to its original form which as I recall threw the exceptions like you described.
        Paul McMahan made changes -
        Status Reopened [ 4 ] Patch Available [ 10002 ]
        Hide
        Paul McMahan added a comment -

        patch reverts behavior of getValue() and passes the currently failing tests.

        Show
        Paul McMahan added a comment - patch reverts behavior of getValue() and passes the currently failing tests.
        Paul McMahan made changes -
        Attachment MYFACES-1577-2.patch [ 12355009 ]
        Paul McMahan made changes -
        Assignee Dennis Byrne [ dennisbyrne ] Paul McMahan [ pmcmahan ]
        Paul McMahan made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Matthias Weßendorf made changes -
        Affects Version/s  1.2.0 [ 12312576 ]
        Fix Version/s 1.2.0-SNAPSHOT [ 12311938 ]
        Affects Version/s 1.2.0-SNAPSHOT [ 12311938 ]
        Fix Version/s  1.2.0 [ 12312576 ]
        Matthias Weßendorf made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Paul McMahan
            Reporter:
            Paul McMahan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development