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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
