Re: Java LDAP LDAPAttribute with null values (ITS#1514)

[Kurt@OpenLDAP.org]

I note that I don't view this as an implementation issue,
but a specification issue.  We should have the specification
clarified as to expected semantics and then implement those
semantics.

IMO, 1) is not a really bad approach.  A value (null) was
provided, it should not be ignored.

I could live with 2) IF there was strong consensus supporting
this approach.  But as many attributes do not allow empty
values, this would to protocol errors not API exceptions.
I think this would confuse many application developers.

However, I believe 3) is the best option.  The constructor
was designed such that String or byte[] were expected, not
null.  As such, an exception should be thrown.  And applications
which are improperly coded will quickly fault, that's a
Good Thing.

Kurt

At 10:52 AM 2002-01-08, vtag@novell.com wrote:
>I've given my opinion, but to be honest I'm not very
>comfortable with it.  Let me review the issue and the
>options.  The issue is: what happens if
>the application specifies null as an attribute value.
>There are three choices.
>
>1) Silently ignore it - i.e. addValue( (String)null)
>    does nothing.
>2) The default behavior of the API is to turn
>    the value into either new String("") or
>    new byte[0].
>3) Treat null as an error and throw a RuntimeException
>    (probably IllegalArgumentException).
>
>Option 1) If this option were used, applications with errors
>could discover it only by observing missing values in the DIT
>or by getting back server errors.  I really don't like this kind
>of error detection and do not choose this option.
>
>Option 2) The thing that bothers me here is that
>applications assume some sort of default behavior
>under the covers.  Someone has to create the
>zero length string or zero length byte array.
>If that is what the application wants, it seems more
>explicit and easier to understand for the application
>to generate the objects rather than for the API to
>generate them.  It would be easy for the application
>to assume (incorrectly) that the API generates the right
>thing for all kinds of attributes, where in reality, the
>API has no concept of what the syntax of the attribute
>is.  Some syntaxes do not even acccept zero length values.
>An example is the numeric string syntax, defined as 1*d.
>
>3) This approach gives consistent and predictable results.
>The application must be specific about what it intends.
>I feel it is better and more consistent to always throw
>an IllegalArgumentException if null is specified as an
>argument value.  It is also simple to debug and to prevent
>from the application point of view.
>
>-Steve
>
>> 
>> >>> <vtag@novell.com> 08-Jan-02 8:44:03 AM >>>
>> > 
>> > 
>> > >>> <dfoster@equitytg.com> 07-Jan-02 7:39:14 PM >>>
>> > I can certainly agree with your argument about throwing an
>Exception
>> but
>> > there are a few things that are preventing me from whole heartedly
>> accepting
>> > it.  The primary reason is draft-ietf-ldapext-ldap-java-api-17.txt
>> does not
>> > provide a checked Exception mechanism for (most if not all) of the
>> > container/wrapper classes (i.e. LDAPAttribute), therefore any
>> Exception that
>> > gets thrown would be have to be a RuntimeException which would not
>> 
>> That is right, a RuntimeException must be thrown.  The
>> RuntimeException
>> indicates what the error is but occurs only at runtime not at
>compile
>> time.
>> It could be argued that the constructors LDAPAttribute("name"), and
>> LDAPAttribute("name", (String)null) are equivalent, i.e. create an
>> LDAPAttribute
>> with no values (values must be added by subsequent calls).  On the
>> other
>> hand it could be argued that it should create an empty String value
>> or an empty byte[] value.  Both are possible and legal.  The draft is
>
>> silent on this behavior.  Based on the principle of least surprise,
>> I think I would go with the idea that a null attribute is the same
>as
>> an
>> empty attribute and would insert an empty value (String or byte[] 
>> depending on the which method is called.
>> 
>> > accomplish what you mentioned about an immediate error,
>additionally
>> the
>> > error would become intermittent.  That is why I propose creating a
>> 
>> I don't really know what you mean by intermittent error.  It should
>> always
>> happen given the same conditions.
>> 
>> 
>> > consistent deterministic behavior pattern for dealing with null
>> values that
>> > does not break with the ietf draft.
>> > 
>> > I have no problem with treating null as an empty string but what
>if
>> that
>> > attribute's value is not a string (.i.e a JPEG photo)?  I think if
>we
>> treat
>> > nulls as what they are (no value) the LDAP server will be forced
>to
>> reject
>> > an entry if an attribute's value is required.  What do you think?
>> 
>> More properly said it is an empty value, either String or byte[]
>> depending
>> on which function is called.  The null must be properly cast of
>> course.
>> 
>> -Steve
>> > 
>> > 
>> > Dane Foster
>> > http://www.equitytg.com 
>> > 954.360.9800
>> > ----- Original Message -----
>> > From: "Kurt D. Zeilenga" <Kurt@OpenLDAP.org>
>> > To: <dfoster@equitytg.com>
>> > Cc: <openldap-its@OpenLDAP.org>
>> > Sent: Monday, January 07, 2002 7:05 PM
>> > Subject: Re: JLDAP Bug n fixes (ITS#1512)
>> > 
>> > 
>> > > At 03:37 PM 2002-01-07, dfoster@equitytg.com wrote:
>> > > >This message is a follow up to my previous message because I
>> forgot to
>> > > >comment on :
>> > > >>At 01:01 PM 2002-01-07, vtag@novell.com wrote:
>> > > >>It certainly doesn't make sense to do something like
>> > > >>  LDAPAttribute("name", new String[]  {"a", "b", null, "c"});
>> > > >>This case should probably throw an exception.
>> > > >
>> > > >> Kurt wrote:
>> > > >>I would have to agree.  But,
>> > > >
>> > > >>LDAPAttribute("mail", new String[]  {"a", "b", "", "c"});
>> > > >
>> > > >>would be valid (IA5 strings allow empty values), null !=
>empty.
>> > > >
>> > > >
>> > > >LDAPAttribute( "name", new String[]{ "a", "b", null, "c"} );
>> > >
>> > > >I don't think it should throw an Exception for two reasons:
>> > > >1 - It does not throw an Exception if you do (assume att was
>> already
>> > > >instantiated): att.setValue( null )//cast as a byte[] or
>String.
>> > >
>> > > That might be an argument for treating the null as an
>> > > empty string.  It not a good reason for treating it
>> > > otherwise.
>> > >
>> > > >2 - It could potentially introduce a really hard to find bug in
>> people's
>> > > >code that uses the API (e.g.,migration scripts, automated
>> scripts).
>> > >
>> > > This is actually a good argument for throwing an exception.
>> > > Exceptions are much easier to debug.
>> > >
>> > >
>> > > >A think the solution should be to ignore null.
>> > >
>> > > I oppose this solution as it would lead to code which would
>> > > cause subsequent errors instead of immediate errors.
>> > >
>> > > Kurt
>> > >
>>