[Freeipa-devel] [PATCH][WIP] LDAP encoding redone

[Rob Crittenden]

Jan Cholasta wrote:
> Hi,
>
> this is the next patch in the input validation & handling series
> <https://fedorahosted.org/freeipa/ticket/2357>. It changes the way
> entries are encoded and decoded in the LDAP backend.
>
> The patch consists of several changes:
>
>    * Refactored the Encoder class to be more universally usable. It uses
> a polymorphic interface, which hopefully makes the encoding code more
> readable.
>
>    * Attribute values now use Python data types matching the syntax of
> the attribute. This removes the need to decode/encode the values from/to
> raw LDAP values in the CallbackInterface callbacks as well as other
> parts of IPA.
>
>    * On command output, attribute values are converted to strings so
> that the resulting entry is the same as it is without the patch. I don't
> like this code and I'd like to get rid of at least some parts of it, but
> I'm not sure how that would affect API compatibility. Removing the
> special case for boolean values would fix
> <https://fedorahosted.org/freeipa/ticket/2025>.
>
>    * Entries are more strictly checked when they are encoded and
> decoded. Values of multi-value attributes must be lists (not tuples!) of
> objects of the appropriate python type, values of single-value
> attributes must be objects of the appropriate python type. This helps
> detecting data type errors that would previously go unnoticed.
>
>    * Some parameters use data type that doesn't match the syntax of the
> according attribute, or are single-value even when the according
> attribute is multi-value. Values of such parameters wouldn't pass the
> new strict checking if they were used in attributes without
> modifications. To remedy this, added a new parameter option
> attr_convertor, which allows specifying a custom function for converting
> parameter values to attribute values.
>
> Note that this is work in progress, some things may be (and certainly
> are) broken, there is some low-quality code and docstrings, comments and
> tests are TBD.
>
> Suggestions and comments are welcome.
>
> Honza

I haven't tried this yet, but this change jumped out at me:

if attr not in ('aciname', 'group', 'memberof', 'nsaccountlock', 
'subtree', 'targetgroup', 'type') and self.obj is not None and attr in 
self.obj.params and 'virtual_attribute' not in self.obj.params[attr].flags:

Why exclude this subset of attributes?

Is the big block of code adding to __call__ meant to maintain backwards 
compatibility?

This seems to make lists out of a lot of things that weren't previously 
lists. Is that to satisfy the schema?

rob