jasonfagerberg-toast commented on code in PR #694:
URL: https://github.com/apache/directory-scimple/pull/694#discussion_r1873318193


##########
scim-core/src/test/java/org/apache/directory/scim/core/repository/PatchHandlerTest.java:
##########
@@ -56,6 +57,43 @@ public PatchHandlerTest() {
     this.patchHandler = new DefaultPatchHandler(schemaRegistry);
   }
 
+  @Test
+  public void applyReplaceEntireEnterpriseExtension() {
+    String enterpriseExtensionUrn = 
"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User";
+    Map<String, String> enterpriseExtensionValue = new HashMap<>();
+    enterpriseExtensionValue.put("costCenter", "New Cost Center");
+    enterpriseExtensionValue.put("department", "New Department");

Review Comment:
   XS Nit:
   
   Again, entirely my fault, but from reading the rest of the tests, it looks 
like most use `Map.ofEntries` or `Map.of` to define an immutable map



##########
scim-core/src/main/java/org/apache/directory/scim/core/repository/DefaultPatchHandler.java:
##########
@@ -132,10 +132,23 @@ private <T extends ScimResource> void apply(T source, 
Map<String, Object> source
     // if the attribute has a URN, assume it's an extension that URN does not 
match the baseUrn
     if (attributeReference.hasUrn() && 
!attributeReference.getUrn().equals(source.getBaseUrn())) {
       Schema schema = 
this.schemaRegistry.getSchema(attributeReference.getUrn());
-      Attribute attribute = 
schema.getAttribute(attributeReference.getAttributeName());
-      
checkMutability(schema.getAttributeFromPath(attributeReference.getFullAttributeName()));
 
-      patchOperationHandler.applyExtensionValue(source, sourceAsMap, schema, 
attribute, valuePathExpression, attributeReference.getUrn(), 
patchOperation.getValue());
+      if (schema != null) {
+        Attribute attribute = 
schema.getAttribute(attributeReference.getAttributeName());
+        
checkMutability(schema.getAttributeFromPath(attributeReference.getFullAttributeName()));
+
+        patchOperationHandler.applyExtensionValue(source, sourceAsMap, schema, 
attribute, valuePathExpression, attributeReference.getUrn(), 
patchOperation.getValue());
+      } else {
+        // If schema is null, it's either the root of an extension, or an 
invalid patch path
+        // It's not possible from the antlr parser to tell the diff between 
'this:is:extension:urn' and 'this:is:extension:urn:attribute'

Review Comment:
   Yeah I was looking at this too, I think the SCIM 
[RFCs](https://datatracker.ietf.org/doc/html/rfc7644) ANBF has this same issue
   
   ```
   attrPath  = [URI ":"] ATTRNAME *1subAttr
                    ; SCIM attribute name
                    ; URI is SCIM "schema" URI
   ``` 
   
   its kinda unfortunate but I think this might be the path of least resistance 
and path of least hack



##########
scim-core/src/test/java/org/apache/directory/scim/core/repository/PatchHandlerTest.java:
##########
@@ -56,6 +57,43 @@ public PatchHandlerTest() {
     this.patchHandler = new DefaultPatchHandler(schemaRegistry);
   }
 
+  @Test
+  public void applyReplaceEntireEnterpriseExtension() {
+    String enterpriseExtensionUrn = 
"urn:ietf:params:scim:schemas:extension:enterprise:2.0:User";
+    Map<String, String> enterpriseExtensionValue = new HashMap<>();
+    enterpriseExtensionValue.put("costCenter", "New Cost Center");
+    enterpriseExtensionValue.put("department", "New Department");
+    PatchOperation op = patchOperation(REPLACE, enterpriseExtensionUrn, 
enterpriseExtensionValue);
+    ScimUser updatedUser = patchHandler.apply(user(), List.of(op));
+    EnterpriseExtension actual = (EnterpriseExtension) 
updatedUser.getExtension("urn:ietf:params:scim:schemas:extension:enterprise:2.0:User");

Review Comment:
   XS Nit:
   
   This one is entirely my fault as I did this when I wrote the test, but we 
could replace the string here with the variable defined above
   ```suggestion
       EnterpriseExtension actual = (EnterpriseExtension) 
updatedUser.getExtension(enterpriseExtensionUrn);
   ```



-- 
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

Reply via email to