[Freeipa-devel] [PATCH] 023 Circular entity dependency

[Endi Sukma Dewata]

On 10/18/2011 10:52 AM, Petr Vobornik wrote:
>  > 3. Another goal is to replace entity names used in spec (see
>  > other_entity & nested_entity spec properties) with the actual entity
>  > objects. In this case it might be better to use the loops described in
>  > #2. This can be done separately.
>
> Wouldn't it lead to the circular dependancy problem again? I think using
> entity names and calling IPA.get_entity at the time when it is needed is
> fine. But we should make some naming conversions of function params or
> object properties to distinguish when we are working with just name or
> entity itself.

It shouldn't. The circular dependency happens when we create an entity 
that has a facet that depends on another entity, and that entity also 
has a facet that depends on the first entity, but the first entity is 
still being created. If we create all entities first separately, by the 
time we create the facets all entities are already created.

Agreed on the parameter naming.

>  > 5. In entity.js:201, the use of entity.title for the breadcrumb tooltip
>  > might not be appropriate because usually the title is plural whereas the
>  > breadcrumb points to a single object. It would be better to use the
>  > entity.metadata.label_singular.
>
> Fixed

The first and last elements in the breadcrumb actually don't have the 
tooltip set so it will show a default tooltip which might not be 
correct. This is an existing issue so we can fix this separately.

>  > 8. The facet_specs and dialog_specs lists can be replaced with
>  > ordered_map. It already has a method to find an element by its name.
>  > This can be done separately.
>
> My intention was to be able specify entity facets and dialogs simply by
> specifying their spec in entity spec (without calling builder function),
> this is possible now. I didn't want to add initialization logic for
> facet_specs (to fill the ordered_map) to entity in order to keep it as
> simple as possible. It would be nice though. Maybe we could make an
> utility object with methods for some simple initialization logic which
> is used on many places - like checking if boolean is set or some more
> complicated like initialization of ordered_map (needs key selector fn as
> parameter).

I didn't mean to replace the spec with a class. I was referring to using 
the ordered_map to replace the plain array (i.e. facet_specs and 
dialog_specs) that contains the specs because the ordered_map already 
has a method to lookup the elements by their name. So instead of:

   entity.facet_specs.push(spec);
   var facet = get_spec_by_name(entity.facet_specs, association_name);

it will be

   entity.facet_specs.put(spec.name, spec);
   var facet = entity.facet_specs.get(association_name);

This is not crucial, we can always improve it later.

ACK and pushed to master.

-- 
Endi S. Dewata