Hi David and Ravi The patch was contributed by Ravi, but the @author tag lists Hiroki Tateno.
I'm not an expert on proper attribution, Craig can correct me where I go wrong :-). Here's my understanding. If Hiroki wrote the code we'll have to add his name to the commit message, if not we'll remote the @author tag. We may want to find out whether Hiroki has a CLA on file with Apache for future patches (same would apply for anyone in an @author tag). It isn't a requirement (AFAIK) but it's always nice to know. -mike On Wed, May 13, 2009 at 5:54 PM, <[email protected]> wrote: > Author: dezzio > Date: Wed May 13 22:54:32 2009 > New Revision: 774580 > > URL: http://svn.apache.org/viewvc?rev=774580&view=rev > Log: > OpenJPA-1051: Fixed MappingDefaultsImpl to avoid column name duplications > when long column names are supplied for a database that accepts only shorter > names. Changes submitted for Ravi Palacherla. > > Added: > openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/ > > > openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java > (with props) > Modified: > > > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java > > > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java > > > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java > > Modified: > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java > URL: > http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java?rev=774580&r1=774579&r2=774580&view=diff > > ============================================================================== > --- > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java > (original) > +++ > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java > Wed May 13 22:54:32 2009 > @@ -539,7 +539,9 @@ > else if (_dsIdName != null) > cols[i].setName(_dsIdName + i); > correctName(table, cols[i]); > + table.addSubColumn(cols[i].getName()); > } > + table.resetSubColumns(); > } > > /** > @@ -582,7 +584,9 @@ > } else if (_versName != null) > cols[i].setName(_versName + i); > correctName(table, cols[i]); > + table.addSubColumn(cols[i].getName()); > } > + table.resetSubColumns(); > } > > public void populateColumns(Discriminator disc, Table table, > @@ -593,7 +597,9 @@ > else if (_discName != null) > cols[i].setName(_discName + i); > correctName(table, cols[i]); > + table.addSubColumn(cols[i].getName()); > } > + table.resetSubColumns(); > } > > public void populateJoinColumn(ClassMapping cm, Table local, Table > foreign, > @@ -618,8 +624,11 @@ > > public void populateColumns(ValueMapping vm, String name, Table table, > Column[] cols) { > - for (int i = 0; i < cols.length; i++) > + for (int i = 0; i < cols.length; i++) { > correctName(table, cols[i]); > + table.addSubColumn(cols[i].getName()); > + } > + table.resetSubColumns(); > } > > public boolean populateOrderColumns(FieldMapping fm, Table table, > @@ -630,7 +639,9 @@ > else if (_orderName != null) > cols[i].setName(_orderName + i); > correctName(table, cols[i]); > + table.addSubColumn(cols[i].getName()); > } > + table.resetSubColumns(); > return _orderLists && (JavaTypes.ARRAY == fm.getTypeCode() > || List.class.isAssignableFrom(fm.getType())); > } > @@ -643,7 +654,9 @@ > else if (_nullIndName != null) > cols[i].setName(_nullIndName + i); > correctName(table, cols[i]); > + table.addSubColumn(cols[i].getName()); > } > + table.resetSubColumns(); > return _addNullInd; > } > > > Modified: > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java > URL: > http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java?rev=774580&r1=774579&r2=774580&view=diff > > ============================================================================== > --- > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java > (original) > +++ > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java > Wed May 13 22:54:32 2009 > @@ -39,13 +39,17 @@ > > private Set _names = null; > > + // an additional names Set for checking name duplication > + private Set _subNames = null; > + > /** > * Return true if the given name is in use already. > */ > public boolean isNameTaken(String name) { > if (name == null) > return true; > - return _names != null && _names.contains(name.toUpperCase()); > + return (_names != null && _names.contains(name.toUpperCase())) || > + (_subNames != null && _subNames.contains(name.toUpperCase())); > } > > /** > @@ -77,4 +81,20 @@ > if (name != null && _names != null) > _names.remove(name.toUpperCase()); > } > + > + /** > + * Attempt to add the given name to the set. > + * > + * @param name the name to add > + */ > + protected void addSubName(String name) { > + if (_subNames == null) { > + _subNames = new HashSet(); > + } > + _subNames.add(name.toUpperCase()); > + } > + > + protected void resetSubNames() { > + _subNames = null; > + } > } > > Modified: > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java > URL: > http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java?rev=774580&r1=774579&r2=774580&view=diff > > ============================================================================== > --- > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java > (original) > +++ > openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java > Wed May 13 22:54:32 2009 > @@ -255,8 +255,8 @@ > } > > public String[] getColumnNames() { > - return _colMap == null ? new String[0] : > - (String[])_colMap.keySet().toArray(new > String[_colMap.size()]); > + return _colMap == null ? new String[0] : > + (String[])_colMap.keySet().toArray(new > String[_colMap.size()]); > } > > /** > @@ -275,8 +275,8 @@ > * for dynamic table implementation. > */ > public boolean containsColumn(String name) { > - return name != null && _colMap != null > - && _colMap.containsKey(name.toUpperCase()); > + return name != null && _colMap != null > + && _colMap.containsKey(name.toUpperCase()); > } > > /** > @@ -756,4 +756,15 @@ > public void setColNumber(int colNum) { > _colNum = colNum; > } > + > + /** > + * Add a column to the subNames set to avoid naming conflict. > + */ > + public void addSubColumn(String name) { > + addSubName(name); > + } > + > + public void resetSubColumns() { > + resetSubNames(); > + } > } > > Added: > openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java > URL: > http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java?rev=774580&view=auto > > ============================================================================== > --- > openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java > (added) > +++ > openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java > Wed May 13 22:54:32 2009 > @@ -0,0 +1,63 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one > + * or more contributor license agreements. See the NOTICE file > + * distributed with this work for additional information > + * regarding copyright ownership. The ASF licenses this file > + * to you under the Apache License, Version 2.0 (the > + * "License"); you may not use this file except in compliance > + * with the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, > + * software distributed under the License is distributed on an > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > + * KIND, either express or implied. See the License for the > + * specific language governing permissions and limitations > + * under the License. > + */ > +package org.apache.openjpa.jdbc.meta; > + > +import org.apache.openjpa.jdbc.schema.Table; > +import org.apache.openjpa.jdbc.schema.Column; > +import org.apache.openjpa.jdbc.conf.JDBCConfiguration; > +import org.apache.openjpa.jdbc.conf.JDBCConfigurationImpl; > + > +import junit.framework.TestCase; > + > +public class TestMappingDefaultsImpl extends TestCase { > + > + public void setUp() { > + } > + > + /** > + * For databases that accept only short column names, test avoidance > of > + * duplicate column names when populating the table with long column > names. > + * > + * @author Hiroki Tateno > + */ > + public void testPopulateWithLongColumnNames() { > + MappingDefaultsImpl mapping = new MappingDefaultsImpl(); > + JDBCConfiguration conf = new JDBCConfigurationImpl(false, false); > + conf.setDBDictionary("oracle"); > + mapping.setConfiguration(conf); > + Table table = new Table("testtable", null); > + Column[] cols = new Column[3]; > + cols[0] = new > + Column("longnamelongnamelongnamelongnamelongnamelongname1", > null); > + cols[1] = new > + Column("longnamelongnamelongnamelongnamelongnamelongname2", > null); > + cols[2] = new > + Column("longnamelongnamelongnamelongnamelongnamelongname3", > null); > + MappingRepository mr = new MappingRepository(); > + mr.setConfiguration(conf); > + Version version = new Version(new ClassMapping(String.class,mr)); > + mapping.populateColumns(version, table, cols); > + assertFalse("column names are conflicted : " + cols[0].getName(), > + cols[0].getName().equals(cols[1].getName())); > + assertFalse("column names are conflicted : " + cols[0].getName(), > + cols[0].getName().equals(cols[2].getName())); > + assertFalse("column names are conflicted : " + cols[1].getName(), > + cols[1].getName().equals(cols[2].getName())); > + } > +} > > Propchange: > openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java > > ------------------------------------------------------------------------------ > svn:eol-style = native > > >
