[dev] [cvs] commit: framework/Group/Group ldap.php

[Ben Klang]

On Nov 24, 2006, at 2:26 AM, Ben Chavet wrote:

> ben         2006-11-23 23:26:07 PST
>
>   Modified files:
>     Group/Group          ldap.php
>   Log:
>   Revert some of the changes that made ldap groups act more like  
> datatree
>   groups.  I can't think of any reason the ldap group driver should  
> have to
>   handle colon-separated parent/child group names.  The ldap driver  
> uses
>   dn's to handle that relationship.
Can you explain this to me?  I'm not sure what you mean.  I didn't  
think that colon-separated names were DataTree-specific, but rather  
the appropriate way to represent nested groups.  What are you  
proposing as the alternative to specify a nested group when  
communicating with the Groups driver (LDAP or otherwise) if not colon- 
delimited group names?

>   In general, it shouldn't matter to an application what the group  
> names
>   or IDs are, or where they come from.  What should matter is that  
> the API
>   is consistent, and the backend should handle the parent/child  
> relationsip
>   in whatever manner is natural to it.  If there is code somewhere  
> that
>   explicitly generates colon-separated group names, it should be  
> fixed to
>   use the group API, not the other way around.
The colon-separated names are required by apps which manipulate or  
read from nested groups.  How is this breaking the API?  I agree with  
you that the DN can represent a parent/child group, but at the  
application level we don't want to be parsing a DN.  Using colons to  
separate names is a backend agnostic way to represent nested groups.   
In fact the UI in many places shows the group name with colons to the  
user when selecting nested groups.

Incidentally this patch breaks things for me.  Specifically where you  
modify getGroupById() (there may be others, I haven't fully tested  
yet).  You added a note that says if you have more than one group  
with the same name, results can become unpredictable.  Before you  
reverted the patch, there was no chance for ambiguity.

Was there something that was broken in the pre-reverted changes or  
are you just trying to make the group API consistent?  I am totally  
on board with making the API match across backends, but I was not  
aware it was broken for LDAP.

/BAK/
-- 
Ben Klang
Alkaloid Networks
ben at alkaloid.net
404.475.4850
http://projects.alkaloid.net