bdemers commented on PR #410: URL: https://github.com/apache/directory-scimple/pull/410#issuecomment-1804812728
Very helpful! I agree, I think this is separate from the `attributes` change I suggested in this PR. It is possibly related, in both cases I think the REST layer is doing too much. In your case the call to `T stored = repository.get(id);` is triggering a lookup without any other context (e.g. do you need to include all the members) The currently logic: https://github.com/apache/directory-scimple/blob/b91fee906b585a7832b6f45f0ea1e70092761d7d/scim-server/src/main/java/org/apache/directory/scim/server/rest/BaseResourceTypeResourceImpl.java#L292-L302 Is basically doing: 1. Get Object 2. Check if the object exists (404) 3. Calculate the etag 4. Update the Object IMHO, all of this should probably be pushed into the repository The etag usage should be an optional optimization if you can/want to support it. That logic may need to be pushed down into a the datastore (pulling the object into memory, and then calculating the hash of the object could be expensive) I'm thinking the REST layer should delegate all of that logic into the `Repository` layer, possibly with separate `update` and `patch` methods 🤔 - Those method could throw a new `ResourceNotFoundException` (which the REST layer could map back to a 404), or return `null` to match the behavior of `repository.get(id)`. - Not sure about the case where the ETAG dictates nothing needs to be done (throw some sort of exception, return an object with the version/etag set, and the REST layer could do the comparison 🤷) I'm not sure any of those are great options... Anyway, something like this would put all the control into the implementors hands: - Need to support etags, implement it efficiently in your datastore - Optimized queries for patch requests Questions: - Would separate `update` and `patch` methods be useful to you? - Do you care about etags? - Do you support patch requests for objects other than Groups? -- 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]
