You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by el...@apache.org on 2012/02/02 23:59:08 UTC
svn commit: r1239907 - in /directory/shared/trunk/ldap/model/src:
main/java/org/apache/directory/shared/ldap/model/name/Rdn.java
test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
Author: elecharny
Date: Thu Feb 2 22:59:08 2012
New Revision: 1239907
URL: http://svn.apache.org/viewvc?rev=1239907&view=rev
Log:
Fix DIRAPI-76 : new Rdn( "A=a,B=b" ) now throws an LdapInvalidDnException
Modified:
directory/shared/trunk/ldap/model/src/main/java/org/apache/directory/shared/ldap/model/name/Rdn.java
directory/shared/trunk/ldap/model/src/test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
Modified: directory/shared/trunk/ldap/model/src/main/java/org/apache/directory/shared/ldap/model/name/Rdn.java
URL: http://svn.apache.org/viewvc/directory/shared/trunk/ldap/model/src/main/java/org/apache/directory/shared/ldap/model/name/Rdn.java?rev=1239907&r1=1239906&r2=1239907&view=diff
==============================================================================
--- directory/shared/trunk/ldap/model/src/main/java/org/apache/directory/shared/ldap/model/name/Rdn.java (original)
+++ directory/shared/trunk/ldap/model/src/main/java/org/apache/directory/shared/ldap/model/name/Rdn.java Thu Feb 2 22:59:08 2012
@@ -249,6 +249,11 @@ public class Rdn implements Cloneable, E
normalized = false;
}
+ if ( upName.length() < rdn.length() )
+ {
+ throw new LdapInvalidDnException( "Invalid RDN" );
+ }
+
upName = rdn;
}
else
@@ -347,7 +352,7 @@ public class Rdn implements Cloneable, E
return;
case 1:
- this.ava = ( Ava ) rdn.ava.clone();
+ this.ava = rdn.ava.clone();
hashCode();
return;
@@ -359,7 +364,7 @@ public class Rdn implements Cloneable, E
for ( Ava currentAva : rdn.avas )
{
- avas.add( ( Ava ) currentAva.clone() );
+ avas.add( currentAva.clone() );
avaTypes.put( currentAva.getNormType(), currentAva );
}
@@ -772,7 +777,7 @@ public class Rdn implements Cloneable, E
break;
case 1:
- rdn.ava = ( Ava ) this.ava.clone();
+ rdn.ava = this.ava.clone();
rdn.avaTypes = avaTypes;
break;
@@ -783,7 +788,7 @@ public class Rdn implements Cloneable, E
for ( Ava currentAva : this.avas )
{
- rdn.avas.add( ( Ava ) currentAva.clone() );
+ rdn.avas.add( currentAva.clone() );
rdn.avaTypes.put( currentAva.getNormType(), currentAva );
}
Modified: directory/shared/trunk/ldap/model/src/test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
URL: http://svn.apache.org/viewvc/directory/shared/trunk/ldap/model/src/test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java?rev=1239907&r1=1239906&r2=1239907&view=diff
==============================================================================
--- directory/shared/trunk/ldap/model/src/test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java (original)
+++ directory/shared/trunk/ldap/model/src/test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java Thu Feb 2 22:59:08 2012
@@ -6,16 +6,16 @@
* 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.
- *
+ * under the License.
+ *
*/
package org.apache.directory.shared.ldap.model.name;
@@ -34,6 +34,7 @@ import java.io.ObjectOutputStream;
import java.util.Iterator;
import org.apache.directory.shared.ldap.model.exception.LdapException;
+import org.apache.directory.shared.ldap.model.exception.LdapInvalidDnException;
import org.apache.directory.shared.ldap.model.schema.SchemaManager;
import org.apache.directory.shared.util.Strings;
import org.junit.Test;
@@ -279,7 +280,7 @@ public class RdnTest
{
Rdn rdn = new Rdn( "a", "b" );
- Rdn rdnClone = ( Rdn ) rdn.clone();
+ Rdn rdnClone = rdn.clone();
rdn = new Rdn( "c=d" );
@@ -311,7 +312,7 @@ public class RdnTest
{
Rdn rdn = new Rdn( "a = b + aa = bb" );
- Rdn rdnClone = ( Rdn ) rdn.clone();
+ Rdn rdnClone = rdn.clone();
rdn.clear();
rdn = new Rdn( "c=d" );
@@ -1153,10 +1154,10 @@ public class RdnTest
@Test
public void testComparingOfClonedMultiValuedRDNs() throws LdapException
{
- // Use upper case attribute types to test if normalized types are used
+ // Use upper case attribute types to test if normalized types are used
// for comparison
Rdn rdn = new Rdn( " A = b + C = d" );
- Rdn clonedRdn = ( Rdn ) rdn.clone();
+ Rdn clonedRdn = rdn.clone();
assertTrue( rdn.equals( clonedRdn ) );
}
@@ -1171,7 +1172,7 @@ public class RdnTest
@Test
public void testComparingOfCopyConstructedMultiValuedRDNs() throws LdapException
{
- // Use upper case attribute types to test if normalized types are used
+ // Use upper case attribute types to test if normalized types are used
// for comparison
Rdn rdn = new Rdn( " A = b + C = d" );
Rdn copiedRdn = new Rdn( rdn );
@@ -1210,4 +1211,14 @@ public class RdnTest
i++;
}
}
+
+
+ /**
+ * test that a RDN with two AVAs throws an exception
+ */
+ @Test( expected=LdapInvalidDnException.class )
+ public void testWrongRdn() throws LdapException
+ {
+ new Rdn( " A = b, C = d " );
+ }
}
Re: svn commit: r1239907 - in /directory/shared/trunk/ldap/model/src:
main/java/org/apache/directory/shared/ldap/model/name/Rdn.java test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
Posted by Alex Karasulu <ak...@apache.org>.
On Sat, Feb 4, 2012 at 10:13 AM, Emmanuel Lecharny <el...@gmail.com>wrote:
> On 2/3/12 11:09 PM, Alex Karasulu wrote:
>
>> On Fri, Feb 3, 2012 at 12:59 AM,<el...@apache.org> wrote:
>>
>> Author: elecharny
>>> Date: Thu Feb 2 22:59:08 2012
>>> New Revision: 1239907
>>>
>>> URL: http://svn.apache.org/viewvc?**rev=1239907&view=rev<http://svn.apache.org/viewvc?rev=1239907&view=rev>
>>> Log:
>>> Fix DIRAPI-76 : new Rdn( "A=a,B=b" ) now throws an LdapInvalidDnException
>>>
>>>
>>> Should the exception not be ... LdapInvalidNameComponent (we can create
>> one
>> if it does not exist).
>>
>> Reason I say this is that the whole issue with the non-intuitive
>> constructor was that the API user was thinking the argument can be a
>> multi-component relative distinguished name or a DN.
>> LdapInvalidDnException
>> might not fit here and it might make the user think they have to use a DN
>> rather than a single name component.
>>
>> WDYT?
>>
>> Rahhh... Not such an easy move. In many many places, we are expecting a
> LdapInvalidDnException. Rdn is considered as a Dn with one single Rdn in
> most of the code.
>
> Question : would it worth the effort to change every part of the code when
> we can simply improve the message contained in the exception ?
>
>
Never thought it would be this bloody hard. Leave it as is then and just
improve the message contained in the exception. This is my 2 cents.
--
Best Regards,
-- Alex
Re: svn commit: r1239907 - in /directory/shared/trunk/ldap/model/src:
main/java/org/apache/directory/shared/ldap/model/name/Rdn.java test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
Posted by Emmanuel Lecharny <el...@gmail.com>.
On 2/3/12 11:09 PM, Alex Karasulu wrote:
> On Fri, Feb 3, 2012 at 12:59 AM,<el...@apache.org> wrote:
>
>> Author: elecharny
>> Date: Thu Feb 2 22:59:08 2012
>> New Revision: 1239907
>>
>> URL: http://svn.apache.org/viewvc?rev=1239907&view=rev
>> Log:
>> Fix DIRAPI-76 : new Rdn( "A=a,B=b" ) now throws an LdapInvalidDnException
>>
>>
> Should the exception not be ... LdapInvalidNameComponent (we can create one
> if it does not exist).
>
> Reason I say this is that the whole issue with the non-intuitive
> constructor was that the API user was thinking the argument can be a
> multi-component relative distinguished name or a DN. LdapInvalidDnException
> might not fit here and it might make the user think they have to use a DN
> rather than a single name component.
>
> WDYT?
>
Rahhh... Not such an easy move. In many many places, we are expecting a
LdapInvalidDnException. Rdn is considered as a Dn with one single Rdn in
most of the code.
Question : would it worth the effort to change every part of the code
when we can simply improve the message contained in the exception ?
Or may be we can go for a more drastic change : get rid of the
LdapDnException and rename it LdapNmeException ?
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: svn commit: r1239907 - in /directory/shared/trunk/ldap/model/src: main/java/org/apache/directory/shared/ldap/model/name/Rdn.java test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
Posted by Pierre-Arnaud Marcelot <pa...@marcelot.net>.
On 4 févr. 2012, at 01:52, Alex Karasulu wrote:
>
>
> On Sat, Feb 4, 2012 at 2:47 AM, Emmanuel Lecharny <el...@gmail.com> wrote:
> On 2/3/12 11:09 PM, Alex Karasulu wrote:
> On Fri, Feb 3, 2012 at 12:59 AM,<el...@apache.org> wrote:
>
> Author: elecharny
> Date: Thu Feb 2 22:59:08 2012
> New Revision: 1239907
>
> URL: http://svn.apache.org/viewvc?rev=1239907&view=rev
> Log:
> Fix DIRAPI-76 : new Rdn( "A=a,B=b" ) now throws an LdapInvalidDnException
>
>
> Should the exception not be ... LdapInvalidNameComponent (we can create one
> if it does not exist).
> Or LdapInvalidRdnException. Yes.
>
>
> Sounds good too.
+1
Regards,
Pierre-Arnaud
>
> Reason I say this is that the whole issue with the non-intuitive
> constructor was that the API user was thinking the argument can be a
> multi-component relative distinguished name or a DN. LdapInvalidDnException
> might not fit here and it might make the user think they have to use a DN
> rather than a single name component.
>
> WDYT?
> I totally agree. The LdapInvalidDnException was picked to have a quick fix for this issue. I was overloaded with many other issues related to the change made in the Rdn constructor fix :
> - DSML parser was not anymore working (a bug in the DSML xml files)
> - some question raised about the ParentIdAnRdn to be double checked (do we support a multiple AVA in a NamingContext, or not)
>
>
> Totally understandable. I just posted this just in case it was not noticed.
>
> --
> Best Regards,
> -- Alex
>
Re: svn commit: r1239907 - in /directory/shared/trunk/ldap/model/src:
main/java/org/apache/directory/shared/ldap/model/name/Rdn.java test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
Posted by Alex Karasulu <ak...@apache.org>.
On Sat, Feb 4, 2012 at 2:47 AM, Emmanuel Lecharny <el...@gmail.com>wrote:
> On 2/3/12 11:09 PM, Alex Karasulu wrote:
>
>> On Fri, Feb 3, 2012 at 12:59 AM,<el...@apache.org> wrote:
>>
>> Author: elecharny
>>> Date: Thu Feb 2 22:59:08 2012
>>> New Revision: 1239907
>>>
>>> URL: http://svn.apache.org/viewvc?**rev=1239907&view=rev<http://svn.apache.org/viewvc?rev=1239907&view=rev>
>>> Log:
>>> Fix DIRAPI-76 : new Rdn( "A=a,B=b" ) now throws an LdapInvalidDnException
>>>
>>>
>>> Should the exception not be ... LdapInvalidNameComponent (we can create
>> one
>> if it does not exist).
>>
> Or LdapInvalidRdnException. Yes.
>
>
Sounds good too.
>
>> Reason I say this is that the whole issue with the non-intuitive
>> constructor was that the API user was thinking the argument can be a
>> multi-component relative distinguished name or a DN.
>> LdapInvalidDnException
>> might not fit here and it might make the user think they have to use a DN
>> rather than a single name component.
>>
>> WDYT?
>>
> I totally agree. The LdapInvalidDnException was picked to have a quick fix
> for this issue. I was overloaded with many other issues related to the
> change made in the Rdn constructor fix :
> - DSML parser was not anymore working (a bug in the DSML xml files)
> - some question raised about the ParentIdAnRdn to be double checked (do we
> support a multiple AVA in a NamingContext, or not)
>
>
Totally understandable. I just posted this just in case it was not noticed.
--
Best Regards,
-- Alex
Re: svn commit: r1239907 - in /directory/shared/trunk/ldap/model/src:
main/java/org/apache/directory/shared/ldap/model/name/Rdn.java test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
Posted by Emmanuel Lecharny <el...@gmail.com>.
On 2/3/12 11:09 PM, Alex Karasulu wrote:
> On Fri, Feb 3, 2012 at 12:59 AM,<el...@apache.org> wrote:
>
>> Author: elecharny
>> Date: Thu Feb 2 22:59:08 2012
>> New Revision: 1239907
>>
>> URL: http://svn.apache.org/viewvc?rev=1239907&view=rev
>> Log:
>> Fix DIRAPI-76 : new Rdn( "A=a,B=b" ) now throws an LdapInvalidDnException
>>
>>
> Should the exception not be ... LdapInvalidNameComponent (we can create one
> if it does not exist).
Or LdapInvalidRdnException. Yes.
>
> Reason I say this is that the whole issue with the non-intuitive
> constructor was that the API user was thinking the argument can be a
> multi-component relative distinguished name or a DN. LdapInvalidDnException
> might not fit here and it might make the user think they have to use a DN
> rather than a single name component.
>
> WDYT?
I totally agree. The LdapInvalidDnException was picked to have a quick
fix for this issue. I was overloaded with many other issues related to
the change made in the Rdn constructor fix :
- DSML parser was not anymore working (a bug in the DSML xml files)
- some question raised about the ParentIdAnRdn to be double checked (do
we support a multiple AVA in a NamingContext, or not)
I'll add the exception.
>
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: svn commit: r1239907 - in /directory/shared/trunk/ldap/model/src:
main/java/org/apache/directory/shared/ldap/model/name/Rdn.java test/java/org/apache/directory/shared/ldap/model/name/RdnTest.java
Posted by Alex Karasulu <ak...@apache.org>.
On Fri, Feb 3, 2012 at 12:59 AM, <el...@apache.org> wrote:
> Author: elecharny
> Date: Thu Feb 2 22:59:08 2012
> New Revision: 1239907
>
> URL: http://svn.apache.org/viewvc?rev=1239907&view=rev
> Log:
> Fix DIRAPI-76 : new Rdn( "A=a,B=b" ) now throws an LdapInvalidDnException
>
>
Should the exception not be ... LdapInvalidNameComponent (we can create one
if it does not exist).
Reason I say this is that the whole issue with the non-intuitive
constructor was that the API user was thinking the argument can be a
multi-component relative distinguished name or a DN. LdapInvalidDnException
might not fit here and it might make the user think they have to use a DN
rather than a single name component.
WDYT?
--
Best Regards,
-- Alex