NerdWalletOSS / apollo-cache-policies

An extension of the Apollo 3 cache with support for advanced cache policies.
Apache License 2.0
152 stars 22 forks source link

add query key args TTL fix #40

Closed danReynolds closed 2 years ago

danReynolds commented 2 years ago

Fixes an issue identified in #39 where a cache fields on the Query type with a custom field policy keyArgs specification would not be evicted when their TTL had expired.

This is because the custom keyArgs would cause the field to be stored differently in the cache. Consider the following example:

Policies:

{
  typePolicies: {
    Query: {
      fields: {
        allFilms: {
          keyArgs: false,
        }
     }
    }
  },
  invalidationPolicies: {
    timeToLive: 10000,
  }
}

Query:

query {
  allFilms({ page: 1}) {
     ...
  }
}

Response:

{
   allFilms: {
      __typename: 'FilmsConnection',
     films: [...],
   }
}

In this case, if the allFilms field is queried for with arguments like { page: 1 }, it would not be stored in the cache as allFilms({page : 1}) like if the keyArgs was unspecified and instead is stored as allFilms. When the TTL eviction later runs for the FilmsConnection type, since there is no keyArgs type policy for the FilmsConnection type, it tries to look up the field by allFilms({page : 1}) and does not find a corresponding entry in the store.

There are a couple potential solutions to this issue:

  1. Run TTL eviction policies on the Query type as well, which does have the keyArgs specified and would find the allFIlms store field and evict it.

This works by making it so that each field directly off the Query type would have two TTL policies run for it:

a) The TTL policy for its own type, in this case it would be the FilmsConnection TTL which since unspecified would fall back to the global TTL and not work since it would look up the field with the args as allFilms({page: 1}) b) The TTL policy as a field of the Query type, which would also fallback to the global TTL and find the field under allFilms.

The problem with this approach is that it would be a breaking change and cause some potentially unexpected behavior for library consumers.

In the README example, we call out a way of specifying a global TTL, as well as type specific TTLs:

  import { InvalidationPolicyCache, RenewalPolicy } from '@nerdwallet/apollo-cache-policies';

  const cache = new InvalidationPolicyCache({
    typePolicies: {...},
    invalidationPolicies: {
      timeToLive: 3600 * 1000; // 1hr TTL on all types in the cache
      renewalPolicy: RenewalPolicy;
      types: {
        Employee: {
          timeToLive: 3600 * 1000 * 24 // 24hr TTL specifically for the Employee type in the cache
        },
      }
    }
  });

In this example, a global TTL of 1hr is declared as well as a custom one specifically for the Employee type. If we were to start running TTLs on the Query type as well, then this type specific override behavior would not be possible, since the global TTL would apply to the Query type and it would still be evicted. This is a breaking change, and I also don't think is very intuitive.

  1. An alternative solution and the one made in this PR is to still only run the TTL eviction policy for the entity type of fields off of the Query, in this case the FilmsConnection type, but check its name using both its own keyArgs and the Query type's keyArgs.

This fixes the issue and retains the TTL overriding ability as outlined in the README.