From 7f582047119782b1a1c173fd0784478169839488 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Fri, 14 Nov 2014 13:51:41 +0000
Subject: [PATCH] OPENDJ-1629 CR-5279: fix integer matching rules so that they handle negative values
---
opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleTest.java | 17 +++
opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleTest.java | 25 ++++-
opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleImpl.java | 22 ++--
opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/CollationMatchingRulesImpl.java | 34 ++++--
opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/AbstractOrderingMatchingRuleImpl.java | 35 ------
opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleImpl.java | 114 ++++++++++++++++++++--
6 files changed, 176 insertions(+), 71 deletions(-)
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/AbstractOrderingMatchingRuleImpl.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/AbstractOrderingMatchingRuleImpl.java
index 0648348..7bf58c5 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/AbstractOrderingMatchingRuleImpl.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/AbstractOrderingMatchingRuleImpl.java
@@ -46,9 +46,7 @@
* this assumption does not hold true.
*/
abstract class AbstractOrderingMatchingRuleImpl extends AbstractMatchingRuleImpl {
-
private final Collection<? extends Indexer> indexers;
-
private final String indexId;
/** Constructor for default matching rules. */
@@ -97,35 +95,6 @@
};
}
- /**
- * Retrieves the normalized form of the provided assertion value, which is
- * best suited for efficiently performing greater than matching
- * operations on that value. The assertion value is guaranteed to be valid
- * against this matching rule's assertion syntax.
- *
- * @param schema
- * The schema in which this matching rule is defined.
- * @param assertionValue
- * The syntax checked assertion value to be normalized.
- * @return The normalized version of the provided assertion value.
- * @throws DecodeException
- * if an syntax error occurred while parsing the value.
- */
- public Assertion getGreaterThanAssertion(Schema schema, ByteSequence assertionValue) throws DecodeException {
- final ByteString normAssertion = normalizeAttributeValue(schema, assertionValue);
- return new Assertion() {
- @Override
- public ConditionResult matches(final ByteSequence attributeValue) {
- return ConditionResult.valueOf(attributeValue.compareTo(normAssertion) > 0);
- }
-
- @Override
- public <T> T createIndexQuery(IndexQueryFactory<T> factory) throws DecodeException {
- return factory.createRangeMatchQuery(indexId, normAssertion, ByteString.empty(), false, false);
- }
- };
- }
-
/** {@inheritDoc} */
@Override
public Assertion getLessOrEqualAssertion(final Schema schema, final ByteSequence value)
@@ -149,4 +118,8 @@
public Collection<? extends Indexer> getIndexers() {
return indexers;
}
+
+ final String getIndexId() {
+ return indexId;
+ }
}
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/CollationMatchingRulesImpl.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/CollationMatchingRulesImpl.java
index 89ee97d..4542d34 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/CollationMatchingRulesImpl.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/CollationMatchingRulesImpl.java
@@ -36,7 +36,9 @@
import org.forgerock.opendj.ldap.Assertion;
import org.forgerock.opendj.ldap.ByteSequence;
import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.ConditionResult;
import org.forgerock.opendj.ldap.DecodeException;
+import org.forgerock.opendj.ldap.spi.IndexQueryFactory;
import org.forgerock.opendj.ldap.spi.Indexer;
/**
@@ -129,9 +131,9 @@
* Defines the base for collation matching rules.
*/
private static abstract class AbstractCollationMatchingRuleImpl extends AbstractMatchingRuleImpl {
-
private final Locale locale;
final Collator collator;
+ final String indexName;
final Indexer indexer;
/**
@@ -143,7 +145,8 @@
AbstractCollationMatchingRuleImpl(Locale locale) {
this.locale = locale;
this.collator = createCollator(locale);
- this.indexer = new DefaultIndexer(getSharedIndexName());
+ this.indexName = getPrefixIndexName() + "." + INDEX_ID_SHARED;
+ this.indexer = new DefaultIndexer(indexName);
}
private Collator createCollator(Locale locale) {
@@ -178,10 +181,6 @@
return builder.toString();
}
- String getSharedIndexName() {
- return getPrefixIndexName() + "." + INDEX_ID_SHARED;
- }
-
/** {@inheritDoc} */
@Override
public Collection<? extends Indexer> getIndexers() {
@@ -220,7 +219,7 @@
@Override
public Assertion getAssertion(final Schema schema, final ByteSequence assertionValue)
throws DecodeException {
- return DefaultAssertion.named(getSharedIndexName(), normalizeAttributeValue(schema, assertionValue));
+ return DefaultAssertion.named(indexName, normalizeAttributeValue(schema, assertionValue));
}
}
@@ -242,7 +241,7 @@
CollationSubstringMatchingRuleImpl(Locale locale) {
super(locale);
substringMatchingRule = new AbstractSubstringMatchingRuleImpl(
- getPrefixIndexName() + "." + INDEX_ID_SUBSTRING, getSharedIndexName()) {
+ getPrefixIndexName() + "." + INDEX_ID_SUBSTRING, indexName) {
@Override
public ByteString normalizeAttributeValue(Schema schema, ByteSequence value)
throws DecodeException {
@@ -288,7 +287,7 @@
CollationOrderingMatchingRuleImpl(Locale locale) {
super(locale);
- orderingMatchingRule = new AbstractOrderingMatchingRuleImpl(getSharedIndexName()) {
+ orderingMatchingRule = new AbstractOrderingMatchingRuleImpl(indexName) {
@Override
public ByteString normalizeAttributeValue(Schema schema, ByteSequence value) throws DecodeException {
return CollationOrderingMatchingRuleImpl.this.normalizeAttributeValue(schema, value);
@@ -340,8 +339,21 @@
/** {@inheritDoc} */
@Override
- public Assertion getAssertion(Schema schema, ByteSequence assertionValue) throws DecodeException {
- return orderingMatchingRule.getGreaterThanAssertion(schema, assertionValue);
+ public Assertion getAssertion(Schema schema, ByteSequence assertionValue)
+ throws DecodeException {
+ final ByteString normAssertion = normalizeAttributeValue(schema, assertionValue);
+ return new Assertion() {
+ @Override
+ public ConditionResult matches(final ByteSequence attributeValue) {
+ return ConditionResult.valueOf(attributeValue.compareTo(normAssertion) > 0);
+ }
+
+ @Override
+ public <T> T createIndexQuery(IndexQueryFactory<T> factory) throws DecodeException {
+ return factory.createRangeMatchQuery(indexName, normAssertion,
+ ByteString.empty(), false, false);
+ }
+ };
}
}
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleImpl.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleImpl.java
index 52351e4..f2a61e0 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleImpl.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleImpl.java
@@ -26,30 +26,30 @@
*/
package org.forgerock.opendj.ldap.schema;
-import static com.forgerock.opendj.ldap.CoreMessages.*;
+import static org.forgerock.opendj.ldap.schema.IntegerOrderingMatchingRuleImpl.*;
-import org.forgerock.i18n.LocalizableMessage;
-import org.forgerock.i18n.slf4j.LocalizedLogger;
+import java.util.Comparator;
+
import org.forgerock.opendj.ldap.ByteSequence;
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.DecodeException;
/**
* This class defines the integerMatch matching rule defined in X.520 and
- * referenced in RFC 2252.
+ * referenced in RFC 2252. The implementation of this matching rule is
+ * intentionally aligned with the ordering matching rule so that they could
+ * potentially share the same index.
*/
final class IntegerEqualityMatchingRuleImpl extends AbstractEqualityMatchingRuleImpl {
- private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
+ @Override
+ public Comparator<ByteSequence> comparator(final Schema schema) {
+ return COMPARATOR;
+ }
@Override
public ByteString normalizeAttributeValue(final Schema schema, final ByteSequence value)
throws DecodeException {
- try {
- return ByteString.valueOf(Integer.parseInt(value.toString()));
- } catch (final Exception e) {
- logger.debug(LocalizableMessage.raw("%s", e));
- throw DecodeException.error(WARN_ATTR_SYNTAX_ILLEGAL_INTEGER.get(value));
- }
+ return normalizeValueAndEncode(value);
}
}
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleImpl.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleImpl.java
index b7fb8c1..3bcb2b5 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleImpl.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleImpl.java
@@ -26,30 +26,122 @@
*/
package org.forgerock.opendj.ldap.schema;
-import static com.forgerock.opendj.ldap.CoreMessages.*;
+import static com.forgerock.opendj.ldap.CoreMessages.WARN_ATTR_SYNTAX_ILLEGAL_INTEGER;
-import org.forgerock.i18n.LocalizableMessage;
-import org.forgerock.i18n.slf4j.LocalizedLogger;
+import java.math.BigInteger;
+import java.util.Comparator;
+
+import org.forgerock.opendj.ldap.Assertion;
import org.forgerock.opendj.ldap.ByteSequence;
import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.ConditionResult;
import org.forgerock.opendj.ldap.DecodeException;
+import org.forgerock.opendj.ldap.spi.IndexQueryFactory;
/**
* This class defines the integerOrderingMatch matching rule defined in X.520
- * and referenced in RFC 4519.
+ * and referenced in RFC 4519. The implementation of this matching rule is
+ * intentionally aligned with the ordering matching rule so that they could
+ * potentially share the same index.
*/
final class IntegerOrderingMatchingRuleImpl extends AbstractOrderingMatchingRuleImpl {
+ static final Comparator<ByteSequence> COMPARATOR = new Comparator<ByteSequence>() {
+ @Override
+ public int compare(final ByteSequence o1, final ByteSequence o2) {
+ final BigInteger i1 = decodeNormalizedValue(o1);
+ final BigInteger i2 = decodeNormalizedValue(o2);
+ return i1.compareTo(i2);
+ }
+ };
- private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
+ static ByteString normalizeValueAndEncode(final ByteSequence value) throws DecodeException {
+ return encodeNormalizedValue(normalizeValue(value));
+ }
+
+ private static BigInteger normalizeValue(final ByteSequence value) throws DecodeException {
+ try {
+ return new BigInteger(value.toString());
+ } catch (final Exception e) {
+ throw DecodeException.error(WARN_ATTR_SYNTAX_ILLEGAL_INTEGER.get(value));
+ }
+ }
+
+ private static BigInteger decodeNormalizedValue(final ByteSequence normalizedValue) {
+ return new BigInteger(normalizedValue.toByteArray());
+ }
+
+ private static ByteString encodeNormalizedValue(final BigInteger normalizedValue) {
+ return ByteString.wrap(normalizedValue.toByteArray());
+ }
+
+ @Override
+ public Comparator<ByteSequence> comparator(final Schema schema) {
+ return COMPARATOR;
+ }
+
+ @Override
+ public Assertion getAssertion(final Schema schema, final ByteSequence value)
+ throws DecodeException {
+ final BigInteger normAssertion = normalizeValue(value);
+ return new Assertion() {
+ @Override
+ public <T> T createIndexQuery(final IndexQueryFactory<T> factory)
+ throws DecodeException {
+ return factory.createRangeMatchQuery(getIndexId(), ByteString.empty(),
+ encodeNormalizedValue(normAssertion), false, false);
+ }
+
+ @Override
+ public ConditionResult matches(final ByteSequence normValue) {
+ return ConditionResult.valueOf(decodeNormalizedValue(normValue).compareTo(
+ normAssertion) < 0);
+ }
+ };
+ }
+
+ @Override
+ public Assertion getGreaterOrEqualAssertion(final Schema schema, final ByteSequence value)
+ throws DecodeException {
+ final BigInteger normAssertion = normalizeValue(value);
+ return new Assertion() {
+ @Override
+ public <T> T createIndexQuery(final IndexQueryFactory<T> factory)
+ throws DecodeException {
+ return factory.createRangeMatchQuery(getIndexId(), encodeNormalizedValue(normAssertion), ByteString
+ .empty(), true, false);
+ }
+
+ @Override
+ public ConditionResult matches(final ByteSequence normValue) {
+ return ConditionResult.valueOf(decodeNormalizedValue(normValue).compareTo(
+ normAssertion) >= 0);
+ }
+ };
+ }
+
+ @Override
+ public Assertion getLessOrEqualAssertion(final Schema schema, final ByteSequence value)
+ throws DecodeException {
+ final BigInteger normAssertion = normalizeValue(value);
+ return new Assertion() {
+ @Override
+ public <T> T createIndexQuery(final IndexQueryFactory<T> factory)
+ throws DecodeException {
+ return factory.createRangeMatchQuery(getIndexId(), ByteString.empty(),
+ encodeNormalizedValue(normAssertion), false, true);
+ }
+
+ @Override
+ public ConditionResult matches(final ByteSequence normValue) {
+ return ConditionResult.valueOf(decodeNormalizedValue(normValue).compareTo(
+ normAssertion) <= 0);
+ }
+ };
+ }
@Override
public ByteString normalizeAttributeValue(final Schema schema, final ByteSequence value)
throws DecodeException {
- try {
- return ByteString.valueOf(Integer.parseInt(value.toString()));
- } catch (final Exception e) {
- logger.debug(LocalizableMessage.raw("%s", e));
- throw DecodeException.error(WARN_ATTR_SYNTAX_ILLEGAL_INTEGER.get(value));
- }
+ return normalizeValueAndEncode(value);
}
}
diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleTest.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleTest.java
index b0c8961..1daa2d6 100644
--- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleTest.java
+++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerEqualityMatchingRuleTest.java
@@ -33,13 +33,17 @@
/**
* Test the IntegerEqualityMatchingRule.
*/
-//TODO: fix matching rule so that commented data in data providers pass
public class IntegerEqualityMatchingRuleTest extends MatchingRuleTest {
/** {@inheritDoc} */
@Override
@DataProvider(name = "matchingRuleInvalidAttributeValues")
public Object[][] createMatchingRuleInvalidAttributeValues() {
+ /*
+ * The JDK8 BigInteger parser is quite tolerant and allows leading zeros
+ * and + characters. It's ok if the matching rule is more tolerant than
+ * the syntax itself (see commented data).
+ */
return new Object[][] {
//{"01"},
//{"00"},
@@ -48,6 +52,11 @@
{"b2"},
{"-"},
{""},
+ {" 63 "},
+ {"- 63"},
+ //{"+63" },
+ {"AB" },
+ {"0xAB"},
};
}
@@ -58,8 +67,12 @@
return new Object[][] {
{"1234567890", "1234567890", ConditionResult.TRUE},
{"-1", "-1", ConditionResult.TRUE},
- //{"-9876543210", "-9876543210", ConditionResult.TRUE},
+ {"-9876543210", "-9876543210", ConditionResult.TRUE},
{"1", "-1", ConditionResult.FALSE},
+ // Values which are greater than 64 bits.
+ { "-987654321987654321987654321", "-987654321987654321987654322", ConditionResult.FALSE },
+ {"987654321987654321987654321", "987654321987654321987654322", ConditionResult.FALSE },
+ { "987654321987654321987654321", "987654321987654321987654321", ConditionResult.TRUE },
};
}
diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleTest.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleTest.java
index 30e7b8f..b2647ba 100644
--- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleTest.java
+++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/IntegerOrderingMatchingRuleTest.java
@@ -22,6 +22,7 @@
*
*
* Copyright 2009 Sun Microsystems, Inc.
+ * Portions copyright 2014 ForgeRock AS.
*/
package org.forgerock.opendj.ldap.schema;
@@ -32,14 +33,25 @@
/**
* Test the IntegerOrderingMatchingRule.
*/
-//TODO: fix matching rule so that commented data in data providers pass
public class IntegerOrderingMatchingRuleTest extends OrderingMatchingRuleTest {
/** {@inheritDoc} */
@Override
@DataProvider(name = "OrderingMatchingRuleInvalidValues")
public Object[][] createOrderingMatchingRuleInvalidValues() {
+ /*
+ * The JDK8 BigInteger parser is quite tolerant and allows leading zeros
+ * and + characters. It's ok if the matching rule is more tolerant than
+ * the syntax itself (see commented data).
+ */
return new Object[][] {
+ //{"01"},
+ //{"00"},
+ //{"-01"},
+ { "1-2" },
+ { "b2" },
+ { "-" },
+ { "" },
{" 63 "},
{"- 63"},
//{"+63" },
@@ -56,11 +68,14 @@
{"1", "0", 1},
{"1", "1", 0},
{"45", "54", -1},
- //{"-63", "63", -1},
- //{"-63", "0", -1},
+ {"-63", "63", -1},
+ {"-63", "0", -1},
{"63", "0", 1},
- //{"0", "-63", 1},
- //{"987654321987654321987654321", "987654321987654321987654322", -1},
+ {"0", "-63", 1},
+ // Values which are greater than 64 bits.
+ { "-987654321987654321987654321", "-987654321987654321987654322", 1 },
+ {"987654321987654321987654321", "987654321987654321987654322", -1},
+ { "987654321987654321987654321", "987654321987654321987654321", 0 },
};
}
--
Gitblit v1.10.0