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

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 <jden...@redhat.com>

Looking to carve out IT costs?

Freeipa-devel mailing list

Reply via email to