You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by bd...@apache.org on 2023/01/11 19:52:52 UTC
[directory-scimple] branch patch_phase1_hacking created (now fa7088a7)
This is an automated email from the ASF dual-hosted git repository.
bdemers pushed a change to branch patch_phase1_hacking
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git
at fa7088a7 Create Patch Operation handlers for each type of OP
This branch includes the following new commits:
new fa7088a7 Create Patch Operation handlers for each type of OP
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[directory-scimple] 01/01: Create Patch Operation handlers for each type of OP
Posted by bd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bdemers pushed a commit to branch patch_phase1_hacking
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git
commit fa7088a77cc3b5891ddba9a0f556872ebcfff4a1
Author: Brian Demers <bd...@apache.org>
AuthorDate: Fri Jan 6 18:38:22 2023 -0500
Create Patch Operation handlers for each type of OP
- FitlerExpressions.inMemory() now returns Predicate<Object> instead of KeyedResource
- Added tests for complex patch operations based on SCIM spec
---
.../scim/core/repository/PatchHandlerImpl.java | 146 +++++++---
.../scim/core/repository/PatchHandlerTest.java | 307 ++++++++++++++++++++-
.../scim/spec/filter/FilterExpressions.java | 3 +-
3 files changed, 417 insertions(+), 39 deletions(-)
diff --git a/scim-core/src/main/java/org/apache/directory/scim/core/repository/PatchHandlerImpl.java b/scim-core/src/main/java/org/apache/directory/scim/core/repository/PatchHandlerImpl.java
index c623c6d5..1253c02a 100644
--- a/scim-core/src/main/java/org/apache/directory/scim/core/repository/PatchHandlerImpl.java
+++ b/scim-core/src/main/java/org/apache/directory/scim/core/repository/PatchHandlerImpl.java
@@ -29,7 +29,6 @@ import org.apache.directory.scim.spec.filter.*;
import org.apache.directory.scim.spec.filter.attribute.AttributeReference;
import org.apache.directory.scim.spec.patch.PatchOperation;
import org.apache.directory.scim.spec.patch.PatchOperationPath;
-import org.apache.directory.scim.spec.resources.KeyedResource;
import org.apache.directory.scim.spec.resources.ScimResource;
import org.apache.directory.scim.spec.schema.Schema;
import org.apache.directory.scim.spec.schema.Schema.Attribute;
@@ -41,7 +40,6 @@ import java.util.stream.Collectors;
@SuppressWarnings("unchecked")
@Slf4j
public class PatchHandlerImpl implements PatchHandler {
-
private static final TypeReference<Map<String, Object>> MAP_TYPE = new TypeReference<>() {
};
@@ -49,6 +47,12 @@ public class PatchHandlerImpl implements PatchHandler {
private final SchemaRegistry schemaRegistry;
+ private final Map<PatchOperation.Type, PatchOperationHandler> patchOperationHandlers = Map.of(
+ PatchOperation.Type.ADD, new AddOperationHandler(),
+ PatchOperation.Type.REPLACE, new ReplaceOperationHandler(),
+ PatchOperation.Type.REMOVE, new RemoveOperationHandler()
+ );
+
public PatchHandlerImpl(SchemaRegistry schemaRegistry) {
this.schemaRegistry = schemaRegistry;
}
@@ -61,9 +65,10 @@ public class PatchHandlerImpl implements PatchHandler {
throw new IllegalArgumentException("patchOperations is null. Cannot apply patch.");
}
- T updatedScimResource;
- updatedScimResource = SerializationUtils.clone(original);
+ T updatedScimResource = SerializationUtils.clone(original);
for (PatchOperation patchOperation : patchOperations) {
+
+ // if path is null, break up the value into multiple patch operations
if (patchOperation.getPath() == null) {
if (!(patchOperation.getValue() instanceof Map)) {
throw new IllegalArgumentException("Cannot apply patch. value is required");
@@ -93,37 +98,15 @@ public class PatchHandlerImpl implements PatchHandler {
final ValuePathExpression valuePathExpression = valuePathExpression(patchOperation);
final AttributeReference attributeReference = attributeReference(valuePathExpression);
- Schema baseSchema = this.schemaRegistry.getSchema(source.getBaseUrn());
- Attribute attribute = baseSchema.getAttribute(attributeReference.getAttributeName());
+ Schema schema = this.schemaRegistry.getSchema(source.getBaseUrn());
+ Attribute attribute = schema.getAttribute(attributeReference.getAttributeName());
- Object attributeObject = attribute.getAccessor().get(source);
- if (attributeObject == null && patchOperation.getOperation().equals(PatchOperation.Type.REPLACE)) {
- throw new IllegalArgumentException("Cannot apply patch replace on missing property: " + attribute.getName());
- }
+ PatchOperationHandler patchOperationHandler = patchOperationHandlers.get(patchOperation.getOperation());
- if (valuePathExpression.getAttributeExpression() != null && attributeObject instanceof Collection<?>) {
- // apply expression filter
- Collection<Object> items = (Collection<Object>) attributeObject;
- Collection<Object> updatedCollection = items.stream().map(item -> {
- KeyedResource keyedResource = (KeyedResource) item;
- Map<String, Object> keyedResourceAsMap = objectAsMap(item);
- Predicate<KeyedResource> pred = FilterExpressions.inMemory(valuePathExpression.getAttributeExpression(), baseSchema);
- if (pred.test(keyedResource)) {
- String subAttributeName = valuePathExpression.getAttributePath().getSubAttributeName();
- if (keyedResourceAsMap.get(subAttributeName) == null && patchOperation.getOperation().equals(PatchOperation.Type.REPLACE)) {
- throw new IllegalArgumentException("Cannot apply patch replace on missing property: " + valuePathExpression.toFilter());
- }
- keyedResourceAsMap.put(subAttributeName, patchOperation.getValue());
- return keyedResourceAsMap;
- } else {
- // filter does not apply
- return item;
- }
- }).collect(Collectors.toList());
- sourceAsMap.put(attribute.getName(), updatedCollection);
+ if (attribute.isMultiValued()) {
+ patchOperationHandler.applyMultiValue(source, sourceAsMap, schema, attribute, valuePathExpression, patchOperation.getValue());
} else {
- // no filter expression
- sourceAsMap.put(attribute.getName(), patchOperation.getValue());
+ patchOperationHandler.applySingleValue(sourceAsMap, attribute, patchOperation.getValue());
}
return (T) mapAsScimResource(sourceAsMap, source.getClass());
}
@@ -156,4 +139,103 @@ public class PatchHandlerImpl implements PatchHandler {
.orElseThrow(() -> new IllegalArgumentException("Patch operation must have an expression with a valid attribute path"));
}
+ private interface PatchOperationHandler {
+ <T extends ScimResource> void applySingleValue(Map<String, Object> sourceAsMap, Attribute attribute, Object value);
+ <T extends ScimResource> void applyMultiValue(final T source, Map<String, Object> sourceAsMap, Schema schema, Attribute attribute, ValuePathExpression valuePathExpression, Object value);
+ }
+
+ private class AddOperationHandler implements PatchOperationHandler {
+ @Override
+ public <T extends ScimResource> void applySingleValue(Map<String, Object> sourceAsMap, Attribute attribute, Object value) {
+ String attributeName = attribute.getName();
+ if (sourceAsMap.get(attributeName) == null) {
+ sourceAsMap.put(attributeName, value);
+ } else {
+ log.debug("Resource '{}' with attribute '{}' already contains value and cannot be patched with an ADD operation", sourceAsMap.get("id"), attribute.getUrn());
+ }
+ }
+
+ @Override
+ public <T extends ScimResource> void applyMultiValue(T source, Map<String, Object> sourceAsMap, Schema schema, Attribute attribute, ValuePathExpression valuePathExpression, Object value) {
+
+ String subAttributeName = valuePathExpression.getAttributePath().getAttributeName();
+ Collection<Object> items = (Collection<Object>) sourceAsMap.get(subAttributeName);
+ if (items == null) {
+ items = new ArrayList<>();
+ sourceAsMap.put(subAttributeName, items);
+ }
+
+ if (value instanceof Collection) {
+ items.addAll((Collection<Object>) value);
+ } else {
+ items.add(value);
+ }
+ }
+ }
+
+ private class ReplaceOperationHandler implements PatchOperationHandler {
+
+ @Override
+ public <T extends ScimResource> void applySingleValue(Map<String, Object> sourceAsMap, Attribute attribute, Object value) {
+ sourceAsMap.put(attribute.getName(), value);
+ }
+
+ @Override
+ public <T extends ScimResource> void applyMultiValue(T source, Map<String, Object> sourceAsMap, Schema schema, Attribute attribute, ValuePathExpression valuePathExpression, Object value) {
+
+ if (valuePathExpression.getAttributeExpression() != null) {
+
+ // apply expression filter
+ Collection<Object> items = attribute.getAccessor().get(source);
+ Predicate<Object> pred = FilterExpressions.inMemory(valuePathExpression.getAttributeExpression(), schema);
+
+ Collection<Object> updatedCollection = items.stream()
+ .map(item -> {
+ Map<String, Object> resourceAsMap = objectAsMap(item);
+ // find items that need to be updated
+ if (pred.test(item)) {
+ String subAttributeName = valuePathExpression.getAttributePath().getSubAttributeName();
+ if (resourceAsMap.get(subAttributeName) == null) {
+ resourceAsMap = (Map<String, Object>) value;
+ } else {
+ resourceAsMap.put(subAttributeName, value);
+ }
+ return resourceAsMap;
+ } else {
+ // filter does not apply
+ return item;
+ }
+ }).collect(Collectors.toList());
+ sourceAsMap.put(attribute.getName(), updatedCollection);
+
+ // replace collection
+ } else {
+ sourceAsMap.put(attribute.getName(), value);
+ }
+ }
+ }
+
+ private class RemoveOperationHandler implements PatchOperationHandler {
+
+ @Override
+ public <T extends ScimResource> void applySingleValue(Map<String, Object> sourceAsMap, Attribute attribute, Object value) {
+ sourceAsMap.remove(attribute.getName());
+ }
+
+ @Override
+ public <T extends ScimResource> void applyMultiValue(T source, Map<String, Object> sourceAsMap, Schema schema, Attribute attribute, ValuePathExpression valuePathExpression, Object value) {
+
+ if (valuePathExpression.getAttributeExpression() != null) {
+ Collection<Object> items = attribute.getAccessor().get(source);
+ Predicate<Object> pred = FilterExpressions.inMemory(valuePathExpression.getAttributeExpression(), schema);
+
+ Collection<Object> updatedCollection = items.stream()
+ .filter(item -> !(pred.test(item)))
+ .collect(Collectors.toList());
+ sourceAsMap.put(attribute.getName(), updatedCollection);
+ } else {
+ sourceAsMap.remove(attribute.getName());
+ }
+ }
+ }
}
diff --git a/scim-core/src/test/java/org/apache/directory/scim/core/repository/PatchHandlerTest.java b/scim-core/src/test/java/org/apache/directory/scim/core/repository/PatchHandlerTest.java
index 508eb59f..8fbe8024 100644
--- a/scim-core/src/test/java/org/apache/directory/scim/core/repository/PatchHandlerTest.java
+++ b/scim-core/src/test/java/org/apache/directory/scim/core/repository/PatchHandlerTest.java
@@ -23,15 +23,18 @@ import org.apache.directory.scim.core.schema.SchemaRegistry;
import org.apache.directory.scim.spec.filter.FilterParseException;
import org.apache.directory.scim.spec.patch.PatchOperation;
import org.apache.directory.scim.spec.patch.PatchOperationPath;
+import org.apache.directory.scim.spec.resources.Email;
import org.apache.directory.scim.spec.resources.ScimUser;
import org.apache.directory.scim.spec.schema.Schemas;
import org.junit.jupiter.api.Test;
import static java.util.Map.entry;
+import static org.apache.directory.scim.spec.patch.PatchOperation.Type.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
@@ -42,7 +45,7 @@ public class PatchHandlerTest {
@Test
public void applyReplaceUserName() throws FilterParseException {
PatchOperation op = new PatchOperation();
- op.setOperation(PatchOperation.Type.REPLACE);
+ op.setOperation(REPLACE);
op.setPath(new PatchOperationPath("userName"));
op.setValue("testUser_updated");
ScimUser updatedUser = performPatch(op);
@@ -52,17 +55,311 @@ public class PatchHandlerTest {
@Test
public void applyReplaceUserNameWithMappedValue() {
PatchOperation op = new PatchOperation();
- op.setOperation(PatchOperation.Type.REPLACE);
+ op.setOperation(REPLACE);
op.setValue(Map.ofEntries(entry("userName", "testUser_updated")));
ScimUser updatedUser = performPatch(op);
assertThat(updatedUser.getUserName()).isEqualTo("testUser_updated");
}
- private ScimUser performPatch(PatchOperation op) {
+ @Test
+ public void applyWithFilterExpression() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(REPLACE);
+ op.setPath(new PatchOperationPath("emails[type EQ \"home\"].value"));
+ op.setValue("new-home@example.com");
+ ScimUser updatedUser = performPatch(op);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com"),
+ new Email()
+ .setType("home")
+ .setValue("new-home@example.com") // updated email
+ ));
+ }
+
+ @Test
+ public void replaceItem() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(REPLACE);
+ op.setPath(new PatchOperationPath("emails[type EQ \"home\"]"));
+ op.setValue(Map.of(
+ "type", "other",
+ "value", "other@example.com"
+ ));
+
+ ScimUser updatedUser = performPatch(op);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com"),
+ new Email()
+ .setType("other")
+ .setValue("other@example.com")
+ ));
+ }
+
+ @Test
+ public void replaceMultipleAttributes() {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(REPLACE);
+ op.setValue(Map.of(
+ "emails", List.of(
+ Map.of(
+ "type", "home",
+ "value", "first@example.com"),
+ Map.of(
+ "type", "work",
+ "value", "second@example.com",
+ "primary", true)
+ ),
+ "nickName", "Babs"
+ ));
+
+ ScimUser updatedUser = performPatch(op);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setType("home")
+ .setValue("first@example.com"),
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("second@example.com")
+ ));
+ assertThat(updatedUser.getNickName()).isEqualTo("Babs");
+ }
+
+ @Test
+ public void replaceCollection() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(REPLACE);
+ op.setPath(new PatchOperationPath("emails"));
+ op.setValue(List.of(
+ Map.of(
+ "value", "first@example.com",
+ "type", "home"),
+ Map.of(
+ "primary", true,
+ "value", "second@example.com",
+ "type", "work")
+ ));
+ ScimUser updatedUser = performPatch(op);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setType("home")
+ .setValue("first@example.com"),
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("second@example.com")
+ ));
+ }
+
+ @Test
+ public void deleteItemWithFilter() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(REMOVE);
+ op.setPath(new PatchOperationPath("emails[type EQ \"home\"]"));
+ ScimUser updatedUser = performPatch(op);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com")
+ ));
+ }
+
+ @Test
+ public void deleteAttributeWithPath() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(REMOVE);
+ op.setPath(new PatchOperationPath("nickName"));
+ ScimUser updatedUser = performPatch(op);
+ assertThat(updatedUser.getNickName()).isNull();
+ }
+
+ @Test
+ public void deleteCollectionWithPath() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(REMOVE);
+ op.setPath(new PatchOperationPath("emails"));
+ ScimUser updatedUser = performPatch(op);
+ assertThat(updatedUser.getEmails()).isNull();
+ }
+
+ @Test
+ public void deleteItemWithComplexFilter() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(REMOVE);
+ op.setPath(new PatchOperationPath("emails[type EQ \"home\"] and value ew \"example.com\""));
+ ScimUser updatedUser = performPatch(op);
+ assertThat(updatedUser.getEmails()).isEqualTo(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com")
+ ));
+ }
+
+ @Test
+ public void addAttribute() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(ADD);
+ op.setPath(new PatchOperationPath("profileUrl"));
+ op.setValue("https://profile.example.com");
+
+ ScimUser updatedUser = performPatch(op);
+ assertThat(updatedUser).isEqualTo(new ScimUser()
+ .setUserName("testUser")
+ .setNickName("tester")
+ .setProfileUrl("https://profile.example.com")
+ .setEmails(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com"),
+ new Email()
+ .setType("home")
+ .setValue("home@example.com")
+ )));
+ }
+
+ @Test
+ public void addItem() throws FilterParseException {
+ PatchOperation op = new PatchOperation();
+ op.setOperation(ADD);
+ op.setPath(new PatchOperationPath("emails"));
+ op.setValue(Map.of(
+ "type", "other",
+ "value", "other@example.com"));
+
+ ScimUser updatedUser = performPatch(op);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com"),
+ new Email()
+ .setType("home")
+ .setValue("home@example.com"),
+ new Email()
+ .setType("other")
+ .setValue("other@example.com")
+ ));
+ }
+
+ @Test
+ public void addMultipleProperties() throws FilterParseException {
+ // From Section 3.5.2.1 Add Operation of SCIM Protocol RFC
+ PatchOperation op = new PatchOperation();
+ op.setOperation(ADD);
+ op.setValue(Map.of(
+ "emails", Map.of(
+ "value", "babs@example.com",
+ "type", "other"),
+ "profileUrl", "https://profile.example.com"
+ ));
+
+ ScimUser updatedUser = performPatch(op);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com"),
+ new Email()
+ .setType("home")
+ .setValue("home@example.com"),
+ new Email()
+ .setType("other")
+ .setValue("babs@example.com")
+ ));
+ assertThat(updatedUser.getProfileUrl()).isEqualTo("https://profile.example.com");
+ }
+
+ @Test
+ public void multiplePatchOperations() throws FilterParseException {
+ PatchOperation opRm = new PatchOperation();
+ opRm.setOperation(REMOVE);
+ opRm.setPath(new PatchOperationPath("emails[type EQ \"home\"]"));
+
+ PatchOperation opAdd = new PatchOperation();
+ opAdd.setOperation(ADD);
+ opAdd.setPath(new PatchOperationPath("emails"));
+ opAdd.setValue(Map.of(
+ "value", "babs@example.com",
+ "type", "other")
+ );
+
+ ScimUser updatedUser = performPatch(opRm, opAdd);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com"),
+ new Email()
+ .setType("other")
+ .setValue("babs@example.com")
+ ));
+ }
+
+ @Test
+ public void replaceCollectionWithMultipleOps() throws FilterParseException {
+ PatchOperation opRm = new PatchOperation();
+ opRm.setOperation(REMOVE);
+ opRm.setPath(new PatchOperationPath("emails"));
+
+ PatchOperation opAdd = new PatchOperation();
+ opAdd.setOperation(ADD);
+ opAdd.setPath(new PatchOperationPath("emails"));
+ opAdd.setValue(List.of(
+ Map.of(
+ "value", "first@example.com",
+ "type", "home"),
+ Map.of(
+ "primary", true,
+ "value", "second@example.com",
+ "type", "work")
+ ));
+
+ ScimUser updatedUser = performPatch(opRm, opAdd);
+ List<Email> emails = updatedUser.getEmails();
+ assertThat(emails).isEqualTo(List.of(
+ new Email()
+ .setType("home")
+ .setValue("first@example.com"),
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("second@example.com")
+ ));
+ }
+
+ private ScimUser performPatch(PatchOperation... op) {
when(mockSchemaRegistry.getSchema(ScimUser.SCHEMA_URI)).thenReturn(Schemas.schemaFor(ScimUser.class));
- PatchHandlerImpl patchHandler = new PatchHandlerImpl(mockSchemaRegistry);
+ PatchHandler patchHandler = new PatchHandlerImpl(mockSchemaRegistry);
ScimUser user = new ScimUser();
user.setUserName("testUser");
- return patchHandler.apply(user, List.of(op));
+ user.setNickName("tester");
+ user.setEmails(List.of(
+ new Email()
+ .setPrimary(true)
+ .setType("work")
+ .setValue("work@example.com"),
+ new Email()
+ .setType("home")
+ .setValue("home@example.com")
+ ));
+
+ return patchHandler.apply(user, Arrays.asList(op));
}
}
diff --git a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/FilterExpressions.java b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/FilterExpressions.java
index e9d2300f..531788dc 100644
--- a/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/FilterExpressions.java
+++ b/scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/FilterExpressions.java
@@ -1,6 +1,5 @@
package org.apache.directory.scim.spec.filter;
-import org.apache.directory.scim.spec.resources.KeyedResource;
import org.apache.directory.scim.spec.resources.ScimResource;
import org.apache.directory.scim.spec.schema.Schema;
@@ -29,7 +28,7 @@ public final class FilterExpressions {
return InMemoryScimFilterMatcher.toPredicate(expression, schema);
}
- public static Predicate<KeyedResource> inMemory(FilterExpression expression, Schema schema) {
+ public static Predicate<Object> inMemory(FilterExpression expression, Schema schema) {
return InMemoryScimFilterMatcher.toPredicate(expression, schema);
}
}