Forum OpenACS Improvement Proposals (TIPs): Re: TIP: Composable rel types
I certainly appreciate the work in this are quite a lot, but i have trouble to understand the consequences based on the two above paragraphs. I don't regard myself as an expert on the composition relationships, so i would prefer a more detailed technical description about the implications (semantically and performance-wise) of the suggested change to be able to assess the implications.
In the patch in the first test, i see that you are adding two relationships for user-1 (membership_rel and admin_rel), similarly in the later examples. Why is this? Every admin should be a member as implied by the relationship hierarchy. How does the relationship type hierarchy of member+admin rels interact with "composable_p"? What are the implications on the membership web interface in OpenAcs and DotLrn? Is it necessary to update the documentation (http://openacs.org/doc/parties.html, http://www.openacs.org/doc/current/groups-design.html, etc.)?
all the best
Regarding the idea of composition relationships in general, there is a reasonably concise explanation at http://www.openacs.org/doc/current/groups-requirements.html in the System Overview section. The relationship allows one to build a group hierarchy where membership in a component group confers membership in the composite group. A similar hierarchy could be achieved using a custom group->group relationship type (there is no default rel_type like this) with relational constraints that force membership in the parent group but the composition relationship is a simpler, more elegant solution.
All of this documentation is a lot of hand-waving, however; although the original, impenetrable groups UI in OpenACS does use composition relationships to relate subgroups to the main subsite application group (see packages/acs-subsite/www/admin/groups/new.tcl), I have a feeling these pages were never used. Nor does there appear to be any use of the relationship type in .LRN communities. I'm pretty sure, based on forum posts and conversations with others that anyone who attempted to use composition_rels to model group hierarchies ran into the issue described in the TIP and abandoned the effort. Those who didn't, would have had to come up with workarounds. We are collaborating with developers who modeled an entire university using composition_rels but ended up using categories for roles rather than relationship types. One side effect (among several) of their workaround is that they cannot utilize administrator relational segments for permissioning. What we're trying to do is model a large organization using composition relationships while retaining the power of roles and relational segments.
You raise an interesting point regarding type hierarchies - while relationships (at least the membership_rel/admin_rel hierarchy) are declared as hierarchies, both the API and UI ignore that information. See packages/acs-subsite/www/members/make-admin.tcl, where the standard member management page adds a new admin_rel relation in addition to the membership relation, likely due to the fact that the members page requires a membership_rel relation to display a user as a member. Also, see http://openacs.org/api-doc/proc-view?proc=group::add_member&source_p=1, where a membership_rel is added any time one tries to add another type of relationship (no check of the type hierarchy! what if the rel_type is not subtyped from membership_rel?). Using the admin/member case again, you can see that adding a user as an admin via the groups API results in the user having both an admin and membership relationship to the group. I'm not sure if this was done because developers didn't consult or understand the design docs or if they ran into some other issues using the type hierarchy in practice. I think .LRN uses additional relationship types but I'm not sure how it handles the management of those relationships together with membership relationships. A quick grep of dotlrn* sources though turns up calls to the same group::add_member proc which leads me to believe that the same redundant rels exist in a typical .LRN install. If that is not the case or you are aware of examples in the toolkit that do take the type hierarchy into account, let me know.
The behavior described above has always bothered me but I've never gotten around to trying to change it. In the end, groups and relational segments still work. The patch was written with the assumption that a membership_rel will be added as a prerequisite for any non-membership_rel. If we were to remove that assumption, I might revise the patch and have the triggers find the closest ancestor rel_type that was composable, if it existed. In the case of the admin/member relationships and with the defaults discussed, the effect would be the same - the admin_rel would not propagate but the trigger would find that membership_rels do and so add one. I think this behavior would line up semantically with the intent of composable rel types, though it would involve a little more overhead. Thoughts?
Having said all the above, I don't think there are major implications for .LRN based largely on the fact the type isn't used in .LRN. The functional changes to the triggers only deal with new relationships to component groups so the only impact for most standard installations will be in the performance of membership_rel_ins_tr, which is called for each new membership relation. The changes there are minimal - the only new overhead coming in the join of acs_rels against acs_rel_types to retrieve composable_p. I timed the new and old version of the membership_rel_ins_tr trigger and got similar times for both. No red flags.
As Dave mentioned, the docs aren't truly accurate. I would commit to updating documentation as necessary. I would also be willing to look at modifying the add_member proc and member management pages to use the type hierarchy, though upgrade scripts to remove redundancies at this point in time might be a bit scary for large installations.
As I'm writing this, I had another thought; set composable default of 't' for *all* rel types which would mean no functional changes to existing installations. Developer docs could be updated to detail what's involved in changing the default behavior.
Let me know if this answers your questions/concerns (or brings up new ones!).
- to use still the relationship type hierarchy for permissions (maninfested in approved_member_map), which ignores the composable flag on the relationship type, and
- avoids, that the transitive property of a component is inherited by the container (admin-rel of the component ends up as admin-rel of the container).
It is still unclear to me, what should actually happen in a deeper relationship hierarchy, if composable_p is set false for a more general relationship type (e.g. dotlrn-admin) but not for a more special one (e.g. dotrln-course-admin).
member admin dotlrn-member dotlrn-admin dotlrn-ta dotlrn-course-admin dotlrn-studentRather than setting the composable to 't' it would make more sense to set the composable flag to 'f' for every relationship type. Or if i think loudly, to set the attribute to +1/0/-1, where +1 is the old behavior, 0 is the composable == 'f' and -1 inherits down, such done does not have to provide a user with both 'admin' and 'member' along the composition hierarchy, but to cause the functions and triggers to add automatically the implied relationships to the component when this relationship is added (if admin is set, add automatically the membership, or similar when dotlrn-admin is added etc.)
Anyhow, each time when i look at the code, it took me a while understanding the existing model and its implications on memberships and permissions. I would love to see a simpler model such as RBAC for roles and permissions to simplify the semantics rather than reading through the source to try to understand, what ends up in what materialized table etc. Adding more flags makes understanding more complex and not simpler.
Regarding the first of your two points re: the patch, the composable flag isn't ignored when membership relations are added; if a membership relation (or subtype) is added to a *component* group, the flag is checked to determine whether or not that relationship should be added to container groups. In practice, however the flag will have no effect on most installations because most installations will have no component groups. The second point is correct.
Your question re: semantics when a given type is composable and an ancestor is not is a good one. I think it's unclear because such an organization of subtypes doesn't make much sense. The approach I think I would favor would be to enforce false on all further subtypes of once composable is set to false (via trigger on acs_rel_types). Would that clarify things in your view?
I'm not sure I understand your comment about membership relationships being at the top of the hierarchy thus never being used. Can you explain?
I think it's true generally that membership is what most developers and users will be interested in seeing propagated from component to container and that is usually implicit when you are creating components as subgroups (there are probably some cases where subgroups shouldn't be components but that is another subject). I can think of some benefits of composition in other types, however. For example, part of our type hierarchy looks like:
and an example group hierarchy might look like:
Some Healthcare Organization
Consider the case where you have have "First Year Residents" in Department Y. When you have an organization with multiple teaching hospitals, it might be useful to see "who are all the first year residents at Hospital X" or even "in the entire organization" or to assign this grouping some privileges at the hospital or organizational level. If the first_year_resident role propagated, this would be easy. I think it's worthwhile to afford (or rather, to not rescind) this flexibility.
I like the idea of automatically adding implied rel types. I think that is what most developers and users would expect when adding a role from a hierarchy. If you look at how the existing code functions however, an admin_rel added using the pgsql functions (or relation_add) results in a new row in both membership_rels and admin_rels but only an admin_rel in group_member_map. This could/should be corrected but I think it is a separate issue and deserves its own discussion/TIP.
Regarding your latest post, I agree about the complexity and its funny you should mention recursive queries; I also wondered whether or not we could use those techniques to simplify the group membership tables/maps while working on the composition issue. That's also a separate issue too but it's related to the type hierarchy issue above.
If we could make a determination regarding the idea of composable relationship types here, we could investigate the other issues in another thread.
i think we have similar goals: to provide for large organization scalable, differentiated and somewhat decentralized membership- and rights management. It seems to me as if the existing code base of OpenACS was developed in various layers (the often deeply nested views), competing concepts (e.g. groups and memberships rels) and APIs (and existing APIs often not used, e.g. in www directories of the admin pages of subsite and friends). Dealing with these code layers is probably the bigger issue than the original problem.
We (mostly victor) have already done some work with membership management and recursive queries that could be used as a basis. Probably, the easiest path might be
- define intended semantics based on recursive queries
- define stored procedures and a new tcl-api
- mark existing infrastruture as deprecated and replace it stepwise
- for the existing sql-structures/queries/api, one has to maintain the materialized tables and triggers for a while (similar as for the recursive queries we did for permissions)
- we have to maintain the existing sql-structures/queries/api for Oracle as well, although i would expect that porting the recursive queries to Oracle is straightforward.
As for the TIP:
- as it is, it does not hurt
- we should make clear that it is not recommended to pursue this way in the future, but to see this tip as a transitory step
- i would recommend to set the flag to false for all reltypes
having this said, i approve if it helps you in a transitional phase...