From 09b4b3b022e722b1c7a34cf376eb2d11d0d1c8b1 Mon Sep 17 00:00:00 2001
From: Nicolas Capponi <nicolas.capponi@forgerock.com>
Date: Mon, 03 Mar 2014 16:07:24 +0000
Subject: [PATCH] Fix OPENDJ-1361 : Matching Rules and Syntaxes have a reference to a schema  that can be changed, leading to unexpected behavior

---
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java                             |   50 +++++++++++++++++++++++--
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CertificateExactMatchingRuleImplTest.java      |    3 -
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/MatchingRuleTest.java                          |    3 -
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java                              |   10 +++++
 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/DistinguishedNameEqualityMatchingRuleTest.java |   10 +++-
 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java                                    |   10 +++++
 6 files changed, 73 insertions(+), 13 deletions(-)

diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java
index 7ae600d..0e7bdcc 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/MatchingRule.java
@@ -540,4 +540,14 @@
 
         this.schema = schema;
     }
+
+    /**
+     * Indicates if the matching rule has been validated, which means it has a
+     * non-null schema.
+     *
+     * @return {@code true} if and only if this matching rule has been validated
+     */
+    boolean isValidated() {
+        return schema != null;
+    }
 }
diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java
index fc5956a..ccbf1f2 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/SchemaBuilder.java
@@ -1476,7 +1476,23 @@
      * @return A matching rule builder.
      */
     public MatchingRule.Builder buildMatchingRule(final MatchingRule matchingRule) {
-        lazyInitBuilder();
+        return buildMatchingRule(matchingRule, true);
+    }
+
+    /**
+     * Duplicates the matching rule.
+     *
+     * @param matchingRule
+     *            The matching rule to duplicate.
+     * @param initialize
+     *            Indicates if initialization should be attempted. Use
+     *            {@code false} to prevent it.
+     * @return A matching rule builder.
+     */
+    private MatchingRule.Builder buildMatchingRule(final MatchingRule matchingRule, final boolean initialize) {
+        if (initialize) {
+            lazyInitBuilder();
+        }
         return new MatchingRule.Builder(matchingRule, this);
     }
 
@@ -1500,7 +1516,24 @@
      * @return A syntax builder.
      */
     public Syntax.Builder buildSyntax(final Syntax syntax) {
-        lazyInitBuilder();
+        return buildSyntax(syntax, true);
+    }
+
+    /**
+     * Duplicates the syntax, with or without lazy initialization
+     * of the schema.
+     *
+     * @param syntax
+     *            The syntax to duplicate.
+     * @param initialize
+     *            Indicates if initialization should be attempted.
+     *            Use {@code false} to prevent it.
+     * @return A syntax builder.
+     */
+    private Syntax.Builder buildSyntax(final Syntax syntax, final boolean initialize) {
+        if (initialize) {
+            lazyInitBuilder();
+        }
         return new Syntax.Builder(syntax, this);
     }
 
@@ -2679,6 +2712,7 @@
     }
 
     SchemaBuilder addMatchingRule(final MatchingRule rule, final boolean overwrite) {
+        assert !rule.isValidated() : "Matching rule has already been validated, it can't be added";
         MatchingRule conflictingRule;
         if (numericOID2MatchingRules.containsKey(rule.getOID())) {
             conflictingRule = numericOID2MatchingRules.get(rule.getOID());
@@ -2773,11 +2807,19 @@
         // unlikely, may be different in the new schema.
 
         for (final Syntax syntax : schema.getSyntaxes()) {
-            addSyntax(syntax, overwrite);
+            if (overwrite) {
+                buildSyntax(syntax, false).addToSchemaOverwrite();
+            } else {
+                buildSyntax(syntax, false).addToSchema();
+            }
         }
 
         for (final MatchingRule matchingRule : schema.getMatchingRules()) {
-            addMatchingRule(matchingRule, overwrite);
+            if (overwrite) {
+                buildMatchingRule(matchingRule, false).addToSchemaOverwrite();
+            } else {
+                buildMatchingRule(matchingRule, false).addToSchema();
+            }
         }
 
         for (final MatchingRuleUse matchingRuleUse : schema.getMatchingRuleUses()) {
diff --git a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java
index 7b8e53d..af61d90 100644
--- a/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java
+++ b/opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/Syntax.java
@@ -453,4 +453,14 @@
             }
         }
     }
+
+    /**
+     * Indicates if the syntax has been validated, which means it has a non-null
+     * schema.
+     *
+     * @return {@code true} if and only if this syntax has been validated
+     */
+    boolean isValidated() {
+        return schema != null;
+    }
 }
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CertificateExactMatchingRuleImplTest.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CertificateExactMatchingRuleImplTest.java
index 1eb2852..bf5eb42 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CertificateExactMatchingRuleImplTest.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/CertificateExactMatchingRuleImplTest.java
@@ -161,9 +161,6 @@
     @Test(dataProvider = "certificateExactMatchingRules")
     public void certificateExactMatchingRules(ByteString attributeValue,
             ByteString assertionValue, ConditionResult result) throws DecodeException {
-        // TODO : workaround to make test pass until issue OPENDJ-1361 is fixed
-        new SchemaBuilder("workaround").addSchema(Schema.getCoreSchema(), true).toSchema();
-
         MatchingRule rule = getRule();
 
         // normalize the 2 provided values and check that they are equal
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/DistinguishedNameEqualityMatchingRuleTest.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/DistinguishedNameEqualityMatchingRuleTest.java
index 1a91ad1..c7d3bbf 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/DistinguishedNameEqualityMatchingRuleTest.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/DistinguishedNameEqualityMatchingRuleTest.java
@@ -27,6 +27,7 @@
 
 package org.forgerock.opendj.ldap.schema;
 
+import static org.fest.assertions.Assertions.*;
 import static org.testng.Assert.assertEquals;
 
 import org.forgerock.opendj.ldap.ByteString;
@@ -185,18 +186,21 @@
         return CoreSchema.getDistinguishedNameMatchingRule();
     }
 
+    @Test
+    public void testIsValidated() {
+        assertThat(getRule().isValidated()).isTrue();
+    }
+
     /**
      * Test the normalized values
      */
     @Test(dataProvider = "testDNs")
     public void testNormalization(final String value1, final String value2) throws Exception {
-        // TODO : workaround to make test pass until issue OPENDJ-1361 is fixed
-        new SchemaBuilder("workaround").addSchema(Schema.getCoreSchema(), true).toSchema();
-
         final MatchingRule rule = getRule();
         final ByteString normalizedValue1 =
                 rule.normalizeAttributeValue(ByteString.valueOf(value1));
         final ByteString expectedValue = ByteString.valueOf(value2);
         assertEquals(normalizedValue1, expectedValue);
     }
+
 }
diff --git a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/MatchingRuleTest.java b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/MatchingRuleTest.java
index 3cf7baf..59e1a8b 100644
--- a/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/MatchingRuleTest.java
+++ b/opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/MatchingRuleTest.java
@@ -71,9 +71,6 @@
     @Test(dataProvider = "matchingrules")
     public void matchingRules(final String value1, final String value2, final ConditionResult result)
             throws Exception {
-        // TODO : workaround to make test pass until issue OPENDJ-1361 is fixed
-        new SchemaBuilder("workaround").addSchema(Schema.getCoreSchema(), true).toSchema();
-
         final MatchingRule rule = getRule();
 
         // normalize the 2 provided values and check that they are equals

--
Gitblit v1.10.0