OPENDJ-3281 Modify operations may not be replayed if case is mixed on attri...

Jean-Noël Rouvignac

02.39.2016 251f9586274ce5f121b0bb596fbb21e1ecd39442

OPENDJ-3281 Modify operations may not be replayed if case is mixed on attribute values

Regression introduced in OPENDJ-1368, when AttributeValue class was removed
and replaced by plain ByteString.

Historical values are wrongly compared for equality by using ByteString.equals(),
while they should take use the attribute type's equality matching rule for determining equality.

AttrValueHistorical.java:
Added attributeType field used in equals() and hashCode().

AttrHistoricalMultipleTest.java:
Fixed the unit test that reproduces the bug.

ValueInfoTest.java: RENAMED to AttrValueHistoricalTest.java

 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistorical.java

@@ -20,8 +20,8 @@
import java.util.Set;

import org.forgerock.opendj.ldap.ByteString;
import org.opends.server.replication.common.CSN;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.opends.server.replication.common.CSN;
import org.opends.server.types.Entry;
import org.opends.server.types.Modification;

@@ -59,12 +59,12 @@
  /**
   * Create a new object from a provided attribute type. Historical is empty.
   *
   * @param type the provided attribute type.
   * @param attrType the provided attribute type.
   * @return a new AttributeInfo object.
   */
  public static AttrHistorical createAttributeHistorical(AttributeType type)
  public static AttrHistorical createAttributeHistorical(AttributeType attrType)
  {
    return type.isSingleValue() ? new AttrHistoricalSingle() : new AttrHistoricalMultiple();
    return attrType.isSingleValue() ? new AttrHistoricalSingle(attrType) : new AttrHistoricalMultiple();
  }

  /**
@@ -85,8 +85,9 @@
   * Assign the provided information to this object.
   *
   * @param histKey the key to assign.
   * @param attrType the associated attribute type.
   * @param value   the associated value or null if there is no value;
   * @param csn     the associated CSN.
   */
  public abstract void assign(HistAttrModificationKey histKey, ByteString value, CSN csn);
  public abstract void assign(HistAttrModificationKey histKey, AttributeType attrType, ByteString value, CSN csn);
}

 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistoricalMultiple.java

@@ -147,21 +147,25 @@
   */
  void delete(Attribute attr, CSN csn)
  {
    AttributeType attrType = attr.getAttributeDescription().getAttributeType();
    for (ByteString val : attr)
    {
      delete(val, csn);
      delete(val, attrType, csn);
    }
  }

   /**
    * Update the historical of this attribute after a delete value.
    *
    * @param val value that was deleted
    * @param csn time when the delete was done
    */
   void delete(ByteString val, CSN csn)
   * Update the historical of this attribute after a delete value.
   *
   * @param val
   *          value that was deleted
   * @param attrType
   * @param csn
   *          time when the delete was done
   */
  void delete(ByteString val, AttributeType attrType, CSN csn)
   {
     update(csn, new AttrValueHistorical(val, null, csn));
     update(csn, new AttrValueHistorical(val, attrType, null, csn));
   }

  /**
@@ -174,9 +178,10 @@
   */
  private void add(Attribute attr, CSN csn)
  {
    AttributeType attrType = attr.getAttributeDescription().getAttributeType();
    for (ByteString val : attr)
    {
      add(val, csn);
      add(val, attrType, csn);
    }
  }

@@ -185,12 +190,13 @@
     *
     * @param addedValue
     *          values that was added
     * @param attrType
     * @param csn
     *          time when the value was added
     */
   void add(ByteString addedValue, CSN csn)
   void add(ByteString addedValue, AttributeType attrType, CSN csn)
   {
     update(csn, new AttrValueHistorical(addedValue, csn, null));
     update(csn, new AttrValueHistorical(addedValue, attrType, csn, null));
   }

  private void update(CSN csn, AttrValueHistorical valInfo)
@@ -425,13 +431,14 @@
      // we are processing DELETE of some attribute values
      AttributeBuilder builder = new AttributeBuilder(modAttr);

      AttributeType attrType = modAttr.getAttributeDescription().getAttributeType();
      for (ByteString val : modAttr)
      {
        boolean deleteIt = true;  // true if the delete must be done
        boolean addedInCurrentOp = false;

        // update historical information
        AttrValueHistorical valInfo = new AttrValueHistorical(val, null, csn);
        AttrValueHistorical valInfo = new AttrValueHistorical(val, attrType, null, csn);
        AttrValueHistorical oldValInfo = valuesHist.get(valInfo);
        if (oldValInfo != null)
        {
@@ -516,10 +523,12 @@
      return false;
    }

    AttributeBuilder builder = new AttributeBuilder(m.getAttribute());
    for (ByteString addVal : m.getAttribute())
    Attribute attribute = m.getAttribute();
    AttributeBuilder builder = new AttributeBuilder(attribute);
    AttributeType attrType = attribute.getAttributeDescription().getAttributeType();
    for (ByteString addVal : attribute)
    {
      AttrValueHistorical valInfo = new AttrValueHistorical(addVal, csn, null);
      AttrValueHistorical valInfo = new AttrValueHistorical(addVal, attrType, csn, null);
      AttrValueHistorical oldValInfo = valuesHist.get(valInfo);
      if (oldValInfo == null)
      {
@@ -582,21 +591,21 @@
  }

  @Override
  public void assign(HistAttrModificationKey histKey, ByteString value, CSN csn)
  public void assign(HistAttrModificationKey histKey, AttributeType attrType, ByteString value, CSN csn)
  {
    switch (histKey)
    {
    case ADD:
      if (value != null)
      {
        add(value, csn);
        add(value, attrType, csn);
      }
      break;

    case DEL:
      if (value != null)
      {
        delete(value, csn);
        delete(value, attrType, csn);
      }
      break;

@@ -604,7 +613,7 @@
      delete(csn);
      if (value != null)
      {
        add(value, csn);
        add(value, attrType, csn);
      }
      break;


 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrHistoricalSingle.java

@@ -24,9 +24,9 @@

import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.ModificationType;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.opends.server.replication.common.CSN;
import org.opends.server.types.Attribute;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.opends.server.types.Entry;
import org.opends.server.types.Modification;

@@ -38,18 +38,31 @@
 */
public class AttrHistoricalSingle extends AttrHistorical
{
  /** Last time when the attribute was deleted. */
  private CSN deleteTime;
  /** Last time when a value was added. */
  private CSN addTime;
  /** Last added value. */
  private ByteString value;
  /** Attribute type for this historical value */
  private AttributeType attributeType;
  /** Last time when a value was added. */
  private CSN addTime;
  /** Last time when the attribute was deleted. */
  private CSN deleteTime;
  /**
   * Last operation applied. This is only used for multiple mods on the same
   * single valued attribute in the same modification.
   */
  private HistAttrModificationKey lastMod;

  /**
   * Builds an {@link AttrHistoricalSingle} object.
   *
   * @param attributeType
   *          the attribute type for this historical value
   */
  public AttrHistoricalSingle(AttributeType attributeType)
  {
    this.attributeType = attributeType;
  }

  @Override
  public CSN getDeleteTime()
  {
@@ -61,7 +74,7 @@
  {
    if (addTime != null)
    {
      return Collections.singleton(new AttrValueHistorical(value, addTime, null));
      return Collections.singleton(new AttrValueHistorical(value, attributeType, addTime, null));
    }
    return Collections.emptySet();
  }
@@ -262,7 +275,7 @@
  }

  @Override
  public void assign(HistAttrModificationKey histKey, ByteString value, CSN csn)
  public void assign(HistAttrModificationKey histKey, AttributeType attrType, ByteString value, CSN csn)
  {
    switch (histKey)
    {

 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/AttrValueHistorical.java

@@ -12,30 +12,38 @@
 * information: "Portions Copyright [year] [name of copyright owner]".
 *
 * Copyright 2006-2010 Sun Microsystems, Inc.
 * Portions Copyright 2013-2015 ForgeRock AS.
 * Portions Copyright 2013-2016 ForgeRock AS.
 */
package org.opends.server.replication.plugin;

import static org.forgerock.util.Reject.*;

import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.DecodeException;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.opends.server.replication.common.CSN;

/** AttrValueHistorical is the historical information of the modification of one attribute value. */
public class AttrValueHistorical
{
  private AttributeType attributeType;
  private ByteString value;
  private ByteString normalizedValue;
  private CSN valueDeleteTime;
  private CSN valueUpdateTime;

  /**
   * Build an AttrValueHistorical for a provided attribute value, providing
   * the last time the provided value is either updated or deleted.
   * @param value    the provided attributeValue
   * @param value    the provided attribute value
   * @param attributeType the provided attribute type
   * @param csnUpdate last time when this value was updated
   * @param csnDelete last time when this value for deleted
   */
  public AttrValueHistorical(ByteString value, CSN csnUpdate, CSN csnDelete)
  public AttrValueHistorical(ByteString value, AttributeType attributeType, CSN csnUpdate, CSN csnDelete)
  {
    this.value = value;
    this.attributeType = checkNotNull(attributeType);
    this.valueUpdateTime = csnUpdate;
    this.valueDeleteTime = csnDelete;
  }
@@ -46,7 +54,14 @@
    if (obj instanceof AttrValueHistorical)
    {
      AttrValueHistorical objVal = (AttrValueHistorical) obj;
      return value.equals(objVal.getAttributeValue());
      try
      {
        return getNormalizedValue().equals(objVal.getNormalizedValue());
      }
      catch (DecodeException e)
      {
        return value.equals(objVal.getAttributeValue());
      }
    }
    return false;
  }
@@ -54,7 +69,23 @@
  @Override
  public int hashCode()
  {
    return value.hashCode();
    try
    {
      return getNormalizedValue().hashCode();
    }
    catch (DecodeException e)
    {
      return value.hashCode();
    }
  }

  private ByteString getNormalizedValue() throws DecodeException
  {
    if (normalizedValue == null)
    {
      normalizedValue = attributeType.getEqualityMatchingRule().normalizeAttributeValue(value);
    }
    return normalizedValue;
  }

  /**

 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/EntryHistorical.java

@@ -581,7 +581,7 @@
              attrInfo = AttrHistorical.createAttributeHistorical(attrDesc.getAttributeType());
              newHistorical.attributesHistorical.put(attrDesc, attrInfo);
            }
            attrInfo.assign(histVal.getHistKey(), histVal.getAttributeValue(), csn);
            attrInfo.assign(histVal.getHistKey(), attrDesc.getAttributeType(), histVal.getAttributeValue(), csn);
          }
        }
      }

 opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/AttrHistoricalMultipleTest.java

@@ -25,12 +25,12 @@
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Set;

import org.forgerock.i18n.LocalizableMessageBuilder;
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.ModificationType;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.forgerock.opendj.ldap.schema.CoreSchema;
import org.opends.server.TestCaseUtils;
import org.opends.server.core.DirectoryServer;
import org.opends.server.replication.ReplicationTestCase;
@@ -113,52 +113,54 @@

  /** Create a AttrInfo and check the methods. */
  @Test(dataProvider = "attrInfo")
  public void attrInfo(ByteString att, CSN deleteTime, CSN updateTime) throws Exception
  public void attrInfo(ByteString attrValue, CSN deleteTime, CSN updateTime) throws Exception
  {
    AttributeType attrType = CoreSchema.getDescriptionAttributeType();
    // Create an empty AttrInfo
    AttrHistoricalMultiple attrInfo1 = new AttrHistoricalMultiple();

    // Check
    attrInfo1.add(att, updateTime);
    Set<AttrValueHistorical> values1 = attrInfo1.getValuesHistorical();
    assertEquals(values1.size(), 1);
    AttrValueHistorical valueInfo1 = new AttrValueHistorical(att, updateTime, null);
    assertTrue(values1.contains(valueInfo1));
    attrInfo1.add(attrValue, attrType, updateTime);
    assertThat(attrInfo1.getValuesHistorical())
        .containsOnly(new AttrValueHistorical(attrValue, attrType, updateTime, null));

    // Check constructor with parameter
    AttrValueHistorical valueInfo2 = new AttrValueHistorical(att, updateTime, deleteTime);
    AttrValueHistorical valueInfo2 = new AttrValueHistorical(attrValue, attrType, updateTime, deleteTime);
    AttrHistoricalMultiple attrInfo2 = new AttrHistoricalMultiple(
        deleteTime, updateTime, Collections.singletonMap(valueInfo2, valueInfo2));
        deleteTime, updateTime, Collections.singleton(valueInfo2));

    // Check equality
    //assertTrue(attrInfo1.getDeleteTime().compareTo(attrInfo2.getDeleteTime())==0);

    //  Check constructor with time parameter and not Value
    AttrHistoricalMultiple attrInfo3 = new AttrHistoricalMultiple(deleteTime, updateTime, null);
    attrInfo3.add(att, updateTime);
    Set<AttrValueHistorical> values3 = attrInfo3.getValuesHistorical();
    assertEquals(values3.size(), 1);
    valueInfo1 = new AttrValueHistorical(att, updateTime, null);
    assertTrue(values3.contains(valueInfo1));
    attrInfo3.add(attrValue, attrType, updateTime);
    assertThat(attrInfo3.getValuesHistorical())
        .containsOnly(new AttrValueHistorical(attrValue, attrType, updateTime, null));

    // Check duplicate
    AttrHistoricalMultiple attrInfo4 = attrInfo3.duplicate();
    Set<AttrValueHistorical> values4 = attrInfo4.getValuesHistorical();
    AttrHistoricalMultiple attrInfo4 = duplicate(attrInfo3);
    assertEquals(attrInfo4.getDeleteTime().compareTo(attrInfo3.getDeleteTime()), 0);
    assertEquals(values4.size(), values3.size());
    assertThat(attrInfo4.getValuesHistorical()).isEqualTo(attrInfo3.getValuesHistorical());

    // Check
    attrInfo4.delete(att, updateTime);
    attrInfo4.delete(attrValue, attrType, updateTime);
    assertEquals(attrInfo4.getValuesHistorical().size(), 1);

    // Check
    AttributeType type = DirectoryServer.getSchema().getAttributeType(ATTRIBUTE_NAME);
    attrInfo3.delete(Attributes.create(type, att), updateTime) ;
    attrInfo3.delete(Attributes.create(type, attrValue), updateTime);
    assertEquals(attrInfo3.getValuesHistorical().size(), 1);

    // Check
    attrInfo2.delete(updateTime);
    assertEquals(attrInfo2.getValuesHistorical().size(), 0);
    assertThat(attrInfo2.getValuesHistorical()).isEmpty();
  }

  private AttrHistoricalMultiple duplicate(AttrHistoricalMultiple attrMul)
  {
    return new AttrHistoricalMultiple(
        attrMul.getDeleteTime(), attrMul.getLastUpdateTime(), attrMul.getValuesHistorical());
  }

  @Test
@@ -253,22 +255,23 @@
    assertAttributeValues(entry, "X");
  }

  /** Use case: user changes the case of a family name for example. */
  @Test
  public void replay_addThenDeleteThenAdd_differentCaseWithCaseIgnoreAttributeType() throws Exception
  {
    CSN[] t = newCSNs(3);

    mod = newModification(ADD, "X");
    mod = newModification(ADD, "x");
    replayOperation(t[0], entry, mod, SUCCESS);
    assertAttributeValues(entry, "X");
    assertAttributeValues(entry, "x");

    mod = newModification(DELETE, "x");
    mod = newModification(DELETE, "X");
    replayOperation(t[1], entry, mod, SUCCESS);
    assertNoAttributeValue(entry);

    mod = newModification(ADD, "X");
    replayOperationSuppressMod(t[2], entry, mod, CONFLICT);
    assertAttributeValues(entry);
    replayOperation(t[2], entry, mod, SUCCESS);
    assertAttributeValues(entry, "X");
  }

  @Test

 opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/AttrHistoricalSingleTest.java

@@ -25,6 +25,8 @@
import org.forgerock.i18n.LocalizableMessageBuilder;
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.ModificationType;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.forgerock.opendj.ldap.schema.Schema;
import org.opends.server.TestCaseUtils;
import org.opends.server.replication.ReplicationTestCase;
import org.opends.server.replication.common.CSN;
@@ -54,7 +56,8 @@
  @BeforeMethod
  public void localSetUp() throws Exception
  {
    attrHist = new AttrHistoricalSingle();
    AttributeType attrType = Schema.getDefaultSchema().getAttributeType(ATTRIBUTE_NAME);
    attrHist = new AttrHistoricalSingle(attrType);
    csn = csnGen.newCSN();
    entry = TestCaseUtils.makeEntry(
        "dn: uid=test.user",
@@ -213,8 +216,9 @@
  @Test
  public void replay_deleteDubious() throws Exception
  {
    HistoricalAttributeValue histAttrVal = new HistoricalAttributeValue("display:" + csn + ":add:X");
    attrHist.assign(histAttrVal.getHistKey(), histAttrVal.getAttributeValue(), csn);
    AttributeType attrType = Schema.getDefaultSchema().getAttributeType(ATTRIBUTE_NAME);
    HistoricalAttributeValue histAttrVal = new HistoricalAttributeValue(ATTRIBUTE_NAME + ":" + csn + ":add:X");
    attrHist.assign(histAttrVal.getHistKey(), attrType, histAttrVal.getAttributeValue(), csn);
    mod = newModification(ADD, "X");
    entry.applyModification(mod);
    assertAttributeValue(entry, "X");

 opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/AttrValueHistoricalTest.java

File was renamed from opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/ValueInfoTest.java
@@ -12,29 +12,27 @@
 * information: "Portions Copyright [year] [name of copyright owner]".
 *
 * Copyright 2006-2010 Sun Microsystems, Inc.
 * Portions Copyright 2013-2015 ForgeRock AS.
 * Portions Copyright 2013-2016 ForgeRock AS.
 */
package org.opends.server.replication.plugin;

import static org.assertj.core.api.Assertions.*;
import static org.testng.Assert.*;

import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.forgerock.opendj.ldap.schema.CoreSchema;
import org.opends.server.replication.ReplicationTestCase;
import org.opends.server.replication.common.CSN;
import org.forgerock.opendj.ldap.ByteString;
import org.opends.server.util.TimeThread;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import static org.testng.Assert.*;

/**
 * Test of ValueInfo.
 */
/** Test of AttrValueHistorical. */
@SuppressWarnings("javadoc")
public class ValueInfoTest extends ReplicationTestCase
public class AttrValueHistoricalTest extends ReplicationTestCase
{

  /**
   * Build some data for the ValueInfo test below.
   */
  /** Build some data for the ValueInfo test below. */
  @DataProvider(name = "valueInfo")
  public Object[][] createData() {
    ByteString att1 = ByteString.valueOfUtf8("string");
@@ -56,16 +54,15 @@
        {att3,upd3,del3}};
  }

  /**
   * Create a ValueInfo and check the methods.
   */
  /** Create a ValueInfo and check the methods. */
  @Test(dataProvider = "valueInfo")
  public void valueInfo(ByteString value, CSN csnUpdate, CSN csnDelete) throws Exception
  {
    AttrValueHistorical valInfo1 = new AttrValueHistorical(value, csnUpdate, csnDelete);
    AttrValueHistorical valInfo2 = new AttrValueHistorical(value, csnUpdate, csnUpdate);
    AttributeType attrType = CoreSchema.getDescriptionAttributeType();
    AttrValueHistorical valInfo1 = new AttrValueHistorical(value, attrType, csnUpdate, csnDelete);
    AttrValueHistorical valInfo2 = new AttrValueHistorical(value, attrType, csnUpdate, csnUpdate);
    AttrValueHistorical valInfo3 = new AttrValueHistorical(ByteString.valueOfUtf8("Test"),
            csnUpdate, csnUpdate);
            attrType, csnUpdate, csnUpdate);

    // Check equals
    assertFalse(valInfo1.equals(new Object()));
@@ -87,4 +84,27 @@
    assertEquals(valInfo1.getAttributeValue(), value);
    assertEquals(valInfo1.isUpdate(), csnUpdate != null);
  }

  @Test
  public void testEqualsHashCode()
  {
    final AttrValueHistorical histVal1 = newAttrValueHistorical("description");
    final AttrValueHistorical histVal2 = newAttrValueHistorical("Description");
    final AttrValueHistorical histVal3 = newAttrValueHistorical("another description");

    assertThat(histVal1).isEqualTo(histVal1);
    assertThat(histVal1).isEqualTo(histVal2);
    assertThat(histVal2).isEqualTo(histVal1);

    assertThat(histVal1).isNotEqualTo(histVal3);

    assertThat(histVal1.hashCode()).isEqualTo(histVal2.hashCode());
    assertThat(histVal1.hashCode()).isNotEqualTo(histVal3.hashCode());
  }

  private AttrValueHistorical newAttrValueHistorical(String pp)
  {
    AttributeType attrType = CoreSchema.getDescriptionAttributeType();
    return new AttrValueHistorical(ByteString.valueOfUtf8(pp), attrType, null, null);
  }
}