Description of problem: When referential integrity plugin removes attributes from CoS template (for example: uniqueMember), objects that are affected by the CoS still have that 'deleted' attribute. If you explicitly remove desired attribute from template, then CoS-affected entries behave as expected. Version-Release number of selected component (if applicable): 1.0.2 How reproducible: All operations were performed using Management Console Steps to Reproduce: 1. Import attached LDIF (base DN is dc=example,dc=com). 2. Enable referential integrity plugin 3. Restart slapd. 4. Ensure that dn: cn=testgroup,ou=posixGroups,dc=example,dc=com has attributes uniqueMember: uid=haizaar,ou=people,dc=example,dc=com uniqueMember: uid=gilran,ou=people,dc=example,dc=com (Propagates by CoS) 5. Delete object dn: uid=gilran, ou=People,dc=example,dc=com 5. Ensure that dn: cn=uids,ou=posixGroups,dc=example,dc=com has only one 'uniqueMember' attribute left - referential integrity plugin should remove the other one. uniqueMember: uid=haizaar,ou=people,dc=example,dc=com Actual results: dn: cn=testgroup,ou=posixGroups,dc=example,dc=com still has _both_ 'uniqueMember' attributes, although one attribute has been removed from template! Expected results: Since attribute was deleted from template, objects affected by corresponding CoS should've seen this change. Additional info:
Created attachment 134332 [details] LDIF for bug reproduction
Created attachment 142873 [details] bug analysis and discussion
Comment from Pete: You can use the start dependency code as a model for call ordering - the plugins just need to be inserted in the list in the right order initially - they will never have to be updated so no other code changes.
Created attachment 142912 [details] cvs diffs Files: plugins/cos/cos_cache.c plugins/referint/referint.c slapd/delete.c slapd/modify.c slapd/modrdn.c slapd/pblock.c slapd/plugin.c slapd/slap.h slapd/slapi-plugin.h
Created attachment 142913 [details] Fix proposal
Created attachment 142914 [details] Test result: CoS is called after Referential Integrity Plugin Attachment referred by the previous comment.
Sorry, it's confusing, isn't it? Let me clarify some more. Comment #4 is the diffs for the reviews. Comment #5 is the description for the diffs. Comment #6 shows just ordering the plugins was not enough for updating the CoS cache. So, Comment #6 is a sort of codicil.
Comment on attachment 142912 [details] cvs diffs found a bug in plugin_create_pluginlist: +int +plugin_create_pluginlist( Slapi_Entry *plugin_entry, char *attr_name, + struct slapdplugin *plugin, struct slapdplugin **plugin_list) +{ + Slapi_Attr *attr = 0; + int hint =0; + int num_vals = 0; + int val_index = 0; + Slapi_Value *val; + + if( 0 == slapi_entry_attr_find( plugin_entry, attr_name, &attr )) + { + /* allocate memory for the string array */ + slapi_attr_get_numvalues( attr, &num_vals ); + + if(num_vals) + { + hint = slapi_attr_first_value( attr, &val ); + while(val_index < num_vals) + { + /* add the plugin to the list at the place befores */ + + add_plugin_to_list_after( plugin_list, plugin, slapi_value_get_string(val) ); + hint = slapi_attr_next_value( attr, hint, &val ); + val_index++; + } + } This does not work when multiple nsslapd-plugin-depends-on-call is set in one plugin entry. I'll fix it and send out a review request again.
Created attachment 143004 [details] cvs diffs (revised) Fixed a bug reported in the previous comment. That is, added a code to delete the plugin if the plugin is already in the list before the plugin with "name". +static void +add_plugin_to_list_after(struct slapdplugin **list, + struct slapdplugin *plugin, const char *name) +{ struct slapdplugin **tmp; - for ( tmp = list; *tmp; tmp = &(*tmp)->plg_next ) + PR_ASSERT(name && *name); + for ( tmp = list; tmp && *tmp; tmp = &(*tmp)->plg_next ) { + if (!strcasecmp(plugin->plg_name, (*tmp)->plg_name)) { + /* plugin is already in the list before the plugin with "name". + delete it first */ + *tmp = (*tmp)->plg_next; + if (NULL == *tmp) + break; + } + else if (!strcasecmp(name, (*tmp)->plg_name))
Based upon the following advice from Rich, fixed in the different way. Richard Megginson wrote: > Could we solve this problem by having CoS (and probably Roles too) register both a SLAPI_PLUGIN_INTERNAL_POST_op_FN and a SLAPI_PLUGIN_POST_op_FN? I think part of the problem is that CoS is not notified of changes during internal operations - SLAPI_PLUGIN_INTERNAL_POST_op_FN might take care of that. > > If that doesn't work, maybe we could use some sort of generic entry state change notification method. I had forgotten about the statechange plugin which could be used for this purpose. It would work something like this: > CoS (and Roles too, I think) registers an entry state change callback using the slapi_apib interface. With CoS, this can just simply call cos_post_op. Then, we change the internal_modify, internal_modrdn, internal_add, and internal_delete functions to call this entry post change function using the apib interface. [...] > I see. Then can we register CoS as a "object" plugintype? Several other plug-ins take this approach, which need to register callbacks for preop, postop, and other types. For example, see the repl5_init.c - the main replication_multimaster_plugin_init() calls slapi_register_plugin() with the different types of plugins it wants to register as.
Created attachment 143177 [details] cvs diff (create_inctance.c cos.c roles_plugin.c) Files: ldap/admin/src/create_instance.c ldap/servers/plugins/cos/cos.c ldap/servers/plugins/roles/roles_plugin.c Changes: 1) registered cos_post_op and roles_post_op as SLAPI_PLUGIN_INTERNAL_POST_op_FN functions. 2) changed the plugin type of CoS and Roles from "postoperation" to "object".
Richard Megginson wrote: > Looks good. Ship it! Thank you, Rich!!
Created attachment 143178 [details] cvs commit message Checked in into HEAD.
Created attachment 143368 [details] problem description + review request
Created attachment 143369 [details] cvs commit message Reviewed by Rich (Thank you!) Checked in into HEAD.
Created attachment 143568 [details] Yet another side effect... Comments from Rich: I worry that there is some cleanup that we won't do if we skip the plugin_closeall, but then we don't have any plugins (other than the database) that run in db2ldif mode. I guess let's see if the acceptance tests like the change. I'm going to check in ldif2ldbm.c (with the dblayer_close call removed) and check the acceptance results tomorrow.
Created attachment 143569 [details] cvs commit ldif2ldbm.c Checked in into HEAD. I'm running the acceptance tests on 32-bit/64-bit RHEL4 from now...
verified aginst: 1193765112 idm-console-framework-1.1.0-5.el5idm Tue Oct 30 2007 1193765112 redhat-idm-console-1.0.0-13.el5idm Tue Oct 30 2007 1194380792 tftp-0.42-3.1 Tue Nov 06 2007 1195006662 subversion-1.4.2-2.el5 Tue Nov 13 2007 1195174828 redhat-ds-base-8.0.0-11.el5dsrv Thu Nov 15 2007