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

[RESTDataSource]: allow per-request caching logic #73

Closed
bennypowers opened this issue Mar 8, 2021 · 0 comments · Fixed by #100
Closed

[RESTDataSource]: allow per-request caching logic #73

bennypowers opened this issue Mar 8, 2021 · 0 comments · Fixed by #100

Comments

@bennypowers
Copy link

bennypowers commented Mar 8, 2021

RESTDataSource caches each GET request, and clears the cache for other HTTP verbs. Most of the time this is exactly what users need. However there are cases where users need to cache POST requests as if they were query-style requests, for example, if they are POSTing a graphql query, or if the resource they are requesting accepts POST bodies as configuration for some computation. In these cases, users would like to configure the caching behaviour on a per-request basis.

API Proposal

class MyDataSource extends RESTDataSource<MyContext> {
  /** @override */
  protected checkRequestCache<T = any>(
    request: Request,
    cacheKey: string,
    performRequest: () => Promise<T>,
  ): Promise<T> {
    if (request.url.includes('/my-posty-query-endpoint')) {
      let promise = this.memoizedResults.get(cacheKey);
      if (promise) return promise;
      promise = performRequest();
      this.memoizedResults.set(cacheKey, promise);
      return promise;
    } else {
      return super.checkRequestCache(request, cacheKey, performRequest);
    }
  }
}

Absent this feature, the user will have to override the entire fetch method, when she really only wants to modify the last 1/5 of its code. Moreover, fetch is private and can't be overridden, so users with this case seemingly have no recourse.


Alternate API Proposal

base class
class RESTDataSource {
  private async fetch<TResult>(
    init: RequestInit & {
      path: string;
      params?: URLSearchParamsInit;
    },
  ): Promise<TResult> {
    // ... yadda yadda ...
  
    const performRequest = async () => { /* ... */ };
  
    if (this.shouldCacheRequest(request)) {
      return this.cacheRequest(cacheKey, performRequest)
    } else {
      this.memoizedResults.delete(cacheKey);
      return performRequest();
    }
  }
  
  /** Override to configure caching behaviour per request */
  protected shouldCacheRequest(request: Request): boolean {
    return request.method === 'GET';
  }
  
  private cacheRequest<T = any>(cacheKey: string, performRequest: () => Promise<T>): Promise<T> {
    let promise = this.memoizedResults.get(cacheKey);
    if (promise) return promise;
  
    promise = performRequest();
    this.memoizedResults.set(cacheKey, promise);
    return promise;
  }
}

Usage:

protected shouldCacheRequest(request: Request): boolean {
  const { pathname } = new URL(request.url);
  switch (pathname) {
    case '/query-post':
    case '/definitely-cache-this':
      return true;
    default:
      return request.method === 'GET';
  }
}
bennypowers referenced this issue in bennypowers/apollo-server Mar 9, 2021
implements `protected shouldCacheRequest`

fixes #5007
@trevor-scheer trevor-scheer transferred this issue from apollographql/apollo-server Oct 20, 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant