openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Craig L Russell <Craig.Russ...@Sun.COM>
Subject Re: svn commit: r774580 - in /openjpa/trunk/openjpa-jdbc/src: main/java/org/apache/openjpa/jdbc/meta/ main/java/org/apache/openjpa/jdbc/schema/ test/java/org/apache/openjpa/jdbc/meta/
Date Thu, 14 May 2009 22:03:12 GMT
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 <dezzio@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, <dezzio@apache.org> wrote:
>>>>>
>>>>> Author: dezzio
>>>>>> Date: Wed May 13 22:54:32 2009
>>>>>> New Revision: 774580
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=774580&view=rev
>>>>>> Log:
>>>>>> OpenJPA-1051: Fixed MappingDefaultsImpl to avoid column name
>>>>>> duplications
>>>>>> when long column names are supplied for a database that accepts 

>>>>>> only
>>>>>> shorter
>>>>>> names.  Changes submitted for Ravi Palacherla.
>>>>>>
>>>>>> Added:
>>>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/
>>>>>>
>>>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/TestMappingDefaultsImpl.java
>>>>>>
>>>>>> (with props)
>>>>>> Modified:
>>>>>>
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/MappingDefaultsImpl.java
>>>>>>
>>>>>>
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/schema/NameSet.java
>>>>>>
>>>>>>
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/schema/Table.java
>>>>>>
>>>>>>
>>>>>> Modified:
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/MappingDefaultsImpl.java
>>>>>>
>>>>>> URL:
>>>>>>
>>>>>> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java?rev=774580&r1=774579&r2=774580&view=diff
>>>>>>
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> =================================================================
>>>>>>
>>>>>> ---
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/MappingDefaultsImpl.java
>>>>>>
>>>>>> (original)
>>>>>> +++
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/MappingDefaultsImpl.java
>>>>>>
>>>>>> Wed May 13 22:54:32 2009
>>>>>> @@ -539,7 +539,9 @@
>>>>>>          else if (_dsIdName != null)
>>>>>>              cols[i].setName(_dsIdName + i);
>>>>>>          correctName(table, cols[i]);
>>>>>> +            table.addSubColumn(cols[i].getName());
>>>>>>      }
>>>>>> +        table.resetSubColumns();
>>>>>>  }
>>>>>>
>>>>>>  /**
>>>>>> @@ -582,7 +584,9 @@
>>>>>>          } else if (_versName != null)
>>>>>>              cols[i].setName(_versName + i);
>>>>>>          correctName(table, cols[i]);
>>>>>> +            table.addSubColumn(cols[i].getName());
>>>>>>      }
>>>>>> +        table.resetSubColumns();
>>>>>>  }
>>>>>>
>>>>>>  public void populateColumns(Discriminator disc, Table table,
>>>>>> @@ -593,7 +597,9 @@
>>>>>>          else if (_discName != null)
>>>>>>              cols[i].setName(_discName + i);
>>>>>>          correctName(table, cols[i]);
>>>>>> +            table.addSubColumn(cols[i].getName());
>>>>>>      }
>>>>>> +        table.resetSubColumns();
>>>>>>  }
>>>>>>
>>>>>>  public void populateJoinColumn(ClassMapping cm, Table local,  
>>>>>> Table
>>>>>> foreign,
>>>>>> @@ -618,8 +624,11 @@
>>>>>>
>>>>>>  public void populateColumns(ValueMapping vm, String name, Table
>>>>>> table,
>>>>>>      Column[] cols) {
>>>>>> -        for (int i = 0; i < cols.length; i++)
>>>>>> +        for (int i = 0; i < cols.length; i++) {
>>>>>>          correctName(table, cols[i]);
>>>>>> +            table.addSubColumn(cols[i].getName());
>>>>>> +        }
>>>>>> +        table.resetSubColumns();
>>>>>>  }
>>>>>>
>>>>>>  public boolean populateOrderColumns(FieldMapping fm, Table  
>>>>>> table,
>>>>>> @@ -630,7 +639,9 @@
>>>>>>          else if (_orderName != null)
>>>>>>              cols[i].setName(_orderName + i);
>>>>>>          correctName(table, cols[i]);
>>>>>> +            table.addSubColumn(cols[i].getName());
>>>>>>      }
>>>>>> +        table.resetSubColumns();
>>>>>>      return _orderLists && (JavaTypes.ARRAY == fm.getTypeCode()
>>>>>>          || List.class.isAssignableFrom(fm.getType()));
>>>>>>  }
>>>>>> @@ -643,7 +654,9 @@
>>>>>>          else if (_nullIndName != null)
>>>>>>              cols[i].setName(_nullIndName + i);
>>>>>>          correctName(table, cols[i]);
>>>>>> +            table.addSubColumn(cols[i].getName());
>>>>>>      }
>>>>>> +        table.resetSubColumns();
>>>>>>      return _addNullInd;
>>>>>>  }
>>>>>>
>>>>>>
>>>>>> Modified:
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/schema/NameSet.java
>>>>>>
>>>>>> URL:
>>>>>>
>>>>>> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java?rev=774580&r1=774579&r2=774580&view=diff
>>>>>>
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> =================================================================
>>>>>>
>>>>>> ---
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/schema/NameSet.java
>>>>>>
>>>>>> (original)
>>>>>> +++
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/schema/NameSet.java
>>>>>>
>>>>>> Wed May 13 22:54:32 2009
>>>>>> @@ -39,13 +39,17 @@
>>>>>>
>>>>>>  private Set _names = null;
>>>>>>
>>>>>> +    // an additional names Set for checking name duplication
>>>>>> +    private Set _subNames = null;
>>>>>> +
>>>>>>  /**
>>>>>>   * Return true if the given name is in use already.
>>>>>>   */
>>>>>>  public boolean isNameTaken(String name) {
>>>>>>      if (name == null)
>>>>>>          return true;
>>>>>> -        return _names != null &&  
>>>>>> _names.contains(name.toUpperCase());
>>>>>> +        return (_names != null &&  
>>>>>> _names.contains(name.toUpperCase()))
>>>>>> ||
>>>>>> +            (_subNames != null &&
>>>>>> _subNames.contains(name.toUpperCase()));
>>>>>>  }
>>>>>>
>>>>>>  /**
>>>>>> @@ -77,4 +81,20 @@
>>>>>>      if (name != null && _names != null)
>>>>>>          _names.remove(name.toUpperCase());
>>>>>>  }
>>>>>> +
>>>>>> +    /**
>>>>>> +    * Attempt to add the given name to the set.
>>>>>> +    *
>>>>>> +    * @param name the name to add
>>>>>> +    */
>>>>>> +    protected void addSubName(String name) {
>>>>>> +        if (_subNames == null) {
>>>>>> +            _subNames = new HashSet();
>>>>>> +        }
>>>>>> +        _subNames.add(name.toUpperCase());
>>>>>> +    }
>>>>>> +
>>>>>> +    protected void resetSubNames() {
>>>>>> +        _subNames = null;
>>>>>> +    }
>>>>>> }
>>>>>>
>>>>>> Modified:
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/schema/Table.java
>>>>>>
>>>>>> URL:
>>>>>>
>>>>>> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java?rev=774580&r1=774579&r2=774580&view=diff
>>>>>>
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> =================================================================
>>>>>>
>>>>>> ---
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/schema/Table.java
>>>>>>
>>>>>> (original)
>>>>>> +++
>>>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/ 
>>>>>> jdbc/schema/Table.java
>>>>>>
>>>>>> Wed May 13 22:54:32 2009
>>>>>> @@ -255,8 +255,8 @@
>>>>>>  }
>>>>>>
>>>>>>  public String[] getColumnNames() {
>>>>>> -       return _colMap == null ? new String[0] :
>>>>>> -               (String[])_colMap.keySet().toArray(new
>>>>>> String[_colMap.size()]);
>>>>>> +        return _colMap == null ? new String[0] :
>>>>>> +            (String[])_colMap.keySet().toArray(new
>>>>>> String[_colMap.size()]);
>>>>>>  }
>>>>>>
>>>>>>  /**
>>>>>> @@ -275,8 +275,8 @@
>>>>>>   * for dynamic table implementation.
>>>>>>   */
>>>>>>  public boolean containsColumn(String name) {
>>>>>> -       return name != null && _colMap != null
>>>>>> -               && _colMap.containsKey(name.toUpperCase());
>>>>>> +        return name != null && _colMap != null
>>>>>> +            && _colMap.containsKey(name.toUpperCase());
>>>>>>  }
>>>>>>
>>>>>>  /**
>>>>>> @@ -756,4 +756,15 @@
>>>>>>  public void setColNumber(int colNum) {
>>>>>>      _colNum = colNum;
>>>>>>  }
>>>>>> +
>>>>>> +    /**
>>>>>> +    * Add a column to the subNames set to avoid naming conflict.
>>>>>> +    */
>>>>>> +    public void addSubColumn(String name) {
>>>>>> +        addSubName(name);
>>>>>> +    }
>>>>>> +
>>>>>> +    public void resetSubColumns() {
>>>>>> +        resetSubNames();
>>>>>> +    }
>>>>>> }
>>>>>>
>>>>>> Added:
>>>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/TestMappingDefaultsImpl.java
>>>>>>
>>>>>> URL:
>>>>>>
>>>>>> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java?rev=774580&view=auto
>>>>>>
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> = 
>>>>>> =================================================================
>>>>>>
>>>>>> ---
>>>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/TestMappingDefaultsImpl.java
>>>>>>
>>>>>> (added)
>>>>>> +++
>>>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/TestMappingDefaultsImpl.java
>>>>>>
>>>>>> Wed May 13 22:54:32 2009
>>>>>> @@ -0,0 +1,63 @@
>>>>>> +/*
>>>>>> + * Licensed to the Apache Software Foundation (ASF) under one
>>>>>> + * or more contributor license agreements.  See the NOTICE file
>>>>>> + * distributed with this work for additional information
>>>>>> + * regarding copyright ownership.  The ASF licenses this file
>>>>>> + * to you under the Apache License, Version 2.0 (the
>>>>>> + * "License"); you may not use this file except in compliance
>>>>>> + * with the License.  You may obtain a copy of the License at
>>>>>> + *
>>>>>> + * http://www.apache.org/licenses/LICENSE-2.0
>>>>>> + *
>>>>>> + * Unless required by applicable law or agreed to in writing,
>>>>>> + * software distributed under the License is distributed on an
>>>>>> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
>>>>>> + * KIND, either express or implied.  See the License for the
>>>>>> + * specific language governing permissions and limitations
>>>>>> + * under the License.
>>>>>> + */
>>>>>> +package org.apache.openjpa.jdbc.meta;
>>>>>> +
>>>>>> +import org.apache.openjpa.jdbc.schema.Table;
>>>>>> +import org.apache.openjpa.jdbc.schema.Column;
>>>>>> +import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
>>>>>> +import org.apache.openjpa.jdbc.conf.JDBCConfigurationImpl;
>>>>>> +
>>>>>> +import junit.framework.TestCase;
>>>>>> +
>>>>>> +public class TestMappingDefaultsImpl extends TestCase {
>>>>>> +
>>>>>> +    public void setUp() {
>>>>>> +    }
>>>>>> +
>>>>>> +    /**
>>>>>> +     * For databases that accept only short column names, test
>>>>>> avoidance
>>>>>> of
>>>>>> +     * duplicate column names when populating the table with  
>>>>>> long
>>>>>> column
>>>>>> names.
>>>>>> +     *
>>>>>> +     * @author Hiroki Tateno
>>>>>> +     */
>>>>>> +    public void testPopulateWithLongColumnNames() {
>>>>>> +        MappingDefaultsImpl mapping = new MappingDefaultsImpl();
>>>>>> +        JDBCConfiguration conf = new  
>>>>>> JDBCConfigurationImpl(false,
>>>>>> false);
>>>>>> +        conf.setDBDictionary("oracle");
>>>>>> +        mapping.setConfiguration(conf);
>>>>>> +        Table table = new Table("testtable", null);
>>>>>> +        Column[] cols = new Column[3];
>>>>>> +        cols[0] = new
>>>>>> +
>>>>>> Column("longnamelongnamelongnamelongnamelongnamelongname1",
>>>>>> null);
>>>>>> +        cols[1] = new
>>>>>> +
>>>>>> Column("longnamelongnamelongnamelongnamelongnamelongname2",
>>>>>> null);
>>>>>> +        cols[2] = new
>>>>>> +
>>>>>> Column("longnamelongnamelongnamelongnamelongnamelongname3",
>>>>>> null);
>>>>>> +        MappingRepository mr = new MappingRepository();
>>>>>> +        mr.setConfiguration(conf);
>>>>>> +        Version version = new Version(new
>>>>>> ClassMapping(String.class,mr));
>>>>>> +        mapping.populateColumns(version, table, cols);
>>>>>> +        assertFalse("column names are conflicted : " +
>>>>>> cols[0].getName(),
>>>>>> +                cols[0].getName().equals(cols[1].getName()));
>>>>>> +        assertFalse("column names are conflicted : " +
>>>>>> cols[0].getName(),
>>>>>> +                cols[0].getName().equals(cols[2].getName()));
>>>>>> +        assertFalse("column names are conflicted : " +
>>>>>> cols[1].getName(),
>>>>>> +                cols[1].getName().equals(cols[2].getName()));
>>>>>> +    }
>>>>>> +}
>>>>>>
>>>>>> Propchange:
>>>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/ 
>>>>>> jdbc/meta/TestMappingDefaultsImpl.java
>>>>>>
>>>>>>
>>>>>> ------------------------------------------------------------------------------
>>>>>>
>>>>>> svn:eol-style = native
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>> 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!


Mime
View raw message