From 17d7e4ed4a190e989e196a81fb36bc4fcc66fbac Mon Sep 17 00:00:00 2001
From: Jean-Noël Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Fri, 29 Jan 2016 10:07:09 +0000
Subject: [PATCH] Code cleanup
---
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java | 133 +++++++++++++++++++-------------
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java | 101 +++++++++++--------------
2 files changed, 123 insertions(+), 111 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java
index 02d4e70..ddc1d0c 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java
+++ b/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()
{
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java
index 389c014..5e1ab2d 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java
+++ b/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:
--
Gitblit v1.10.0