Ahn nath/cache option apis

[ahn-nath]

Closes #294
We have explored a caching option with the library axios-cache-interceptor to “cache API calls to reduce the number of credits we use”. For this caching option we need to see the lifetime of a cached response, a way to persist the day permanently and across different user sessions and, keep in mind, the margin to consider for cutting expenses.

• Add pull request description
• Add persistent storage
• Set options to invalidate cache and the lifetime of a cached response

Description of relevant aspects:

Set options to invalidate the cache and the lifetime of a cached response
To define a limit or deadline for a cached response to be valid and updated, we make use of the property "ttl", as part of the “per request configuration”. We set it to the reasonable time limit of 1 week in milliseconds: 604800000.

// The time until the cached value is expired in milliseconds.
    ttl: 1000 * 60 * 10080,

Source: https://axios-cache-interceptor.js.org/#/pages/per-request-configuration?id=cachettl

Add persistent storage
The lifetime of a request will only make sense and work if we use persistent storage. There are several options they provide, such as:

• In memory: that will not be persistent and will easily be lost with a page reload.
• Web storage: it has sessions and across sessions (works with a browser Window and dies as soon as it is closed)
• Custom storage: which can last more time.

I am not considering using a database. Instead, I think we can use documents that are stored in the file system for persistence or variables that are initialized once during the server lifetime. I am using a Map structure that is working with the buildStorage function of the library.

Considerations:

• If the app/amplify project uses several servers to work with the representatives' data, libraries that help you persist the data elsewhere may be a better option, such as Node Redis. Now, this simpler solution can prevent the complexity such a library would add and provide a working option that fits our case. I am open to your comments and suggestions if my judgment is incorrect.

• I discussed with my mentors that in some cases, when important data changes, we may need to update the website data as soon as possible, and not wait until the scheduled time to refresh the API comes. Such a case is when we need to send a letter and require a valid address. In those cases, as suggested by my mentors @thyeggman and @paramsiddharth, an alternative would be to make a real API call and update the caching storage as per demand, or when the user triggers the function responsible for sending the letter.

Results and the effect of this implementation

On average, we could assume that I make 9 calls per day, with a total of 63 calls per week. If we are only making real API calls (updating the cache) once a week, that is, only making one call to the API per week, we are using, roughly, 1.58% of the current average. For that case, we would be cutting 98.5% of our weekly use.

[paramsiddharth]

@ahn-nath Make sure to also look into the conflicts in the draft PR once you return. :)

[thyeggman]

For a feature like this, it would be good to add some tests. You can mock the interceptor if needed to make sure that the cache contents change, and then that the API call is not made on a subsequent request after the first one is cached

[paramsiddharth]

@thyeggman Hey Jacob! I think #296 should be completed before we start writing the tests for this update. Maybe we can also have optional tests that use the actual Cicero API instead of the mocked version that will be made in #296.

[ahn-nath]

Then, if I understand correctly, this PR can be approved once the other issue is closed, correct? El vie., 28 de octubre de 2022 10:49 p. m., Param Siddharth < ***@***.***> escribió:

…

@thyeggman <https://github.com/thyeggman> Hey Jacob! I think #296 <#296> should be completed before we start writing the tests for this update. Maybe we can also have optional tests that use the actual Cicero API instead of the mocked version that will be made in #296 <#296>. — Reply to this email directly, view it on GitHub <#373 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQBB52UEKBCLK4REWYK4P53WFSGELANCNFSM6AAAAAARPRYJEE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[paramsiddharth]

Then, if I understand correctly, this PR can be approved once the other issue is closed, correct? El vie., 28 de octubre de 2022 10:49 p. m., Param Siddharth < @.> escribió:
…
@thyeggman https://github.com/thyeggman Hey Jacob! I think #296 <#296> should be completed before we start writing the tests for this update. Maybe we can also have optional tests that use the actual Cicero API instead of the mocked version that will be made in #296 <#296>. — Reply to this email directly, view it on GitHub <#373 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQBB52UEKBCLK4REWYK4P53WFSGELANCNFSM6AAAAAARPRYJEE . You are receiving this because you were mentioned.Message ID: @.>

@ahn-nath Yes. However, there are some changes requested. You'll need to make them before the PR is ready to merge. :) But you can look at it once you are back from the conference.

[ahn-nath]

As request by my mentor, Jacob, I have added some tests to guarantee that the library works as expected. 3 tests were added.

Description

1. we test that the configuration we set is valid and available to check
In this test, we just want to check that the configuration of the Axios instanxe is properly configured and that the contents of the instance change.

describe('test Axios configuration', () => {
  test('tests argument composition', () => {
  . . .
 })
})

2. we test that the cache is working as expected with concurrent requests
In this test, we check that the library/interceptor in successfully caching responses, and we also test that it is able to do it with concurrent requests.

describe('test Axios cache with concurrent requests', () => {
  test('tests cache', async () => {
    . . .
  })
})

3. we test that the cache is invalidated when the ttl expires
In this test, we set a time for when the caching/cache response will be invalidated and no longer cached. We set the expiry time to 10 sec and make a request after 10 sec from that statement to validate that the response is no longer cached.

describe('test Axios cache invalidation option', () => {
  test('tests cache invalidation', async () => {
  . . .
  }, 70000)
})

Looking forward to your thoughts and review @thyeggman, @paramsiddharth.

[paramsiddharth]

@ahn-nath I added a comment. Please check it out. :) Thank you.

[ahn-nath]

@ahn-nath I added a comment. Please check it out. :) Thank you.

Checked 👍

[paramsiddharth]

@ahn-nath Looks good to me, Nath! :) Good work!

[thyeggman]

The tests you wrote are good test cases to have, although there are come basic cases that would be good to test as well, such as a simple test to make a single request, verify that it is not cached, and then repeat the request and make sure it is cached.

I left a few comments on the behavior of Promise.all as well

[ahn-nath]

The tests you wrote are good test cases to have, although there are come basic cases that would be good to test as well, such as a simple test to make a single request, verify that it is not cached, and then repeat the request and make sure it is cached.

I left a few comments on the behavior of Promise.all as well

@thyeggman
I feel that a simple test to check for a single request is unnecessary and repetitive, so I did not create a test for that. In the second test, with the description "test Axios cache with concurrent requests", I am verifying that the first request was not cached and then assuring the second was. Let me know your thoughts.

[thyeggman]

@ahn-nath

I feel that a simple test to check for a single request is unnecessary and repetitive

You're right, it does become redundant if you change the third test as I suggested. This looks good to me now, thanks for making the changes!