From d2ffb3eb61ac14df25d56086bb36b544890bb69e Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Mon, 27 Jun 2011 16:42:54 +0000
Subject: [PATCH] Fix OPENDJ-217: Unpredictable failures during schema validation using SDK

---
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaElement.java     |    8 -
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java            |    5 
 opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/schema/SchemaBuilderTest.java |    8 
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/AttributeType.java     |   69 +++++++++--
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/NameForm.java          |    3 
 opendj3/opendj-ldap-sdk/src/main/resources/org/forgerock/opendj/ldap/core.properties          |   15 ++
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java     |  116 ++++++++++---------
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITStructureRule.java  |   42 +++++-
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java      |    3 
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRuleUse.java   |    3 
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITContentRule.java    |    3 
 opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java       |   67 ++++++++--
 12 files changed, 225 insertions(+), 117 deletions(-)

diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/AttributeType.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/AttributeType.java
index d697d00..3bc424b 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/AttributeType.java
+++ b/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));
   }
 }
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITContentRule.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITContentRule.java
index bb898db..a45e2ba 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITContentRule.java
+++ b/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
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITStructureRule.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITStructureRule.java
index 36bddbc..0d2cb87 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/DITStructureRule.java
+++ b/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));
   }
 }
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java
index fcd4ae8..c77a0ca 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java
+++ b/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
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRuleUse.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRuleUse.java
index 54eec8c..1bd90c5 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRuleUse.java
+++ b/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
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/NameForm.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/NameForm.java
index e876bf6..cce207d 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/NameForm.java
+++ b/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
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java
index ece9db7..4bc909a 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java
+++ b/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));
   }
 }
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java
index d9613e1..187aaea 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java
+++ b/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);
-      }
-      catch (final SchemaException e)
-      {
-        removeObjectClass(oc);
-        warnings.add(ERR_OC_VALIDATION_FAIL.get(oc.toString(),
-            e.getMessageObject()));
-      }
+      removeAttributeType(attributeType);
+    }
+
+    // Object classes need special processing because they have hierarchical
+    // dependencies.
+    List<ObjectClass> invalidObjectClasses = new LinkedList<ObjectClass>();
+    for (final ObjectClass objectClass : numericOID2ObjectClasses.values())
+    {
+      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,36 +3572,37 @@
       }
     }
 
-    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);
+    }
 
-        // build the nameForm2StructureRules map
-        List<DITStructureRule> rules;
-        final String ocOID = rule.getNameForm().getOID();
-        if ((rules = nameForm2StructureRules.get(ocOID)) == null)
-        {
-          nameForm2StructureRules.put(ocOID, Collections.singletonList(rule));
-        }
-        else if (rules.size() == 1)
-        {
-          rules = new ArrayList<DITStructureRule>(rules);
-          rules.add(rule);
-          nameForm2StructureRules.put(ocOID, rules);
-        }
-        else
-        {
-          rules.add(rule);
-        }
-      }
-      catch (final SchemaException e)
+    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();
+      if ((rules = nameForm2StructureRules.get(ocOID)) == null)
       {
-        removeDITStructureRule(rule);
-        warnings.add(ERR_DSR_VALIDATION_FAIL.get(rule.toString(),
-            e.getMessageObject()));
+        nameForm2StructureRules.put(ocOID, Collections.singletonList(rule));
+      }
+      else if (rules.size() == 1)
+      {
+        rules = new ArrayList<DITStructureRule>(rules);
+        rules.add(rule);
+        nameForm2StructureRules.put(ocOID, rules);
+      }
+      else
+      {
+        rules.add(rule);
       }
     }
   }
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaElement.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaElement.java
index f608246..09f1340 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/SchemaElement.java
+++ b/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;
 }
diff --git a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java b/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java
index 630235c..f200816 100644
--- a/opendj3/opendj-ldap-sdk/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java
+++ b/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;
           }
diff --git a/opendj3/opendj-ldap-sdk/src/main/resources/org/forgerock/opendj/ldap/core.properties b/opendj3/opendj-ldap-sdk/src/main/resources/org/forgerock/opendj/ldap/core.properties
index 5a6cc2f..0c3d7fd 100755
--- a/opendj3/opendj-ldap-sdk/src/main/resources/org/forgerock/opendj/ldap/core.properties
+++ b/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
diff --git a/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/schema/SchemaBuilderTest.java b/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/schema/SchemaBuilderTest.java
index 1aa4537..e2374cb 100644
--- a/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/schema/SchemaBuilderTest.java
+++ b/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())

--
Gitblit v1.10.0