Fix issue OPENDJ-263 : Wrong search results when searching draft change log...

Ludovic Poitou

24.04.2011 2569b771a69bddc8cf0e83daf8705a45be2087da

Fix issue OPENDJ-263 : Wrong search results when searching draft change log using filter of the form "(changeNumber>=xxxx)" where xxxx<firstChangeNumber.

Matt pointed to the incriminated piece of code, which didn't consider that specific case.

We should add some test for this, but since it involves purging, delays, I recommend to add that in functional test, not unit test.

 opends/src/server/org/opends/server/replication/server/ECLServerHandler.java

@@ -291,7 +291,8 @@
  private String clDomCtxtsToString(String msg)
  {
    StringBuilder buffer = new StringBuilder();
    buffer.append(msg+"\n");
    buffer.append(msg);
    buffer.append("\n");
    for (int i=0;i<domainCtxts.length;i++)
    {
      domainCtxts[i].toString(buffer);
@@ -624,17 +625,38 @@
        {
          // startDraftCN provided in the request IS NOT in the DraftCNDb

          // Is the provided startDraftCN <= the potential last DraftCN

          // Get the draftLimits (from the eligibleCN got at the beginning of
          // the operation) in order to have the potential last DraftCN.
          // the operation) in order to have the first and possible last
          // DraftCN.
          int[] limits = replicationServer.getECLDraftCNLimits(
              eligibleCN, excludedServiceIDs);

          if (startDraftCN<=limits[1])
          // If the startDraftCN provided is lower than the first Draft CN in
          // the DB, let's use the lower limit.
          if (startDraftCN < limits[0])
          {
          // startDraftCN is between first and potential last and has never been
            // returned yet
            crossDomainStartState = draftCNDb.getValue(limits[0]);

            if (crossDomainStartState != null)
            {
              // startDraftCN (from the request filter) is present in the
              // draftCnDb.
              // Get an iterator to traverse it.
              draftCNDbIter =
                draftCNDb.generateIterator(limits[0]);
            }
            else
            {
              // This shouldn't happen
              // Let's start from what we have in the changelog db.
              isEndOfDraftCNReached = true;
              crossDomainStartState = null;
            }
          }
          else if (startDraftCN<=limits[1])
          {
            // startDraftCN is between first and potential last and has never
            // been returned yet
            if (draftCNDb.count() == 0)
            {
              // db is empty
@@ -789,12 +811,12 @@
                // changelog db has been trimed and the cookie is not valid
                // anymore.
                boolean cookieTooOld = false;
                for (int serverId : rsd.getStartState())
                for (int aServerId : rsd.getStartState())
                {
                  ChangeNumber dbOldestChange =
                    rsd.getStartState().getMaxChangeNumber(serverId);
                    rsd.getStartState().getMaxChangeNumber(aServerId);
                  ChangeNumber providedChange =
                    newDomainCtxt.startState.getMaxChangeNumber(serverId);
                    newDomainCtxt.startState.getMaxChangeNumber(aServerId);
                  if ((providedChange != null)
                      && (providedChange.older(dbOldestChange)))
                  {
@@ -928,6 +950,7 @@
  /**
   * Shutdown this handler.
   */
  @Override
  public void shutdown()
  {
    if (debugEnabled())
@@ -954,6 +977,7 @@
  /**
   * Request to shutdown the associated writer.
   */
  @Override
  protected void shutdownWriter()
  {
    shutdownWriter = true;
@@ -1211,6 +1235,7 @@
   * @param synchronous - not used
   * @return the next message
   */
  @Override
  protected UpdateMsg getnextMessage(boolean synchronous)
  {
    UpdateMsg msg = null;

 opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java

@@ -1355,9 +1355,14 @@
    startCLmsg.setLastDraftChangeNumber(-1);
    startCLmsg.setChangeNumber(new ChangeNumber(0,0,(short)0));

    // If there's no filter, just return
    if (sf == null)
    {
      return startCLmsg;
    }

    // Here are the 3 elementary cases we know how to optimize
    if ((sf != null)
        && (sf.getFilterType() == FilterType.GREATER_OR_EQUAL)
    if ((sf.getFilterType() == FilterType.GREATER_OR_EQUAL)
        && (sf.getAttributeType() != null)
        && (sf.getAttributeType().getPrimaryName().
            equalsIgnoreCase("changeNumber")))
@@ -1367,8 +1372,7 @@
      startCLmsg.setFirstDraftChangeNumber(sn);
      return startCLmsg;
    }
    else if ((sf != null)
        && (sf.getFilterType() == FilterType.LESS_OR_EQUAL)
    else if ((sf.getFilterType() == FilterType.LESS_OR_EQUAL)
        && (sf.getAttributeType() != null)
        && (sf.getAttributeType().getPrimaryName().
            equalsIgnoreCase("changeNumber")))
@@ -1378,8 +1382,7 @@
      startCLmsg.setLastDraftChangeNumber(sn);
      return startCLmsg;
    }
    else if ((sf != null)
        && (sf.getFilterType() == FilterType.EQUALITY)
    else if ((sf.getFilterType() == FilterType.EQUALITY)
        && (sf.getAttributeType() != null)
        && (sf.getAttributeType().getPrimaryName().
            equalsIgnoreCase("replicationcsn")))
@@ -1389,8 +1392,7 @@
      startCLmsg.setChangeNumber(cn);
      return startCLmsg;
    }
    else if ((sf != null)
        && (sf.getFilterType() == FilterType.EQUALITY)
    else if ((sf.getFilterType() == FilterType.EQUALITY)
        && (sf.getAttributeType() != null)
        && (sf.getAttributeType().getPrimaryName().
            equalsIgnoreCase("changenumber")))
@@ -1401,8 +1403,7 @@
      startCLmsg.setLastDraftChangeNumber(sn);
      return startCLmsg;
    }
    else if ((sf != null)
        && (sf.getFilterType() == FilterType.AND))
    else if (sf.getFilterType() == FilterType.AND)
    {
      // Here is the only binary operation we know how to optimize
      Collection<SearchFilter> comps = sf.getFilterComponents();

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

@@ -175,6 +175,7 @@
   *           If the environment could not be set up.
   */
  @BeforeClass
  @Override
  public void setUp() throws Exception
  {
    super.setUp();
@@ -436,7 +437,7 @@
    ECLCompatTestLimits(1,8, true);

    // Test first and last draft changenumber, a dd a new change, do not
    // search again the ECL, but search fro first and last
    // search again the ECL, but search for first and last
    ECLCompatTestLimitsAndAdd(1,8, ts);

    // Test DraftCNDb is purged when replication change log is purged
@@ -929,7 +930,7 @@
    ReplicationBroker s1test2 = null;
    ReplicationBroker s2test = null;
    ReplicationBroker s2test2 = null;
    

    try
    {
      // Initialize a second test backend
@@ -2699,7 +2700,7 @@
      if (s1 != null)
      {
        try { s1.close(); } catch (Exception ignored) {};
        while (!s1.isClosed()) sleep(100);      
        while (!s1.isClosed()) sleep(100);
      }
      if (s2 != null)
      {
@@ -2868,12 +2869,12 @@

    // Initialize a second test backend
    Backend backend2 = null;
    

    try
    {
      backend2 = initializeTestBackend(true, TEST_ROOT_DN_STRING2,
          TEST_BACKEND_ID2);
          

      // --
      s1test = openReplicationSession(
          DN.decode(TEST_ROOT_DN_STRING),  1201,