search

aep-dev / aeps

AEPs help developers and organizations build clear, consistent network APIs and clients by providing an extensible set of design guidelines.
http://aep.dev/
Other
21 stars 15 forks source link

replacing usage of update etag with AIP-154 ifMatch? #241

Open toumorokoshi opened 2 weeks ago

toumorokoshi commented 2 weeks ago

Today, there are two sightly different mechanics for validation of resource freshness. Those are:

in https://aep.dev/update, using an etag field in the resource body (proto), and rejecting the request if that does match.

In https://aep.dev/154/#guidance, using an etag header in the response, and an optional if-match header in the request for resource validation.

These are fundamentally incompatible methods, and we should provide a single way to do so. As an initial proposal, I propose aligning with AIP-154 and:

toumorokoshi commented 2 weeks ago

one disadvantage of putting etags in the header is you won't be able to get it from the resource body (e.g. when you curl and write the output to disk for stage purpose). So it might make sense to still have an etag field in the body as place where the value is duplicated.

rofrankel commented 2 weeks ago

From https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Match:

The comparison with the stored ETag uses the strong comparison algorithm, meaning two files are considered identical byte by byte only. If a listed ETag has the W/ prefix indicating a weak entity tag, this comparison algorithm will never match it.

It sounds like this means that weak etags would no longer be supported. That seems like an unfortunate limitation.

dv-stewarts commented 2 weeks ago

Not as important as the actual decision, but I think there some contradiction in the text of https://aep.dev/154/ which should also be resolved:

https://aep.dev/154/#guidance

Resources must support the If-Match header (and may support the If-None-Match header) if and only if resources provide the ETag.

https://aep.dev/154/#condition-headers

Services that provide ETags should support the If-Match and If-None-Match headers on incoming requests:

One is must/may for If-Match/If-None-Match and the other is should for both. Or am I reading it wrong?

toumorokoshi commented 1 week ago

Not as important as the actual decision, but I think there some contradiction in the text of https://aep.dev/154/ which should also be resolved:

https://aep.dev/154/#guidance

Resources must support the If-Match header (and may support the If-None-Match header) if and only if resources provide the ETag.

https://aep.dev/154/#condition-headers

Services that provide ETags should support the If-Match and If-None-Match headers on incoming requests:

One is must/may for If-Match/If-None-Match and the other is should for both. Or am I reading it wrong?

Good callout! In the future I'd suggest filing a separate issue so we can keep the conversation focused.

To address your point though: Yes I think those statements are not consistent, and should be. I'd probably say the latter statement is what I'd lean toward, since etags are useful in their own right for identifying whether a resource changed, even if it's not used as a check-and-set.