[jira] [Commented] (DIRAPI-328) Fix inconsistency in Attribute and Value

["Emmanuel Lecharny (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/DIRAPI-328?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16836759#comment-16836759 ] 

Emmanuel Lecharny commented on DIRAPI-328:
------------------------------------------

Regarding the suggestion to throw an {{LdapInvalidAttributeValueException}} for {{Value.getString()}}, it's really problematic. Code like :

{code:java}
    public static javax.naming.directory.Attribute toJndiAttribute( Attribute attribute )
    {
        if ( attribute != null )
        {
            javax.naming.directory.Attribute jndiAttribute = new BasicAttribute( attribute.getUpId() );

            // Looping on values
            for ( Iterator<Value> valueIterator = attribute.iterator(); valueIterator.hasNext(); )
            {
                Value value = valueIterator.next();
                jndiAttribute.add( value.getString() );
            }
        ...
{code}

would definitively have problem with the thrown exception. This code is clearly wrong - it should check beforehand if the value is {{H/R}} -, but there are other pieces of code like that which would suffer if we throw an exception.

Side note, I reviewed many of the places where {{Value.getString()}} is called in the API and in the server, most of the time a check is done beforehand to know if the value is {{H/R}}, or assume it's {{H/R}}. There are some corner cases that have to be double checked, typically when we process a {{DN}}/{{RDN}} or {{AVA}}, where it assumes the value to be {{H/R}} but it may well be binary too (quite not frequent, but this is also the reason we never get a failure.

So bottom line, yes, there is more to do, but I don't think throwing an exception is the way to go.

> Fix inconsistency in Attribute and Value
> ----------------------------------------
>
>                 Key: DIRAPI-328
>                 URL: https://issues.apache.org/jira/browse/DIRAPI-328
>             Project: Directory Client API
>          Issue Type: Improvement
>            Reporter: Stefan Seelmann
>            Assignee: Emmanuel Lecharny
>            Priority: Major
>             Fix For: 2.0.0.AM3
>
>
> Inconsistent method names:
> * Attribute: getString() and getBytes()
> * Value: getValue() and getBytes()
> * Proposal: Rename Value.getValue() to Value.getString()
> Inconsistent and wrong documented error handling:
> * Attribute: getString() and getBytes() check for isHumanReadable and throw LdapInvalidAttributeValueException
> * Value: getValue()/getString() tries to UTF-8 encode bytes if not human readable, which doesn't work always, and javadoc says "returns null"
> * Value: getBytes() doesn't check if is human readable
> * Proposal: Make Value behave like Attribute, i.e. throw LdapInvalidAttributeValueException
> Opinions?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)