[GitHub] [directory-scimple] bdemers commented on a diff in pull request #213: Draft: Applying Patches to resources

[via GitHub]

bdemers commented on code in PR #213:
URL: https://github.com/apache/directory-scimple/pull/213#discussion_r1084722794


##
scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/InMemoryScimFilterMatcher.java:
##
@@ -158,8 +159,11 @@ public boolean test(R actual) {
 
   // now walk the attribute path again to get the accessor and value
   Schema.Attribute schemaAttribute = 
attributeContainer.getAttribute(attributeReference.getAttributeName());
-  actual = schemaAttribute.getAccessor().get(actual);
-
+  if (actual instanceof ScimResource) {
+// actual is the top level scim resource - need to extract the top 
attribute.
+// otherwise we can move on directly to the sub-attribute
+actual = schemaAttribute.getAccessor().get(actual);
+  }

Review Comment:
   @erant10 
   樂 This bit has me confused.
   Locally, I reverted this back the previous try/catch/todo, and all the tests 
pass.
   
   But I haven't been able to figure out what condition causes this error. Any 
ideas?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@directory.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: dev-unsubscr...@directory.apache.org
For additional commands, e-mail: dev-h...@directory.apache.org

[GitHub] [directory-scimple] bdemers commented on a diff in pull request #213: Draft: Applying Patches to resources

[GitBox]

bdemers commented on code in PR #213:
URL: https://github.com/apache/directory-scimple/pull/213#discussion_r1060871770


##
scim-core/src/main/java/org/apache/directory/scim/core/repository/ObjectMapperProvider.java:
##
@@ -0,0 +1,27 @@
+package org.apache.directory.scim.core.repository;
+
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.databind.AnnotationIntrospector;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.introspect.AnnotationIntrospectorPair;
+import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
+import 
com.fasterxml.jackson.module.jakarta.xmlbind.JakartaXmlBindAnnotationIntrospector;
+import 
com.fasterxml.jackson.module.jakarta.xmlbind.JakartaXmlBindAnnotationModule;
+
+public class ObjectMapperProvider {

Review Comment:
   I recently refactored some of the JSON/ObjectMapper bits.
   I'm not 100% that will help with this usage, but it is something we can 
investigate.
   
   
https://github.com/apache/directory-scimple/blob/develop/scim-core/src/main/java/org/apache/directory/scim/core/json/ObjectMapperFactory.java
 
   
   This _may_ come into play when dealing with patches that contain values from 
SCIM Extensions.
   That said, with your approach using `objectAsMap` it _may_ not make a 
difference.
   (Mostly a note to myself, to go back and look at the ObjectMapper-related 
bits )



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@directory.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: dev-unsubscr...@directory.apache.org
For additional commands, e-mail: dev-h...@directory.apache.org