Bug 515329 - Correct attribute value inconsistency on replica

When performing operations with multiple mods to the same multi-valued
attribute on a single modify operation, a replica was not resolving
the attribute values correctly.  This would lead to an inconsistency
between the master the change was initially performed against and the
replicas.  The problem would occur with a modify operation such as
this:

  dn: uid=testuser,dc=example,dc=com
  changetype: modify
  add: cn
  cn: 2
  -
  replace: cn
  cn: 3

The problem is that we use the CSNs from the attribute state data
to determine which values should remain after the operation (this is
done to merge with later occuring changes from other masters).  The
CSN for all mods within the same modify operation is exactly the same.
The old code was looking for attributes older than the deletion that
occurs as a part of the replace, then deleting those values.  This
would cause the value of "2" in the above example to remain.  Simply
changing this comparision to look for values with the same or older
CSN to delete would cause the new value of "3" to be removed as well
when we get around to resolving the attribute after the second half
of the replace operation.

The fix is to use a different CSN comparison when we are removing all
values of an attribute during attribute resolution (remove values with
the same or older CSN).  This is safe becuse the only present values
at this time are older values or values added in a previous mod in the
same modify operation.  When processing other mods that are not
removing all values of an attribute, we only want to remove values
with a CSN older that that of the current modify operation.  This
prevents us from removing a newly added value, such as "3" in the
example above.  This is safe since we resolve the attribute after
each mod in the modify operation.

ldap/servers/slapd/entrywsi.c

@@ -46,7 +46,7 @@
 #include "slap.h"
 #include "slapi-plugin.h"
 
-static void resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state);
+static void resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state, int delete_priority);
 
 static int
 entry_present_value_to_deleted_value(Slapi_Attr *a, Slapi_Value *v)
@@ -477,7 +477,7 @@ entry_add_present_values_wsi(Slapi_Entry *e, const char *type, struct berval **b
 			 * Now delete non-RDN values from a->a_present_values; and
 			 * restore possible RDN values from a->a_deleted_values
 			 */
-			resolve_attribute_state(e, a, attr_state);
+			resolve_attribute_state(e, a, attr_state, 0);
 			retVal= LDAP_SUCCESS;
 		}
 		else
@@ -547,7 +547,7 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval
 			attr_set_deletion_csn(a,csn);
 			if(urp)
 			{
-				resolve_attribute_state(e, a, attr_state); /* ABSOLVED */
+				resolve_attribute_state(e, a, attr_state, 1 /* set delete priority */); /* ABSOLVED */
 			}
 			else
 			{
@@ -589,7 +589,7 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval
 				 * should free valuestodelete only (don't call valuearray_free)
 				 * [622023] */
 				slapi_ch_free((void **)&valuestodelete);
-				resolve_attribute_state(e, a, attr_state);
+				resolve_attribute_state(e, a, attr_state, 0);
 				retVal= LDAP_SUCCESS;
 			}
 			else
@@ -955,7 +955,7 @@ value_distinguished_at_csn(const Slapi_Entry *e, const Slapi_Attr *original_attr
 }
 
 static void
-resolve_attribute_state_multi_valued(Slapi_Entry *e, Slapi_Attr *a, int attribute_state)
+resolve_attribute_state_multi_valued(Slapi_Entry *e, Slapi_Attr *a, int attribute_state, int delete_priority)
 {
 	int i;
 	const CSN *adcsn= attr_get_deletion_csn(a);
@@ -976,7 +976,17 @@ resolve_attribute_state_multi_valued(Slapi_Entry *e, Slapi_Attr *a, int attribut
 		vdcsn= value_get_csn(v, CSN_TYPE_VALUE_DELETED);
 		vucsn= value_get_csn(v, CSN_TYPE_VALUE_UPDATED);
 		deletedcsn= csn_max(vdcsn, adcsn);
-		if(csn_compare(vucsn,deletedcsn)<0) /* check if the attribute or value was deleted after the value was last updated */
+
+		/* Check if the attribute or value was deleted after the value was
+		 * last updated.  If the value update CSN and the deleted CSN are
+		 * the same (meaning they are separate mods from the same exact
+		 * operation), we should only delete the value if delete priority
+		 * is set.  Delete priority should only be set when we are deleting
+		 * all value of an attribute.  This prevents us from leaving a value
+		 * that was added as a previous mod in the same exact modify
+		 * operation as the subsequent delete.*/
+		if((csn_compare(vucsn,deletedcsn)<0) ||
+			(delete_priority && (csn_compare(vucsn,deletedcsn) == 0)))
 		{
 	        if(!value_distinguished_at_csn(e, a, v, deletedcsn))
 			{
@@ -1000,7 +1010,9 @@ resolve_attribute_state_multi_valued(Slapi_Entry *e, Slapi_Attr *a, int attribut
 		vdcsn= value_get_csn(v, CSN_TYPE_VALUE_DELETED);
 		vucsn= value_get_csn(v, CSN_TYPE_VALUE_UPDATED);
 		deletedcsn= csn_max(vdcsn, adcsn);
-		if((csn_compare(vucsn,deletedcsn)>=0) || /* check if the attribute or value was deleted after the value was last updated */
+
+		/* check if the attribute or value was deleted after the value was last updated */
+		if((csn_compare(vucsn,deletedcsn)>0) ||
 	        value_distinguished_at_csn(e, a, v, deletedcsn))
 		{
 			entry_deleted_value_to_present_value(a,v);
@@ -1178,7 +1190,7 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu
 }
 
 static void
-resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state)
+resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state, int delete_priority)
 {
 	if(slapi_attr_flag_is_set(a,SLAPI_ATTR_FLAG_SINGLE))
 	{
@@ -1186,6 +1198,6 @@ resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state)
 	}
 	else
 	{
-		resolve_attribute_state_multi_valued(e,a,attribute_state);
+		resolve_attribute_state_multi_valued(e,a,attribute_state, delete_priority);
 	}
 }