[Freeipa-devel] [PATCH 24/24] Add utility classes for handling DN's along with their, unittest.

[John Dennis]

On 06/20/2011 04:51 PM, John Dennis wrote:
> On 06/20/2011 04:06 PM, Rob Crittenden wrote:
>> Take a look at ipalib/constants.py, it is full of containers like this.
>> It is hard to review this patch without seeing how it will be used in
>> the framework, are you planning on replacing all of these with DN
>> constructors?
>
> Yup, I'm aware of these. There are two easy solutions:
>
> 1) Leave the containers as they are. They can always be used with DN
> class. This is another one of the reasons the DN class accepts DN syntax
> (for legacy and simplicity). The existing containers are all simple
> DN's, their encoded value and decoded values are identical. So as long
> as any programmer who adds a new container understands the encoding
> rules all will be good. (The problem with your example test was simply
> you didn't use the constructor correctly. See "[PATCH 28/28]" for just
> one way to construct a DN using the existing container and base strings
> as we currently have them defined.)
>
> 2) Convert the containers to DN objects. From a robustness point of view
> this is preferred. Converting them would be trivial. Once the containers
> are DN objects the programmer can't make unintentional mistakes and the
> objects combine correctly. The problem we were having is you CANNOT
> treat DN's as simple strings, they aren't simple strings, they are
> complex objects which in some instances are equivalent to simple strings.

I meant to add that if the container and base definitions in 
constants.py are converted to DN objects (a good idea I believe and 
easy) then in theory everything should still "just work" because when a 
DN object is evaluated in a string context (the only way these constants 
are used I believe) you get the identical string as to what we currently 
have in constants.py.

The pay-off comes mostly with user supplied values which get used in 
conjunction with the container+base DN's because unlike what's in 
constants.py user supplied values are not crafted by programmers aware 
of the rules of LDAP syntax. It's the DN's which result from user 
supplied values which are the primary problem areas. It really doesn't 
make much sense to use DN objects in selected known problem areas, for 
consistency and robustness we should use just one idiom throughout the 
code base, things should just work much better all around. But the 
design of the classes allow for incremental conversion of the code as we 
converge on more consistent DN handling (a win/win situation).

BTW, it was very difficult to track down how some values were getting 
"corrupted" along the way. After looking at both the client and server 
side of things and the way we designed the RPC API mechanism it became 
clear to me there was no easy fix, no band-aids, you really have to 
treat a DN string as data with known properties and behavior, e.g. an 
object that knows how to operate on it's internal data, everything else 
just has different failure modes.

> My thought was to do the conversion to DN objects incrementally. I
> deliberately wrote the classes to support incremental migration. We
> start with the bugs which we know are due to problems with DN handling
> and convert those first on an as needed basis rather than as a
> potentially large disruptive modification.
>
> The bottom line is we need to have some way to form DN's correctly from
> pieces and pick DN components apart into component pieces again. We want
> common utility code to do this and not have everybody take a crack at it
> in isolated cases when trying to fix bugs. We also want it to support
> our legacy implementation and be simple to use (at least those were the
> goals I tried to hit).
>
>> Multi-valued RDNs are 100% guaranteed in IPA so the easier it is to work
>> with them the better.
>
> I believe the classes make handling multi-valued RDN's quite easy.
>
> It's just when you start to try and explain things it seems easier to
> not fill the explanation with a bunch of caveats. If you understand
> mutli-valued RDN's and the AVA's they're composed from the classes will
> make perfect sense and combine easily.
>
>


-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/