rexyai / RestRserve

R web API framework for building high-performance microservices and app backends
https://restrserve.org
271 stars 31 forks source link

Add further ETag headers #188

Closed DavZim closed 2 years ago

DavZim commented 2 years ago

This PR adds the If-Match and If-Unmodified-Since headers that were still missing from the earlier PR. NOTE that all the functionality is handled by process_response not process_request, therefore the PUT functionality described in MDN cannot be performed (Eg update a database only if it matches something or so) or in other words the ETag we have at the moment is catered towards values being returned and not values being send to the API.

The additional headers are documented and tested. This PR also adds a small safeguard against split date headers.

codecov[bot] commented 2 years ago

Codecov Report

Merging #188 (159cd9d) into dev (26c0537) will increase coverage by 0.05%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #188      +/-   ##
==========================================
+ Coverage   94.86%   94.92%   +0.05%     
==========================================
  Files          28       28              
  Lines        1364     1378      +14     
==========================================
+ Hits         1294     1308      +14     
  Misses         70       70              
Impacted Files Coverage Δ
R/ETagMiddleware.R 91.78% <100.00%> (+1.95%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 26c0537...159cd9d. Read the comment docs.

artemklevtsov commented 2 years ago

Do we really need to test all these formats with tryFormats?

DavZim commented 2 years ago

What do you mean by that?

Edit

ah, I get it. you mean the as.POSIXlt(... tryFormats = c("..."). Not sure if we need all of them. I took the default values from as.POSIXlt and prepended the time_fmt (the ugly format that is the standard), and %FT%TZ as might also be used and works much nicer. I did not see any drawbacks from including this, including no slower speeds but only a miniscule increase in package size. BUt we can take them out if we want.

dselivanov commented 2 years ago

@DavZim thanks for clean PR with tests!