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

[Endi Sukma Dewata]

On 10/10/2011 10:13 AM, Petr Vobornik wrote:
> https://fedorahosted.org/freeipa/ticket/1531
>
> (3.0 Core Effort Iteration 01 September Y11 Release)
>
> Implemented solution:
> * all entities are created on application start
> * dependant objects (facets and dialogs) are created at once on their
> first use in entity.
>
> Note(patch naming): patch 022 was second part of 021, but the file name
> was wrong(021-1)

Some comments/issues:

1. One of the goals of this bug is to remove the temporary workaround in 
IPA.search_facet.create_content(). We should now be able to call the 
initialize_table_columns() during facet initialization.

2. Using lazy-loading to create entities, facets, and dialogs makes 
object creations a little bit unpredictable. This is probably fine for 
now, but if there's a problem the other option is to create all objects 
during application initialization. We can use a loop to create all 
entities first, then use another loop to create all dependent objects in 
each entity.

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.

4. In the original code, when creating a facet for indirect association 
it will try to find the corresponding direct facet and use it instead of 
creating a new one. In the new code, the indirect facet will always be 
created, but since there is no indirect facet group the facet will never 
appear. It would be better if we can avoid unnecessary creation of 
indirect facets.

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.

6. Invoking a method by concatenating the method name dynamically such 
as prepare_<facet type>_spec will work, but it's more error prone and 
will clutter up the namespace. It would be better to store the methods 
in a map like this:

   that.map.put('search', function(spec) {
     ...
   });

and use it like this:

   var method = that.map.get('search');
   method(spec);

This can be done separately.

7. The code in entity.js:474,998,1000 should have a deeper indentation 
because it's a continuation of the previous line.

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.

-- 
Endi S. Dewata