openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Ezzio <dez...@apache.org>
Subject Re: svn commit: r774580 - in /openjpa/trunk/openjpa-jdbc/src: main/java/org/apache/openjpa/jdbc/meta/ main/java/org/apache/openjpa/jdbc/schema/ test/java/org/apache/openjpa/jdbc/meta/
Date Thu, 14 May 2009 15:12:14 GMT
Ravi,

Could you clarify whether Hiroki should remain as the author of the test 
class?  I checked whether he was an Oracle employee, but I didn't go any 
further.

Thanks,

David

Michael Dick wrote:
> 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, <dezzio@apache.org> 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
>>
>>
>>
> 

Mime
View raw message