Hi Craig,

No questions. We'll be in compliance for this contribution. Give us a day or two.

Thanks,

David

Donald Woods wrote:
For #1 - Patch contributions via JIRA do not require having an ICLA on file, as long as the author created and submitted the patch with the "Grant license to ASF for inclusion in ASF works" selected when attaching the patch.



-Donald


Craig L Russell wrote:
We have to be very careful about this. Apache needs to track the provenance of every contribution. Patches should only be uploaded to JIRA issues by the author.

1. It is against the rules to commit contributions without the author of the contribution signing an ICLA.

2. It is against the rules to commit contributions without acknowledging the author in the commit message (if the committer is not the author).

3. We don't want @author tags. These tags don't foster community. If tags exist in contributions, they should not be committed until the tags are removed.

If there are any questions about the above, please raise them now.

Craig

On May 14, 2009, at 8:34 AM, Ravi Palacherla wrote:

Hi Mike,

Hiroki Tateno is the author of this test class.

Regarding CLA on file with Apache, I am not sure about it and will check with him and update accordingly.

Regards,
Ravi.

-----Original Message-----
From: Michael Dick [mailto:[email protected]]
Sent: Thursday, May 14, 2009 8:54 AM
To: [email protected]; Ravi Palacherla
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/

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




Craig L Russell
Architect, Sun Java Enterprise System http://db.apache.org/jdo
408 276-5638 mailto:[email protected]
P.S. A good JDO? O, Gasp!


Reply via email to