You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by Michael Dick <mi...@gmail.com> on 2009/05/14 16:53:56 UTC

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

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/

Posted by Ravi Palacherla <ra...@oracle.com>.
Hi Mike,

How to add Hiroki's name in commit message ?

Regards,
Ravi.

-----Original Message-----
From: Michael Dick [mailto:michael.d.dick@gmail.com] 
Sent: Thursday, May 14, 2009 8:54 AM
To: dev@openjpa.apache.org; 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, <de...@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
>
>
>

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/

Posted by David Ezzio <de...@apache.org>.
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, <de...@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
>>
>>
>>
> 

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/

Posted by Craig L Russell <Cr...@Sun.COM>.
Hi Ravi,

On May 15, 2009, at 6:56 AM, Ravi Palacherla wrote:

> Hi David,
>
> Thanks for removing the tag.
> Hiroki Tateno faxed a signed ICLA.

Thanks, I think we're ok now.

Craig
>
>
> Regards,
> Ravi.
>
> -----Original Message-----
> From: David Ezzio [mailto:dezzio@apache.org]
> Sent: Friday, May 15, 2009 7:34 AM
> To: dev@openjpa.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/
>
> Hi Craig,
>
> I've removed the errant author tag in TestMappingDefaultsImpl that was
> the source of the legal lesson for all of us.
>
> Regarding removing existing author tags, my 2 cents: Most of them
> attribute authorship to Patrick, Abe, Marc, and Steve, who as we know
> were the original authors of the code.  I think we should leave these
> tags in, and following your guidance, not add any more.
>
> Regarding the contributions that I and and others at Oracle are
> contributing, I am taking steps to ensure that we have a ICLA on file
> with Apache for all contributors, and we will do our best to comply  
> with
> all other guidelines identified in this mail thread.  If there are any
> problems in carrying through with this, I'll raise them either  
> privately
> with you or in this forum.
>
> I want to thank everyone for catching this error on my part and
> illuminating for all of us the proper procedures.
>
> David
>
>
> Craig L Russell wrote:
>> Hi Mike,
>>
>> On May 14, 2009, at 2:35 PM, Michael Dick wrote:
>>
>>> Thanks for the clarifications Craig and Donald.
>>>
>>> I think we've corrected the commit message now and I did verify  
>>> that the
>>> patch had granted the copyright so we should be okay.
>>
>> I saw your corrective action and appreciate it.
>>>
>>>
>>> How far do we want to go regarding @author tags. Should we go  
>>> ahead and
>>> remove them from already committed code or just make sure that we
>>> don't add
>>> any more?
>>
>> There are too many in the code to remove IMHO. We should not accept  
>> any
>> more.
>>
>> Craig
>>>
>>>
>>> -mike
>>>
>>> On Thu, May 14, 2009 at 3:44 PM, David Ezzio <de...@apache.org>  
>>> wrote:
>>>
>>>> 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:michael.d.dick@gmail.com]
>>>>>>> Sent: Thursday, May 14, 2009 8:54 AM
>>>>>>> To: dev@openjpa.apache.org; 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, <de...@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
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>> Craig L Russell
>>>>>> Architect, Sun Java Enterprise System http://db.apache.org/jdo
>>>>>> 408 276-5638 mailto:Craig.Russell@sun.com
>>>>>> P.S. A good JDO? O, Gasp!
>>>>>>
>>>>>>
>>>>>
>>
>> Craig L Russell
>> Architect, Sun Java Enterprise System http://db.apache.org/jdo
>> 408 276-5638 mailto:Craig.Russell@sun.com
>> P.S. A good JDO? O, Gasp!
>>

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


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/

Posted by Ravi Palacherla <ra...@oracle.com>.
Hi David,

Thanks for removing the tag.
Hiroki Tateno faxed a signed ICLA.

Regards,
Ravi.

-----Original Message-----
From: David Ezzio [mailto:dezzio@apache.org] 
Sent: Friday, May 15, 2009 7:34 AM
To: dev@openjpa.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/

Hi Craig,

I've removed the errant author tag in TestMappingDefaultsImpl that was 
the source of the legal lesson for all of us.

Regarding removing existing author tags, my 2 cents: Most of them 
attribute authorship to Patrick, Abe, Marc, and Steve, who as we know 
were the original authors of the code.  I think we should leave these 
tags in, and following your guidance, not add any more.

Regarding the contributions that I and and others at Oracle are 
contributing, I am taking steps to ensure that we have a ICLA on file 
with Apache for all contributors, and we will do our best to comply with 
all other guidelines identified in this mail thread.  If there are any 
problems in carrying through with this, I'll raise them either privately 
with you or in this forum.

I want to thank everyone for catching this error on my part and 
illuminating for all of us the proper procedures.

David


Craig L Russell wrote:
> Hi Mike,
> 
> On May 14, 2009, at 2:35 PM, Michael Dick wrote:
> 
>> Thanks for the clarifications Craig and Donald.
>>
>> I think we've corrected the commit message now and I did verify that the
>> patch had granted the copyright so we should be okay.
> 
> I saw your corrective action and appreciate it.
>>
>>
>> How far do we want to go regarding @author tags. Should we go ahead and
>> remove them from already committed code or just make sure that we 
>> don't add
>> any more?
> 
> There are too many in the code to remove IMHO. We should not accept any 
> more.
> 
> Craig
>>
>>
>> -mike
>>
>> On Thu, May 14, 2009 at 3:44 PM, David Ezzio <de...@apache.org> wrote:
>>
>>> 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:michael.d.dick@gmail.com]
>>>>>> Sent: Thursday, May 14, 2009 8:54 AM
>>>>>> To: dev@openjpa.apache.org; 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, <de...@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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>> Craig L Russell
>>>>> Architect, Sun Java Enterprise System http://db.apache.org/jdo
>>>>> 408 276-5638 mailto:Craig.Russell@sun.com
>>>>> P.S. A good JDO? O, Gasp!
>>>>>
>>>>>
>>>>
> 
> Craig L Russell
> Architect, Sun Java Enterprise System http://db.apache.org/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!
> 

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/

Posted by David Ezzio <de...@apache.org>.
Hi Craig,

I've removed the errant author tag in TestMappingDefaultsImpl that was 
the source of the legal lesson for all of us.

Regarding removing existing author tags, my 2 cents: Most of them 
attribute authorship to Patrick, Abe, Marc, and Steve, who as we know 
were the original authors of the code.  I think we should leave these 
tags in, and following your guidance, not add any more.

Regarding the contributions that I and and others at Oracle are 
contributing, I am taking steps to ensure that we have a ICLA on file 
with Apache for all contributors, and we will do our best to comply with 
all other guidelines identified in this mail thread.  If there are any 
problems in carrying through with this, I'll raise them either privately 
with you or in this forum.

I want to thank everyone for catching this error on my part and 
illuminating for all of us the proper procedures.

David


Craig L Russell wrote:
> Hi Mike,
> 
> On May 14, 2009, at 2:35 PM, Michael Dick wrote:
> 
>> Thanks for the clarifications Craig and Donald.
>>
>> I think we've corrected the commit message now and I did verify that the
>> patch had granted the copyright so we should be okay.
> 
> I saw your corrective action and appreciate it.
>>
>>
>> How far do we want to go regarding @author tags. Should we go ahead and
>> remove them from already committed code or just make sure that we 
>> don't add
>> any more?
> 
> There are too many in the code to remove IMHO. We should not accept any 
> more.
> 
> Craig
>>
>>
>> -mike
>>
>> On Thu, May 14, 2009 at 3:44 PM, David Ezzio <de...@apache.org> wrote:
>>
>>> 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:michael.d.dick@gmail.com]
>>>>>> Sent: Thursday, May 14, 2009 8:54 AM
>>>>>> To: dev@openjpa.apache.org; 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, <de...@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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>> Craig L Russell
>>>>> Architect, Sun Java Enterprise System http://db.apache.org/jdo
>>>>> 408 276-5638 mailto:Craig.Russell@sun.com
>>>>> P.S. A good JDO? O, Gasp!
>>>>>
>>>>>
>>>>
> 
> Craig L Russell
> Architect, Sun Java Enterprise System http://db.apache.org/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!
> 

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/

Posted by Craig L Russell <Cr...@Sun.COM>.
Hi Mike,

On May 14, 2009, at 2:35 PM, Michael Dick wrote:

> Thanks for the clarifications Craig and Donald.
>
> I think we've corrected the commit message now and I did verify that  
> the
> patch had granted the copyright so we should be okay.

I saw your corrective action and appreciate it.
>
>
> How far do we want to go regarding @author tags. Should we go ahead  
> and
> remove them from already committed code or just make sure that we  
> don't add
> any more?

There are too many in the code to remove IMHO. We should not accept  
any more.

Craig
>
>
> -mike
>
> On Thu, May 14, 2009 at 3:44 PM, David Ezzio <de...@apache.org>  
> wrote:
>
>> 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:michael.d.dick@gmail.com]
>>>>> Sent: Thursday, May 14, 2009 8:54 AM
>>>>> To: dev@openjpa.apache.org; 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, <de...@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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>> Craig L Russell
>>>> Architect, Sun Java Enterprise System http://db.apache.org/jdo
>>>> 408 276-5638 mailto:Craig.Russell@sun.com
>>>> P.S. A good JDO? O, Gasp!
>>>>
>>>>
>>>

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


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/

Posted by Michael Dick <mi...@gmail.com>.
Thanks for the clarifications Craig and Donald.

I think we've corrected the commit message now and I did verify that the
patch had granted the copyright so we should be okay.

How far do we want to go regarding @author tags. Should we go ahead and
remove them from already committed code or just make sure that we don't add
any more?

-mike

On Thu, May 14, 2009 at 3:44 PM, David Ezzio <de...@apache.org> wrote:

> 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:michael.d.dick@gmail.com]
>>>> Sent: Thursday, May 14, 2009 8:54 AM
>>>> To: dev@openjpa.apache.org; 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, <de...@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
>>>>>
>>>>>
>>>>>
>>>>>
>>> Craig L Russell
>>> Architect, Sun Java Enterprise System http://db.apache.org/jdo
>>> 408 276-5638 mailto:Craig.Russell@sun.com
>>> P.S. A good JDO? O, Gasp!
>>>
>>>
>>

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/

Posted by David Ezzio <de...@apache.org>.
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:michael.d.dick@gmail.com]
>>> Sent: Thursday, May 14, 2009 8:54 AM
>>> To: dev@openjpa.apache.org; 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, <de...@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
>>>>
>>>>
>>>>
>>
>> Craig L Russell
>> Architect, Sun Java Enterprise System http://db.apache.org/jdo
>> 408 276-5638 mailto:Craig.Russell@sun.com
>> P.S. A good JDO? O, Gasp!
>>
> 

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/

Posted by Craig L Russell <Cr...@Sun.COM>.
For trivial patches, maybe.

For significant patches, we really need to have the ICLA.

Craig

On May 14, 2009, at 1:16 PM, 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:michael.d.dick@gmail.com]
>>> Sent: Thursday, May 14, 2009 8:54 AM
>>> To: dev@openjpa.apache.org; 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, <de...@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
>>>>
>>>>
>>>>
>> Craig L Russell
>> Architect, Sun Java Enterprise System http://db.apache.org/jdo
>> 408 276-5638 mailto:Craig.Russell@sun.com
>> P.S. A good JDO? O, Gasp!

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


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/

Posted by Donald Woods <dw...@apache.org>.
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:michael.d.dick@gmail.com]
>> Sent: Thursday, May 14, 2009 8:54 AM
>> To: dev@openjpa.apache.org; 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, <de...@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
>>>
>>>
>>>
> 
> Craig L Russell
> Architect, Sun Java Enterprise System http://db.apache.org/jdo
> 408 276-5638 mailto:Craig.Russell@sun.com
> P.S. A good JDO? O, Gasp!
> 

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/

Posted by Craig L Russell <Cr...@Sun.COM>.
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:michael.d.dick@gmail.com]
> Sent: Thursday, May 14, 2009 8:54 AM
> To: dev@openjpa.apache.org; 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, <de...@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
>>
>>
>>

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


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/

Posted by Ravi Palacherla <ra...@oracle.com>.
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:michael.d.dick@gmail.com] 
Sent: Thursday, May 14, 2009 8:54 AM
To: dev@openjpa.apache.org; 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, <de...@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
>
>
>