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