Code cleanup

Jean-Noël Rouvignac

15.17.2016 17d7e4ed4a190e989e196a81fb36bc4fcc66fbac

Code cleanup

VLVIndex.java:
In modifyEntry(), do not check shouldInclude() several times.
Extracted addEntry0() and removeEntry0() which do not include the shouldInclude() check.
Extracted method getLowestAttributeValue().
Moved encodeVLVKey() to avoid jumping up and down when reading the code.

EntryContainer.java:
extracted methods add*Control() and isPageFull()

 opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java

@@ -96,7 +96,6 @@
import org.opends.server.core.ModifyDNOperation;
import org.opends.server.core.ModifyOperation;
import org.opends.server.core.SearchOperation;
import org.opends.server.protocols.ldap.LDAPResultCode;
import org.opends.server.types.Attribute;
import org.opends.server.types.Attributes;
import org.opends.server.types.CanceledOperationException;
@@ -718,7 +717,7 @@
             * sortKeyResponseControl in the searchResultDone message, and not
             * send back any search result entries.
             */
            searchOperation.addResponseControl(newServerSideSortControl(NO_SUCH_ATTRIBUTE));
            addServerSideSortControl(searchOperation, NO_SUCH_ATTRIBUTE);
            searchOperation.setResultCode(ResultCode.UNAVAILABLE_CRITICAL_EXTENSION);
            return null;
          }
@@ -735,8 +734,7 @@
          {
            if (pageRequest.getSize() == 0)
            {
              Control control = new PagedResultsControl(pageRequest.isCritical(), 0, null);
              searchOperation.getResponseControls().add(control);
              addPagedResultsControl(searchOperation, pageRequest, null);
              return null;
            }
            if (searchOperation.getSizeLimit() > 0 && pageRequest.getSize() >= searchOperation.getSizeLimit())
@@ -774,7 +772,7 @@
                candidateEntryIDs = vlvIndex.evaluate(txn, searchOperation, sortRequest, vlvRequest, debugBuffer);
                if (candidateEntryIDs != null)
                {
                  searchOperation.addResponseControl(newServerSideSortControl(SUCCESS));
                  addServerSideSortControl(searchOperation, SUCCESS);
                  candidatesAreInScope = true;
                  break;
                }
@@ -839,7 +837,7 @@
              {
                if (sortRequest.containsSortKeys())
                {
                  searchOperation.addResponseControl(newServerSideSortControl(SUCCESS));
                  addServerSideSortControl(searchOperation, SUCCESS);
                }
                else
                {
@@ -850,7 +848,7 @@
                   * include the sortKeyResponseControl in the searchResultDone
                   * message.
                   */
                  searchOperation.addResponseControl(newServerSideSortControl(NO_SUCH_ATTRIBUTE));
                  addServerSideSortControl(searchOperation, NO_SUCH_ATTRIBUTE);
                }
              }
              catch (DirectoryException de)
@@ -906,7 +904,7 @@
            if (sortRequest != null)
            {
              // FIXME OPENDJ-2628: Add support for sorting unindexed searches using indexes like DSEE currently does
              searchOperation.addResponseControl(newServerSideSortControl(UNWILLING_TO_PERFORM));
              addServerSideSortControl(searchOperation, UNWILLING_TO_PERFORM);
              if (sortRequest.isCritical())
              {
                throw new DirectoryException(
@@ -940,28 +938,23 @@
            searchOperation.returnEntry(baseEntry, null);
          }

          if (pageRequest != null)
          {
            // Indicate no more pages.
            Control control = new PagedResultsControl(pageRequest.isCritical(), 0, null);
            searchOperation.getResponseControls().add(control);
          }
          // Indicate no more pages.
          addPagedResultsControl(searchOperation, pageRequest, null);
        }

        private void serverSideSortControlError(final SearchOperation searchOperation,
            ServerSideSortRequestControl sortRequest, DirectoryException de) throws DirectoryException
        {
          searchOperation.addResponseControl(newServerSideSortControl(de.getResultCode().intValue()));

          addServerSideSortControl(searchOperation, de.getResultCode().intValue());
          if (sortRequest.isCritical())
          {
            throw de;
          }
        }

        private ServerSideSortResponseControl newServerSideSortControl(int resultCode)
        private void addServerSideSortControl(SearchOperation searchOp, int resultCode)
        {
          return new ServerSideSortResponseControl(resultCode, null);
          searchOp.addResponseControl(new ServerSideSortResponseControl(resultCode, null));
        }

        private EntryIDSet getIDSetFromScope(final ReadableTransaction txn, DN aBaseDN, SearchScope searchScope,
@@ -1134,13 +1127,10 @@
        searchOperation.returnEntry(baseEntry, null);
      }

      if (!manageDsaIT
          && !dn2uri.returnSearchReferences(txn, searchOperation)
          && pageRequest != null)
      if (!manageDsaIT && !dn2uri.returnSearchReferences(txn, searchOperation))
      {
        // Indicate no more pages.
        Control control = new PagedResultsControl(pageRequest.isCritical(), 0, null);
        searchOperation.getResponseControls().add(control);
        addPagedResultsControl(searchOperation, pageRequest, null);
      }
    }

@@ -1214,22 +1204,17 @@
            if ((manageDsaIT || entry.getReferralURLs() == null)
                && searchOperation.getFilter().matchesEntry(entry))
            {
              if (pageRequest != null
                  && searchOperation.getEntriesSent() == pageRequest.getSize())
              if (isPageFull(searchOperation, pageRequest))
              {
                // The current page is full.
                // Set the cookie to remember where we were.
                ByteString cookie = cursor.getKey();
                Control control = new PagedResultsControl(pageRequest.isCritical(), 0, cookie);
                searchOperation.getResponseControls().add(control);
                addPagedResultsControl(searchOperation, pageRequest, cursor.getKey());
                return;
              }

              if (!searchOperation.returnEntry(entry, null))
              {
                // We have been told to discontinue processing of the
                // search. This could be due to size limit exceeded or
                // operation cancelled.
                // We have been told to discontinue processing of the search.
                // This could be due to size limit exceeded or operation cancelled
                return;
              }
            }
@@ -1247,11 +1232,20 @@
      logger.traceException(e);
    }

    // Indicate no more pages.
    addPagedResultsControl(searchOperation, pageRequest, null);
  }

  private boolean isPageFull(SearchOperation searchOperation, PagedResultsControl pageRequest)
  {
    return pageRequest != null && searchOperation.getEntriesSent() == pageRequest.getSize();
  }

  private void addPagedResultsControl(SearchOperation searchOp, PagedResultsControl pageRequest, ByteString cookie)
  {
    if (pageRequest != null)
    {
      // Indicate no more pages.
      Control control = new PagedResultsControl(pageRequest.isCritical(), 0, null);
      searchOperation.getResponseControls().add(control);
      searchOp.addResponseControl(new PagedResultsControl(pageRequest.isCritical(), 0, cookie));
    }
  }

@@ -1373,22 +1367,17 @@
              && (manageDsaIT || entry.getReferralURLs() == null)
              && filter.matchesEntry(entry))
          {
            if (pageRequest != null
                && searchOperation.getEntriesSent() == pageRequest.getSize())
            if (isPageFull(searchOperation, pageRequest))
            {
              // The current page is full.
              // Set the cookie to remember where we were.
              ByteString cookie = entryID.toByteString();
              Control control = new PagedResultsControl(pageRequest.isCritical(), 0, cookie);
              searchOperation.getResponseControls().add(control);
              addPagedResultsControl(searchOperation, pageRequest, entryID.toByteString());
              return;
            }

            if (!searchOperation.returnEntry(entry, null))
            {
              // We have been told to discontinue processing of the
              // search. This could be due to size limit exceeded or
              // operation cancelled.
              // We have been told to discontinue processing of the search.
              // This could be due to size limit exceeded or operation cancelled
              break;
            }
          }
@@ -1409,12 +1398,8 @@
      }
    }

    if (pageRequest != null)
    {
      // Indicate no more pages.
      Control control = new PagedResultsControl(pageRequest.isCritical(), 0, null);
      searchOperation.getResponseControls().add(control);
    }
    // Indicate no more pages.
    addPagedResultsControl(searchOperation, pageRequest, null);
  }

  private int findStartIndex(Long beginEntryID, long[] entryIDReorderedSet)
@@ -2705,7 +2690,7 @@
      targetIndex = sortMap.size() + 1;
      result = new long[0];
    }
    searchOperation.addResponseControl(new VLVResponseControl(targetIndex, sortMap.size(), LDAPResultCode.SUCCESS));
    addVLVResponseControl(searchOperation, targetIndex, sortMap.size(), SUCCESS);
    return result;
  }

@@ -2715,10 +2700,8 @@
    int targetOffset = vlvRequest.getOffset();
    if (targetOffset < 0)
    {
      // The client specified a negative target offset. This
      // should never be allowed.
      searchOperation.addResponseControl(new VLVResponseControl(targetOffset, sortMap.size(),
          LDAPResultCode.OFFSET_RANGE_ERROR));
      // The client specified a negative target offset. This should never be allowed.
      addVLVResponseControl(searchOperation, targetOffset, sortMap.size(), OFFSET_RANGE_ERROR);

      LocalizableMessage message = ERR_ENTRYIDSORTER_NEGATIVE_START_POS.get();
      throw new DirectoryException(ResultCode.VIRTUAL_LIST_VIEW_ERROR, message);
@@ -2773,10 +2756,16 @@
      sortedIDs = Arrays.copyOf(sortedIDs, arrayPos);
    }

    searchOperation.addResponseControl(new VLVResponseControl(targetOffset, sortMap.size(), LDAPResultCode.SUCCESS));
    addVLVResponseControl(searchOperation, targetOffset, sortMap.size(), SUCCESS);
    return sortedIDs;
  }

  private static void addVLVResponseControl(SearchOperation searchOp, int targetPosition, int contentCount,
      int vlvResultCode)
  {
    searchOp.addResponseControl(new VLVResponseControl(targetPosition, contentCount, vlvResultCode));
  }

  /** Get the exclusive lock. */
  void lock()
  {

 opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java

@@ -81,12 +81,14 @@
import org.opends.server.util.StaticUtils;

/**
 * This class represents a VLV index. Each record corresponds to a single entry matching
 * the VLV criteria. Keys are a sequence of the entry's normalized attribute values corresponding to
 * the VLV sort order, followed by the entry's entry ID. Records do not have a "value" since all
 * required information is held within the key. The entry ID is included in the key as a
 * "tie-breaker" and ensures that keys correspond to one and only one entry. This ensures that all
 * tree updates can be performed using lock-free operations.
 * This class represents a VLV index.
 * <p>
 * Each record corresponds to a single entry matching the VLV criteria.
 * Keys are a sequence of the entry's normalized attribute values corresponding to the VLV sort order,
 * followed by the entry's entry ID.
 * Records do not have a "value" since all required information is held within the key.
 * The entry ID is included in the key as a "tie-breaker" and ensures that keys correspond to one and only one entry.
 * This ensures that all tree updates can be performed using lock-free operations.
 */
class VLVIndex extends AbstractTree implements ConfigurationChangeListener<BackendVLVIndexCfg>, Closeable
{
@@ -364,10 +366,15 @@
  {
    if (shouldInclude(entry))
    {
      buffer.put(this, toKey(entry, entryID));
      addEntry0(buffer, entryID, entry);
    }
  }

  private void addEntry0(final IndexBuffer buffer, final EntryID entryID, final Entry entry)
  {
    buffer.put(this, toKey(entry, entryID));
  }

  ByteString toKey(final Entry entry, final EntryID entryID)
  {
    return encodeVLVKey(entry, entryID.longValue());
@@ -394,20 +401,20 @@
        if (isSortAttributeModified(mods))
        {
          // Sorted attributes have changed. Reindex the entry.
          removeEntry(buffer, entryID, oldEntry);
          addEntry(buffer, entryID, newEntry);
          removeEntry0(buffer, entryID, oldEntry);
          addEntry0(buffer, entryID, newEntry);
        }
      }
      else
      {
        // The modifications caused the new entry to be unindexed. Remove from vlvIndex.
        removeEntry(buffer, entryID, oldEntry);
        removeEntry0(buffer, entryID, oldEntry);
      }
    }
    else if (shouldInclude(newEntry))
    {
      // The modifications caused the new entry to be indexed. Add to vlvIndex
      addEntry(buffer, entryID, newEntry);
      addEntry0(buffer, entryID, newEntry);
    }
  }

@@ -434,10 +441,15 @@
  {
    if (shouldInclude(entry))
    {
      buffer.remove(this, toKey(entry, entryID));
      removeEntry0(buffer, entryID, entry);
    }
  }

  private void removeEntry0(final IndexBuffer buffer, final EntryID entryID, final Entry entry)
  {
    buffer.remove(this, toKey(entry, entryID));
  }

  void updateIndex(final WriteableTransaction txn, final TreeSet<ByteString> addedkeys,
      final TreeSet<ByteString> deletedKeys) throws StorageRuntimeException
  {
@@ -516,6 +528,7 @@
      {
        if (cursor.next())
        {
          // FIXME the array returned by readRange() is not ordered like a defined EntryIDSet expects
          return newDefinedSet(readRange(cursor, entryCount, debugBuilder));
        }
      }
@@ -595,9 +608,8 @@
        // Treat a non-matching assertion as matching beyond the end of the index.
        targetPosition = currentCount;
      }
      searchOperation.addResponseControl(new VLVResponseControl(targetPosition + 1, currentCount,
          LDAPResultCode.SUCCESS));
      return newDefinedSet(toPrimitiveLongArray(selectedIDs));
      addVLVResponseControl(searchOperation, targetPosition + 1, currentCount, LDAPResultCode.SUCCESS);
      return newDefinedSet(toPrimitiveLongArray(selectedIDs)); // FIXME not ordered like a defined EntryIDSet expects
    }
  }

@@ -632,7 +644,7 @@
    }
    catch (final DecodeException e)
    {
      searchOperation.addResponseControl(new VLVResponseControl(0, resultSetSize, LDAPResultCode.OFFSET_RANGE_ERROR));
      addVLVResponseControl(searchOperation, 0, resultSetSize, LDAPResultCode.OFFSET_RANGE_ERROR);
      final String attributeName = primarySortKey.getAttributeType().getNameOrOID();
      throw new DirectoryException(ResultCode.VIRTUAL_LIST_VIEW_ERROR, ERR_VLV_BAD_ASSERTION.get(attributeName));
    }
@@ -649,8 +661,7 @@
    if (targetOffset < 0)
    {
      // The client specified a negative target offset. This should never be allowed.
      searchOperation.addResponseControl(new VLVResponseControl(targetOffset, currentCount,
          LDAPResultCode.OFFSET_RANGE_ERROR));
      addVLVResponseControl(searchOperation, targetOffset, currentCount, LDAPResultCode.OFFSET_RANGE_ERROR);
      final LocalizableMessage message = ERR_ENTRYIDSORTER_NEGATIVE_START_POS.get();
      throw new DirectoryException(ResultCode.VIRTUAL_LIST_VIEW_ERROR, message);
    }
@@ -689,10 +700,10 @@
      afterCount = 0;
    }

    final long[] selectedIDs;
    final int count = 1 + beforeCount + afterCount;
    try (Cursor<ByteString, ByteString> cursor = txn.openCursor(getName()))
    {
      final long[] selectedIDs;
      if (cursor.positionToIndex(startPos))
      {
        selectedIDs = readRange(cursor, count, debugBuilder);
@@ -701,9 +712,15 @@
      {
        selectedIDs = new long[0];
      }
      searchOperation.addResponseControl(new VLVResponseControl(targetOffset, currentCount, LDAPResultCode.SUCCESS));
      return newDefinedSet(selectedIDs);
    }
    addVLVResponseControl(searchOperation, targetOffset, currentCount, LDAPResultCode.SUCCESS);
    return newDefinedSet(selectedIDs); // FIXME not ordered like a defined EntryIDSet expects
  }

  private static void addVLVResponseControl(SearchOperation searchOp, int targetPosition, int contentCount,
      int vlvResultCode)
  {
    searchOp.addResponseControl(new VLVResponseControl(targetPosition, contentCount, vlvResultCode));
  }

  private long[] readRange(final Cursor<ByteString, ByteString> definedCursor, final int count,
@@ -769,6 +786,11 @@
    return false;
  }

  private ByteString encodeVLVKey(final Entry entry, final long entryID)
  {
    return encodeVLVKey(sortOrder, entry, entryID);
  }

  static ByteString encodeVLVKey(final SortOrder sortOrder, final Entry entry, final long entryID)
  {
    final ByteStringBuilder builder = new ByteStringBuilder();
@@ -777,49 +799,50 @@
    return builder.toByteString();
  }

  private ByteString encodeVLVKey(final Entry entry, final long entryID)
  {
    return encodeVLVKey(sortOrder, entry, entryID);
  }

  private static void encodeVLVKey0(final SortOrder sortOrder, final Entry entry, final ByteStringBuilder builder)
  {
    for (final SortKey sortKey : sortOrder.getSortKeys())
    {
      final AttributeType attributeType = sortKey.getAttributeType();
      final MatchingRule matchingRule = sortKey.getEffectiveOrderingRule();
      ByteString sortValue = null;
      for (Attribute a : entry.getAttribute(attributeType))
      {
        for (ByteString v : a)
        {
          try
          {
            /*
             * The RFC states that the lowest value of a multi-valued attribute should be used,
             * regardless of the sort order.
             */
            final ByteString nv = matchingRule.normalizeAttributeValue(v);
            if (sortValue == null || nv.compareTo(sortValue) < 0)
            {
              sortValue = nv;
            }
          }
          catch (final DecodeException e)
          {
            /*
             * This shouldn't happen because the attribute should have already been validated.
             * If it does then treat the value as missing.
             */
            continue;
          }
        }
      }
      ByteString sortValue = getLowestAttributeValue(entry, sortKey);
      encodeVLVKeyValue(sortValue, builder, sortKey.ascending());
    }
  }

  /**
   * The RFC states that the lowest value of a multi-valued attribute should be used,
   * regardless of the sort order.
   */
  private static ByteString getLowestAttributeValue(final Entry entry, final SortKey sortKey)
  {
    final AttributeType attributeType = sortKey.getAttributeType();
    final MatchingRule matchingRule = sortKey.getEffectiveOrderingRule();
    ByteString sortValue = null;
    for (Attribute a : entry.getAttribute(attributeType))
    {
      for (ByteString v : a)
      {
        try
        {
          final ByteString nv = matchingRule.normalizeAttributeValue(v);
          if (sortValue == null || nv.compareTo(sortValue) < 0)
          {
            sortValue = nv;
          }
        }
        catch (final DecodeException e)
        {
          /*
           * This shouldn't happen because the attribute should have already been validated.
           * If it does then treat the value as missing.
           */
          continue;
        }
      }
    }
    return sortValue;
  }

  /**
   * Package private for testing.
   * <p>
   * Keys are logically encoded as follows: