Fix file cache store delete_matched to work on keys longer than allowed filename size

[fatkodima]

Fixes #49690.

For file cache store we url-encode keys to not to clash to with the file system path separators, like /. When the resulting key is longer than the FS max file name length, we split it into parts. But this splitting can be done incorrectly and made in-between the encode-sequence, while it should be done only on the boundaries.

Example: for key foo%20bar splitting into ['foo%2', '0bar'] is incorrect, should be ['foo', '%20bar'] or ['foo%20', 'bar'].

Then when we use delete_matched, we traverse the FS directories and try to decode dir names and get an error if we previously made a split in the incorrect place.

[jonathanhefner]

rindex with a regexp is a bit slower:

Benchmark script

# frozen_string_literal: true

require "benchmark/ips"
require "active_support/all"

FILENAME_MAX_SIZE = 226
FILENAME = "x" * (FILENAME_MAX_SIZE + 1)

Benchmark.ips do |x|
  x.report("original") do
    fname = FILENAME
    fname_paths = []
    begin
      fname_paths << fname[0, FILENAME_MAX_SIZE]
      fname = fname[FILENAME_MAX_SIZE..-1]
    end until fname.blank?
  end

  x.report("rindex") do
    fname = FILENAME
    fname_paths = []
    loop do
      if fname.length < FILENAME_MAX_SIZE
        fname_paths << fname
        break
      else
        index = fname.rindex(/%\h\h/, FILENAME_MAX_SIZE) || FILENAME_MAX_SIZE
        fname_paths << fname[0, index]
        fname = fname[index..-1]
      end
    end
  end

  x.compare!
end

Warming up --------------------------------------
            original    62.853k i/100ms
              rindex    42.464k i/100ms
Calculating -------------------------------------
            original    626.351k (± 2.0%) i/s -      3.143M in   5.019546s
              rindex    428.920k (± 1.2%) i/s -      2.166M in   5.049889s

Comparison:
            original:   626351.3 i/s
              rindex:   428919.5 i/s - 1.46x  slower

It also changes the cache keys, which might be an issue for existing deploys.

But, digging further, I think there is actually another bug in delete_matched. Consider the case where the matcher pattern spans the file separator boundary:

cache.write("A" * FILENAME_MAX_SIZE + "B", "value")
cache.delete_matched(/AB/)

The file path will be "A...A/B" so /AB/ will not match it.

But we can fix both that bug and the decode issue by removing the file separators from the key. I've pushed a commit with an updated test to cover both cases. Let me know what you think.

[fatkodima]

You are right! Squashed commits into one and added you as a co-author (author actually 😅)

[jonathanhefner]

Thank you, @fatkodima! 📂

Backported to 7-1-stable in 86104e0 and 7-0-stable in f630ca2.