[directory-scimple] 01/01: Remove the use of Jackson when processing patches

[bd...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bdemers pushed a commit to branch patch_phase1_no_jackson
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git

commit ba09f2018cbe8c2a45a6c091bec7bbecb52597d6
Author: Brian Demers <bd...@apache.org>
AuthorDate: Fri Jan 6 18:38:22 2023 -0500

    Remove the use of Jackson when processing patches
    
    - FitlerExpressions.inMemory() now returns Predicate<Object> instead of KeyedResource
    - Added test for complex patch filter
---
 .../scim/core/repository/PatchHandlerImpl.java     | 48 ++++------------------
 .../scim/core/repository/PatchHandlerTest.java     | 32 ++++++++++++++-
 .../scim/spec/filter/FilterExpressions.java        |  3 +-
 3 files changed, 41 insertions(+), 42 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..aa10e27e 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
@@ -19,34 +19,23 @@
 
 package org.apache.directory.scim.core.repository;
 
-import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.SerializationUtils;
-import org.apache.directory.scim.core.json.ObjectMapperFactory;
 import org.apache.directory.scim.core.schema.SchemaRegistry;
 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;
 
 import java.util.*;
 import java.util.function.Predicate;
-import java.util.stream.Collectors;
 
 @SuppressWarnings("unchecked")
 @Slf4j
 public class PatchHandlerImpl implements PatchHandler {
-
-  private static final TypeReference<Map<String, Object>> MAP_TYPE = new TypeReference<>() {
-  };
-
-  private final ObjectMapper objectMapper = ObjectMapperFactory.getObjectMapper();
-
   private final SchemaRegistry schemaRegistry;
 
   public PatchHandlerImpl(SchemaRegistry schemaRegistry) {
@@ -88,7 +77,6 @@ public class PatchHandlerImpl implements PatchHandler {
   }
 
   private <T extends ScimResource> T apply(final T source, final PatchOperation patchOperation) {
-    Map<String, Object> sourceAsMap = objectAsMap(source);
 
     final ValuePathExpression valuePathExpression = valuePathExpression(patchOperation);
     final AttributeReference attributeReference = attributeReference(valuePathExpression);
@@ -104,28 +92,19 @@ public class PatchHandlerImpl implements PatchHandler {
     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)) {
+      items.forEach(item -> {
+        Predicate<Object> pred = FilterExpressions.inMemory(valuePathExpression.getAttributeExpression(), baseSchema);
+        if (pred.test(item)) {
           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;
+          Schema.AttributeAccessor subAttributeAccessor = attribute.getAttribute(subAttributeName).getAccessor();
+          subAttributeAccessor.set(item, patchOperation.getValue());
         }
-      }).collect(Collectors.toList());
-      sourceAsMap.put(attribute.getName(), updatedCollection);
+      });
     } else {
-      // no filter expression
-      sourceAsMap.put(attribute.getName(), patchOperation.getValue());
+      // no filter expression, just set the value
+      attribute.getAccessor().set(source, patchOperation.getValue());
     }
-    return (T) mapAsScimResource(sourceAsMap, source.getClass());
+    return source;
   }
 
   private PatchOperationPath tryGetOperationPath(String key) {
@@ -137,14 +116,6 @@ public class PatchHandlerImpl implements PatchHandler {
     }
   }
 
-  private <T extends ScimResource> T mapAsScimResource(final Map<String, Object> scimResourceAsMap, final Class<T> clazz) {
-    return objectMapper.convertValue(scimResourceAsMap, clazz);
-  }
-
-  private Map<String, Object> objectAsMap(final Object object) {
-    return objectMapper.convertValue(object, MAP_TYPE);
-  }
-
   public static ValuePathExpression valuePathExpression(final PatchOperation operation) {
     return Optional.ofNullable(operation.getPath())
       .map(PatchOperationPath::getValuePathExpression)
@@ -155,5 +126,4 @@ public class PatchHandlerImpl implements PatchHandler {
     return Optional.ofNullable(expression.getAttributePath())
       .orElseThrow(() -> new IllegalArgumentException("Patch operation must have an expression with a valid attribute path"));
   }
-
 }
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..4d8d8d66 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,6 +23,7 @@ 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;
@@ -58,11 +59,40 @@ public class PatchHandlerTest {
     assertThat(updatedUser.getUserName()).isEqualTo("testUser_updated");
   }
 
+  @Test
+  public void applyWithFilterExpression() throws FilterParseException {
+    PatchOperation op = new PatchOperation();
+    op.setOperation(PatchOperation.Type.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
+    ));
+  }
+
   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");
+    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, List.of(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);
   }
 }