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

[Adam Young]

On 10/18/2011 02:25 PM, Endi Sukma Dewata wrote:
> 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.
>
Nice work.  I am not 100% satisfied with the solution, but that is due 
to limitations in the Javascript language.  I think this is the best we 
can get with the tools we have today.

A more perfect solution would allow us to use a lazy load proxy, and to 
hide from an end component that the object is a proxy.  the getters code 
we had a while back sort of gave us that, but as we found, browser specific.

Ideally, when we set a Facet on an entity, that would be a lazy load 
proxy, as would be the case where we explicitly link one entity up with 
another.