Fix OPENDJ-217: Unpredictable failures during schema validation using SDK

Matthew Swift

27.42.2011 d2ffb3eb61ac14df25d56086bb36b544890bb69e

Fix OPENDJ-217: Unpredictable failures during schema validation using SDK

Fix validation of attribute types, object classes, and DIT structure rules.
Re-enable tests.

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/AttributeType.java

@@ -31,7 +31,7 @@


import static org.forgerock.opendj.ldap.CoreMessages.*;
import static org.forgerock.opendj.ldap.schema.SchemaConstants.SCHEMA_PROPERTY_APPROX_RULE;
import static org.forgerock.opendj.ldap.schema.SchemaConstants.*;

import java.util.Collections;
import java.util.Iterator;
@@ -128,6 +128,12 @@
  // The syntax for this attribute type.
  private Syntax syntax;

  // Indicates whether or not validation has been performed.
  private boolean needsValidating = true;

  // The indicates whether or not validation failed.
  private boolean isValid = false;



  AttributeType(final String oid, final List<String> names,
@@ -698,13 +704,20 @@



  /**
   * {@inheritDoc}
   */
  @Override
  void validate(final List<LocalizableMessage> warnings, final Schema schema)
      throws SchemaException
  boolean validate(final Schema schema,
      final List<AttributeType> invalidSchemaElements,
      final List<LocalizableMessage> warnings)
  {
    // Avoid validating this schema element more than once. This may occur if
    // multiple attributes specify the same superior.
    if (!needsValidating)
    {
      return isValid;
    }

    // Prevent re-validation.
    needsValidating = false;

    if (superiorTypeOID != null)
    {
      try
@@ -715,7 +728,18 @@
      {
        final LocalizableMessage message = WARN_ATTR_SYNTAX_ATTRTYPE_UNKNOWN_SUPERIOR_TYPE1
            .get(getNameOrOID(), superiorTypeOID);
        throw new SchemaException(message);
        failValidation(invalidSchemaElements, warnings, message);
        return false;
      }

      // First ensure that the superior has been validated and fail if it is
      // invalid.
      if (!superiorType.validate(schema, invalidSchemaElements, warnings))
      {
        final LocalizableMessage message = WARN_ATTR_SYNTAX_ATTRTYPE_INVALID_SUPERIOR_TYPE
            .get(getNameOrOID(), superiorTypeOID);
        failValidation(invalidSchemaElements, warnings, message);
        return false;
      }

      // If there is a superior type, then it must have the same usage
@@ -726,7 +750,8 @@
        final LocalizableMessage message = WARN_ATTR_SYNTAX_ATTRTYPE_INVALID_SUPERIOR_USAGE
            .get(getNameOrOID(), getUsage().toString(), superiorType
                .getNameOrOID());
        throw new SchemaException(message);
        failValidation(invalidSchemaElements, warnings, message);
        return false;
      }

      if (superiorType.isCollective() != isCollective())
@@ -736,7 +761,8 @@
          LocalizableMessage message =
            WARN_ATTR_SYNTAX_ATTRTYPE_NONCOLLECTIVE_FROM_COLLECTIVE
              .get(getNameOrOID(), superiorType.getNameOrOID());
          throw new SchemaException(message);
          failValidation(invalidSchemaElements, warnings, message);
          return false;
        }
      }
    }
@@ -775,7 +801,8 @@
      {
        final LocalizableMessage message = WARN_ATTR_SYNTAX_ATTRTYPE_UNKNOWN_EQUALITY_MR1
            .get(getNameOrOID(), equalityMatchingRuleOID);
        throw new SchemaException(message);
        failValidation(invalidSchemaElements, warnings, message);
        return false;
      }
    }
    else if (getSuperiorType() != null
@@ -802,7 +829,8 @@
      {
        final LocalizableMessage message = WARN_ATTR_SYNTAX_ATTRTYPE_UNKNOWN_ORDERING_MR1
            .get(getNameOrOID(), orderingMatchingRuleOID);
        throw new SchemaException(message);
        failValidation(invalidSchemaElements, warnings, message);
        return false;
      }
    }
    else if (getSuperiorType() != null
@@ -830,7 +858,8 @@
      {
        final LocalizableMessage message = WARN_ATTR_SYNTAX_ATTRTYPE_UNKNOWN_SUBSTRING_MR1
            .get(getNameOrOID(), substringMatchingRuleOID);
        throw new SchemaException(message);
        failValidation(invalidSchemaElements, warnings, message);
        return false;
      }
    }
    else if (getSuperiorType() != null
@@ -858,7 +887,8 @@
      {
        final LocalizableMessage message = WARN_ATTR_SYNTAX_ATTRTYPE_UNKNOWN_APPROXIMATE_MR1
            .get(getNameOrOID(), approximateMatchingRuleOID);
        throw new SchemaException(message);
        failValidation(invalidSchemaElements, warnings, message);
        return false;
      }
    }
    else if (getSuperiorType() != null
@@ -892,5 +922,16 @@
          .get(getNameOrOID());
      warnings.add(message);
    }

    return (isValid = true);
  }



  private void failValidation(final List<AttributeType> invalidSchemaElements,
      final List<LocalizableMessage> warnings, final LocalizableMessage message)
  {
    invalidSchemaElements.add(this);
    warnings.add(ERR_ATTR_TYPE_VALIDATION_FAIL.get(toString(), message));
  }
}

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITContentRule.java

@@ -507,8 +507,7 @@



  @Override
  void validate(final List<LocalizableMessage> warnings, final Schema schema)
  void validate(final Schema schema, final List<LocalizableMessage> warnings)
      throws SchemaException
  {
    // Get the objectclass with the specified OID. If it does not exist

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITStructureRule.java

@@ -29,8 +29,7 @@



import static org.forgerock.opendj.ldap.CoreMessages.ERR_ATTR_SYNTAX_DSR_UNKNOWN_NAME_FORM;
import static org.forgerock.opendj.ldap.CoreMessages.ERR_ATTR_SYNTAX_DSR_UNKNOWN_RULE_ID;
import static org.forgerock.opendj.ldap.CoreMessages.*;

import java.util.*;

@@ -67,6 +66,12 @@
  private NameForm nameForm;
  private Set<DITStructureRule> superiorRules = Collections.emptySet();

  // Indicates whether or not validation has been performed.
  private boolean needsValidating = true;

  // The indicates whether or not validation failed.
  private boolean isValid = false;



  DITStructureRule(final Integer ruleID, final List<String> names,
@@ -305,10 +310,20 @@



  @Override
  void validate(final List<LocalizableMessage> warnings, final Schema schema)
      throws SchemaException
  boolean validate(final Schema schema,
      final List<DITStructureRule> invalidSchemaElements,
      final List<LocalizableMessage> warnings)
  {
    // Avoid validating this schema element more than once. This may occur if
    // multiple rules specify the same superior.
    if (!needsValidating)
    {
      return isValid;
    }

    // Prevent re-validation.
    needsValidating = false;

    try
    {
      nameForm = schema.getNameForm(nameFormOID);
@@ -317,7 +332,8 @@
    {
      final LocalizableMessage message = ERR_ATTR_SYNTAX_DSR_UNKNOWN_NAME_FORM
          .get(getNameOrRuleID(), nameFormOID);
      throw new SchemaException(message, e);
      failValidation(invalidSchemaElements, warnings, message);
      return false;
    }

    if (!superiorRuleIDs.isEmpty())
@@ -334,11 +350,23 @@
        {
          final LocalizableMessage message = ERR_ATTR_SYNTAX_DSR_UNKNOWN_RULE_ID
              .get(getNameOrRuleID(), id);
          throw new SchemaException(message, e);
          failValidation(invalidSchemaElements, warnings, message);
          return false;
        }
        superiorRules.add(rule);
      }
    }
    superiorRules = Collections.unmodifiableSet(superiorRules);

    return (isValid = true);
  }



  private void failValidation(final List<DITStructureRule> invalidSchemaElements,
      final List<LocalizableMessage> warnings, final LocalizableMessage message)
  {
    invalidSchemaElements.add(this);
    warnings.add(ERR_DSR_VALIDATION_FAIL.get(toString(), message));
  }
}

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java

@@ -439,8 +439,7 @@



  @Override
  void validate(final List<LocalizableMessage> warnings, final Schema schema)
  void validate(final Schema schema, final List<LocalizableMessage> warnings)
      throws SchemaException
  {
    // Try finding an implementation in the core schema

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRuleUse.java

@@ -333,8 +333,7 @@



  @Override
  void validate(final List<LocalizableMessage> warnings, final Schema schema)
  void validate(final Schema schema, final List<LocalizableMessage> warnings)
      throws SchemaException
  {
    try

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/NameForm.java

@@ -422,8 +422,7 @@



  @Override
  void validate(final List<LocalizableMessage> warnings, final Schema schema)
  void validate(final Schema schema, final List<LocalizableMessage> warnings)
      throws SchemaException
  {
    try

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java

@@ -84,7 +84,12 @@
  private Set<AttributeType> declaredOptionalAttributes = Collections
      .emptySet();
  private Set<AttributeType> optionalAttributes = Collections.emptySet();
  private boolean validated = false;

  // Indicates whether or not validation has been performed.
  private boolean needsValidating = true;

  // The indicates whether or not validation failed.
  private boolean isValid = false;



@@ -580,15 +585,19 @@



  @Override
  void validate(final List<LocalizableMessage> warnings, final Schema schema)
      throws SchemaException
  boolean validate(final Schema schema,
      final List<ObjectClass> invalidSchemaElements,
      final List<LocalizableMessage> warnings)
  {
    if (validated)
    // Avoid validating this schema element more than once. This may occur if
    // multiple object classes specify the same superior.
    if (!needsValidating)
    {
      return;
      return isValid;
    }
    validated = true;

    // Prevent re-validation.
    needsValidating = false;

    // Init a flag to check to inheritance from top (only needed for
    // structural object classes) per RFC 4512
@@ -608,7 +617,8 @@
        {
          final LocalizableMessage message = WARN_ATTR_SYNTAX_OBJECTCLASS_UNKNOWN_SUPERIOR_CLASS1
              .get(getNameOrOID(), superClassOid);
          throw new SchemaException(message, e);
          failValidation(invalidSchemaElements, warnings, message);
          return false;
        }

        // Make sure that the inheritance configuration is acceptable.
@@ -623,7 +633,8 @@
            final LocalizableMessage message = WARN_ATTR_SYNTAX_OBJECTCLASS_INVALID_SUPERIOR_TYPE1
                .get(getNameOrOID(), objectClassType.toString(), superiorType.toString(),
                    superiorClass.getNameOrOID());
            throw new SchemaException(message);
            failValidation(invalidSchemaElements, warnings, message);
            return false;
          }
          break;

@@ -636,7 +647,8 @@
            final LocalizableMessage message = WARN_ATTR_SYNTAX_OBJECTCLASS_INVALID_SUPERIOR_TYPE1
                .get(getNameOrOID(), objectClassType.toString(), superiorType.toString(),
                    superiorClass.getNameOrOID());
            throw new SchemaException(message);
            failValidation(invalidSchemaElements, warnings, message);
            return false;
          }
          break;

@@ -649,7 +661,8 @@
            final LocalizableMessage message = WARN_ATTR_SYNTAX_OBJECTCLASS_INVALID_SUPERIOR_TYPE1
                .get(getNameOrOID(), objectClassType.toString(), superiorType.toString(),
                    superiorClass.getNameOrOID());
            throw new SchemaException(message);
            failValidation(invalidSchemaElements, warnings, message);
            return false;
          }
          break;
        }
@@ -661,9 +674,15 @@
          derivesTop = true;
        }

        // Validate superior object class so we can inherit its
        // attributes.
        superiorClass.validate(warnings, schema);
        // First ensure that the superior has been validated and fail if it is
        // invalid.
        if (!superiorClass.validate(schema, invalidSchemaElements, warnings))
        {
          final LocalizableMessage message = WARN_ATTR_SYNTAX_OBJECTCLASS_INVALID_SUPERIOR_CLASS
              .get(getNameOrOID(), superClassOid);
          failValidation(invalidSchemaElements, warnings, message);
          return false;
        }

        // Inherit all required attributes from superior class.
        Iterator<AttributeType> i = superiorClass.getRequiredAttributes()
@@ -703,7 +722,8 @@
    {
      final LocalizableMessage message = WARN_ATTR_SYNTAX_OBJECTCLASS_STRUCTURAL_SUPERIOR_NOT_TOP1
          .get(getNameOrOID());
      throw new SchemaException(message);
      failValidation(invalidSchemaElements, warnings, message);
      return false;
    }

    if (oid.equals(EXTENSIBLE_OBJECT_OBJECTCLASS_OID))
@@ -739,7 +759,8 @@
            // about.
            final LocalizableMessage message = WARN_ATTR_SYNTAX_OBJECTCLASS_UNKNOWN_REQUIRED_ATTR1
                .get(getNameOrOID(), requiredAttribute);
            throw new SchemaException(message, e);
            failValidation(invalidSchemaElements, warnings, message);
            return false;
          }
          declaredRequiredAttributes.add(attributeType);
        }
@@ -771,7 +792,8 @@
            // about.
            final LocalizableMessage message = WARN_ATTR_SYNTAX_OBJECTCLASS_UNKNOWN_OPTIONAL_ATTR1
                .get(getNameOrOID(), optionalAttribute);
            throw new SchemaException(message, e);
            failValidation(invalidSchemaElements, warnings, message);
            return false;
          }
          declaredOptionalAttributes.add(attributeType);
        }
@@ -793,5 +815,16 @@
    optionalAttributes = Collections.unmodifiableSet(optionalAttributes);
    requiredAttributes = Collections.unmodifiableSet(requiredAttributes);
    superiorClasses = Collections.unmodifiableSet(superiorClasses);

    return (isValid = true);
  }



  private void failValidation(final List<ObjectClass> invalidSchemaElements,
      final List<LocalizableMessage> warnings, final LocalizableMessage message)
  {
    invalidSchemaElements.add(this);
    warnings.add(ERR_OC_VALIDATION_FAIL.get(toString(), message));
  }
}

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java

@@ -492,6 +492,13 @@
        extraProperties = Collections.unmodifiableMap(extraProperties);
      }

      if (superiorType == null && syntax == null)
      {
        final LocalizableMessage msg = WARN_ATTR_SYNTAX_ATTRTYPE_MISSING_SYNTAX_AND_SUPERIOR
            .get(definition);
        throw new LocalizedIllegalArgumentException(msg);
      }

      final AttributeType attrType = new AttributeType(oid, names, description,
          isObsolete, superiorType, equalityMatchingRule, orderingMatchingRule,
          substringMatchingRule, approximateMatchingRule, syntax,
@@ -3450,7 +3457,7 @@
    {
      try
      {
        syntax.validate(warnings, schema);
        syntax.validate(schema, warnings);
      }
      catch (final SchemaException e)
      {
@@ -3465,7 +3472,7 @@
    {
      try
      {
        rule.validate(warnings, schema);
        rule.validate(schema, warnings);
      }
      catch (final SchemaException e)
      {
@@ -3475,34 +3482,30 @@
      }
    }

    for (final AttributeType attribute : numericOID2AttributeTypes.values()
        .toArray(new AttributeType[numericOID2AttributeTypes.values().size()]))
    // Attribute types need special processing because they have hierarchical
    // dependencies.
    List<AttributeType> invalidAttributeTypes = new LinkedList<AttributeType>();
    for (final AttributeType attributeType : numericOID2AttributeTypes.values())
    {
      try
      {
        attribute.validate(warnings, schema);
      }
      catch (final SchemaException e)
      {
        removeAttributeType(attribute);
        warnings.add(ERR_ATTR_TYPE_VALIDATION_FAIL.get(attribute.toString(),
            e.getMessageObject()));
      }
      attributeType.validate(schema, invalidAttributeTypes, warnings);
    }

    for (final ObjectClass oc : numericOID2ObjectClasses.values().toArray(
        new ObjectClass[numericOID2ObjectClasses.values().size()]))
    for (AttributeType attributeType : invalidAttributeTypes)
    {
      try
      {
        oc.validate(warnings, schema);
      removeAttributeType(attributeType);
      }
      catch (final SchemaException e)

    // Object classes need special processing because they have hierarchical
    // dependencies.
    List<ObjectClass> invalidObjectClasses = new LinkedList<ObjectClass>();
    for (final ObjectClass objectClass : numericOID2ObjectClasses.values())
      {
        removeObjectClass(oc);
        warnings.add(ERR_OC_VALIDATION_FAIL.get(oc.toString(),
            e.getMessageObject()));
      objectClass.validate(schema, invalidObjectClasses, warnings);
      }

    for (ObjectClass objectClass : invalidObjectClasses)
    {
      removeObjectClass(objectClass);
    }

    for (final MatchingRuleUse use : numericOID2MatchingRuleUses.values()
@@ -3511,7 +3514,7 @@
    {
      try
      {
        use.validate(warnings, schema);
        use.validate(schema, warnings);
      }
      catch (final SchemaException e)
      {
@@ -3526,7 +3529,7 @@
    {
      try
      {
        form.validate(warnings, schema);
        form.validate(schema, warnings);

        // build the objectClass2NameForms map
        List<NameForm> forms;
@@ -3559,7 +3562,7 @@
    {
      try
      {
        rule.validate(warnings, schema);
        rule.validate(schema, warnings);
      }
      catch (final SchemaException e)
      {
@@ -3569,13 +3572,21 @@
      }
    }

    for (final DITStructureRule rule : id2StructureRules.values().toArray(
        new DITStructureRule[id2StructureRules.values().size()]))
    // DIT structure rules need special processing because they have hierarchical
    // dependencies.
    List<DITStructureRule> invalidStructureRules = new LinkedList<DITStructureRule>();
    for (final DITStructureRule rule : id2StructureRules.values())
    {
      try
      {
        rule.validate(warnings, schema);
      rule.validate(schema, invalidStructureRules, warnings);
    }

    for (DITStructureRule rule : invalidStructureRules)
    {
      removeDITStructureRule(rule);
    }

    for (final DITStructureRule rule : id2StructureRules.values())
    {
        // build the nameForm2StructureRules map
        List<DITStructureRule> rules;
        final String ocOID = rule.getNameForm().getOID();
@@ -3594,12 +3605,5 @@
          rules.add(rule);
        }
      }
      catch (final SchemaException e)
      {
        removeDITStructureRule(rule);
        warnings.add(ERR_DSR_VALIDATION_FAIL.get(rule.toString(),
            e.getMessageObject()));
      }
    }
  }
}

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaElement.java

@@ -23,6 +23,7 @@
 *
 *
 *      Copyright 2009 Sun Microsystems, Inc.
 *      Portions copyright 2011 ForgeRock AS
 */

package org.forgerock.opendj.ldap.schema;
@@ -34,8 +35,6 @@
import java.util.Map;
import java.util.Set;

import org.forgerock.i18n.LocalizableMessage;

import com.forgerock.opendj.util.Validator;


@@ -181,9 +180,4 @@
   *          The buffer to which the information should be appended.
   */
  abstract void toStringContent(StringBuilder buffer);



  abstract void validate(List<LocalizableMessage> warnings, Schema schema)
      throws SchemaException;
}

 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java

@@ -281,8 +281,7 @@



  @Override
  void validate(final List<LocalizableMessage> warnings, final Schema schema)
  void validate(final Schema schema, final List<LocalizableMessage> warnings)
      throws SchemaException
  {
    this.schema = schema;
@@ -320,7 +319,7 @@
            if (subSyntax.impl == null)
            {
              // The substitution syntax was never validated.
              subSyntax.validate(warnings, schema);
              subSyntax.validate(schema, warnings);
            }
            impl = subSyntax.impl;
          }

 opendj3/opendj-ldap-sdk/src/main/resources/org/forgerock/opendj/ldap/core.properties

@@ -1151,7 +1151,7 @@
 parsed as a valid attribute type description because it was empty or \
 contained only whitespace
WARN_ATTR_SYNTAX_ATTRTYPE_INVALID_ATTRIBUTE_USAGE1=The provided value "%s" \
 definition could not be parsed as a valid attribute type description because \
 could not be parsed as a valid attribute type description because \
 it declared that it should have an attribute usage of %s. This is an invalid usage
ERR_ATTR_SYNTAX_OBJECTCLASS_EMPTY_VALUE1=The provided value "%s" could not \
 be parsed as a valid object class description because it was empty or \
@@ -1372,3 +1372,16 @@
ERR_ENTRY_SCHEMA_DSR_MISSING_DSR=Entry "%s" violates the schema because \
 there is no DIT structure rule that applies to the entry, but there is a DIT \
 structure rule "%s" which applies to the parent entry
WARN_ATTR_SYNTAX_ATTRTYPE_MISSING_SYNTAX_AND_SUPERIOR=The provided value "%s" \
 could not be parsed as a valid attribute type description because \
 it does not declare a syntax nor a superior type. Attribute type descriptions \
 must declare a superior type or a syntax
WARN_ATTR_SYNTAX_ATTRTYPE_INVALID_SUPERIOR_TYPE=The definition for \
 the attribute type "%s" declared a superior type "%s" which has been removed \
 from the schema because it is invalid
WARN_ATTR_SYNTAX_OBJECTCLASS_INVALID_SUPERIOR_CLASS=The definition for \
 the object class "%s" declared a superior object class "%s" which has been \
 removed from the schema because it is invalid
WARN_ATTR_SYNTAX_DSR_INVALID_SUPERIOR_RULE=The definition for \
 the DIT structure rule "%s" declared a superior rule "%s" which has been \
 removed from the schema because it is invalid

 opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/schema/SchemaBuilderTest.java

@@ -46,7 +46,7 @@
   * Tests that schema validation resolves dependencies between parent/child
   * attribute types regardless of the order in which they were added.
   */
  @Test(enabled = false)
  @Test
  public void testAttributeTypeDependenciesChildThenParent()
  {
    final Schema schema = new SchemaBuilder(Schema.getCoreSchema())
@@ -85,7 +85,7 @@
  /**
   * Tests that attribute types must have a syntax or a superior.
   */
  @Test(enabled = false, expectedExceptions = LocalizedIllegalArgumentException.class)
  @Test(expectedExceptions = LocalizedIllegalArgumentException.class)
  public void testAttributeTypeNoSuperiorNoSyntax()
  {
    new SchemaBuilder(Schema.getCoreSchema()).addAttributeType(
@@ -98,7 +98,7 @@
   * Tests that schema validation handles validation failures for superior
   * attribute types regardless of the order.
   */
  @Test(enabled = false)
  @Test
  public void testAttributeTypeSuperiorFailureChildThenParent()
  {
    final Schema schema = new SchemaBuilder(Schema.getCoreSchema())
@@ -233,7 +233,7 @@
   * Tests that schema validation handles validation failures for superior
   * object classes regardless of the order.
   */
  @Test(enabled = false)
  @Test
  public void testObjectClassSuperiorFailureChildThenParent()
  {
    final Schema schema = new SchemaBuilder(Schema.getCoreSchema())