Re: [PR] eTag support with If-Match [directory-scimple]
bdemers commented on PR #541: URL: https://github.com/apache/directory-scimple/pull/541#issuecomment-1997604195 @symphony-enrico SCIMple's ETag support is an area that I think could be improved. (and this Pull Request is very. appreciated in moving that forward!!) SCIMple _basically_ has 2 types of resources: - In Memory (Schema, Configuration, these are created when the application starts, and _likely_ will not change) - Repository controlled (e.g. the persistence of `ScimUser`/`ScimGroup`) For the in memory items, the `ETagGenerator` should be used. For Repository controlled objects, it's up to the implementation, they can use the `ETagGenerator`, but they could also use something else (e.g. functionality tied to a database) > [!NOTE] > This was relatively recent change (before the last release). For implementations that don't return all fields based on optimized lookups based on the [`attributes`/`excludedAttributes` params](https://datatracker.ietf.org/doc/html/rfc7644#section-3.4.2.5) using the `ETagGenerator` would have resulted in an _incorrect_ version/etag. For example, if a ScimUser Repository that did NOT return group memberships, that data would be missing for hashing in the `ETagGenerator`. All that being said, if you have thoughts around this, ways to make this easier, or think my above assumption is wrong. Please let us know! -- 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
Re: [PR] eTag support with If-Match [directory-scimple]
symphony-enrico commented on code in PR #541:
URL: https://github.com/apache/directory-scimple/pull/541#discussion_r1524509240
##
scim-core/src/main/java/org/apache/directory/scim/core/repository/Repository.java:
##
@@ -69,14 +70,14 @@ public interface Repository {
*
*
* @param id the identifier of the ScimResource to update and persist.
- * @param version an optional version (usually used as an ETag) that can be
used to optimize update requests, may be compared against, the current {@code
ScimResource.meta.version}.
+ * @param etags optional ETag(s) in 'If-Match' header. If not null, to avoid
dirty writing, {@code ScimResource.meta.version} must match one of this set
(the set should contain only one element, and it must be not weak)
Review Comment:
BTW I added into the comment the it must be not weak... this also because
the eta is generated by scimple framework and it is generated as not weak in
https://github.com/apache/directory-scimple/blob/a2e61a994187b09940bfa338248af66f987fd5aa/scim-server/src/main/java/org/apache/directory/scim/server/rest/EtagGenerator.java#L68
However I double checked now and I see that all examples in RFC are weak and
in:
https://datatracker.ietf.org/doc/html/rfc7644#section-3.14
> The SCIM protocol supports resource versioning via standard HTTP
>ETags ([Section 2.3 of
[RFC7232]](https://datatracker.ietf.org/doc/html/rfc7232#section-2.3)).
Service providers MAY **support weak ETags**
>as the preferred mechanism for performing conditional
>retrievals and ensuring that clients do not inadvertently overwrite
>each other's changes, respectively
However, it is strange use weak etag for versioning. Anyway one more reason
to dont enforce and let implementation to decide
--
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
Re: [PR] eTag support with If-Match [directory-scimple]
symphony-enrico commented on PR #541: URL: https://github.com/apache/directory-scimple/pull/541#issuecomment-1996895797 ok to test -- 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
Re: [PR] eTag support with If-Match [directory-scimple]
symphony-enrico commented on code in PR #541:
URL: https://github.com/apache/directory-scimple/pull/541#discussion_r1524470637
##
scim-core/src/main/java/org/apache/directory/scim/core/repository/Repository.java:
##
@@ -69,14 +70,14 @@ public interface Repository {
*
*
* @param id the identifier of the ScimResource to update and persist.
- * @param version an optional version (usually used as an ETag) that can be
used to optimize update requests, may be compared against, the current {@code
ScimResource.meta.version}.
+ * @param etags optional ETag(s) in 'If-Match' header. If not null, to avoid
dirty writing, {@code ScimResource.meta.version} must match one of this set
(the set should contain only one element, and it must be not weak)
Review Comment:
IMHO we can let implementation decide. There are too much variables (maybe a
list of etag is provided, some weak, other not... how decide? Implementation
may decide to discard weak one, check only the other... or may decide to refuse
all -after all for dirty write protection, it is very strange to provide a list
of version, even if allowed by http protocol- ... and may be it can also decide
to dont implement versioning at all... or to enforce it, refusing empty etag
parameter or refusing *)
--
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
