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 <[email protected]> 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:[email protected]] >>>> Sent: Thursday, May 14, 2009 8:54 AM >>>> To: [email protected]; Ravi Palacherla >>>> Subject: Re: svn commit: r774580 - in /openjpa/trunk/openjpa-jdbc/src: >>>> main/java/org/apache/openjpa/jdbc/meta/ >>>> main/java/org/apache/openjpa/jdbc/schema/ >>>> test/java/org/apache/openjpa/jdbc/meta/ >>>> >>>> Hi David and Ravi >>>> >>>> The patch was contributed by Ravi, but the @author tag lists Hiroki >>>> Tateno. >>>> >>>> I'm not an expert on proper attribution, Craig can correct me where I go >>>> wrong :-). Here's my understanding. >>>> >>>> If Hiroki wrote the code we'll have to add his name to the commit >>>> message, >>>> if not we'll remote the @author tag. >>>> >>>> We may want to find out whether Hiroki has a CLA on file with Apache for >>>> future patches (same would apply for anyone in an @author tag). It isn't >>>> a >>>> requirement (AFAIK) but it's always nice to know. >>>> >>>> -mike >>>> >>>> On Wed, May 13, 2009 at 5:54 PM, <[email protected]> wrote: >>>> >>>> Author: dezzio >>>>> Date: Wed May 13 22:54:32 2009 >>>>> New Revision: 774580 >>>>> >>>>> URL: http://svn.apache.org/viewvc?rev=774580&view=rev >>>>> Log: >>>>> OpenJPA-1051: Fixed MappingDefaultsImpl to avoid column name >>>>> duplications >>>>> when long column names are supplied for a database that accepts only >>>>> shorter >>>>> names. Changes submitted for Ravi Palacherla. >>>>> >>>>> Added: >>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/ >>>>> >>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java >>>>> >>>>> (with props) >>>>> Modified: >>>>> >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java >>>>> >>>>> >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java >>>>> >>>>> >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java >>>>> >>>>> >>>>> Modified: >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java >>>>> >>>>> URL: >>>>> >>>>> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java?rev=774580&r1=774579&r2=774580&view=diff >>>>> >>>>> ============================================================================== >>>>> >>>>> --- >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java >>>>> >>>>> (original) >>>>> +++ >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingDefaultsImpl.java >>>>> >>>>> Wed May 13 22:54:32 2009 >>>>> @@ -539,7 +539,9 @@ >>>>> else if (_dsIdName != null) >>>>> cols[i].setName(_dsIdName + i); >>>>> correctName(table, cols[i]); >>>>> + table.addSubColumn(cols[i].getName()); >>>>> } >>>>> + table.resetSubColumns(); >>>>> } >>>>> >>>>> /** >>>>> @@ -582,7 +584,9 @@ >>>>> } else if (_versName != null) >>>>> cols[i].setName(_versName + i); >>>>> correctName(table, cols[i]); >>>>> + table.addSubColumn(cols[i].getName()); >>>>> } >>>>> + table.resetSubColumns(); >>>>> } >>>>> >>>>> public void populateColumns(Discriminator disc, Table table, >>>>> @@ -593,7 +597,9 @@ >>>>> else if (_discName != null) >>>>> cols[i].setName(_discName + i); >>>>> correctName(table, cols[i]); >>>>> + table.addSubColumn(cols[i].getName()); >>>>> } >>>>> + table.resetSubColumns(); >>>>> } >>>>> >>>>> public void populateJoinColumn(ClassMapping cm, Table local, Table >>>>> foreign, >>>>> @@ -618,8 +624,11 @@ >>>>> >>>>> public void populateColumns(ValueMapping vm, String name, Table >>>>> table, >>>>> Column[] cols) { >>>>> - for (int i = 0; i < cols.length; i++) >>>>> + for (int i = 0; i < cols.length; i++) { >>>>> correctName(table, cols[i]); >>>>> + table.addSubColumn(cols[i].getName()); >>>>> + } >>>>> + table.resetSubColumns(); >>>>> } >>>>> >>>>> public boolean populateOrderColumns(FieldMapping fm, Table table, >>>>> @@ -630,7 +639,9 @@ >>>>> else if (_orderName != null) >>>>> cols[i].setName(_orderName + i); >>>>> correctName(table, cols[i]); >>>>> + table.addSubColumn(cols[i].getName()); >>>>> } >>>>> + table.resetSubColumns(); >>>>> return _orderLists && (JavaTypes.ARRAY == fm.getTypeCode() >>>>> || List.class.isAssignableFrom(fm.getType())); >>>>> } >>>>> @@ -643,7 +654,9 @@ >>>>> else if (_nullIndName != null) >>>>> cols[i].setName(_nullIndName + i); >>>>> correctName(table, cols[i]); >>>>> + table.addSubColumn(cols[i].getName()); >>>>> } >>>>> + table.resetSubColumns(); >>>>> return _addNullInd; >>>>> } >>>>> >>>>> >>>>> Modified: >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java >>>>> >>>>> URL: >>>>> >>>>> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java?rev=774580&r1=774579&r2=774580&view=diff >>>>> >>>>> ============================================================================== >>>>> >>>>> --- >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java >>>>> >>>>> (original) >>>>> +++ >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/NameSet.java >>>>> >>>>> Wed May 13 22:54:32 2009 >>>>> @@ -39,13 +39,17 @@ >>>>> >>>>> private Set _names = null; >>>>> >>>>> + // an additional names Set for checking name duplication >>>>> + private Set _subNames = null; >>>>> + >>>>> /** >>>>> * Return true if the given name is in use already. >>>>> */ >>>>> public boolean isNameTaken(String name) { >>>>> if (name == null) >>>>> return true; >>>>> - return _names != null && _names.contains(name.toUpperCase()); >>>>> + return (_names != null && _names.contains(name.toUpperCase())) >>>>> || >>>>> + (_subNames != null && >>>>> _subNames.contains(name.toUpperCase())); >>>>> } >>>>> >>>>> /** >>>>> @@ -77,4 +81,20 @@ >>>>> if (name != null && _names != null) >>>>> _names.remove(name.toUpperCase()); >>>>> } >>>>> + >>>>> + /** >>>>> + * Attempt to add the given name to the set. >>>>> + * >>>>> + * @param name the name to add >>>>> + */ >>>>> + protected void addSubName(String name) { >>>>> + if (_subNames == null) { >>>>> + _subNames = new HashSet(); >>>>> + } >>>>> + _subNames.add(name.toUpperCase()); >>>>> + } >>>>> + >>>>> + protected void resetSubNames() { >>>>> + _subNames = null; >>>>> + } >>>>> } >>>>> >>>>> Modified: >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java >>>>> >>>>> URL: >>>>> >>>>> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java?rev=774580&r1=774579&r2=774580&view=diff >>>>> >>>>> ============================================================================== >>>>> >>>>> --- >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java >>>>> >>>>> (original) >>>>> +++ >>>>> openjpa/trunk/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/Table.java >>>>> >>>>> Wed May 13 22:54:32 2009 >>>>> @@ -255,8 +255,8 @@ >>>>> } >>>>> >>>>> public String[] getColumnNames() { >>>>> - return _colMap == null ? new String[0] : >>>>> - (String[])_colMap.keySet().toArray(new >>>>> String[_colMap.size()]); >>>>> + return _colMap == null ? new String[0] : >>>>> + (String[])_colMap.keySet().toArray(new >>>>> String[_colMap.size()]); >>>>> } >>>>> >>>>> /** >>>>> @@ -275,8 +275,8 @@ >>>>> * for dynamic table implementation. >>>>> */ >>>>> public boolean containsColumn(String name) { >>>>> - return name != null && _colMap != null >>>>> - && _colMap.containsKey(name.toUpperCase()); >>>>> + return name != null && _colMap != null >>>>> + && _colMap.containsKey(name.toUpperCase()); >>>>> } >>>>> >>>>> /** >>>>> @@ -756,4 +756,15 @@ >>>>> public void setColNumber(int colNum) { >>>>> _colNum = colNum; >>>>> } >>>>> + >>>>> + /** >>>>> + * Add a column to the subNames set to avoid naming conflict. >>>>> + */ >>>>> + public void addSubColumn(String name) { >>>>> + addSubName(name); >>>>> + } >>>>> + >>>>> + public void resetSubColumns() { >>>>> + resetSubNames(); >>>>> + } >>>>> } >>>>> >>>>> Added: >>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java >>>>> >>>>> URL: >>>>> >>>>> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java?rev=774580&view=auto >>>>> >>>>> ============================================================================== >>>>> >>>>> --- >>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java >>>>> >>>>> (added) >>>>> +++ >>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java >>>>> >>>>> Wed May 13 22:54:32 2009 >>>>> @@ -0,0 +1,63 @@ >>>>> +/* >>>>> + * Licensed to the Apache Software Foundation (ASF) under one >>>>> + * or more contributor license agreements. See the NOTICE file >>>>> + * distributed with this work for additional information >>>>> + * regarding copyright ownership. The ASF licenses this file >>>>> + * to you under the Apache License, Version 2.0 (the >>>>> + * "License"); you may not use this file except in compliance >>>>> + * with the License. You may obtain a copy of the License at >>>>> + * >>>>> + * http://www.apache.org/licenses/LICENSE-2.0 >>>>> + * >>>>> + * Unless required by applicable law or agreed to in writing, >>>>> + * software distributed under the License is distributed on an >>>>> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY >>>>> + * KIND, either express or implied. See the License for the >>>>> + * specific language governing permissions and limitations >>>>> + * under the License. >>>>> + */ >>>>> +package org.apache.openjpa.jdbc.meta; >>>>> + >>>>> +import org.apache.openjpa.jdbc.schema.Table; >>>>> +import org.apache.openjpa.jdbc.schema.Column; >>>>> +import org.apache.openjpa.jdbc.conf.JDBCConfiguration; >>>>> +import org.apache.openjpa.jdbc.conf.JDBCConfigurationImpl; >>>>> + >>>>> +import junit.framework.TestCase; >>>>> + >>>>> +public class TestMappingDefaultsImpl extends TestCase { >>>>> + >>>>> + public void setUp() { >>>>> + } >>>>> + >>>>> + /** >>>>> + * For databases that accept only short column names, test >>>>> avoidance >>>>> of >>>>> + * duplicate column names when populating the table with long >>>>> column >>>>> names. >>>>> + * >>>>> + * @author Hiroki Tateno >>>>> + */ >>>>> + public void testPopulateWithLongColumnNames() { >>>>> + MappingDefaultsImpl mapping = new MappingDefaultsImpl(); >>>>> + JDBCConfiguration conf = new JDBCConfigurationImpl(false, >>>>> false); >>>>> + conf.setDBDictionary("oracle"); >>>>> + mapping.setConfiguration(conf); >>>>> + Table table = new Table("testtable", null); >>>>> + Column[] cols = new Column[3]; >>>>> + cols[0] = new >>>>> + >>>>> Column("longnamelongnamelongnamelongnamelongnamelongname1", >>>>> null); >>>>> + cols[1] = new >>>>> + >>>>> Column("longnamelongnamelongnamelongnamelongnamelongname2", >>>>> null); >>>>> + cols[2] = new >>>>> + >>>>> Column("longnamelongnamelongnamelongnamelongnamelongname3", >>>>> null); >>>>> + MappingRepository mr = new MappingRepository(); >>>>> + mr.setConfiguration(conf); >>>>> + Version version = new Version(new >>>>> ClassMapping(String.class,mr)); >>>>> + mapping.populateColumns(version, table, cols); >>>>> + assertFalse("column names are conflicted : " + >>>>> cols[0].getName(), >>>>> + cols[0].getName().equals(cols[1].getName())); >>>>> + assertFalse("column names are conflicted : " + >>>>> cols[0].getName(), >>>>> + cols[0].getName().equals(cols[2].getName())); >>>>> + assertFalse("column names are conflicted : " + >>>>> cols[1].getName(), >>>>> + cols[1].getName().equals(cols[2].getName())); >>>>> + } >>>>> +} >>>>> >>>>> Propchange: >>>>> openjpa/trunk/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/meta/TestMappingDefaultsImpl.java >>>>> >>>>> >>>>> ------------------------------------------------------------------------------ >>>>> >>>>> svn:eol-style = native >>>>> >>>>> >>>>> >>>>> >>> Craig L Russell >>> Architect, Sun Java Enterprise System http://db.apache.org/jdo >>> 408 276-5638 mailto:[email protected] >>> P.S. A good JDO? O, Gasp! >>> >>> >>
