Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[apollo-datasource-rest] Http cache semantics not honored as expected #40

Closed
Jolville opened this issue Jun 26, 2020 · 0 comments · Fixed by #100
Closed

[apollo-datasource-rest] Http cache semantics not honored as expected #40

Jolville opened this issue Jun 26, 2020 · 0 comments · Fixed by #100

Comments

@Jolville
Copy link

Package version
"apollo-datasource-rest": "^0.6.11"

Expected behavior
When a GET request is made using RESTDataSource and a ttl value is provided, the response is cached for the given time.

const response = await this.get('some-path', {}, { cacheOptions: { ttl: 600 } });

Then a second request is made while the request is still cached, but the reload option is given:

const response = await this.get('some-path', {}, { cache: 'reload', cacheOptions: { ttl: 600 } });

I am expecting this will fetch the response from the datasource and then save the response to the cache with a ttl of 600.

Actual behaviour
The response from the second request is returned from the cache (and the cache isn't updated). I believe that this if statement here should check whether the request needs to be remade regardless of the ttlOverride from the first cached request.

Please let me know if this behavior is deliberate.

Thanks!

@Jolville Jolville changed the title [apollo-datasource-test] Http cache semantics not honored as expected [apollo-datasource-rest] Http cache semantics not honored as expected Jun 26, 2020
@glasser glasser transferred this issue from apollographql/apollo-server Oct 11, 2022
glasser added a commit that referenced this issue Nov 24, 2022
RESTDataSource contained a memoization cache (separate from the
HTTP-header-sensitive shared cache) which caches all GET requests
forever. This wasn't even documented for the first four years of
RESTDataSource's existence and led to a lot of confusion.

In this new major version, this cache will instead by default only
de-duplicate *concurrent* GET requests.

This also introduces `deduplicationPolicyFor()` which lets you tune how
de-duplication works, including restoring v4 semantics (see the
changeset for code snippets).

Fixes #40.
Fixes #72.
Fixes #73.
Fixes #46.

Maybe fi-xes #39 but we could use a unit test.

Somewhat addresses #65 but only for the de-duplication cache (we should
still allow direct control over the cache key for the HTTP).
glasser added a commit that referenced this issue Nov 28, 2022
RESTDataSource contained a memoization cache (separate from the
HTTP-header-sensitive shared cache) which caches all GET requests
forever. This wasn't even documented for the first four years of
RESTDataSource's existence and led to a lot of confusion.

In this new major version, this cache will instead by default only
de-duplicate *concurrent* GET requests.

This also introduces `deduplicationPolicyFor()` which lets you tune how
de-duplication works, including restoring v4 semantics (see the
changeset for code snippets).

Fixes #40.
Fixes #72.
Fixes #73.
Fixes #46.

Maybe fi-xes #39 but we could use a unit test.

Somewhat addresses #65 but only for the de-duplication cache (we should
still allow direct control over the cache key for the HTTP).
glasser added a commit that referenced this issue Nov 28, 2022
RESTDataSource contained a memoization cache (separate from the
HTTP-header-sensitive shared cache) which caches all GET requests
forever. This wasn't even documented for the first four years of
RESTDataSource's existence and led to a lot of confusion.

In this new major version, this cache will instead by default only
de-duplicate *concurrent* GET requests.

This also introduces `deduplicationPolicyFor()` which lets you tune how
de-duplication works, including restoring v4 semantics (see the
changeset for code snippets).

Fixes #40.
Fixes #72.
Fixes #73.
Fixes #46.
Fixes #39 (though it "introduces" #102).

Somewhat addresses #65 but only for the de-duplication cache (we should
still allow direct control over the cache key for the HTTP).
glasser added a commit that referenced this issue Nov 29, 2022
RESTDataSource contained a memoization cache (separate from the
HTTP-header-sensitive shared cache) which caches all GET requests
forever. This wasn't even documented for the first four years of
RESTDataSource's existence and led to a lot of confusion.

In this new major version, this cache will instead by default only
de-duplicate *concurrent* GET requests.

This also introduces `deduplicationPolicyFor()` which lets you tune how
de-duplication works, including restoring v4 semantics (see the
changeset for code snippets).

Fixes #40.
Fixes #72.
Fixes #73.
Fixes #46.
Fixes #39 (though it "introduces" #102).

Somewhat addresses #65 but only for the de-duplication cache (we should
still allow direct control over the cache key for the HTTP).
glasser added a commit that referenced this issue Nov 30, 2022
RESTDataSource contained a memoization cache (separate from the
HTTP-header-sensitive shared cache) which caches all GET requests
forever. This wasn't even documented for the first four years of
RESTDataSource's existence and led to a lot of confusion.

In this new major version, this cache will instead by default only
de-duplicate *concurrent* GET requests.

This also introduces `deduplicationPolicyFor()` which lets you tune how
de-duplication works, including restoring v4 semantics (see the
changeset for code snippets).

Fixes #40.
Fixes #72.
Fixes #73.
Fixes #46.
Fixes #39 (though it "introduces" #102).

Somewhat addresses #65 but only for the de-duplication cache (we should
still allow direct control over the cache key for the HTTP).

Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant