fix: isolate Cache.Sandbox scan results per sandbox#40
Merged
Conversation
Previously scan/1 returned every key in the shared sandbox Agent, including keys written by concurrent async tests registered to the same cache module. This caused leakage between tests (duplicate results from one sandbox, or keys from another sandbox appearing in scan output). The scan macro now passes the current sandbox_id as a sandbox_prefix option, and Cache.Sandbox.scan filters stored keys to only those matching that prefix. Non-sandbox mode and the Redis adapter are unaffected (the opt is only set when sandbox? is true).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
=======================================
Coverage 83.46% 83.46%
=======================================
Files 22 22
Lines 617 617
=======================================
Hits 515 515
Misses 102 102 ☔ View full report in Codecov by Sentry. |
Previously, per-sandbox isolation was layered on top of the sandbox Agent
by prefixing keys with the sandbox_id in Cache (maybe_sandbox_key) and
filtering scan results via a sandbox_prefix option threaded through
Cache.Redis. This spread sandbox concerns across three modules and still
leaked data through operations that bypassed the prefix path.
Cache.Sandbox now owns isolation directly: its Agent state is shaped as
%{sandbox_id => %{key => value}} and every operation (core cache, hash,
json, scan, plus all ETS/DETS-style helpers) reads and writes only the
sub-map for the current caller's sandbox_id. The sandbox_id is resolved
via SandboxRegistry in the caller process before entering the Agent
callback, so caller $callers/$ancestors resolution still works. Falls
back to :__unscoped__ when no registry entry exists.
With isolation handled inside the adapter, Cache no longer needs to
prefix keys in sandbox mode (maybe_sandbox_key is now a plain no-op) and
Cache.Redis no longer threads a sandbox_prefix into scan opts.
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
1 participant
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.
Summary
Cache.Sandbox.scan/3read every key from the shared sandboxAgentregardless of which sandbox registered it. Withasync: truetests sharing a cache module, keys written in sandbox A leaked into scan output in sandbox B — producing duplicates or missing entries depending on timing.scan/1macro now passes the current sandbox id as a new:sandbox_prefixscan option, andCache.Sandbox.scan/3filters stored keys to only those starting with that prefix before the existing match/type filters run.maybe_sandbox_scan_opts/1is a no-op), so the Redis adapter keeps its current behavior and ignores the unknown option.Repro
Two async tests each writing packages to the same cache module and calling
get_package_list()(which usesscan/0to enumerate keys) would see each other's writes.Test plan
CacheSandboxTest > scan only returns keys from the current sandboxverifies scans from two concurrently-registered sandboxes are isolatedmainand passes with this change