mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Matthew Swift
22.23.2016 be346ea061098aebdddc2788c017b9815444ea07
OPENDJ-2877: improve AttributeParser.requireValue() error message

Allocate a temporary empty attribute when the attribute is not present
so that the attribute name can be included in the exception thrown by
requireValue().
4 files modified
56 ■■■■ changed files
opendj-core/src/main/java/org/forgerock/opendj/ldap/AbstractEntry.java 12 ●●●●● patch | view | raw | blame | history
opendj-core/src/main/java/org/forgerock/opendj/ldap/AttributeParser.java 23 ●●●●● patch | view | raw | blame | history
opendj-core/src/main/resources/com/forgerock/opendj/ldap/core.properties 1 ●●●● patch | view | raw | blame | history
opendj-core/src/test/java/org/forgerock/opendj/ldap/AttributeParserTestCase.java 20 ●●●●● patch | view | raw | blame | history
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
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;
        }
        return this;
    }
opendj-core/src/main/resources/com/forgerock/opendj/ldap/core.properties
@@ -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
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();