Uploaded image for project: 'OpenJPA'
  1. OpenJPA
  2. OPENJPA-2467

No setter was found for method like tStart

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.2
    • Fix Version/s: 2.4.0
    • Component/s: jpa
    • Labels:
      None

      Description

      Suppose having an entity class with fields like tStart, tEnd, tModify (with second letter in upper case). The corresponding getter and setter are gettStart and settStart, gettEnd, ... and not getTStart or setTStart.

      Inside class PersistenceMetaDataDefaults use of StringUtils.capitalize in method isDefaultPersistent generate the mistake.

      Look at this pdf, section 8.8
      http://download.oracle.com/otn-pub/jcp/7224-javabeans-1.01-fr-spec-oth-JSpec/beans.101.pdf

      1. OPENJPA-2467.txt
        22 kB
        Dalia Abo Sheasha
      2. OPENJPA-2467.txt
        20 kB
        Dalia Abo Sheasha

        Activity

        Hide
        curtisr7 Rick Curtis added a comment -

        Since the first two characters of tStart, tEnd, etc are not capitalized, the convention(per section 8.8) is to capitalize the first character of the property name.

        getTStart, getTEnd, etc is correct. Please reopen this JIRA with additional data if you disagree.

        Show
        curtisr7 Rick Curtis added a comment - Since the first two characters of tStart, tEnd, etc are not capitalized, the convention(per section 8.8) is to capitalize the first character of the property name. getTStart, getTEnd, etc is correct. Please reopen this JIRA with additional data if you disagree.
        Hide
        daniele.pirola@icteam.it Daniele Pirola added a comment -

        Specs at section 8.8 start explanation from getter and setter so the first two characters are the characters of the method, not of the property. In fact in the example there is "FooBah becomes fooBah" and not "fooBah becomes FooBah". Please see the code of method Introspector.decapitalize for a correct implementation of the rule.
        Please see also https://hibernate.atlassian.net/browse/HHH-8716
        this is another issue for the same problem

        Show
        daniele.pirola@icteam.it Daniele Pirola added a comment - Specs at section 8.8 start explanation from getter and setter so the first two characters are the characters of the method, not of the property. In fact in the example there is "FooBah becomes fooBah" and not "fooBah becomes FooBah". Please see the code of method Introspector.decapitalize for a correct implementation of the rule. Please see also https://hibernate.atlassian.net/browse/HHH-8716 this is another issue for the same problem
        Hide
        ayl2 Aron Lurie added a comment -

        Daniele,

        I believe you are misinterpreting the bean spec. As it says prior to the section that you referenced, "...Thus when we extract a property or event name from the middle of an existing Java name, we normally convert the first character to lower case." The situation that they are referring to is the one where they are trying to generate a field name FROM a getter name. This is the opposite of the situation that you are describing.

        If this is not clear please see also section 8.3.1 of the bean spec. where you will see a case of property name inference.

        -Aron

        Show
        ayl2 Aron Lurie added a comment - Daniele, I believe you are misinterpreting the bean spec. As it says prior to the section that you referenced, "...Thus when we extract a property or event name from the middle of an existing Java name, we normally convert the first character to lower case." The situation that they are referring to is the one where they are trying to generate a field name FROM a getter name. This is the opposite of the situation that you are describing. If this is not clear please see also section 8.3.1 of the bean spec. where you will see a case of property name inference. -Aron
        Hide
        daniele.pirola@icteam.it Daniele Pirola added a comment -

        This is a very common mistake I found many times in many java library and also in earliest version of IDE like Eclipse or IntelliJ.
        Please read this
        http://stackoverflow.com/questions/2948083/naming-convention-for-getters-setters-in-java
        or this
        http://dertompson.com/2013/04/29/java-bean-getterssetters/

        The spec are not so clear but please fix this bug. Also try to use BeanInfo introspector classes to find the correct property from a getter and viceversa, you will find that the correct setter for property like tStart is gettStart, also try in a JSF page

        Show
        daniele.pirola@icteam.it Daniele Pirola added a comment - This is a very common mistake I found many times in many java library and also in earliest version of IDE like Eclipse or IntelliJ. Please read this http://stackoverflow.com/questions/2948083/naming-convention-for-getters-setters-in-java or this http://dertompson.com/2013/04/29/java-bean-getterssetters/ The spec are not so clear but please fix this bug. Also try to use BeanInfo introspector classes to find the correct property from a getter and viceversa, you will find that the correct setter for property like tStart is gettStart, also try in a JSF page
        Hide
        curtisr7 Rick Curtis added a comment -

        After looking at it again this is an edge case that we (and many others) missed.

        Show
        curtisr7 Rick Curtis added a comment - After looking at it again this is an edge case that we (and many others) missed.
        Hide
        daniele.pirola@icteam.it Daniele Pirola added a comment -

        yes I undestand, for me it's usual because we have database field like T_START or K_PRODUCT that we convert into tStart and kProduct java property

        Show
        daniele.pirola@icteam.it Daniele Pirola added a comment - yes I undestand, for me it's usual because we have database field like T_START or K_PRODUCT that we convert into tStart and kProduct java property
        Hide
        dalia Dalia Abo Sheasha added a comment -

        I have attached a patch that includes my fix to this issue. FieldMetaData now handles generating the correct setter name for any case where the second letter of the variable is capitalized. Otherwise, the old behavior is observed. I also added code to look for getters that use the correct naming convention.
        To test the changes, I created two entities. One uses the correct naming convention where a variable named tStart would have the corresponding gettStart(). The other entity uses the old wrong way of naming where tStart would have the corresponding getTStart() method. The second entity is there to make sure the old behavior is still supported.

        Show
        dalia Dalia Abo Sheasha added a comment - I have attached a patch that includes my fix to this issue. FieldMetaData now handles generating the correct setter name for any case where the second letter of the variable is capitalized. Otherwise, the old behavior is observed. I also added code to look for getters that use the correct naming convention. To test the changes, I created two entities. One uses the correct naming convention where a variable named tStart would have the corresponding gettStart(). The other entity uses the old wrong way of naming where tStart would have the corresponding getTStart() method. The second entity is there to make sure the old behavior is still supported.
        Hide
        kwsutter Kevin Sutter added a comment -

        Thanks, Dalia. A couple of suggestions... In the code where you explain that we're now looking for both getaStart and getAStart, could you include the JavaBeans section as a reference? Down the road, if somebody looks at this code, they are going to wonder why this "exception" was put in place. Also, the text above made me think about another scenario... What happens if the attribute is named a_Start? That is, the second character is not an alphabetic character? Does the JavaBean spec outline this behavior? And, how would we respond? Other than that, this looks like a good patch. We're fixing this edge case, but leaving the current behavior.

        On a related note... What happens when we generate Entity classes? If we read an attribute from the database named aStart, which getter/setter methods get generated? Is this a completely separate code path and, thus, your code changes don't affect that processing? Or, are they related and now we're (accidentally) changing how the Entity classes are getting generated?

        Show
        kwsutter Kevin Sutter added a comment - Thanks, Dalia. A couple of suggestions... In the code where you explain that we're now looking for both getaStart and getAStart, could you include the JavaBeans section as a reference? Down the road, if somebody looks at this code, they are going to wonder why this "exception" was put in place. Also, the text above made me think about another scenario... What happens if the attribute is named a_Start? That is, the second character is not an alphabetic character? Does the JavaBean spec outline this behavior? And, how would we respond? Other than that, this looks like a good patch. We're fixing this edge case, but leaving the current behavior. On a related note... What happens when we generate Entity classes? If we read an attribute from the database named aStart, which getter/setter methods get generated? Is this a completely separate code path and, thus, your code changes don't affect that processing? Or, are they related and now we're (accidentally) changing how the Entity classes are getting generated?
        Hide
        dalia Dalia Abo Sheasha added a comment -

        Thank you for the suggestions, Kevin. I have attached a new patch that includes the line referring to the section of the JavaBeans section that should be explaining the naming convention.

        According to my understanding, the scenario where the field name is a_Start should yield the getter name getA_Start(). To double check, I generated the getter/setter in Eclipse and that's what gets generated by default (granted Eclipse isn't always correct). In the JUnit test, I indirectly test this by having a letter then a number.

        I am glad you pointed out the ReverseMappingTool. I went in and made a minor change in the CodeGenerator class so that the getters and setters follow the correct naming convention. I ran the tool and it generated the classes correctly.

        Show
        dalia Dalia Abo Sheasha added a comment - Thank you for the suggestions, Kevin. I have attached a new patch that includes the line referring to the section of the JavaBeans section that should be explaining the naming convention. According to my understanding, the scenario where the field name is a_Start should yield the getter name getA_Start(). To double check, I generated the getter/setter in Eclipse and that's what gets generated by default (granted Eclipse isn't always correct). In the JUnit test, I indirectly test this by having a letter then a number. I am glad you pointed out the ReverseMappingTool. I went in and made a minor change in the CodeGenerator class so that the getters and setters follow the correct naming convention. I ran the tool and it generated the classes correctly.
        Hide
        kwsutter Kevin Sutter added a comment -

        I'm good with your latest patch, Dalia. Since I know you are working with Rick, I'll let him take care of committing on your behalf. Thanks.

        Show
        kwsutter Kevin Sutter added a comment - I'm good with your latest patch, Dalia. Since I know you are working with Rick, I'll let him take care of committing on your behalf. Thanks.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1564931 from Rick Curtis in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1564931 ]

        OPENJPA-2467: Fix detection of property access method names. Patch contributed by Dalia Abo Sheasha.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1564931 from Rick Curtis in branch 'openjpa/trunk' [ https://svn.apache.org/r1564931 ] OPENJPA-2467 : Fix detection of property access method names. Patch contributed by Dalia Abo Sheasha.
        Hide
        curtisr7 Rick Curtis added a comment -

        Committed revision 1564931 to trunk. Thanks for the patch Dalia!

        Show
        curtisr7 Rick Curtis added a comment - Committed revision 1564931 to trunk. Thanks for the patch Dalia!

          People

          • Assignee:
            curtisr7 Rick Curtis
            Reporter:
            daniele.pirola@icteam.it Daniele Pirola
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development