EntryHistorical.java: In encodeAndPurge(), made the code more readable by e...

Jean-Noel Rouvignac

03.39.2013 ffeb697e9e57ced650f704728b4e661fae4fe266

EntryHistorical.java:
In encodeAndPurge(), made the code more readable by extracting methods needsPurge() and encode().

HistoricalTest.java:
Cleaned up the test a bit.

 opends/src/server/org/opends/server/replication/plugin/EntryHistorical.java

@@ -477,57 +477,46 @@
        for (AttrValueHistorical attrValHist : attrHist.getValuesHistorical()
            .keySet())
        {
          attrValHist.getAttributeValue();
          final AttributeValue value = attrValHist.getAttributeValue();

          // Encode an attribute value
          final String strValue;
          if (attrValHist.getValueDeleteTime() != null)
          {
            // this hist must be purged now, so skip its encoding
            if ((purgeDelayInMillisec>0) &&
                (attrValHist.getValueDeleteTime().getTime()<=purgeDate))
            if (needsPurge(attrValHist.getValueDeleteTime(), purgeDate))
            {
              this.lastPurgedValuesCount++;
              // this hist must be purged now, so skip its encoding
              continue;
            }
            strValue = type.getNormalizedPrimaryName() + optionsString + ":" +
            attrValHist.getValueDeleteTime().toString() +
            ":del:" + attrValHist.getAttributeValue().toString();
            AttributeValue val = AttributeValues.create(historicalAttrType,
                                                    strValue);
            builder.add(val);
            strValue = encode("del", type, optionsString, attrValHist
                    .getValueDeleteTime(), value);
            builder.add(AttributeValues.create(historicalAttrType, strValue));
          }
          else if (attrValHist.getValueUpdateTime() != null)
          {
            if ((purgeDelayInMillisec>0) &&
                (attrValHist.getValueUpdateTime().getTime()<=purgeDate))
            if (needsPurge(attrValHist.getValueUpdateTime(), purgeDate))
            {
              // this hist must be purged now, so skip its encoding
              this.lastPurgedValuesCount++;
              continue;
            }
            if ((delAttr && attrValHist.getValueUpdateTime() == deleteTime)
               && (attrValHist.getAttributeValue() != null))

            final ChangeNumber updateTime = attrValHist.getValueUpdateTime();
            // FIXME very suspicious use of == in the next if statement,
            // unit tests do not like changing it
            if (delAttr && updateTime == deleteTime && value != null)
            {
              strValue = type.getNormalizedPrimaryName() + optionsString + ":" +
              attrValHist.getValueUpdateTime().toString() +  ":repl:" +
              attrValHist.getAttributeValue().toString();
              strValue = encode("repl", type, optionsString, updateTime, value);
              delAttr = false;
            }
            else
            else if (value != null)
            {
              if (attrValHist.getAttributeValue() == null)
              {
                strValue = type.getNormalizedPrimaryName() + optionsString
                           + ":" + attrValHist.getValueUpdateTime().toString() +
                           ":add";
              strValue = encode("add", type, optionsString, updateTime, value);
              }
              else
              {
                strValue = type.getNormalizedPrimaryName() + optionsString
                           + ":" + attrValHist.getValueUpdateTime().toString() +
                           ":add:" + attrValHist.getAttributeValue().toString();
              }
              // "add" without any value is suspicious. Tests never go there.
              // Is this used to encode "add" with an empty string?
              strValue = encode("add", type, optionsString, updateTime);
            }

            builder.add(AttributeValues.create(historicalAttrType, strValue));
@@ -536,57 +525,59 @@

        if (delAttr)
        {
          // Stores the attr deletion hist when not older than the purge delay
          if ((purgeDelayInMillisec>0) &&
              (deleteTime.getTime()<=purgeDate))
          if (needsPurge(deleteTime, purgeDate))
          {
            // this hist must be purged now, so skip its encoding
            this.lastPurgedValuesCount++;
            continue;
          }
          String strValue = type.getNormalizedPrimaryName()
              + optionsString + ":" + deleteTime.toString()
              + ":attrDel";
          AttributeValue val =
              AttributeValues.create(historicalAttrType, strValue);
          builder.add(val);
          String strValue = encode("attrDel", type, optionsString, deleteTime);
          builder.add(AttributeValues.create(historicalAttrType, strValue));
        }
      }
    }

    if (entryADDDate != null && !needsPurge(entryADDDate, purgeDate))
    {
    // Encode the historical information for the ADD Operation.
    if (entryADDDate != null)
    {
      // Stores the ADDDate when not older than the purge delay
      if ((purgeDelayInMillisec>0) &&
          (entryADDDate.getTime()<=purgeDate))
      {
        this.lastPurgedValuesCount++;
      }
      else
      {
        builder.add(encodeHistorical(entryADDDate, "add"));
      }
    }

    if (entryMODDNDate != null && !needsPurge(entryMODDNDate, purgeDate))
    {
    // Encode the historical information for the MODDN Operation.
    if (entryMODDNDate != null)
    {
      // Stores the MODDNDate when not older than the purge delay
      if ((purgeDelayInMillisec>0) &&
          (entryMODDNDate.getTime()<=purgeDate))
      {
        this.lastPurgedValuesCount++;
      }
      else
      {
        builder.add(encodeHistorical(entryMODDNDate, "moddn"));
      }
    }

    return builder.toAttribute();
  }

  private boolean needsPurge(ChangeNumber cn, long purgeDate)
  {
    boolean needsPurge = purgeDelayInMillisec > 0 && cn.getTime() <= purgeDate;
    if (needsPurge)
    {
      // this hist must be purged now, because older than the purge delay
      this.lastPurgedValuesCount++;
    }
    return needsPurge;
  }

  private String encode(String operation, AttributeType type,
      String optionsString, ChangeNumber changeTime)
  {
    return type.getNormalizedPrimaryName() + optionsString + ":" + changeTime
        + ":" + operation;
  }

  private String encode(String operation, AttributeType type,
      String optionsString, ChangeNumber changeTime, AttributeValue value)
  {
    return type.getNormalizedPrimaryName() + optionsString + ":" + changeTime
        + ":" + operation + ":" + value;
  }

  /**
   * Set the delay to purge the historical information. The purge is applied
   * only when historical attribute is updated (write operations).

 opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/HistoricalTest.java

@@ -29,6 +29,9 @@
package org.opends.server.replication.plugin;


import static org.opends.server.TestCaseUtils.*;
import static org.testng.Assert.*;

import org.opends.server.TestCaseUtils;
import org.opends.server.core.DirectoryServer;
import org.opends.server.protocols.internal.InternalClientConnection;
@@ -41,23 +44,10 @@
import org.opends.server.replication.protocol.ModifyMsg;
import org.opends.server.replication.service.ReplicationBroker;
import org.opends.server.tools.LDAPModify;
import org.opends.server.types.Attribute;
import org.opends.server.types.AttributeType;
import org.opends.server.types.Attributes;
import org.opends.server.types.ByteString;
import org.opends.server.types.DN;
import org.opends.server.types.Entry;
import org.opends.server.types.Modification;
import org.opends.server.types.ModificationType;
import org.opends.server.types.Operation;
import org.opends.server.types.ResultCode;
import org.opends.server.types.SearchResultEntry;
import org.opends.server.types.SearchScope;
import org.opends.server.types.*;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import static org.testng.Assert.*;
import static org.opends.server.TestCaseUtils.*;
import java.net.ServerSocket;
import java.util.ArrayList;
import java.util.LinkedList;
@@ -67,20 +57,19 @@
/**
 * Tests the Historical class.
 */
@SuppressWarnings("javadoc")
public class HistoricalTest
     extends ReplicationTestCase
{
  private int replServerPort;
  String testName = "historicalTest";
  private String testName = "historicalTest";

  /**
   * Set up replication on the test backend.
   * @throws Exception If an error occurs.
   */
  @BeforeClass
  @Override
  public void setUp()
       throws Exception
  public void setUp() throws Exception
  {
    super.setUp();

@@ -124,11 +113,9 @@
   * and written to an operational attribute of the entry.
   * Also test that historical is purged according to the purge delay that
   * is provided.
   * @throws Exception If the test fails.
   */
  @Test(enabled=true)
  public void testEncodingAndPurge()
       throws Exception
  public void testEncodingAndPurge() throws Exception
  {
    //  Add a test entry.
    TestCaseUtils.addEntry(
@@ -202,8 +189,6 @@
    assertEquals(hist.getLastPurgedValuesCount(),0);
    assertEquals(after, before);

    LDAPReplicationDomain domain = MultimasterReplication.findDomain(
        DN.decode("uid=user.1," + TEST_ROOT_DN_STRING), null);
    Thread.sleep(1000);

    args[9] = TestCaseUtils.createTempFile(
@@ -249,11 +234,9 @@
   * broker API to simulate the ordering that would happen on the first server
   * on one entry, and the reverse ordering that would happen on the
   * second server on a different entry.  Confused yet?
   * @throws Exception If the test fails.
   */
  @Test(enabled=true, groups="slow")
  public void conflictSingleValue()
       throws Exception
  public void conflictSingleValue() throws Exception
  {
    final DN dn1 = DN.decode("cn=test1," + TEST_ROOT_DN_STRING);
    final DN dn2 = DN.decode("cn=test2," + TEST_ROOT_DN_STRING);
@@ -382,8 +365,7 @@
  {
    List<Modification> mods = new ArrayList<Modification>(1);
    mods.add(mod);
    ModifyMsg modMsg = new ModifyMsg(changeNum, dn, mods, entryuuid);
    broker.publish(modMsg);
    broker.publish(new ModifyMsg(changeNum, dn, mods, entryuuid));
  }

  /**
@@ -504,16 +486,14 @@
        String parentId = LDAPReplicationDomain.findEntryUUID(dn1.getParent());
        assertTrue(addmsg.getParentEntryUUID().equals(parentId));
        addmsg.createOperation(InternalClientConnection.getRootConnection());
      } else
      {
        if (count == 1)
      }
      else if (count == 1)
        {
          // The first operation should be an ADD operation.
          fail("FakeAddOperation was not correctly generated"
              + " from historical information");
        }
      }
    }

      assertEquals(count, assertCount);
    }
@@ -531,18 +511,18 @@
   * the time to purge everything in 1 run .. and thus to relauch it to finish
   * the purge. And verify that the second run starts on the changeNumber where
   * the previous task run had stopped.
   *
   * @throws Exception If the test fails.
   */
  @Test(enabled=true)
  public void testRecurringPurgeIn1Run()
  throws Exception
  public void testRecurringPurgeIn1Run() throws Exception
  {
    int entryCnt = 10;

    addEntriesWithHistorical(1, entryCnt);

    // set the purge delay to 1 sec
    // set the purge delay to 1 minute
    // FIXME could we change this setting to also accept seconds?
    // This way this test would not take one minute to run
    // (and it could also fail less often in jenkins).
    TestCaseUtils.dsconfig(
        "set-replication-domain-prop",
        "--provider-name","Multimaster Synchronization",
@@ -565,8 +545,6 @@
    addTask(taskInit, ResultCode.SUCCESS, null);

    // every entry should be purged from its hist
    try
    {
      // Search for matching entries in config backend
      InternalSearchOperation op = connection.processSearch(
          ByteString.valueOf(TEST_ROOT_DN_STRING),
@@ -579,17 +557,12 @@
      LinkedList<SearchResultEntry> entries = op.getSearchEntries();
      assertTrue(entries != null);
      assertEquals(entries.size(), 0);
    } catch (Exception e)
    {
      fail("assertNoConfigEntriesWithFilter: could not search config backend" + e.getMessage());
    }
  }

  /**
   * Add a provided number of generated entries containing historical.
   * @param dnSuffix A suffix to be added to the dn
   * @param entryCnt The number of entries to create
   * @throws Exception
   */
  private void addEntriesWithHistorical(int dnSuffix, int entryCnt)
  throws Exception