Refresh OverDrive collection token every 30 days (PP-3937)

[dbernstein]

Description

Add a two-layer cache for OverDrive collection tokens so they are
automatically refreshed as OverDrive rotates them, without requiring a
process restart or configuration change.

Motivation and Context

OverDrive is retiring legacy collection tokens within the next 3–4 months
and moving to a model where tokens must be refreshed periodically.
collectionToken is embedded in the library account response
(GET /v1/libraries/{id}) and is required for all collection-scoped API
calls (search, product listings, availability, metadata).
Two problems existed in the prior implementation:

1. DB cache had no TTL. get_library() and get_advantage_accounts()
   called Representation.get() with no max_age, so the library document
   (and the collectionToken within it) could persist in the
   representations table indefinitely.
2. In-memory cache was unbounded. _collection_token was a plain
   str | None set once and never cleared. Flask workers hold one
   OverdriveAPI instance per collection for the entire process lifetime
   (CirculationManager.load_settings), so a rotated token would never be
   picked up without a full process restart or an admin config change.

Solution: two-layer cache

Layer	Constant	TTL	Purpose
In-memory (_cached_collection_token)	COLLECTION_TOKEN_MAX_AGE	5 minutes	Avoids a DB hit on every request
DB (representations table)	LIBRARY_MAX_AGE	30 days	Avoids a network hit on every 5-minute miss
A long-lived Flask worker re-checks the DB every 5 minutes and the DB
re-fetches from OverDrive every 30 days. Rotated tokens are picked up
within 5 minutes.

How Has This Been Tested?

• test_collection_token — verifies the in-memory cache is populated on
  first access and reused on subsequent calls within COLLECTION_TOKEN_MAX_AGE.
• test_collection_token_cache_expires — verifies that aging the
  in-memory cache past COLLECTION_TOKEN_MAX_AGE causes get_library to
  be called again and returns the new token.
• test_collection_token_error — verifies that an errorCode in the
  library response raises CannotLoadConfiguration.
• test_get_library_passes_max_age — verifies LIBRARY_MAX_AGE is
  forwarded to Representation.get() in get_library().
• test_get_library_refreshes_stale_token — end-to-end DB staleness test:
  ages the Representation record past LIBRARY_MAX_AGE and verifies the
  next call re-fetches from the network.
• test_get_advantage_accounts_passes_max_age — verifies LIBRARY_MAX_AGE
  is also forwarded in the advantage accounts Representation.get() call.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.30%. Comparing base (0312188) to head (96b09f4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3204   +/-   ##
=======================================
  Coverage   93.30%   93.30%           
=======================================
  Files         497      497           
  Lines       46163    46166    +3     
  Branches     6326     6326           
=======================================
+ Hits        43074    43077    +3     
+ Misses       2004     2003    -1     
- Partials     1085     1086    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jonathangreen]

I approved this @dbernstein, but I think there is some interaction between the two layers of caching here that are not ideal. I added some comments with my thoughts for consideration.

I don't think we really need two layers of caching here. It makes it kind of hard to reason about what is going on. I think we might be better off just dropping the in-memory cache and caching via Representation.get.

[jonathangreen]

Because of the two layer caching here, if the self.get_library returns a cached representation near the end of its TTL, then we will cache it here for an additional LIBRARY_MAX_AGE. So a token could be used for 2 * LIBRARY_MAX_AGE.

[jonathangreen]

Representation.get returns the old document on error, even if max_age has passed. So its possible that a failure will make the old document stick around. Combined with the second layer of caching, I think this means old tokens can hang around for some time.

[dbernstein]

@jonathangreen : thanks for the review. I'll drop the in-memory cacheing and we can always bring it (or something similar) back if necessary.

[dbernstein]

@jonathangreen : in the current state of the app (ie what is currently deployed in production), calls to the OverDrive API is caching the token once it has been read and it doesn't refresh it for the life of the OverdriveAPI object. For celery this is not a problem since overdrive api instances are shortlived. For the flask layer calls, the OD API hangs around for the entire life of the process which is only restarted if a uwsgi process runs into trouble. So if we didn't restart every two weeks, we could hit a snag. That was the motivation for the second level cache: basically keep the existing caching functionality but also try to guarantee we'll recycle the token every X days.

I just want to make sure you're clear that by removing the second layer, we would be losing an existing performance optimization for both the flask calls (not a big deal since there are so many fewer) and the celery import calls (multiple API calls within a single celery task). I agree with your assessment about being hard to reason about and the possible document failure you identified. Just want to make sure we're aligned.

Another option is to have the second level cache work on a different frequency than the first level cache - ie just enough time for the celery task to enjoy the caching of the token (from the database) - ie couple of minutes.

[jonathangreen]

Thanks @dbernstein , we're aligned. I understand the tradeoff, without the in-memory layer, each collection_token access during a Celery import will do a DB query via Representation.get().

I'm fine with dropping the in-memory cache to keep things simple, but I'd also be okay with keeping it at a shorter TTL as you suggested. My concern wasn't the in-memory cache itself, just the long lifetime causing the unexpected interactions I flagged.