openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Dick <michael.d.d...@gmail.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 21:35:23 GMT
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 <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!
>>>
>>>
>>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message