Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the fetch_multi behavior fully match default cache store #16

Merged
merged 5 commits into from
Oct 13, 2023

Conversation

marcoadkins
Copy link
Contributor

Things Done:

  • Ensure fetch_multi passes the input name back to the block even if its an object.
  • Make the overall behavior of fetch_multi match AR cache fetch_multi

@pawurb
Copy link
Owner

pawurb commented Oct 12, 2023

@marcoadkins thanks for PR. But please rebase to current main, you're also changing unrelated tests.

@marcoadkins
Copy link
Contributor Author

@pawurb Just merged in main. And those unrelated spec updates were to fix an order dependency issue and some typos in the unit tests no behavior change other than ensuring the multi test has a large enough value to compress.

@pawurb
Copy link
Owner

pawurb commented Oct 12, 2023

Can you add a test case that will fail with current implementation but is fixed by this PR?

@marcoadkins
Copy link
Contributor Author

@pawurb Done.

Previous Failure:

~/projects/rails-brotli-cache (main*) » bundle exec rspec spec/rails-brotli-cache/store_spec.rb                                                macrobook@Marcos-MBP-2
.........F.............

Failures:

  1) RailsBrotliCache fetch_multi with a complex object that responds to #cache_key works for store and reread
     Failure/Error: big_enough_to_compress_value + key.id

     NoMethodError:
       undefined method `id' for "key_1":String
     # ./spec/rails-brotli-cache/store_spec.rb:126:in `block (5 levels) in <top (required)>'
     # ./lib/rails-brotli-cache/store.rb:118:in `block in fetch_multi'
     # ./lib/rails-brotli-cache/store.rb:115:in `fetch_multi'
     # ./spec/rails-brotli-cache/store_spec.rb:125:in `block (4 levels) in <top (required)>'
     # ./spec/rails-brotli-cache/store_spec.rb:144:in `block (4 levels) in <top (required)>'

Finished in 0.01292 seconds (files took 0.32236 seconds to load)
23 examples, 1 failure

Failed examples:

rspec ./spec/rails-brotli-cache/store_spec.rb:143 # RailsBrotliCache fetch_multi with a complex object that responds to #cache_key works for store and reread

@pawurb pawurb merged commit 3ea7de3 into pawurb:main Oct 13, 2023
5 checks passed
@pawurb
Copy link
Owner

pawurb commented Oct 13, 2023

Thanks! I've released as 0.6.2

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.

None yet

2 participants