Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

New: Use ETAG in header for reading releases from the cache #12

Merged
merged 2 commits into from
Sep 11, 2019

Conversation

swatikode
Copy link
Contributor

What is the purpose of this pull request? (put an "X" next to item)

[] Documentation update
[] Bug fix
[x] New functionality
[] Changes an existing functionality
[] Other, please explain:

What changes did you make? (Give an overview)
An HTTP call to retrieve the releases does not use any authentication, as a result of this there is a rate limit of how much requests can be made. To solve a problem, I decided to capture the ETAG from the first retrieve releases request that is returned with a status of 200. The response of this request is cached in a localStorage. And then use this ETAG in the subsequent requests for retrieving releases. If the resource (releases data) has not changed then the github API server will return with 304-status and no response. In this situation we will return the cached version of data.

Add any screenshots
localStorage used for storing releases data and ETAG

image

I refreshed the page and you can see releases returned with 304
image

When clicked the versions chip then also releases returned with 304 because resource is not changed
image

@abhijit945 abhijit945 added the enhancement New feature or request label Sep 11, 2019
Copy link
Contributor

@abhijit945 abhijit945 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment.
Looks good to me 👍

)
.then(response => {
if (response.status === 200) {
localStorage.setItem(constants.LOCAL_STORAGE.GET_RELEASES_ETAG, response.headers.etag.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if we should clear the localStorage on unmount of the app? Not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that is necessary because the data in local storage is not dependent on anything happening in the DOM

VERSIONS_PATH: "versions"
VERSIONS_PATH: "versions",
LOCAL_STORAGE: {
GET_RELEASES_ETAG: "GET_RELEASES_ETAG",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value can be camelCased.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. ETAG is left all upper case because it is an abbreviation

@abhijit945 abhijit945 merged commit 2dae5d4 into cerner:master Sep 11, 2019
abhijit945 pushed a commit that referenced this pull request Sep 11, 2019
# [1.4.0](v1.3.2...v1.4.0) (2019-09-11)

### New

* Use ETAG in header for reading releases from the cache (#12) ([2dae5d4](2dae5d4)), closes [#12](#12)
@abhijit945
Copy link
Contributor

🎉 This PR is included in version 1.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants