persistence@glassfish.java.net

Code reviews for Issues 139 and 335

From: Pramod Gopinath <Pramod.Gopinath_at_Sun.COM>
Date: Thu, 09 Mar 2006 17:57:16 -0800

Hi Gordon / Peter

Issues being addressed :

139 - Java2DB does not drop sequences during undeploy/redeploy
335 - Generator Table of TableGenerator getting dirty + no PK on name
column with derby

Peter had suggested in issue 139 to provide some code snippets that
would help to drop the sequence table. But doing this would cause
problems, specifically in cases where there are multiple applications
sharing the same sequence generator table. So instead of dropping the
entire table I have made changes to the code to ensure that we just
issue a delete statement to just drop the row that is associated for
this particular applicaton. In short for entities in this application
that are using sequence generator table would have the row dropped.

In case of issue 335, ensured that we would create the sequence table
with the sequence name as a primary key for that sequence. This would
ensure that we do not insert multiple rows of the same sequence name
into the generator table.

As part of these code changes I had refactored code and ensured that I
define new methods and retain the behavior of the old methods as is (for
backward compatibility.

I have attached a jar containing the sources as well as the diff files.
Thanks
Pramod



Index: src/java/oracle/toplink/essentials/tools/schemaframework/SchemaManager.java
===================================================================
RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/SchemaManager.java,v
retrieving revision 1.7
diff -r1.7 SchemaManager.java
196d195
< Iterator descriptors = getSession().getDescriptors().values().iterator();
198,210c197,206
< while (descriptors.hasNext()) {
< ClassDescriptor descriptor = (ClassDescriptor)descriptors.next();
<
< if (descriptor.usesSequenceNumbers()) {
< String seqName = descriptor.getSequenceNumberName();
<
< if (seqName == null) {
< seqName = getSession().getDatasourcePlatform().getDefaultSequence().getName();
< }
<
< if (processedSequenceNames.contains(seqName)) {
< continue;
< }
---
>         buildTableAndSequenceDefinitions(sequenceDefinitions, tableDefinitions, processedSequenceNames);
>         processTableDefinitions(tableDefinitions, create);
>         processSequenceDefinitions(sequenceDefinitions, create);
>     }
>     
>     /**
>      * Common implementor for createSequence and replaceSequence
>      */
>     protected void createOrReplaceSequences(boolean create, boolean drop) throws TopLinkException {
>         Sequencing sequencing = getSession().getSequencing();
212c208,211
<                 processedSequenceNames.add(seqName);
---
>         if ((sequencing == null) || (sequencing.whenShouldAcquireValueForAll() == Sequencing.AFTER_INSERT)) {
>             // Not required on Sybase native etc.
>             return;
>         }
214c213,215
<                 Sequence sequence = getSession().getDatasourcePlatform().getSequence(seqName);
---
>         // Prepare table and sequence definitions
>         // table name mapped to TableDefinition
>         HashMap tableDefinitions = new HashMap();
216,218c217,218
<                 if (sequence.shouldAcquireValueAfterInsert()) {
<                     continue;
<                 }
---
>         // sequence name to SequenceDefinition
>         HashSet sequenceDefinitions = new HashSet();
220c220,221
<                 SequenceDefinition sequenceDefinition = buildSequenceDefinition(sequence);
---
>         // remember the processed - to handle each sequence just once.
>         HashSet processedSequenceNames = new HashSet();
222,224c223,226
<                 if (sequenceDefinition == null) {
<                     continue;
<                 }
---
>         buildTableAndSequenceDefinitions(sequenceDefinitions, tableDefinitions, processedSequenceNames);
>         processTableDefinitions(tableDefinitions, create);
>         processSequenceDefinitions(sequenceDefinitions, drop);
>     }    
226c228
<                 sequenceDefinitions.add(sequenceDefinition);
---
>     private void processSequenceDefinitions(final HashSet sequenceDefinitions, final boolean drop) throws TopLinkException {
228c230,231
<                 TableDefinition tableDefinition = sequenceDefinition.buildTableDefinition();
---
>         // create sequence objects
>         Iterator itSequenceDefinitions = sequenceDefinitions.iterator();
230,232c233,234
<                 if (tableDefinition != null) {
<                     String tableName = tableDefinition.getName();
<                     TableDefinition otherTableDefinition = (TableDefinition)tableDefinitions.get(tableName);
---
>         while (itSequenceDefinitions.hasNext()) {
>             SequenceDefinition sequenceDefinition = (SequenceDefinition)itSequenceDefinitions.next();
234,238c236,240
<                     if (otherTableDefinition != null) {
<                         // check for a conflict; if there is one - throw a ValidationException
<                     } else {
<                         tableDefinitions.put(tableName, tableDefinition);
<                     }
---
>             if (drop) {
>                 try {
>                     dropObject(sequenceDefinition);
>                 } catch (DatabaseException exception) {
>                     // Ignore sequence not found for first creation
240a243,244
> 
>             createObject(sequenceDefinition);
241a246,248
>     }
> 
>     private void processTableDefinitions(final HashMap tableDefinitions, final boolean create) throws TopLinkException {
256,266c263
<             if (create) {
<                 try {
<                     createObject(tableDefinition);
<                 } catch (DatabaseException exception) {
<                     // Ignore already created
<                 } finally {
<                     if (shouldLogExceptionStackTrace) {
<                         session.getSessionLog().setShouldLogExceptionStackTrace(true);
<                     }
<                 }
<             } else {
---
>             if (!create) {
276c273,275
< 
---
>             }
>             
>             try {
277a277,282
>             } catch (DatabaseException exception) {
>                 // Ignore already created
>             } finally {
>                 if (shouldLogExceptionStackTrace) {
>                     session.getSessionLog().setShouldLogExceptionStackTrace(true);
>                 }
278a284
> 
279a286
>     }
281,282c288,292
<         // create sequence objects
<         Iterator itSequenceDefinitions = sequenceDefinitions.iterator();
---
>     private void buildTableAndSequenceDefinitions(final HashSet sequenceDefinitions, 
>             final HashMap tableDefinitions, final HashSet processedSequenceNames) {
>         Iterator descriptors = getSession().getDescriptors().values().iterator();
>         while (descriptors.hasNext()) {
>             ClassDescriptor descriptor = (ClassDescriptor)descriptors.next();
284,285c294,295
<         while (itSequenceDefinitions.hasNext()) {
<             SequenceDefinition sequenceDefinition = (SequenceDefinition)itSequenceDefinitions.next();
---
>             if (descriptor.usesSequenceNumbers()) {
>                 String seqName = descriptor.getSequenceNumberName();
287,291c297,298
<             if (!create) {
<                 try {
<                     dropObject(sequenceDefinition);
<                 } catch (DatabaseException exception) {
<                     // Ignore sequence not found for first creation
---
>                 if (seqName == null) {
>                     seqName = getSession().getDatasourcePlatform().getDefaultSequence().getName();
293d299
<             }
295c301,333
<             createObject(sequenceDefinition);
---
>                 if (processedSequenceNames.contains(seqName)) {
>                     continue;
>                 }
> 
>                 processedSequenceNames.add(seqName);
> 
>                 Sequence sequence = getSession().getDatasourcePlatform().getSequence(seqName);
> 
>                 if (sequence.shouldAcquireValueAfterInsert()) {
>                     continue;
>                 }
> 
>                 SequenceDefinition sequenceDefinition = buildSequenceDefinition(sequence);
> 
>                 if (sequenceDefinition == null) {
>                     continue;
>                 }
> 
>                 sequenceDefinitions.add(sequenceDefinition);
> 
>                 TableDefinition tableDefinition = sequenceDefinition.buildTableDefinition();
> 
>                 if (tableDefinition != null) {
>                     String tableName = tableDefinition.getName();
>                     TableDefinition otherTableDefinition = (TableDefinition)tableDefinitions.get(tableName);
> 
>                     if (otherTableDefinition != null) {
>                         // check for a conflict; if there is one - throw a ValidationException
>                     } else {
>                         tableDefinitions.put(tableName, tableDefinition);
>                     }
>                 }
>             }
763a802,818
>     
>     /**
>      * Drop and recreate the default table schema for the TopLink project this session associated with.
>      */
>     public void replaceDefaultTables(boolean keepSequenceTables) throws TopLinkException {
>         boolean shouldLogExceptionStackTrace = getSession().getSessionLog().shouldLogExceptionStackTrace();
>         getSession().getSessionLog().setShouldLogExceptionStackTrace(false);
> 
>         try {
>             DefaultTableGenerator gen = new DefaultTableGenerator(session.getProject());
>             gen.generateDefaultTableCreator().replaceTables(session, this, keepSequenceTables);
>         } catch (DatabaseException exception) {
>             // Ignore error
>         } finally {
>             getSession().getSessionLog().setShouldLogExceptionStackTrace(shouldLogExceptionStackTrace);
>         }
>     }    
Index: src/java/oracle/toplink/essentials/tools/schemaframework/TableCreator.java
===================================================================
RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/TableCreator.java,v
retrieving revision 1.1.1.1
diff -r1.1.1.1 TableCreator.java
202a203,222
>         replaceTablesAndConstraints(schemaManager, session);
> 
>         schemaManager.createSequences();
> 
>     }
>     
>     /**
>      * Recreate the tables on the database.
>      * This will drop the tables if they exist and recreate them.
>      */
>     public void replaceTables(oracle.toplink.essentials.sessions.DatabaseSession session, 
>             SchemaManager schemaManager, boolean keepSequenceTable) {
>         replaceTablesAndConstraints(schemaManager, session);
> 
>         schemaManager.createOrReplaceSequences(keepSequenceTable, true);
>     }    
>     
> 
>     private void replaceTablesAndConstraints(final SchemaManager schemaManager, 
>             final oracle.toplink.essentials.sessions.DatabaseSession session) {
238,239d257
< 
<         schemaManager.createSequences();
Index: src/java/oracle/toplink/essentials/tools/schemaframework/TableSequenceDefinition.java
===================================================================
RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/TableSequenceDefinition.java,v
retrieving revision 1.2
diff -r1.2 TableSequenceDefinition.java
157c157
<         definition.addField(getSequenceNameFieldName(), String.class, 50);
---
>         definition.addPrimaryKeyField(getSequenceNameFieldName(), String.class, 50);