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/

[David Ezzio]

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, <dez...@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