Fix bug with caching in variables filter

[vol1ura]

Here was a bug and cache was never used

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.50%. Comparing base (ae5e542) to head (a0a224c).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #106 +/- ## ======================================= Coverage 96.50% 96.50% ======================================= Files 83 83 Lines 2147 2147 ======================================= Hits 2072 2072 Misses 75 75 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[AlexWayfer]

Please, add specs (test) which will fail in the current main branch and work in your's one.

[simonneutert]

I can be wrong, but I thought I'd give it a quick glance and see if I can find a fix quickly.

Well... I couldn't come up with a simple way to spec it. But here's how this can be reproduced ...

The underlying spec I used to reproduce

When reading through the codebase it felt natural, that the caching needed substitution.

So I glanced in the yaml files and took a key where the value contained a "%" symbol.

Precisely I settled on the params key.

# spec/filters_spec.rb added in line 36
it 'adds cached filter' do
   filter = R18n::Filters.add('my', :params) { |i, _config| i }

   expect(filter).to be_kind_of(R18n::Filters::Filter)
   expect(filter.name).to eq(:params)
   expect(filter.types).to eq(['my'])
   expect(filter).to be_enabled

   expect(R18n::Filters.defined).to have_key(:params)

   expect(i18n.params).to eq('Is  between  and ?')
   expect(i18n.my_tree_filter).to eq('name' => 'value')
 end

Current state

Adding a counter var to the loop

Here's the thing when adding a counter var to lib/r18n-core/filters.rb

Filters.add(String, :variables) do |content, config, *params|
  cached_params = []
  counter = 1
  content.to_s.gsub(/%\d/) do |key|
    i = key[1..].to_i
    unless cached_params.include? i - 1
      param = config[:locale].localize(params[i - 1])
      param = ActiveSupport::SafeBuffer.new + param if defined? ActiveSupport::SafeBuffer

      cached_params[i - 1] = param
      puts counter
      counter += 1
    end
    cached_params[i - 1]
  end
end

The logs show:

The loop runs three times.

What I think this PR had in mind

Caching only plays a role when having multiple yaml values with duplicate "%{int}" values (I guess).

When implementing a slightly modified version of the code in the PR

Filters.add(String, :variables) do |content, config, *params|
  cached_params = {}
  counter = 1
  content.to_s.gsub(/%\d/) do |key|
    i = key[1..].to_i
    unless cached_params.key?(i - 1)
      puts "inner: #{i - 1}"
      param = config[:locale].localize(params[i - 1])
      param = ActiveSupport::SafeBuffer.new + param if defined? ActiveSupport::SafeBuffer

      cached_params[i - 1] = param
      puts counter
      counter += 1
    end
    cached_params[i - 1]
  end
end

Output with the changes

The loop runs to times, once for every key, being: %1 and %2.

---

Hope this helps! If so, I will happily chip in, maybe you have a vision @AlexWayfer for a clean and easy way to spec this out 😬 🤞

Factoring bits out might enable us to use RSpec's #receive, maybe?

https://rubydoc.info/github/rspec/rspec-mocks/RSpec%2FMocks%2FExampleMethods:receive