Bug 675265 - preventryusn gets added to entries on a failed delete

https://bugzilla.redhat.com/show_bug.cgi?id=675265

Description: When an entry is deleted with Entry USN plugin enabled,
an operational attribute preventryusn is added to handle indexes and
entryusn tombstone.  The attribute must have been added only when
the delete was successful, but it was added regardless of the result
from the operation.  This patch checks the delete result in the
newly added entryusn delete bepost plugin (usn_bepostop_delete).
If it is not successful, the bepost plugin cleans up the attribute.

ldap/servers/plugins/usn/usn.c

@@ -59,6 +59,7 @@ static int usn_bepreop_add(Slapi_PBlock *pb);
 static int usn_bepreop_delete(Slapi_PBlock *pb);
 static int usn_bepreop_modify(Slapi_PBlock *pb);
 static int usn_bepostop(Slapi_PBlock *pb);
+static int usn_bepostop_delete (Slapi_PBlock *pb);
 static int usn_start(Slapi_PBlock *pb);
 static int usn_close(Slapi_PBlock *pb);
 static int usn_get_attr(Slapi_PBlock *pb, const char* type, void *value);
@@ -177,7 +178,7 @@ usn_bepostop_init(Slapi_PBlock *pb)
     if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_ADD_FN,
                                  (void *)usn_bepostop) != 0 ||
         slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_DELETE_FN,
-                                 (void *)usn_bepostop) != 0 ||
+                                 (void *)usn_bepostop_delete) != 0 ||
         slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_MODIFY_FN,
                                  (void *)usn_bepostop) != 0 ||
         slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_MODRDN_FN,
@@ -516,6 +517,47 @@ bail:
     return rc;
 }
 
+/* count up the counter */
+/* if the op is delete and the op was not successful, remove preventryusn */
+static int
+usn_bepostop_delete (Slapi_PBlock *pb)
+{
+    int rc = -1;
+    Slapi_Backend *be = NULL;
+
+    slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM,
+                    "--> usn_bepostop\n");
+
+    /* if op is not successful, don't increment the counter */
+    slapi_pblock_get(pb, SLAPI_RESULT_CODE, &rc);
+    if (LDAP_SUCCESS != rc) {
+        Slapi_Entry *e = NULL;
+
+        slapi_pblock_get(pb, SLAPI_DELETE_BEPOSTOP_ENTRY, &e);
+        if (NULL == e) {
+            rc = LDAP_NO_SUCH_OBJECT;    
+            goto bail;
+        }
+        /* okay to return the rc from slapi_entry_delete_values */
+        rc = slapi_entry_delete_values(e, SLAPI_ATTR_ENTRYUSN_PREV, NULL);
+        goto bail;
+    }
+
+    slapi_pblock_get(pb, SLAPI_BACKEND, &be);
+    if (NULL == be) {
+        rc = LDAP_PARAM_ERROR;    
+        goto bail;
+    }
+
+    if (be->be_usn_counter) {
+        slapi_counter_increment(be->be_usn_counter);
+    }
+bail:
+    slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM,
+                    "<-- usn_bepostop\n");
+    return rc;
+}
+
 /* mimic replication to turn on create_tombstone_entry */
 static int
 usn_get_attr(Slapi_PBlock *pb, const char* type, void *value)

ldap/servers/slapd/back-ldbm/ldbm_delete.c

@@ -116,6 +116,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
 
 	if (NULL == addr)
 	{
+		/* retval is -1 */
 		goto error_return;
 	}
 	ldap_result_code = slapi_dn_syntax_check(pb, addr->dn, 1);
@@ -123,6 +124,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
 	{
 		ldap_result_code = LDAP_INVALID_DN_SYNTAX;
 		slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message);
+		/* retval is -1 */
 		goto error_return;
 	}
 
@@ -158,19 +160,22 @@ ldbm_back_delete( Slapi_PBlock *pb )
 		operation->o_status = SLAPI_OP_STATUS_WILL_COMPLETE;
 	}
 	if ( slapi_op_abandoned( pb ) ) {
+		/* retval is -1 */
 		goto error_return;
 	}
 
 	/* find and lock the entry we are about to modify */
 	if ( (e = find_entry2modify( pb, be, addr, NULL )) == NULL )
 	{
-    	ldap_result_code= -1; 
+		ldap_result_code= LDAP_NO_SUCH_OBJECT; 
+		/* retval is -1 */
 		goto error_return; /* error result sent by find_entry2modify() */
 	}
 
 	if ( slapi_entry_has_children( e->ep_entry ) )
 	{
 		ldap_result_code= LDAP_NOT_ALLOWED_ON_NONLEAF;
+		/* retval is -1 */
 		goto error_return;
 	}
 
@@ -194,6 +199,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
 		{
 			/* restore original entry so the front-end delete code can free it */
 			slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry );
+			/* retval is -1 */
 			goto error_return;
 		}
 		slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code);
@@ -211,6 +217,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
 			slapi_pblock_get(pb, SLAPI_RESULT_CODE, &ldap_result_code);
 			/* restore original entry so the front-end delete code can free it */
 			slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry );
+			/* retval is -1 */
 			goto error_return;
 		}
 		/* the flag could be set in a preop plugin (e.g., USN) */
@@ -294,6 +301,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
 	if ( ldap_result_code != LDAP_SUCCESS )
 	{
 		ldap_result_message= errbuf;
+		/* retval is -1 */
 		goto error_return;
 	}
 
@@ -766,8 +774,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
 			}
 			if (entryrdn_get_switch()) /* subtree-rename: on */
 			{
-				retval =
-						entryrdn_index_entry(be, e, BE_INDEX_DEL, &txn);
+				retval = entryrdn_index_entry(be, e, BE_INDEX_DEL, &txn);
 				if (DB_LOCK_DEADLOCK == retval) {
 					LDAPDebug0Args( LDAP_DEBUG_ARGS,
 							"delete (deleting entryrdn) DB_LOCK_DEADLOCK\n");
@@ -852,6 +859,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
 		/* Failed */
 		LDAPDebug( LDAP_DEBUG_ANY, "Retry count exceeded in delete\n", 0, 0, 0 );
 		ldap_result_code= LDAP_OPERATIONS_ERROR;
+		retval = -1;
 		goto error_return;
 	}
 
@@ -885,6 +893,7 @@ ldbm_back_delete( Slapi_PBlock *pb )
 			ldap_result_code= LDAP_OPERATIONS_ERROR;
 			LDAPDebug( LDAP_DEBUG_ANY,
 				"ldbm_back_delete: modify_switch_entries failed\n", 0, 0, 0);
+			retval = -1;
 			goto error_return;
 		}
 	}
@@ -893,10 +902,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
 	goto common_return;
 
 error_return:
-	if (e!=NULL) {
-	    cache_unlock_entry( &inst->inst_cache, e );
-	    CACHE_RETURN( &inst->inst_cache, &e );
-	}
 	if (tombstone_in_cache)
 	{
 		CACHE_REMOVE( &inst->inst_cache, tombstone );
@@ -941,9 +946,27 @@ common_return:
 	 * but not if the operation is purging tombstones.
 	 */
 	if (!delete_tombstone_entry) {
+		if (e) {
+			/* set entry in case be-postop plugins need to work on it
+			 * (e.g., USN) */
+			slapi_pblock_get( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, &orig_entry );
+			slapi_pblock_set( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, e->ep_entry );
+		}
 		plugin_call_plugins (pb, SLAPI_PLUGIN_BE_POST_DELETE_FN);
+		/* set original entry back */
+		if (e) {
+			slapi_pblock_set( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, orig_entry );
+		}
 	}
 
+	/* Need to return to cache after post op plugins are called */
+	if (retval) { /* error case */
+		if (e) {
+			cache_unlock_entry( &inst->inst_cache, e );
+			CACHE_RETURN( &inst->inst_cache, &e );
+		}
+	}
+	
 	if (ruv_c_init) {
 		modify_term(&ruv_c, be);
 	}

ldap/servers/slapd/slapi-plugin.h

@@ -5931,6 +5931,7 @@ typedef struct slapi_plugindesc {
 #define SLAPI_DELETE_EXISTING_ENTRY		SLAPI_ADD_EXISTING_DN_ENTRY
 #define SLAPI_DELETE_GLUE_PARENT_ENTRY	SLAPI_ADD_PARENT_ENTRY
 #define SLAPI_DELETE_BEPREOP_ENTRY			SLAPI_ENTRY_PRE_OP
+#define SLAPI_DELETE_BEPOSTOP_ENTRY			SLAPI_ENTRY_POST_OP
 
 /* modify arguments */
 #define SLAPI_MODIFY_TARGET			SLAPI_TARGET_DN