From be346ea061098aebdddc2788c017b9815444ea07 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Thu, 22 Sep 2016 22:06:04 +0000
Subject: [PATCH] OPENDJ-2877: improve AttributeParser.requireValue() error message
---
opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeParser.java | 25 ++++++++++---------------
opendj-core/src/main/resources/com/forgerock/opendj/ldap/core.properties | 3 ++-
opendj-core/src/main/java/org/forgerock/opendj/ldap/AbstractEntry.java | 12 ++++++++----
opendj-core/src/test/java/org/forgerock/opendj/ldap/AttributeParserTestCase.java | 20 +++++++++-----------
4 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/AbstractEntry.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/AbstractEntry.java
index 766107b..3c85008 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/AbstractEntry.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/AbstractEntry.java
@@ -12,11 +12,13 @@
* information: "Portions Copyright [year] [name of copyright owner]".
*
* Copyright 2009-2010 Sun Microsystems, Inc.
- * Portions copyright 2012-2013 ForgeRock AS.
+ * Portions copyright 2012-2016 ForgeRock AS.
*/
package org.forgerock.opendj.ldap;
+import static org.forgerock.opendj.ldap.Attributes.emptyAttribute;
+
import java.util.Collection;
import java.util.Iterator;
@@ -153,12 +155,14 @@
@Override
public AttributeParser parseAttribute(final AttributeDescription attributeDescription) {
- return AttributeParser.parseAttribute(getAttribute(attributeDescription));
+ final Attribute attribute = getAttribute(attributeDescription);
+ return AttributeParser.parseAttribute(attribute != null ? attribute : emptyAttribute(attributeDescription));
}
@Override
public AttributeParser parseAttribute(final String attributeDescription) {
- return AttributeParser.parseAttribute(getAttribute(attributeDescription));
+ final Attribute attribute = getAttribute(attributeDescription);
+ return AttributeParser.parseAttribute(attribute != null ? attribute : emptyAttribute(attributeDescription));
}
@Override
@@ -191,7 +195,7 @@
@Override
public boolean removeAttribute(final AttributeDescription attributeDescription) {
- return removeAttribute(Attributes.emptyAttribute(attributeDescription), null);
+ return removeAttribute(emptyAttribute(attributeDescription), null);
}
@Override
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeParser.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeParser.java
index a916433..8387622 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeParser.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeParser.java
@@ -20,9 +20,11 @@
import java.util.LinkedHashSet;
import java.util.Set;
+import org.forgerock.i18n.LocalizedIllegalArgumentException;
import org.forgerock.opendj.ldap.schema.Schema;
import org.forgerock.util.Function;
+import static com.forgerock.opendj.ldap.CoreMessages.ERR_ATTRIBUTE_PARSER_MISSING_ATTRIBUTE;
import static com.forgerock.opendj.util.Collections2.*;
import static java.util.Arrays.asList;
import static org.forgerock.opendj.ldap.Functions.*;
@@ -57,9 +59,6 @@
*/
public final class AttributeParser {
// TODO: enums, filters, rdns?
-
- private static final AttributeParser NULL_INSTANCE = new AttributeParser(null);
-
/**
* Returns an attribute parser for the provided attribute. {@code null}
* attributes are permitted and will be treated as if an empty attribute was
@@ -70,7 +69,7 @@
* @return The attribute parser.
*/
public static AttributeParser parseAttribute(final Attribute attribute) {
- return isEmpty(attribute) ? NULL_INSTANCE : new AttributeParser(attribute);
+ return new AttributeParser(attribute);
}
private static boolean isEmpty(final Attribute attribute) {
@@ -641,17 +640,17 @@
}
/**
- * Throws a {@code NoSuchElementException} if the attribute referenced by
- * this parser is {@code null} or empty.
+ * Throws a {@code LocalizedIllegalArgumentException} if the attribute referenced by this parser is {@code null} or
+ * empty.
*
* @return A reference to this attribute parser.
- * @throws NoSuchElementException
- * If the attribute referenced by this parser is {@code null} or
- * empty.
+ * @throws LocalizedIllegalArgumentException
+ * If the attribute referenced by this parser is {@code null} or empty.
*/
public AttributeParser requireValue() {
if (isEmpty(attribute)) {
- throw new NoSuchElementException();
+ final String attributeName = attribute.getAttributeDescriptionAsString();
+ throw new LocalizedIllegalArgumentException(ERR_ATTRIBUTE_PARSER_MISSING_ATTRIBUTE.get(attributeName));
} else {
return this;
}
@@ -667,11 +666,7 @@
* @return This attribute parser.
*/
public AttributeParser usingSchema(final Schema schema) {
- // Avoid modifying the null instance: a schema will not be needed
- // anyway.
- if (this != NULL_INSTANCE) {
- this.schema = schema;
- }
+ this.schema = schema;
return this;
}
diff --git a/opendj-core/src/main/resources/com/forgerock/opendj/ldap/core.properties b/opendj-core/src/main/resources/com/forgerock/opendj/ldap/core.properties
index a2384c0..fded58f 100644
--- a/opendj-core/src/main/resources/com/forgerock/opendj/ldap/core.properties
+++ b/opendj-core/src/main/resources/com/forgerock/opendj/ldap/core.properties
@@ -1683,7 +1683,7 @@
name "%s": %s. To avoid ambiguity the name "%s" will only be associated with \
DIT content rule "%s"
WARN_DIT_SR_UNKNOWN=No DIT structure rule with name "%s" exists in the schema
-
+
# Labels for generated documentation
DOC_LOCALE_TAG=Code tag: %s
DOC_LOCALE_OID=Collation order object identifier: %s
@@ -1712,3 +1712,4 @@
can be matched against '%s'
ERR_CERT_NO_MATCH_SUBJECT=The host name contained in the subject DN '%s' \
does not match the host name '%s'
+ERR_ATTRIBUTE_PARSER_MISSING_ATTRIBUTE=The entry could not be parsed because the '%s' is missing
diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/AttributeParserTestCase.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/AttributeParserTestCase.java
index 56da512..a420c53 100644
--- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/AttributeParserTestCase.java
+++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/AttributeParserTestCase.java
@@ -11,15 +11,13 @@
* Header, with the fields enclosed by brackets [] replaced by your own identifying
* information: "Portions Copyright [year] [name of copyright owner]".
*
- * Copyright 2012-2015 ForgeRock AS.
+ * Copyright 2012-2016 ForgeRock AS.
*/
package org.forgerock.opendj.ldap;
import static org.fest.assertions.Assertions.assertThat;
-import java.util.NoSuchElementException;
-
import org.fest.util.Collections;
import org.forgerock.i18n.LocalizedIllegalArgumentException;
import org.testng.annotations.Test;
@@ -72,7 +70,7 @@
assertThat(e.parseAttribute("enabled").asBoolean(false)).isFalse();
}
- @Test(expectedExceptions = { NoSuchElementException.class })
+ @Test(expectedExceptions = { LocalizedIllegalArgumentException.class })
public void testAsBooleanMissingRequired() {
Entry e = new LinkedHashMapEntry("dn: cn=test", "objectClass: test");
e.parseAttribute("enabled").requireValue().asBoolean();
@@ -108,7 +106,7 @@
assertThat(e.parseAttribute("age").asInteger(100)).isEqualTo(100);
}
- @Test(expectedExceptions = { NoSuchElementException.class })
+ @Test(expectedExceptions = { LocalizedIllegalArgumentException.class })
public void testAsIntegerMissingRequired() {
Entry e = new LinkedHashMapEntry("dn: cn=test", "objectClass: test");
e.parseAttribute("age").requireValue().asInteger();
@@ -144,7 +142,7 @@
assertThat(e.parseAttribute("age").asLong(100)).isEqualTo(100);
}
- @Test(expectedExceptions = { NoSuchElementException.class })
+ @Test(expectedExceptions = { LocalizedIllegalArgumentException.class })
public void testAsLongMissingRequired() {
Entry e = new LinkedHashMapEntry("dn: cn=test", "objectClass: test");
e.parseAttribute("age").requireValue().asLong();
@@ -182,7 +180,7 @@
DN.valueOf("cn=boss"));
}
- @Test(expectedExceptions = { NoSuchElementException.class })
+ @Test(expectedExceptions = { LocalizedIllegalArgumentException.class })
public void testAsDNMissingRequired() {
Entry e = new LinkedHashMapEntry("dn: cn=test", "objectClass: test");
e.parseAttribute("manager").requireValue().asDN();
@@ -222,7 +220,7 @@
.isEqualTo(AttributeDescription.valueOf("sn"));
}
- @Test(expectedExceptions = { NoSuchElementException.class })
+ @Test(expectedExceptions = { LocalizedIllegalArgumentException.class })
public void testAsAttributeDescriptionMissingRequired() {
Entry e = new LinkedHashMapEntry("dn: cn=test", "objectClass: test");
e.parseAttribute("type").requireValue().asAttributeDescription();
@@ -259,7 +257,7 @@
String.valueOf("sn"));
}
- @Test(expectedExceptions = { NoSuchElementException.class })
+ @Test(expectedExceptions = { LocalizedIllegalArgumentException.class })
public void testAsStringMissingRequired() {
Entry e = new LinkedHashMapEntry("dn: cn=test", "objectClass: test");
e.parseAttribute("type").requireValue().asString();
@@ -291,7 +289,7 @@
ByteString.valueOfUtf8("sn"));
}
- @Test(expectedExceptions = { NoSuchElementException.class })
+ @Test(expectedExceptions = { LocalizedIllegalArgumentException.class })
public void testAsByteStringMissingRequired() {
Entry e = new LinkedHashMapEntry("dn: cn=test", "objectClass: test");
e.parseAttribute("type").requireValue().asByteString();
@@ -335,7 +333,7 @@
Collections.set(DN.valueOf("cn=dummy1"), DN.valueOf("cn=dummy2")));
}
- @Test(expectedExceptions = { NoSuchElementException.class })
+ @Test(expectedExceptions = { LocalizedIllegalArgumentException.class })
public void testAsSetOfDNMissingRequired() {
Entry e = new LinkedHashMapEntry("dn: cn=group", "objectClass: group");
e.parseAttribute("member").requireValue().asSetOfDN();
--
Gitblit v1.10.0