From 053e00fd90104453e80c90f6e8cda26471198e0e Mon Sep 17 00:00:00 2001
From: Nicolas Capponi <nicolas.capponi@forgerock.com>
Date: Fri, 13 May 2016 14:27:28 +0000
Subject: [PATCH] OPENDJ-3010 Fix Extensible ObjectClass to be consistent wrt optional attributes
---
opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/ObjectClassTestCase.java | 22 +++++++++--
opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/EntrySchemaCheckingTestCase.java | 19 +++++++--
opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java | 44 +++++++++++++++-------
3 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java
index 10eddf3..1b9e3df 100644
--- a/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java
+++ b/opendj-core/src/main/java/org/forgerock/opendj/ldap/schema/ObjectClass.java
@@ -67,7 +67,12 @@
this.type = oc.objectClassType;
this.superiorClasses.addAll(oc.superiorClassOIDs);
this.requiredAttributes.addAll(oc.requiredAttributeOIDs);
- this.optionalAttributes.addAll(oc.optionalAttributeOIDs);
+ // Don't copy optional attributes for extensibleObject because they will
+ // prevent attribute types from being removed from the schema.
+ // The optional attributes will be refreshed during validation.
+ if (!oc.isExtensible()) {
+ this.optionalAttributes.addAll(oc.optionalAttributeOIDs);
+ }
}
Builder(final String oid, final SchemaBuilder builder) {
@@ -392,10 +397,11 @@
/** Indicates whether or not validation has been performed. */
private boolean needsValidating = true;
- /** The indicates whether or not validation failed. */
+ /** Indicates whether or not validation failed. */
private boolean isValid;
- /** Whether this is the extensible object class, which allows any attribute types. */
- private boolean isExtensibleObject;
+
+ /** Indicates whether this object class is the extensibleObject class. */
+ private final boolean isExtensibleObject;
/**
* Construct a extensibleObject object class where the set of allowed
@@ -432,11 +438,7 @@
this.objectClassType = builder.type;
this.requiredAttributeOIDs = unmodifiableCopyOfSet(builder.requiredAttributes);
this.optionalAttributeOIDs = unmodifiableCopyOfSet(builder.optionalAttributes);
- this.isExtensibleObject = isExtensible();
- }
-
- private boolean isExtensible() {
- return hasName(EXTENSIBLE_OBJECT_OBJECTCLASS_NAME) || hasNameOrOID(EXTENSIBLE_OBJECT_OBJECTCLASS_OID);
+ this.isExtensibleObject = oid.equals(EXTENSIBLE_OBJECT_OBJECTCLASS_OID);
}
/**
@@ -619,6 +621,20 @@
}
/**
+ * Indicates whether this object class is extensibleObject class.
+ * <p>
+ * An extensible object class has an optional attributes list corresponding
+ * to all the attributes types defined in the schema. It means any attribute
+ * type can be used with this object class.
+ *
+ * @return {@code true} if this object class is extensible.
+ */
+ public boolean isExtensible() {
+ return isExtensibleObject;
+ }
+
+
+ /**
* Indicates whether this schema definition is declared "obsolete".
*
* @return <code>true</code> if this schema definition is declared
@@ -639,7 +655,7 @@
* <code>false</code> if not.
*/
public boolean isOptional(final AttributeType attributeType) {
- return isExtensibleObject || optionalAttributes.contains(attributeType);
+ return optionalAttributes.contains(attributeType);
}
/**
@@ -653,7 +669,7 @@
* <code>false</code> if not.
*/
public boolean isRequired(final AttributeType attributeType) {
- return isExtensibleObject || requiredAttributes.contains(attributeType);
+ return requiredAttributes.contains(attributeType);
}
/**
@@ -747,7 +763,7 @@
}
}
- if (!optionalAttributeOIDs.isEmpty()) {
+ if (!isExtensible() && !optionalAttributeOIDs.isEmpty()) {
final Iterator<String> iterator = optionalAttributeOIDs.iterator();
final String firstName = iterator.next();
@@ -893,14 +909,14 @@
return false;
}
- if (oid.equals(EXTENSIBLE_OBJECT_OBJECTCLASS_OID)) {
+ if (isExtensible()) {
declaredOptionalAttributes = new HashSet<>(requiredAttributeOIDs.size());
for (final AttributeType attributeType : schema.getAttributeTypes()) {
if (attributeType.getUsage() == AttributeUsage.USER_APPLICATIONS) {
declaredOptionalAttributes.add(attributeType);
}
}
- optionalAttributes = declaredRequiredAttributes;
+ optionalAttributes = declaredOptionalAttributes;
} else {
if (!requiredAttributeOIDs.isEmpty()) {
declaredRequiredAttributes = new HashSet<>(requiredAttributeOIDs.size());
diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/EntrySchemaCheckingTestCase.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/EntrySchemaCheckingTestCase.java
index 5284dc2..b2c04a1 100644
--- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/EntrySchemaCheckingTestCase.java
+++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/EntrySchemaCheckingTestCase.java
@@ -38,6 +38,7 @@
* Test schema validation using {@link Schema#validateEntry}.
*/
@Test
+@SuppressWarnings("javadoc")
public class EntrySchemaCheckingTestCase extends AbstractSchemaTestCase {
/**
@@ -957,7 +958,18 @@
}
@Test
- public void testExtensibleObjectAcceptsAnyAttribute() throws Exception {
+ public void testExtensibleObjectAcceptsAnyAttributeButNotPlaceholderAttribute() throws Exception {
+ // @formatter:off
+ final Entry e2 = newEntry(
+ "dn: dc=example,dc=com",
+ "objectClass: top",
+ "objectClass: organization",
+ "objectClass: extensibleObject",
+ "o: example",
+ "telephonenumber: 123456");
+ // @formatter:on
+ assertConformsToSchema(e2, defaultPolicy());
+
// @formatter:off
final Entry e = newEntry(
"dn: dc=example,dc=com",
@@ -965,10 +977,9 @@
"objectClass: organization",
"objectClass: extensibleObject",
"o: example",
- "dummy: it works too!!");
+ "dummy: unknown attribute!!");
// @formatter:on
-
- assertConformsToSchema(e, defaultPolicy());
+ assertDoesNotConformToSchema(e, defaultPolicy());
}
/**
diff --git a/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/ObjectClassTestCase.java b/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/ObjectClassTestCase.java
index 7607458..7bf6eb5 100644
--- a/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/ObjectClassTestCase.java
+++ b/opendj-core/src/test/java/org/forgerock/opendj/ldap/schema/ObjectClassTestCase.java
@@ -23,14 +23,28 @@
@SuppressWarnings("javadoc")
public class ObjectClassTestCase extends AbstractSchemaTestCase {
+
@Test
- public void extensibleObjectAcceptsAllAttributes() {
+ public void extensibleObjectShouldNotAcceptPlaceholderAttribute() {
Schema schema = getCoreSchema();
ObjectClass extensibleObject = schema.getObjectClass(EXTENSIBLE_OBJECT_OBJECTCLASS_OID);
+
AttributeType attributeType = schema.getAttributeType("dummy");
assertThat(attributeType.isPlaceHolder()).isTrue();
- assertThat(extensibleObject.isRequired(attributeType)).isTrue();
- assertThat(extensibleObject.isOptional(attributeType)).isTrue();
- assertThat(extensibleObject.isRequiredOrOptional(attributeType)).isTrue();
+ assertThat(extensibleObject.isRequired(attributeType)).isFalse();
+ assertThat(extensibleObject.isOptional(attributeType)).isFalse();
+ assertThat(extensibleObject.isRequiredOrOptional(attributeType)).isFalse();
+ }
+
+ @Test
+ public void extensibleObjectShouldAcceptAnyValidAttribute() {
+ Schema schema = getCoreSchema();
+ ObjectClass extensibleObject = schema.getObjectClass(EXTENSIBLE_OBJECT_OBJECTCLASS_OID);
+
+ AttributeType cn = schema.getAttributeType("cn");
+ assertThat(cn.isPlaceHolder()).isFalse();
+ assertThat(extensibleObject.isRequired(cn)).isFalse();
+ assertThat(extensibleObject.isOptional(cn)).isTrue();
+ assertThat(extensibleObject.isRequiredOrOptional(cn)).isTrue();
}
}
--
Gitblit v1.10.0