Skip to content

fix(sandbox): implement Redis set ops and raw command/pipeline instead of raising - #53

Open
MikaAK wants to merge 2 commits into
mainfrom
fix/sandbox-redis-set-ops
Open

fix(sandbox): implement Redis set ops and raw command/pipeline instead of raising#53
MikaAK wants to merge 2 commits into
mainfrom
fix/sandbox-redis-set-ops

Conversation

@MikaAK

@MikaAK MikaAK commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Summary

Cache.Sandbox raised "Not Implemented" from smembers/3, sadd/4, command/3, command!/3, pipeline/3 and pipeline!/3.

Under sandbox?: Mix.env() === :test the use Cache wrappers delegate to Cache.Sandbox, so dialyzer (running in test) has been correctly inferring that every Redis cache module's injected command/1,2, command!/1,2, pipeline/1,2, pipeline!/1,2, sadd/2,3, smembers/2 have no local return — 11 no_return warnings per Redis cache module, forcing a :no_return ignore entry per module in every consuming app.

  • sadd/smembers — real in-memory sets (MapSet under the key), Redis semantics: SADD returns the new-member count, SMEMBERS of a missing key is [].
  • command/pipelinePING, GET, EXISTS, DEL mapped onto the sandbox map; anything else returns {:error, %ErrorMessage{code: :not_implemented}} — the same error shape the Redis adapter produces on failure — instead of raising. Raw GET returns the stored binary undecoded, exactly as Redis would (the Redis-backed sandbox term-encodes on put). pipeline runs commands in order and halts on the first error.
  • Bang variants raise only on that error branch, so they have a real success typing too.

Verification

  • New test/cache/redis_sandbox_test.exs (11 tests, sandbox only, no Redis needed) — green.
  • Dialyzer on a sandbox?: true Redis cache module: main produces the 11 no_return warnings above; this branch produces 0.
  • Consuming umbrella (cheddar_flow_ex, 6 Redis cache modules): with this branch as a path dep and all six :no_return ignore entries deleted, dialyzer goes 174 → 108 total warnings, all remaining ones already skipped, 0 unnecessary skips.
  • Full suite: the 13 pre-existing real-Redis (RedisJSONTest/RedisHashTest under parallel contention) failures are unchanged from main; everything else green.

MikaAK added 2 commits August 17, 2026 18:02
…d of raising

Cache.Sandbox raised "Not Implemented" from smembers/3, sadd/4, command/3,
command!/3, pipeline/3 and pipeline!/3. Under sandbox?: Mix.env() === :test
the use-Cache wrappers delegate to Cache.Sandbox, so dialyzer (running in
test) correctly inferred every Redis cache module's injected command/1,
pipeline/1, sadd/2, smembers/2 ... had no local return — forcing a
:no_return ignore entry per Redis cache module in every consuming app.

- sadd/smembers: real in-memory sets (MapSet under the key), Redis
  semantics — SADD returns the new-member count, SMEMBERS of a missing key
  is [].
- command/pipeline: PING, GET, EXISTS, DEL mapped onto the sandbox map;
  anything else returns {:error, %ErrorMessage{code: :not_implemented}} —
  the same error shape the Redis adapter produces — instead of raising.
  The bang variants raise only on that error branch, so they have a real
  success typing too.
- pipeline runs commands in order and halts on the first error.
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.87%. Comparing base (3c6c060) to head (16bbe13).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #53   +/-   ##
=======================================
  Coverage   83.87%   83.87%           
=======================================
  Files          24       24           
  Lines         707      707           
=======================================
  Hits          593      593           
  Misses        114      114           

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

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.

1 participant