#47792 database plugins need a way to call betxn plugins
Closed: Fixed None Opened 5 years ago by mreynolds.

Database plugins, like chaining, handle ldap operation on their own. So they never call any pre/post op plugins(or backend txn pre/post op plugins for that matter). This can break existing deployments that were using chaining before 389 switched over to using backend txn plugins across the board.

In the case of chaining, the RI plugin is a possible chaining component, but since it is now a backend txn plugin it will never get called for delete operations on a chaining backend.

Also, the RI plugin, just like the attribute uniqueness plugin, will have to know to look at all possible sub-suffixes/backends to properly handle the group cleanup (See ticket 47777).


ok, but note that, in general, large whitespace only commits will make cherry-picking/backporting harder

Code cleanup

git merge ticket47792
Updating 2b98dca..d341b77
Fast-forward
ldap/servers/plugins/chainingdb/cb_add.c | 128 +++++++++++++--------------
ldap/servers/plugins/chainingdb/cb_delete.c | 133 ++++++++++++++--------------
ldap/servers/plugins/chainingdb/cb_modify.c | 172 +++++++++++++++++-------------------
ldap/servers/plugins/chainingdb/cb_modrdn.c | 158 +++++++++++++++++----------------
ldap/servers/plugins/chainingdb/cb_search.c | 385 +++++++++++++++++++++++++++++++++++++++------------------------------------------

2b98dca..d341b77 master -> master
commit d341b77

9cf2aa3..65c5e58 389-ds-base-1.3.2 -> 389-ds-base-1.3.2
commit 65c5e58

f94bfa1..ea1b127 389-ds-base-1.3.1 -> 389-ds-base-1.3.1
commit ea1b127112efd3e962d363f12fcbae6f3b18c06a

The function names are misleading e.g.
slapi_plugin_call_preop_betxn_plugins calls both the be and betxn plugins. Maybe just slapi_plugin_call_preop_be_plugins? Not sure about a good name.

{{{
165 if ( NULL != ctrls)
166 ldap_controls_free(ctrls);
}}}
It is ok to call ldap_controls_free with NULL.

I do not like the way the calls to be_txn plugins are made public. In the cb case you probably call the plugins at the right place, but if a function to call thes plugins is public it could be used to call be txn plugins at any time.

Replying to [comment:7 lkrispen]:

I do not like the way the calls to be_txn plugins are made public. In the cb case you probably call the plugins at the right place, but if a function to call these plugins is public it could be used to call be txn plugins at any time.

I see your point, but database plugins need to be able to call these be/betxn plugins(starting in 1.3.1 most plugins are now betxn plugins). Someone writing their own plugin is already doing so at their own risk. We can't stop them from crashing the server or introducing memory leaks, and I think the same applies in this case. I think what we need here is proper documentation on how to use these functions.

Replying to [comment:6 rmeggins]:

The function names are misleading e.g.
slapi_plugin_call_preop_betxn_plugins calls both the be and betxn plugins. Maybe just slapi_plugin_call_preop_be_plugins? Not sure about a good name.

Sure, I'll work on a better name.

{{{
165 if ( NULL != ctrls)
166 ldap_controls_free(ctrls);
}}}
It is ok to call ldap_controls_free with NULL.

I just checked mozldap and openldap, and yes it is safe to call this function if the controls are NULL. Maybe in the past it wasn't, as most places in the server code do this NULL check.

Moved the functions into slapi-private.h, later we can easily move them to the public api if there is a need to do so.

New patch attached.

git merge ticket47792
Updating 087356f..9929b43
Fast-forward
ldap/servers/plugins/chainingdb/cb_add.c | 76 ++++++++++++++++++++++++++++++++++++++++++----------------------------------
ldap/servers/plugins/chainingdb/cb_close.c | 4 ++--
ldap/servers/plugins/chainingdb/cb_delete.c | 58 ++++++++++++++++++++++++++++++----------------------------
ldap/servers/plugins/chainingdb/cb_modify.c | 60 +++++++++++++++++++++++++++++++-----------------------------
ldap/servers/plugins/chainingdb/cb_modrdn.c | 61 ++++++++++++++++++++++++++++++++-----------------------------
ldap/servers/plugins/chainingdb/cb_search.c | 44 ++++++++++++++++----------------------------
ldap/servers/slapd/plugin.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ldap/servers/slapd/slapi-plugin.h | 6 ++++++
ldap/servers/slapd/slapi-private.h | 34 ++++++++++++++++++++++++++++++++++
9 files changed, 286 insertions(+), 150 deletions(-)

git push origin master
087356f..9929b43 master -> master

commit 9929b43
Author: Mark Reynolds mreynolds@redhat.com
Date: Wed May 7 12:29:03 2014 -0400

1.3.2

2132875..8eeb738 389-ds-base-1.3.2 -> 389-ds-base-1.3.2
commit 8eeb738

1.3.1

40e86e7..ba53b57 389-ds-base-1.3.1 -> 389-ds-base-1.3.1
commit ba53b5761f37074454723e83c70e5ec0ebd6ab6f

Metadata Update from @rmeggins:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.3.1.23

2 years ago

Login to comment on this ticket.

Metadata