persistence@glassfish.java.net

Code review for issue 2262 - Primary keys in JoinTable by Java2DB do not have specified length

From: Wonseok Kim <guruwons_at_gmail.com>
Date: Tue, 6 Mar 2007 22:27:15 +0900

Hi Tom,
Please review this attached fix.
https://glassfish.dev.java.net/issues/show_bug.cgi?id=2262

Cause: when DefaultTableGenerator handles a relation table, the generated
field doesn't follow the type definitions(defined in DatabaseField) of the
target table.

Fix summary:
See buildRelationTableFields() and resolveDatabaseField() in
DefaultTableGenerator
* Now the DatabaseField of relation table is constructed fully by copying
type definitions of the target DatabaseField.
* This also fixes the problem that the columnDefinition of @JoinColumn
(inside of @JoinTable) does not apply.

Tested locally with the attached testcase and entity-persistence-tests.

Note also: I added JDK6 methods to DataSource impl inside
ValidationTestSuite of entity-persistence-tests to be compiled in JDK6.

[DIFF]

Index:
entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java
===================================================================
RCS file:
/cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java,v
retrieving revision 1.13
diff -c -w -r1.13 DefaultTableGenerator.java
***
entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java
 15 Dec 2006 06:52:19 -0000 1.13
---
entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java
 6 Mar 2007 13:04:08 -0000
***************
*** 104,109 ****
--- 104,111 ----
      //used to track th field definition: keyed by the database field
object, and
      //valued by the field definition.
      private Map<DatabaseField, FieldDefinition> fieldMap = null;
+     //DatabaseField pool (synchronized with above 'fieldMap')
+     private Map<DatabaseField, DatabaseField> databaseFields;
      /**
       * Default construcotr
***************
*** 113,118 ****
--- 115,121 ----
          databasePlatform = project.getLogin().getPlatform();
          tableMap = new HashMap();
          fieldMap = new HashMap();
+         databaseFields = new HashMap();
      }
      /**
***************
*** 336,342 ****
              setFieldToRelationTable(fkField, tblDef);
          }
!         // add a foreign key constraint
          DatabaseTable targetTable = targetField.getTable();
          TableDefinition targetTblDef =
getTableDefFromDBTable(targetTable);
--- 339,345 ----
              setFieldToRelationTable(fkField, tblDef);
          }
!         // add a foreign key constraint from fk field to target field
          DatabaseTable targetTable = targetField.getTable();
          TableDefinition targetTblDef =
getTableDefFromDBTable(targetTable);
***************
*** 442,455 ****
       */
      private DatabaseField resolveDatabaseField(DatabaseField childField,
DatabaseField parentField) {
          //set through the type from the source table key field to the
relation or direct collection table key field.
!         DatabaseField reslovedDatabaseField = new DatabaseField();
!         reslovedDatabaseField.setName(childField.getName());
!         reslovedDatabaseField.setType(getFieldDefFromDBField(parentField,
true).getType());
          //Table should be set, otherwise other same name field will be
used wrongly because equals() is true.
          //Fix for GF#1392 the same name column for the entity and
many-to-many table cause wrong pk constraint.
!         reslovedDatabaseField.setTable(childField.getTable());
!         return reslovedDatabaseField;
      }
      /**
--- 445,485 ----
       */
      private DatabaseField resolveDatabaseField(DatabaseField childField,
DatabaseField parentField) {
          //set through the type from the source table key field to the
relation or direct collection table key field.
!         DatabaseField resolvedDatabaseField = new DatabaseField();
!         // find original field in the parent table, which contains actual
type definitions
!         // if 'resolvedParentField' is null, there is no corresponding
field definition (typo?)
!         DatabaseField resolvedParentField = databaseFields.get
(parentField);
!
!         resolvedDatabaseField.setName(childField.getName());
          //Table should be set, otherwise other same name field will be
used wrongly because equals() is true.
          //Fix for GF#1392 the same name column for the entity and
many-to-many table cause wrong pk constraint.
!         resolvedDatabaseField.setTable(childField.getTable());
!
!         // type definitions from parent field definition
!         if(resolvedParentField != null) {
!             resolvedDatabaseField.setType(resolvedParentField.getType());
!             resolvedDatabaseField.setScale(resolvedParentField.getScale
());
!             resolvedDatabaseField.setLength(resolvedParentField.getLength
());
!             resolvedDatabaseField.setPrecision(
resolvedParentField.getPrecision());
!         }
!
!         // these are defined in childField definition(see @JoinColumn)
!         resolvedDatabaseField.setUnique(childField.isUnique());
!         resolvedDatabaseField.setNullable(childField.isNullable());
!         resolvedDatabaseField.setUpdatable(childField.isUpdatable());
!         resolvedDatabaseField.setInsertable(childField.isInsertable());
!
!         String columnDef = childField.getColumnDefinition();
!         if(columnDef == null || columnDef.trim().equals("")) {
!             // if childField has no column definition, follow the
definition of the parent field
!             if(resolvedParentField != null) {
!                 resolvedDatabaseField.setColumnDefinition(
resolvedParentField.getColumnDefinition());
!             }
!         } else {
!             resolvedDatabaseField.setColumnDefinition(columnDef);
!         }
!         return resolvedDatabaseField;
      }
      /**
***************
*** 505,510 ****
--- 535,541 ----
              fieldDef.setIsPrimaryKey(isPrimaryKey);
              fieldMap.put(dbField, fieldDef);
+             databaseFields.put(dbField, dbField);
          }
          return fieldDef;
Index:
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/cmp3/validation/ValidationTestSuite.java
===================================================================
RCS file:
/cvs/glassfish/entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/cmp3/validation/ValidationTestSuite.java,v
retrieving revision 1.7
diff -c -w -r1.7 ValidationTestSuite.java
***
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/cmp3/validation/ValidationTestSuite.java
 1 Mar 2007 22:17:13 -0000    1.7
---
entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/cmp3/validation/ValidationTestSuite.java
 6 Mar 2007 13:04:08 -0000
***************
*** 172,176 ****
--- 172,178 ----
          public void setLogWriter(java.io.PrintWriter out) throws
SQLException{}
          public void setLoginTimeout(int seconds) throws SQLException{}
          public int getLoginTimeout() throws SQLException{return 1;}
+         public <T> T unwrap(Class<T> iface) throws SQLException { return
null; }
+         public boolean isWrapperFor(Class<?> iface) throws SQLException {
return false; }
      }
  }