fix(backends): give ttl one meaning across backends - #136
Merged
Merged
Conversation
Zero or negative TTLs meant 'never expire' on Memcached, raised on Redis and expired at once in memory. Reject them with ValueError through a shared validate_ttl() in every backend, the base fallbacks, CacheManager and StateManager. @cache(ttl=0) keeps sending max-age=0 but stores the entry like ttl=None, so the backend never sees 0; a negative @cache ttl raises CacheXError at decoration time. Closes #102
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #102.
Problem
Nothing validated
ttl, and each backend read0or a negative value differently:0means never expire.SET ... EX 0fails withinvalid expire time.So
@cache(ttl=0), a legalmax-age=0, answered 500 on every miss with Redis and replayed the first response forever with Memcached.Change
This implements the direction agreed in the issue comment.
validate_ttl(ttl)infastapi_cachex.backends.base: returnsNoneor a positivettlunchanged, and raisesValueErrorfor zero or negative values.set,set_if_absentandincrementon Memory, Redis and Memcached, before any I/O.set_if_absent/incrementfallbacks call it too.setdocstring asks third-party backends to use it.CacheManager: validatesdefault_ttlin__init__, and the effective ttl inset/add.get_or_setvalidates before running the factory.StateManager: validatesdefault_ttlin__init__and the effective ttl increate_state.@cache:ttl < 0raisesCacheXErrorat decoration time, like the other argument checks.ttl=0still sendsmax-age=0, but the entry is stored likettl=None: without expiry, and never replayed directly. A matchingIf-None-Matchgets a 304. The backend never receives0.docs/BACKENDS.mdgets a "TTL values" section.docs/HTTP_CACHING.mdanddocs/CACHE_FLOW.mddescribettl=0.SessionManageralready clamps its TTL withmax(ttl, 1)and is unchanged.Tests
tests/backends/test_ttl_contract.py:0and-1are rejected byset/set_if_absent/incrementon every built-in backend and on the base fallbacks. Redis points at an unconnected port and Memcached's client is a stub, which proves the check happens before I/O.CacheManager(including that theget_or_setfactory is not called) andStateManagerreject them too.tests/test_cache.py:test_ttl_zero_entry_expires_immediatelyasserted the old memory-only behaviour. It is replaced by a test that expectsmax-age=0, a stored entry with no expiry, a handler rerun without a validator, and a 304 with one. A second new test covers the negative-ttlCacheXError.tests/backends/test_redis.py: a live end-to-end@cache(ttl=0)route. Against the old code it fails with the Redis error.tests/backends/test_memcached.py: the(0, 0)case oftest_short_ttls_stay_relativerecorded the "0 = never expire" behaviour and is removed.Against the old code, the three
@cachetests fail (the contract tests can't importvalidate_ttl).Checks
fastapi_cachex,testsandscriptstestsandscriptsCACHEX_REQUIRE_LIVE_SERVERS=1): 744 passed, 100% coveragezensical build --strictCompatibility
ttl <= 0used to "work" on the memory backend, where the entry expired immediately. It now raises. That input already crashed on Redis and meant the opposite on Memcached, so the issue scheduled this for 0.3.x.