Re: [Freeipa-devel] [PATCH 31/31] Ticket 1485 - DN pairwise grouping

2011-07-21 Thread Martin Kosek
On Wed, 2011-07-20 at 19:59 -0400, John Dennis wrote:
 The pairwise grouping used to form RDN's and AVA's proved to be
 confusing in practice, this patch removes that functionality thus
 requiring programmers to explicitly pair attr,value using a tuple or
 list.
 
 In addition it was discovered additional functionality was needed to
 support some DN operations in freeipa. DN objects now support
 startswith(), endswith() and the in membership test. These functions
 and operators will accept either a DN or RDN.
 
 The unittest was modified to remove the pairwise tests and add new
 explicit tests. The unittest was augmented to test the new
 functionality. In addition the unittest was cleaned up a bit to use
 common utilty functions for improved readabilty and robustness.
 
 The documentation was updated.
 

The patch looks good to me.

The removed form of creating DN's was indeed confusing. I went through
current uses of DN class, I didn't find any using the removed form. DN
tests also passes correctly.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 31/31] Ticket 1485 - DN pairwise grouping

2011-07-21 Thread John Dennis

On 07/21/2011 08:27 AM, Martin Kosek wrote:

On Wed, 2011-07-20 at 19:59 -0400, John Dennis wrote:

The pairwise grouping used to form RDN's and AVA's proved to be
confusing in practice, this patch removes that functionality thus
requiring programmers to explicitly pair attr,value using a tuple or
list.

In addition it was discovered additional functionality was needed to
support some DN operations in freeipa. DN objects now support
startswith(), endswith() and the in membership test. These functions
and operators will accept either a DN or RDN.

The unittest was modified to remove the pairwise tests and add new
explicit tests. The unittest was augmented to test the new
functionality. In addition the unittest was cleaned up a bit to use
common utilty functions for improved readabilty and robustness.

The documentation was updated.



The patch looks good to me.

The removed form of creating DN's was indeed confusing. I went through
current uses of DN class, I didn't find any using the removed form. DN
tests also passes correctly.

Martin



Actually one of the tests was failing due to the removed form, not sure 
how I missed it (or how you missed it either). But in any case it's a 
one line fix test_role_plugin.py, I've rebased the patch to include that 
and attached it. Whoever commits please use this version with the 
test_role_plugin.


Also, the patch comments failed to mention even though we had a unittest 
for dn.py, test/test_ipalib/test_dn.py it was not getting called by 
run-tests because it had the execute permission bit set, the patch fixes 
that so the unittest gets run by make-test.


--
John Dennis jden...@redhat.com

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
From aca9f730ece272c7f62eedc0931552241e4ff3bf Mon Sep 17 00:00:00 2001
From: John Dennis jden...@redhat.com
Date: Wed, 20 Jul 2011 19:39:05 -0400
Subject: [PATCH 31/31] Ticket 1485 - DN pairwise grouping
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

The pairwise grouping used to form RDN's and AVA's proved to be
confusing in practice, this patch removes that functionality thus
requiring programmers to explicitly pair attr,value using a tuple or
list.

In addition it was discovered additional functionality was needed to
support some DN operations in freeipa. DN objects now support
startswith(), endswith() and the in membership test. These functions
and operators will accept either a DN or RDN.

The unittest was modified to remove the pairwise tests and add new
explicit tests. The unittest was augmented to test the new
functionality. In addition the unittest was cleaned up a bit to use
common utilty functions for improved readabilty and robustness.

The documentation was updated.

fix test_role_plugin use of DN to avoid pairwise grouping
---
 ipalib/dn.py  |  434 +
 tests/test_ipalib/test_dn.py  |  184 +-
 tests/test_xmlrpc/test_role_plugin.py |2 +-
 3 files changed, 398 insertions(+), 222 deletions(-)
 mode change 100755 = 100644 tests/test_ipalib/test_dn.py

diff --git a/ipalib/dn.py b/ipalib/dn.py
index 89248ca..47c6619 100644
--- a/ipalib/dn.py
+++ b/ipalib/dn.py
@@ -20,6 +20,7 @@
 from ldap.dn import str2dn, dn2str
 from ldap import DECODING_ERROR
 from copy import deepcopy
+import sys
 
 __all__ = ['AVA', 'RDN', 'DN']
 
@@ -104,10 +105,10 @@ Or compare a value returned by an LDAP query to a known value:
 
 if value == 'Bob'
 
-All of these simple coding assumptions are WRONG and will FAIL when
-a DN is not one of the simple DN's which are probably the 95% of all
-DN's. This is what makes DN handling pernicious. What works in 95% of
-the cases and is simple, fails for the 5% of DN's which are not
+All of these simple coding assumptions are WRONG and will FAIL when a
+DN is not one of the simple DN's (simple DN's are probably the 95% of
+all DN's). This is what makes DN handling pernicious. What works in
+95% of the cases and is simple, fails for the 5% of DN's which are not
 simple.
 
 Examples of where the simple assumptions fail are:
@@ -127,13 +128,13 @@ Examples of where the simple assumptions fail are:
 To complicate matters a bit more the RFC for the string representation
 of DN's (RFC 4514) permits a variety of different syntax's each of
 which can evaluate to exactly the same DN but have different string
-representations. For example, the attr R,W which contains a reserved
+representations. For example, the attr r,w which contains a reserved
 character (the comma) can be encoded as a string in these different
 ways:
 
-'R\,W'  # backslash escape
-'R\2cW' # hexadecimal ascii escape
-'#522C57'   # binary encoded
+'r\,w'  # backslash escape
+'r\2cw' # hexadecimal ascii escape
+'#722C77'   # binary encoded
 
 It should be clear a DN string may NOT be a simple string, rather a DN
 string is ENCODED. For simple strings the encoding of the DN is
@@ -143,10 +144,10 @@