On Wed, Feb 13, 2013 at 01:22:40PM +0100, Pavel Březina wrote:
Hi,
I started working on
https://fedorahosted.org/sssd/ticket/1784 and I'd
like to get some comments so I know I'm heading the right direction.
*Current state:*
There is one tevent request per a nesting level. The request continues
with many "step" and other-named functions. The path is different for
deref and noderef processing, but the two branches intersect many
times. It uses a lot of recursion. It often translates return codes.
All of this make it hard to understand the process, hard to maintain
and especially it is hard to step out of the process at chosen point.
*New approach:*
There will be several tevent request per nesting level:
main-req (from sdap_nested_group_send)
|
nesting-level-req
/ \
noderef-req deref-req
|
possible-noderef-req
^^^^^
Shouldn't this request get back to the nesting-level-req ? Keep in mind
that on one level you can have several groups, some of them might have
enough missing members to be dereferenced some of them might not.
What is the nesting-level-req going to do? Check if the entry is already
in the hash (or the sysdb as we can search both users and groups by
originalDN in one go) and count how many members are missing?
The noderef branch is going to search bythe DN for users first and then
groups, right?
It is closed API with the following interface:
- sdap_nested_group_send()
- sdap_nested_group_recv()
When is the groups being saved? In sdap_nested_group_done() or in the
consumer of sdap_nested_group_recv() ?
The code is far from complete, I have finished only the main
interface
so far. Otherwise it is just a skeleton. Especially
sdap_nested_group_(deref|single)* functions are just a generated code
so you don't have to bother reading them.
Looks good, but please don't name the functions that save the group into
hash with _save_ that makes it sound like they are saving into the
sysdb. Also the attr variable could have been named name for better
readability.
But I think the concept is sound.
I'm looking forward to see the complete patch along with unit tests :-)