[ 
https://issues.apache.org/jira/browse/OPENJPA-2366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13731375#comment-13731375
 ] 

Kevin Sutter commented on OPENJPA-2366:
---------------------------------------

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);


                
> provide option to exclude schema name from generated @Table annotation for 
> generated entities
> ---------------------------------------------------------------------------------------------
>
>                 Key: OPENJPA-2366
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-2366
>             Project: OpenJPA
>          Issue Type: New Feature
>          Components: jpa
>    Affects Versions: 2.2.0, 2.2.1
>            Reporter: John Mysak
>            Assignee: Austin Dorenkamp
>            Priority: Minor
>         Attachments: OPENJPA-2366doc.patch, OPENJPA-2366.patch
>
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> 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.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to