Skip to content

fix(ets): route ETS-specific functions through sandbox adapter#37

Open
BobbieBarker wants to merge 1 commit intoMikaAK:mainfrom
BobbieBarker:fix/ets-sandbox-isolation
Open

fix(ets): route ETS-specific functions through sandbox adapter#37
BobbieBarker wants to merge 1 commit intoMikaAK:mainfrom
BobbieBarker:fix/ets-sandbox-isolation

Conversation

@BobbieBarker
Copy link
Contributor

Summary

  • ETS-specific functions (update_counter/2, update_counter/3, insert_raw/1, match_object/1, match_object/2) were bypassing sandbox isolation by calling :ets directly instead of routing through @cache_adapter
  • Added defoverridable wrappers in the __using__ macro (lib/cache.ex) that prefix keys via maybe_sandbox_key and dispatch through @cache_adapter when sandbox?: true and adapter: Cache.ETS
  • The sandbox adapter (Cache.Sandbox) already had implementations for these functions — they just were never called from the generated module

Test plan

  • Added test/cache/ets_sandbox_test.exs with 11 tests verifying sandbox isolation for all affected functions
  • Each function has a test that writes data and a separate test verifying the data is not visible (isolation)
  • All existing tests pass (126/127 — the 1 failure in redis_json_test.exs is pre-existing and unrelated)

ETS-specific functions (update_counter, insert_raw, match_object)
were bypassing sandbox isolation by calling :ets directly. This
caused concurrent async tests to collide when using sandbox?: true
with Cache.ETS adapter.

Add defoverridable wrappers in the __using__ macro that prefix keys
via maybe_sandbox_key and route through @cache_adapter when sandbox
mode is enabled.
@BobbieBarker BobbieBarker force-pushed the fix/ets-sandbox-isolation branch from f201b7c to 5688897 Compare March 7, 2026 18:39
@MikaAK
Copy link
Owner

MikaAK commented Mar 9, 2026

I think this is a fundamental problem with the ets adapter, it probably should be calling something the module attribute to allow the sandbox switchover 🤔

@cking222
Copy link
Collaborator

cking222 commented Mar 9, 2026

I think this is a fundamental problem with the ets adapter, it probably should be calling something the module attribute to allow the sandbox switchover 🤔

I am open to feedback. If you want this to work differently let me know. I was optimistically trying to use the new ETS functionality you added, and found out that these functions don't play nice with the sandbox, so i whipped something up to cover the gap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants