OpenJPA
  1. OpenJPA
  2. OPENJPA-2366

provide option to exclude schema name from generated @Table annotation for generated entities

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.0, 2.2.1
    • Fix Version/s: 2.3.0
    • Component/s: jpa
    • Labels:
      None

      Description

      Feature request:

      Provide an option for org.apache.openjpa.jdbc.meta.ReverseMappingTool that will exclude the schema name for the @Table annotation on generated entities. For example, if the schema name is not required and needs to be dynamically specified at deployment time via openjpa.jdbc.Schema.

      1. OPENJPA-2366-3.patch
        17 kB
        Austin Dorenkamp
      2. OPENJPA-2366doc-3.patch
        1 kB
        Austin Dorenkamp

        Activity

        Hide
        Austin Dorenkamp added a comment -

        Thanks Jeremy!

        Show
        Austin Dorenkamp added a comment - Thanks Jeremy!
        Jeremy Bauer made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 2.3.0 [ 12319463 ]
        Resolution Fixed [ 1 ]
        Hide
        Jeremy Bauer added a comment -

        Committed OPENJPA-2366-3.patch and OPENJPA-2366doc-3.patch to trunk (2.3.0) under rev 1519550.

        Show
        Jeremy Bauer added a comment - Committed OPENJPA-2366 -3.patch and OPENJPA-2366 doc-3.patch to trunk (2.3.0) under rev 1519550.
        Hide
        ASF subversion and git services added a comment -

        Commit 1519550 from Jeremy Bauer in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1519550 ]

        OPENJPA-2366 Committing code and unit tests contributed by Austin Dorenkamp

        Show
        ASF subversion and git services added a comment - Commit 1519550 from Jeremy Bauer in branch 'openjpa/trunk' [ https://svn.apache.org/r1519550 ] OPENJPA-2366 Committing code and unit tests contributed by Austin Dorenkamp
        Hide
        Jeremy Bauer added a comment -

        I'll get Austin's latest patches committed within the next few days. Working through a few dev env issues...

        Show
        Jeremy Bauer added a comment - I'll get Austin's latest patches committed within the next few days. Working through a few dev env issues...
        Hide
        Jeremy Bauer added a comment -

        The latest patch looks good. Thanks for submitting it. Adding a specialized field to classmetadata is a little quirky, but it is less invasive than passing the setting all the way through the call stack.

        Show
        Jeremy Bauer added a comment - The latest patch looks good. Thanks for submitting it. Adding a specialized field to classmetadata is a little quirky, but it is less invasive than passing the setting all the way through the call stack.
        Austin Dorenkamp made changes -
        Attachment OPENJPA-2366-3.patch [ 12597546 ]
        Attachment OPENJPA-2366doc-3.patch [ 12597547 ]
        Hide
        Austin Dorenkamp added a comment - - edited

        The updated patches have just been attached. In this comment I will briefly walk through the changes:
        ~TestUseSchemaElement (TUSE) is a new class that test my changes
        ~ReverseMappingTool (RMT) contains the majority of the changes - including initially setting the UseSchemaElement (USE) flag. From the RMT class, the USE flag is stored in the ClassMetaData object where it is eventually retrieved from Annotation/XMLPersistenceMetaDataSerializer (where it is stored) by the Annotation/XMLPersistenceMappingSerializer (where the information for the .java and .xml files are being created).
        ~FieldMetaData is a class that the comments in CMD said to update with any changes to CMD
        ~ReverseMappingToolTask was updated to mirror changes to RMT
        ~persistence.xml was updated to enable the use of a local derby database in TUSE

        Show
        Austin Dorenkamp added a comment - - edited The updated patches have just been attached. In this comment I will briefly walk through the changes: ~TestUseSchemaElement (TUSE) is a new class that test my changes ~ReverseMappingTool (RMT) contains the majority of the changes - including initially setting the UseSchemaElement (USE) flag. From the RMT class, the USE flag is stored in the ClassMetaData object where it is eventually retrieved from Annotation/XMLPersistenceMetaDataSerializer (where it is stored) by the Annotation/XMLPersistenceMappingSerializer (where the information for the .java and .xml files are being created). ~FieldMetaData is a class that the comments in CMD said to update with any changes to CMD ~ReverseMappingToolTask was updated to mirror changes to RMT ~persistence.xml was updated to enable the use of a local derby database in TUSE
        Austin Dorenkamp made changes -
        Attachment OPENJPA-2366doc.patch [ 12596420 ]
        Austin Dorenkamp made changes -
        Attachment OPENJPA-2366.patch [ 12596419 ]
        Hide
        Kevin Sutter added a comment -

        Austin, Thanks for the patch. It looks pretty good. I have a couple of comments that need addressing before we could commit it.

        o Doc changes. I would add the following (to tie the propery setting to the xml updates)...

        + If the useSchemaElement is set to false,
        + the schema name will also be removed from the corresponding XML mapping files

        o Same type of comment applies to the corresponding java code for the tool comments in ReverseMappingTool.java

        o It looks like you're modifying the generated @Table source line and removing any generated Schema element...

        + if(!_useSchemaElement && s.contains("@Table"))
        + {
        + int counter = -1; // Stores length of "schemaName" (minus quotes)
        + char current = 0;
        + while (current != '"')
        +

        { + current = s.charAt(counter+16); + counter++; + }

        + s = s.replaceAll(s.substring(7, 18+counter), "");
        + }

        Since this generated @Table code line may change over time, we should move this logic higher up in the call stack. That is, when the @Table source line is first being generated, determine that we don't want the Schema element and don't generate it in the first place.

        o Same type of comment for the XML generation in XMLPersistenceMappingSerializer. Why wouldn't we detect the condition in the serializeTable() method instead of resetting the names in the table identifier? Actually, this latter approach looks quite scary. It looks like we're modifying the actual class mapping data, which we probably don't want to do. Did the full JUnit bucket run successfully after these changes were applied?

        If we clean this up, then we can remove that ugly "publicResetNames" method...

        o Verify the indentation. In Eclipse, have you changed tabs to be spaces? There are a couple of areas like this that look to be formatting issues:

        + public boolean getUseSchemaElement()

        { + return _useSchemaElement; + }

        +
        + public void setUseSchemaElement(boolean useSchemaElement)

        { + this._useSchemaElement = useSchemaElement; + }

        +
        + /**

        • Internal parser interface.
          */

        o Remove extra comments attributing to you as an author...
        + <!-- Added the following code (ajdorenk) -->

        o In the ReverseMappingTool.java file, shouldn't you use the getter method instead of accessing the field directly?
        + gen.setUseSchemaElement(_useSchemaElement);

        Show
        Kevin Sutter added a comment - Austin, Thanks for the patch. It looks pretty good. I have a couple of comments that need addressing before we could commit it. o Doc changes. I would add the following (to tie the propery setting to the xml updates)... + If the useSchemaElement is set to false, + the schema name will also be removed from the corresponding XML mapping files o Same type of comment applies to the corresponding java code for the tool comments in ReverseMappingTool.java o It looks like you're modifying the generated @Table source line and removing any generated Schema element... + if(!_useSchemaElement && s.contains("@Table")) + { + int counter = -1; // Stores length of "schemaName" (minus quotes) + char current = 0; + while (current != '"') + { + current = s.charAt(counter+16); + counter++; + } + s = s.replaceAll(s.substring(7, 18+counter), ""); + } Since this generated @Table code line may change over time, we should move this logic higher up in the call stack. That is, when the @Table source line is first being generated, determine that we don't want the Schema element and don't generate it in the first place. o Same type of comment for the XML generation in XMLPersistenceMappingSerializer. Why wouldn't we detect the condition in the serializeTable() method instead of resetting the names in the table identifier? Actually, this latter approach looks quite scary. It looks like we're modifying the actual class mapping data, which we probably don't want to do. Did the full JUnit bucket run successfully after these changes were applied? If we clean this up, then we can remove that ugly "publicResetNames" method... o Verify the indentation. In Eclipse, have you changed tabs to be spaces? There are a couple of areas like this that look to be formatting issues: + public boolean getUseSchemaElement() { + return _useSchemaElement; + } + + public void setUseSchemaElement(boolean useSchemaElement) { + this._useSchemaElement = useSchemaElement; + } + + /** Internal parser interface. */ o Remove extra comments attributing to you as an author... + <!-- Added the following code (ajdorenk) --> o In the ReverseMappingTool.java file, shouldn't you use the getter method instead of accessing the field directly? + gen.setUseSchemaElement(_useSchemaElement);
        Austin Dorenkamp made changes -
        Status In Progress [ 3 ] Open [ 1 ]
        Austin Dorenkamp made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Austin Dorenkamp made changes -
        Attachment OPENJPA-2366.patch [ 12596419 ]
        Attachment OPENJPA-2366doc.patch [ 12596420 ]
        Hide
        Austin Dorenkamp added a comment -

        This patch deals with the ReverseMappingTool and associated classes. The added code creates the option for users to exclude the schema name (via a command line flag: "-useSchemaElement") from the @Table annotation in the .java files generated for each table that is being mapped. The schema name will also be removed from the corresponding XML mapping file(s) (i.e., orm.xml) that are generated by the tool. The initialized value for "useSchemaElement" is true in order to preserve backwards compatibility.

        I am requesting a review so that these changes can be incorporated into future Apache OpenJPA releases.

        Show
        Austin Dorenkamp added a comment - This patch deals with the ReverseMappingTool and associated classes. The added code creates the option for users to exclude the schema name (via a command line flag: "-useSchemaElement") from the @Table annotation in the .java files generated for each table that is being mapped. The schema name will also be removed from the corresponding XML mapping file(s) (i.e., orm.xml) that are generated by the tool. The initialized value for "useSchemaElement" is true in order to preserve backwards compatibility. I am requesting a review so that these changes can be incorporated into future Apache OpenJPA releases.
        Hide
        Jeremy Bauer added a comment -

        I like this approach. The options "includeSchema" and "includeSchemaName" and the inverses "excludeSchema" or "excludeSchemaName" also came to mind as possibilities. The use of one of those may help further differentiate them from "useSchemaName". I'm OK with "useSchema" as well though, good documentation will be the key.

        Show
        Jeremy Bauer added a comment - I like this approach. The options "includeSchema" and "includeSchemaName" and the inverses "excludeSchema" or "excludeSchemaName" also came to mind as possibilities. The use of one of those may help further differentiate them from "useSchemaName". I'm OK with "useSchema" as well though, good documentation will be the key.
        Hide
        Austin Dorenkamp added a comment -

        My current thought for implementing this feature is to add a "useSchema" flag that will be set via command line {-useSchema = <true/false>}. With useSchema=false, the schema name for the @Table annotation will be excluded. However, useSchema will default to true if not specified so that it won't disrupt current OpenJPA users. UseSchema is different from useSchemaName as the latter only affects whether or not the schema name is included in the class name. I welcome any other thoughts and ideas regarding the implementation of this feature.

        Show
        Austin Dorenkamp added a comment - My current thought for implementing this feature is to add a "useSchema" flag that will be set via command line {-useSchema = <true/false>}. With useSchema=false, the schema name for the @Table annotation will be excluded. However, useSchema will default to true if not specified so that it won't disrupt current OpenJPA users. UseSchema is different from useSchemaName as the latter only affects whether or not the schema name is included in the class name. I welcome any other thoughts and ideas regarding the implementation of this feature.
        Kevin Sutter made changes -
        Field Original Value New Value
        Assignee Austin Dorenkamp [ ajdorenk ]
        John Mysak created issue -

          People

          • Assignee:
            Austin Dorenkamp
            Reporter:
            John Mysak
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 2h
              2h
              Remaining:
              Remaining Estimate - 2h
              2h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development