From 7c3e0d7c327e1379d531e1955bdf426f7409180b Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Thu, 11 Apr 2013 09:05:06 +0000
Subject: [PATCH] Fix for OPENDJ-693: Implement modify/update support
---
opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/AbstractLDAPAttributeMapper.java | 41 +++++++--
opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/JSONConstantAttributeMapper.java | 13 +++
opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java | 8 +
opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java | 5 +
opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java | 31 +++----
opendj-sdk/opendj3/opendj-rest2ldap/src/test/java/org/forgerock/opendj/rest2ldap/BasicRequestsTest.java | 97 ++++++++++++++++++++++-
6 files changed, 156 insertions(+), 39 deletions(-)
diff --git a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/AbstractLDAPAttributeMapper.java b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/AbstractLDAPAttributeMapper.java
index 0a5398f..cd5bb12 100644
--- a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/AbstractLDAPAttributeMapper.java
+++ b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/AbstractLDAPAttributeMapper.java
@@ -19,6 +19,7 @@
import static java.util.Collections.singletonList;
import static org.forgerock.opendj.ldap.Attributes.emptyAttribute;
import static org.forgerock.opendj.rest2ldap.Utils.i18n;
+import static org.forgerock.opendj.rest2ldap.Utils.isNullOrEmpty;
import static org.forgerock.opendj.rest2ldap.WritabilityPolicy.READ_WRITE;
import java.util.ArrayList;
@@ -33,6 +34,7 @@
import org.forgerock.json.resource.ResultHandler;
import org.forgerock.opendj.ldap.Attribute;
import org.forgerock.opendj.ldap.AttributeDescription;
+import org.forgerock.opendj.ldap.Attributes;
import org.forgerock.opendj.ldap.Entry;
import org.forgerock.opendj.ldap.LinkedAttribute;
import org.forgerock.opendj.ldap.Modification;
@@ -44,7 +46,6 @@
*/
abstract class AbstractLDAPAttributeMapper<T extends AbstractLDAPAttributeMapper<T>> extends
AttributeMapper {
- Object defaultJSONValue = null;
List<Object> defaultJSONValues = emptyList();
final AttributeDescription ldapAttributeName;
private boolean isRequired = false;
@@ -90,10 +91,6 @@
return getThis();
}
- boolean attributeIsRequired() {
- return isRequired && defaultJSONValue == null;
- }
-
boolean attributeIsSingleValued() {
return isSingleValued || ldapAttributeName.getAttributeType().isSingleValue();
}
@@ -131,7 +128,7 @@
"The request cannot be processed because an array of values was "
+ "provided for the single valued field '%s'", path)));
} else {
- getNewLDAPAttributes(c, path, asList(v), new ResultHandler<Attribute>() {
+ final ResultHandler<Attribute> attributeHandler = new ResultHandler<Attribute>() {
@Override
public void handleError(final ResourceException error) {
h.handleError(error);
@@ -166,11 +163,26 @@
if (oldLDAPAttribute.isEmpty() && newLDAPAttribute.isEmpty()) {
// No change.
modifications = Collections.<Modification> emptyList();
- } else if (oldLDAPAttribute.isEmpty() || newLDAPAttribute.isEmpty()) {
- // Delete or add.
+ } else if (oldLDAPAttribute.isEmpty()) {
+ // The attribute is being added.
modifications =
singletonList(new Modification(ModificationType.REPLACE,
newLDAPAttribute));
+ } else if (newLDAPAttribute.isEmpty()) {
+ /*
+ * The attribute is being deleted - this is not
+ * allowed if the attribute is required.
+ */
+ if (isRequired) {
+ h.handleError(new BadRequestException(i18n(
+ "The request cannot be processed because it attempts to remove "
+ + "the required field '%s'", path)));
+ return;
+ } else {
+ modifications =
+ singletonList(new Modification(ModificationType.REPLACE,
+ newLDAPAttribute));
+ }
} else {
/*
* We could do a replace, but try to save bandwidth
@@ -200,12 +212,20 @@
h.handleResult(modifications);
}
}
- });
+ };
+
+ final List<Object> newValues = asList(v);
+ if (newValues.isEmpty()) {
+ // Skip sub-class implementation if there are no values.
+ attributeHandler.handleResult(Attributes.emptyAttribute(ldapAttributeName));
+ } else {
+ getNewLDAPAttributes(c, path, asList(v), attributeHandler);
+ }
}
}
private List<Object> asList(final JsonValue v) {
- if (v == null || v.isNull() || (v.isList() && v.size() == 0)) {
+ if (isNullOrEmpty(v)) {
return defaultJSONValues;
} else if (v.isList()) {
return v.asList();
@@ -213,5 +233,4 @@
return singletonList(v.getObject());
}
}
-
}
diff --git a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/JSONConstantAttributeMapper.java b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/JSONConstantAttributeMapper.java
index e87bf66..da36b92 100644
--- a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/JSONConstantAttributeMapper.java
+++ b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/JSONConstantAttributeMapper.java
@@ -17,6 +17,8 @@
import static org.forgerock.opendj.ldap.Filter.alwaysFalse;
import static org.forgerock.opendj.ldap.Filter.alwaysTrue;
+import static org.forgerock.opendj.rest2ldap.Utils.i18n;
+import static org.forgerock.opendj.rest2ldap.Utils.isNullOrEmpty;
import static org.forgerock.opendj.rest2ldap.Utils.toFilter;
import static org.forgerock.opendj.rest2ldap.Utils.toLowerCase;
@@ -26,6 +28,7 @@
import org.forgerock.json.fluent.JsonPointer;
import org.forgerock.json.fluent.JsonValue;
+import org.forgerock.json.resource.BadRequestException;
import org.forgerock.json.resource.ResultHandler;
import org.forgerock.opendj.ldap.Entry;
import org.forgerock.opendj.ldap.Filter;
@@ -95,7 +98,15 @@
@Override
void toLDAP(final Context c, final JsonPointer path, final Entry e, final JsonValue v,
final ResultHandler<List<Modification>> h) {
- // FIXME: should we check if the provided value matches the constant?
+ if (!isNullOrEmpty(v)) {
+ // A value was provided so it must match.
+ if (!v.getObject().equals(value.getObject())) {
+ h.handleError(new BadRequestException(i18n(
+ "The request cannot be processed because it attempts to modify "
+ + "the read-only field '%s'", path)));
+ return;
+ }
+ }
h.handleResult(Collections.<Modification> emptyList());
}
diff --git a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java
index 3ac297f..c899a5d 100644
--- a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java
+++ b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/ReferenceAttributeMapper.java
@@ -38,7 +38,6 @@
import org.forgerock.json.resource.ResultHandler;
import org.forgerock.opendj.ldap.Attribute;
import org.forgerock.opendj.ldap.AttributeDescription;
-import org.forgerock.opendj.ldap.Attributes;
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.DN;
import org.forgerock.opendj.ldap.Entry;
@@ -183,12 +182,6 @@
@Override
void getNewLDAPAttributes(final Context c, final JsonPointer path,
final List<Object> newValues, final ResultHandler<Attribute> h) {
- // No need to do anything if there are no values.
- if (newValues.isEmpty()) {
- h.handleResult(Attributes.emptyAttribute(ldapAttributeName));
- return;
- }
-
/*
* For each value use the subordinate mapper to obtain the LDAP primary
* key, the perform a search for each one to find the corresponding
@@ -252,21 +245,25 @@
try {
throw error;
} catch (final EntryNotFoundException e) {
- // FIXME: improve error message.
re =
new BadRequestException(
- "the resource referenced by '"
- + primaryKeyValue
- .toString()
- + "' does not exist");
+ i18n("The request cannot be processed "
+ + "because the resource '%s' "
+ + "referenced in field '%s' does "
+ + "not exist",
+ primaryKeyValue
+ .toString(),
+ path));
} catch (final MultipleEntriesFoundException e) {
- // FIXME: improve error message.
re =
new BadRequestException(
- "the resource referenced by '"
- + primaryKeyValue
- .toString()
- + "' is ambiguous");
+ i18n("The request cannot be processed "
+ + "because the resource '%s' "
+ + "referenced in field '%s' is "
+ + "ambiguous",
+ primaryKeyValue
+ .toString(),
+ path));
} catch (final ErrorResultException e) {
re = asResourceException(e);
}
diff --git a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java
index b59180f..c7bc8c2 100644
--- a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java
+++ b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/SimpleAttributeMapper.java
@@ -28,6 +28,7 @@
import static org.forgerock.opendj.rest2ldap.Utils.jsonToByteString;
import static org.forgerock.opendj.rest2ldap.Utils.toFilter;
+import java.util.ArrayList;
import java.util.List;
import java.util.Set;
@@ -77,7 +78,6 @@
* @return This attribute mapper.
*/
public SimpleAttributeMapper defaultJSONValue(final Object defaultValue) {
- this.defaultJSONValue = defaultValue;
this.defaultJSONValues = defaultValue != null ? singletonList(defaultValue) : emptyList();
return this;
}
@@ -157,11 +157,13 @@
try {
final Object value;
if (attributeIsSingleValued()) {
- value = e.parseAttribute(ldapAttributeName).as(decoder(), defaultJSONValue);
+ value =
+ e.parseAttribute(ldapAttributeName).as(decoder(),
+ defaultJSONValues.isEmpty() ? null : defaultJSONValues.get(0));
} else {
final Set<Object> s =
e.parseAttribute(ldapAttributeName).asSetOf(decoder(), defaultJSONValues);
- value = s.isEmpty() ? null : s;
+ value = s.isEmpty() ? null : new ArrayList<Object>(s);
}
h.handleResult(value != null ? new JsonValue(value) : null);
} catch (final Exception ex) {
diff --git a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java
index 51a9adb..3558ac3 100644
--- a/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java
+++ b/opendj-sdk/opendj3/opendj-rest2ldap/src/main/java/org/forgerock/opendj/rest2ldap/Utils.java
@@ -35,6 +35,7 @@
import java.util.Locale;
import java.util.concurrent.atomic.AtomicInteger;
+import org.forgerock.json.fluent.JsonValue;
import org.forgerock.json.resource.ResourceException;
import org.forgerock.json.resource.ResultHandler;
import org.forgerock.opendj.ldap.Attribute;
@@ -237,6 +238,10 @@
return value instanceof String || value instanceof Boolean || value instanceof Number;
}
+ static boolean isNullOrEmpty(final JsonValue v) {
+ return v == null || v.isNull() || (v.isList() && v.size() == 0);
+ }
+
static Attribute jsonToAttribute(final Object value, final AttributeDescription ad) {
return jsonToAttribute(value, ad, fixedFunction(jsonToByteString(), ad));
}
diff --git a/opendj-sdk/opendj3/opendj-rest2ldap/src/test/java/org/forgerock/opendj/rest2ldap/BasicRequestsTest.java b/opendj-sdk/opendj3/opendj-rest2ldap/src/test/java/org/forgerock/opendj/rest2ldap/BasicRequestsTest.java
index 2123a74..fb88904 100644
--- a/opendj-sdk/opendj3/opendj-rest2ldap/src/test/java/org/forgerock/opendj/rest2ldap/BasicRequestsTest.java
+++ b/opendj-sdk/opendj3/opendj-rest2ldap/src/test/java/org/forgerock/opendj/rest2ldap/BasicRequestsTest.java
@@ -15,6 +15,7 @@
*/
package org.forgerock.opendj.rest2ldap;
+import static java.util.Arrays.asList;
import static org.fest.assertions.Assertions.assertThat;
import static org.fest.assertions.Fail.fail;
import static org.forgerock.json.resource.Requests.newDeleteRequest;
@@ -23,6 +24,7 @@
import static org.forgerock.json.resource.Resources.newCollection;
import static org.forgerock.json.resource.Resources.newInternalConnection;
import static org.forgerock.opendj.ldap.Connections.newInternalConnectionFactory;
+import static org.forgerock.opendj.rest2ldap.Rest2LDAP.constant;
import static org.forgerock.opendj.rest2ldap.Rest2LDAP.object;
import static org.forgerock.opendj.rest2ldap.Rest2LDAP.simple;
import static org.forgerock.opendj.rest2ldap.TestUtils.asResource;
@@ -163,6 +165,64 @@
}
@Test
+ public void testUpdateAddOptionalAttribute() throws Exception {
+ final RequestHandler handler = newCollection(builder().build());
+ final Connection connection = newInternalConnection(handler);
+ final JsonValue newContent = getTestUser1Updated(12345);
+ newContent.put("description", asList("one", "two"));
+ final Resource resource1 = connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ checkResourcesAreEqual(resource1, newContent);
+ final Resource resource2 = connection.read(ctx(), newReadRequest("/test1"));
+ checkResourcesAreEqual(resource2, newContent);
+ }
+
+ @Test(expectedExceptions = BadRequestException.class)
+ public void testUpdateConstantAttribute() throws Exception {
+ final RequestHandler handler = newCollection(builder().build());
+ final Connection connection = newInternalConnection(handler);
+ final JsonValue newContent = getTestUser1Updated(12345);
+ newContent.put("schemas", asList("junk"));
+ connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ }
+
+ @Test
+ public void testUpdateDeleteOptionalAttribute() throws Exception {
+ final RequestHandler handler = newCollection(builder().build());
+ final Connection connection = newInternalConnection(handler);
+ final JsonValue newContent = getTestUser1Updated(12345);
+ newContent.put("description", asList("one", "two"));
+ connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ newContent.remove("description");
+ final Resource resource1 = connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ checkResourcesAreEqual(resource1, newContent);
+ final Resource resource2 = connection.read(ctx(), newReadRequest("/test1"));
+ checkResourcesAreEqual(resource2, newContent);
+ }
+
+ @Test(expectedExceptions = BadRequestException.class)
+ public void testUpdateMissingRequiredAttribute() throws Exception {
+ final RequestHandler handler = newCollection(builder().build());
+ final Connection connection = newInternalConnection(handler);
+ final JsonValue newContent = getTestUser1Updated(12345);
+ newContent.remove("surname");
+ connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ }
+
+ @Test
+ public void testUpdateModifyOptionalAttribute() throws Exception {
+ final RequestHandler handler = newCollection(builder().build());
+ final Connection connection = newInternalConnection(handler);
+ final JsonValue newContent = getTestUser1Updated(12345);
+ newContent.put("description", asList("one", "two"));
+ connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ newContent.put("description", asList("three"));
+ final Resource resource1 = connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ checkResourcesAreEqual(resource1, newContent);
+ final Resource resource2 = connection.read(ctx(), newReadRequest("/test1"));
+ checkResourcesAreEqual(resource2, newContent);
+ }
+
+ @Test
public void testUpdateMVCCMatch() throws Exception {
final RequestHandler handler = newCollection(builder().build());
final Connection connection = newInternalConnection(handler);
@@ -197,12 +257,32 @@
connection.update(ctx(), newUpdateRequest("/test1", getTestUser1Updated(99999)));
}
+ @Test(expectedExceptions = BadRequestException.class)
+ public void testUpdateSingleValuedAttributeWithMultipleValues() throws Exception {
+ final RequestHandler handler = newCollection(builder().build());
+ final Connection connection = newInternalConnection(handler);
+ final JsonValue newContent = getTestUser1Updated(12345);
+ newContent.put("surname", asList("black", "white"));
+ connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ }
+
+ @Test(expectedExceptions = BadRequestException.class)
+ public void testUpdateUnknownAttribute() throws Exception {
+ final RequestHandler handler = newCollection(builder().build());
+ final Connection connection = newInternalConnection(handler);
+ final JsonValue newContent = getTestUser1Updated(12345);
+ newContent.add("dummy", "junk");
+ connection.update(ctx(), newUpdateRequest("/test1", newContent));
+ }
+
private Builder builder() throws IOException {
return Rest2LDAP.builder().ldapConnectionFactory(getConnectionFactory()).baseDN("dc=test")
.useEtagAttribute().useClientDNNaming("uid").readOnUpdatePolicy(
ReadOnUpdatePolicy.CONTROLS).authorizationPolicy(AuthorizationPolicy.NONE)
- .additionalLDAPAttribute("objectClass", "top", "person").mapper(
- object().attribute(
+ .additionalLDAPAttribute("objectClass", "top", "person")
+ .mapper(object()
+ .attribute("schemas", constant(asList("urn:scim:schemas:core:1.0")))
+ .attribute(
"_id",
simple("uid").isSingleValued().isRequired().writability(
WritabilityPolicy.CREATE_ONLY)).attribute("displayName",
@@ -210,7 +290,8 @@
simple("sn").isSingleValued().isRequired()).attribute(
"_rev",
simple("etag").isSingleValued().isRequired().writability(
- WritabilityPolicy.READ_ONLY)));
+ WritabilityPolicy.READ_ONLY)).attribute("description",
+ simple("description")));
}
private void checkResourcesAreEqual(final Resource actual, final JsonValue expected) {
@@ -254,12 +335,14 @@
}
private JsonValue getTestUser1(final int rev) {
- return content(object(field("_id", "test1"), field("_rev", String.valueOf(rev)), field(
- "displayName", "test user 1"), field("surname", "user 1")));
+ return content(object(field("schemas", asList("urn:scim:schemas:core:1.0")), field("_id",
+ "test1"), field("_rev", String.valueOf(rev)), field("displayName", "test user 1"),
+ field("surname", "user 1")));
}
private JsonValue getTestUser1Updated(final int rev) {
- return content(object(field("_id", "test1"), field("_rev", String.valueOf(rev)), field(
- "displayName", "changed"), field("surname", "user 1")));
+ return content(object(field("schemas", asList("urn:scim:schemas:core:1.0")), field("_id",
+ "test1"), field("_rev", String.valueOf(rev)), field("displayName", "changed"),
+ field("surname", "user 1")));
}
}
--
Gitblit v1.10.0