[Freeipa-devel] [PATCH] Check if attribute is single-value before trying to add values to it.

Rob Crittenden rcritten at redhat.com
Fri Oct 15 13:16:37 UTC 2010


Pavel Zůna wrote:
> On 2010-10-14 19:20, Rob Crittenden wrote:
>> Pavel Zuna wrote:
>>> On 10/14/2010 12:01 AM, Rob Crittenden wrote:
>>>> Pavel Zuna wrote:
>>>>> This patch adds a check in ldap2 for single-value attributes. DS
>>>>> doesn't
>>>>> seem to care much about attributes being defined as SINGLE-VALUE
>>>>> except
>>>>> for things like uidNumber and gidNumber (I suspect this is handled by
>>>>> the DNA plugin).
>>>>>
>>>>> Ticket #246
>>>>>
>>>>> Pavel
>>>>
>>>> This is similar to ticket 220 which I have a pending patch for (patch
>>>> 552). I think both patches are valid but we should test them
>>>> together to
>>>> be sure. Can you do that?
>>>>
>>>> rob
>>>
>>> I had to NACK your patch number 552, because the check was in the wrong
>>> place.
>>>
>>> Both patches overlap in functionality, so I decided to merge them into a
>>> new version of my original patch.
>>>
>>> I split the single-value check into two parts:
>>>
>>> First part is in baseldap classes (LDAPCreate, LDAPUpdate) and it checks
>>> if we're not trying to add more values to a Param defined attribute,
>>> that is not flagged as multivalue.
>>>
>>> Second part is in the ldap2 backend. It checks if we're not trying to
>>> add more values to an attribute, that is defined as SINGLE-VALUE in the
>>> schema. Unfortunately, it seems that python-ldap isn't capable of
>>> reporting the SINGLE-VALUE flag reliably and DS doesn't enforce it at
>>> all. In other words, this check is a bit weak, but still better than
>>> nothing.
>>>
>>> I hope you don't mind I merged both patches, but it seemed simpler and
>>> we can knock out 2 tickets in one commit. :)
>>>
>>> Ticket #230
>>> Ticket #246
>>>
>>> Pavel
>>
>> Ack if you fix 2 things:
>>
>> 1. Change the error message of the exception to match the exception
>> name, 'only one value allowed' instead of 'attribute is single-value'
> Ok.
>
>> 2. You added a space between desc and info in the DatabaseError
>> exception. The example fails because there is no space after the colon
>> (at least for me, since my editor wipes out trailing white space
>> automatically). Can we either drop the space or add something for info
>> to the example?
> I choose to add something for info, because other exceptions make use of
> a space after colon in their formats.
>
>>
>> rob
>
> Version 3 attached.
>
> Pavel

Ack, just fix the doctest case for OnlyOneValueAllowed() before pushing. 
The doctest still has the old text for the exception.

rob




More information about the Freeipa-devel mailing list