search

sourcelevel / faraday-http-cache

A Faraday middleware that respects HTTP cache
Other
343 stars 86 forks source link

Only update cache when the headers have changed on validation requests #91

Closed chrisface closed 4 years ago

chrisface commented 7 years ago

As mentioned in this issue #77 a store to the cache is triggered to update headers even if the response was not modified. This makes sense to keep the headers in the cache up to date. If there have been no changes to the headers then this causes wasteful writes. Depending on how often a validation occurs and the speed of the cache this can become quite expensive.

Adding a check to see if the 304 response headers match those in the cache allows us to skip the store if nothing has changed. We check for a subset of the headers as 304 should not include certain headers and may be missing others depending on the server.

Some additional configuration to ignore changes in specific headers when determining if response should be cached was also added. This is useful when a server returns headers which change on every request but are not relevant for caching. An example of this could be headers containing the address of the server that handled the response. This allows flagged headers to change on validate requests without triggering a cache store.

One of the specs with the Vary header was changed as the test_app was returning a different value for this header for a 200 and a 304. I think that this header is meant to be consistent across responses for the same resource. If the Vary header has changed and you are now taking into account different headers to determine what cached response should be then it's not possible for this response to be a 304 while at the same time having a different vary header to the 200. Updating the stored entry here doesn't seem to correct to me but this my interpretation of the spec and there many have been another reason for this.

ebert-bot commented 7 years ago

Hello, @chrisface! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

chrisface commented 7 years ago

@lucasmazza Do you have any thoughts on the suggested changes in this PR?