openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Abe White <awh...@bea.com>
Subject Re: svn commit: r525006 - in /incubator/openjpa/trunk: openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/ openjpa-persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/
Date Tue, 03 Apr 2007 17:17:19 GMT
I see several possible problems with this commit.  Are there tests  
for this functionality checked in?

1. The property and logic for using the DefaultSchemaName are defined  
in MappingInfo, but the default schema name is only ever set into  
ClassMappingInfos.  Not FieldMappingInfos, DiscriminatorMappingInfos,  
etc.
2.  MappingInfo uses the default schema name sometimes (line 445) but  
ignores it other times (line 469).
3. It should not be necessary to prepend the schema name to the table  
name in PersistenceMappingDefaults.  The schema is already known.   
Was that code added based on any testing?  If so, perhaps it is  
because of problem #2 above.
4. The XMLPersistenceMappingParser should not override the endClass 
(String) method.  It already overrides the endClassMapping 
(ClassMetaData) method, which is a much more efficient place to set  
the needed info.  You could also do it in the startClassMapping method.

I'm happy to fix things myself, but I don't see any tests in the  
commit to verify my fixes.

On Apr 2, 2007, at 9:48 PM, mikedd@apache.org wrote:

> Author: mikedd
> Date: Mon Apr  2 19:48:10 2007
> New Revision: 525006
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=525006
> Log:
> OpenJPA-179 store defaultSchemaName in ClassMapping
>
> Modified:
>     incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/ 
> openjpa/jdbc/meta/MappingInfo.java
>     incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/ 
> org/apache/openjpa/persistence/jdbc/PersistenceMappingDefaults.java
>     incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/ 
> org/apache/openjpa/persistence/jdbc/XMLPersistenceMappingParser.java
>
> Modified: incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/ 
> apache/openjpa/jdbc/meta/MappingInfo.java
> URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa- 
> jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingInfo.java? 
> view=diff&rev=525006&r1=525005&r2=525006
> ====================================================================== 
> ========
> --- incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/ 
> openjpa/jdbc/meta/MappingInfo.java (original)
> +++ incubator/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/ 
> openjpa/jdbc/meta/MappingInfo.java Mon Apr  2 19:48:10 2007
> @@ -21,6 +21,7 @@
>  import java.util.Collections;
>  import java.util.List;
>
> +import org.apache.commons.lang.StringUtils;
>  import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
>  import org.apache.openjpa.jdbc.schema.Column;
>  import org.apache.openjpa.jdbc.schema.ColumnIO;
> @@ -69,6 +70,7 @@
>      private boolean _canFK = true;
>      private int _join = JOIN_NONE;
>      private ColumnIO _io = null;
> +    private String _defaultSchemaName = null;
>
>      /**
>       * Mapping strategy name.
> @@ -439,6 +441,9 @@
>              if (schema == null) {
>                  schemaName = Schemas.getNewTableSchema 
> ((JDBCConfiguration)
>                      repos.getConfiguration());
> +                if(StringUtils.isEmpty(schemaName)) {
> +                   schemaName = _defaultSchemaName;
> +                }
>                  schema = group.getSchema(schemaName);
>                  if (schema == null)
>                      schema = group.addSchema(schemaName);
> @@ -1764,4 +1769,12 @@
>          public void populate(Table local, Table foreign, Column col,
>              Object target, boolean inverse, int pos, int cols);
>  	}
> +
> +    public String getDefaultSchemaName() {
> +        return _defaultSchemaName;
> +    }
> +
> +    public void setDefaultSchemaName(String schemaName) {
> +        _defaultSchemaName = schemaName;
> +    }
>  }
>
> Modified: incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/ 
> java/org/apache/openjpa/persistence/jdbc/ 
> PersistenceMappingDefaults.java
> URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa- 
> persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/ 
> PersistenceMappingDefaults.java? 
> view=diff&rev=525006&r1=525005&r2=525006
> ====================================================================== 
> ========
> --- incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/ 
> org/apache/openjpa/persistence/jdbc/PersistenceMappingDefaults.java  
> (original)
> +++ incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/ 
> org/apache/openjpa/persistence/jdbc/PersistenceMappingDefaults.java  
> Mon Apr  2 19:48:10 2007
> @@ -15,6 +15,7 @@
>   */
>  package org.apache.openjpa.persistence.jdbc;
>
> +import org.apache.commons.lang.StringUtils;
>  import org.apache.openjpa.jdbc.meta.ClassMapping;
>  import org.apache.openjpa.jdbc.meta.Discriminator;
>  import org.apache.openjpa.jdbc.meta.FieldMapping;
> @@ -114,17 +115,31 @@
>
>      @Override
>      public String getTableName(ClassMapping cls, Schema schema) {
> +        String name = "";
> +
> +        if(StringUtils.isNotEmpty(schema.getName())) {
> +            name +=schema.getName() + '.';
> +        }
> +
>          if (cls.getTypeAlias() != null)
> -            return cls.getTypeAlias();
> +            name += cls.getTypeAlias();
> +
>          else
> -            return Strings.getClassName(
> -                cls.getDescribedType()).replace('$', '_');
> +            name += Strings.getClassName(cls.getDescribedType 
> ()).replace('$',
> +                    '_');
> +
> +        return name;
>      }
>
>      @Override
>      public String getTableName(FieldMapping fm, Schema schema) {
> +        String name = "";
> +        if(StringUtils.isNotEmpty(schema.getName())) {
> +            name +=schema.getName() + '.';
> +        }
> +
>          // base name is table of defining type + '_'
> -        String name = fm.getDefiningMapping().getTable().getName()  
> + "_";
> +        name += fm.getDefiningMapping().getTable().getName() + "_";
>
>          // if this is an assocation table, spec says to suffix  
> with table of
>          // the related type. spec doesn't cover other cases; we're  
> going to
>
> Modified: incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/ 
> java/org/apache/openjpa/persistence/jdbc/ 
> XMLPersistenceMappingParser.java
> URL: http://svn.apache.org/viewvc/incubator/openjpa/trunk/openjpa- 
> persistence-jdbc/src/main/java/org/apache/openjpa/persistence/jdbc/ 
> XMLPersistenceMappingParser.java? 
> view=diff&rev=525006&r1=525005&r2=525006
> ====================================================================== 
> ========
> --- incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/ 
> org/apache/openjpa/persistence/jdbc/ 
> XMLPersistenceMappingParser.java (original)
> +++ incubator/openjpa/trunk/openjpa-persistence-jdbc/src/main/java/ 
> org/apache/openjpa/persistence/jdbc/ 
> XMLPersistenceMappingParser.java Mon Apr  2 19:48:10 2007
> @@ -50,6 +50,7 @@
>  import org.apache.openjpa.meta.ClassMetaData;
>  import org.apache.openjpa.meta.FieldMetaData;
>  import org.apache.openjpa.meta.JavaTypes;
> +import org.apache.openjpa.meta.MetaDataRepository;
>  import org.apache.openjpa.persistence.XMLPersistenceMetaDataParser;
>  import static org.apache.openjpa.persistence.jdbc.MappingTag.*;
>
> @@ -910,4 +911,18 @@
>  		TRUE,
>  		FALSE
>  	}
> +
> +    @Override
> +    protected void endClass(String elem)
> +        throws SAXException {
> +        if (StringUtils.isNotEmpty(_schema)) {
> +            Class cls = classForName(currentClassName());
> +
> +            MetaDataRepository repos = getRepository();
> +            ClassMapping meta = (ClassMapping)  
> repos.getCachedMetaData(cls);
> +
> +            meta.getMappingInfo().setDefaultSchemaName(_schema);
> +        }
> +        super.endClass(elem);
> +    }
>  }
>
>


Notice:  This email message, together with any attachments, may contain information  of  BEA
Systems,  Inc.,  its subsidiaries  and  affiliated entities,  that may be confidential,  proprietary,
 copyrighted  and/or legally privileged, and is intended solely for the use of the individual
or entity named in this message. If you are not the intended recipient, and have received
this message in error, please immediately return this by email and then delete it.

Mime
View raw message