active_support/cache/file_store.rb is this correct?

This should be anyways changed to "match.ord.to_s(16)" to preserve consistency with rest of the methods using gsub.
Attached a patch for this.

OK, I dug into this a bit deeper and I think I have both a test case showing failure and a fix. My prior suggested change just prevents a whiny nil dump, but doesn't fix the inherent problem.

I am running ruby 1.9.2-p0, Rails 3.0.0, on a Ubuntu box. I ran into problem as my former (2.3.x) action caching and cache sweeping was failing.

Below is a log showing failure from running "rails c", the fix, and then it apparently working.

Fragments from active_support/cache/file_store.rb. Current code:

      UNESCAPE_FILENAME_CHARS = /%[0-9A-F]{2}/
        # Translate a file path into a key.
        def file_path_key(path)
          fname = path[cache_path.size, path.size].split(File::SEPARATOR, 4).last
          fname.gsub(UNESCAPE_FILENAME_CHARS){|match| $1.ord.to_s(16)}
        end

jwilson@osprey:~/wg/bridge$ rails c
>> t = ActionController::Base.cache_store.send(:key_file_path, "views/index")
=> "/home/jwilson/wg/bridge/tmp/cache/4E4/5C0/views%2Findex"
>> ActionController::Base.cache_store.send(:file_path_key, t)
NoMethodError: undefined method `ord' for nil:NilClass
    from /home/jwilson/.rvm/gems/ruby-1.9.2-p0@cosb/gems/activesupport-3.0.0/lib/active_support/whiny_nil.rb:48:in `method_missing'
    from /home/jwilson/.rvm/gems/ruby-1.9.2-p0@cosb/gems/activesupport-3.0.0/lib/active_support/cache/file_store.rb:164:in `block in file_path_key'
    from /home/jwilson/.rvm/gems/ruby-1.9.2-p0@cosb/gems/activesupport-3.0.0/lib/active_support/cache/file_store.rb:164:in `gsub'
    from /home/jwilson/.rvm/gems/ruby-1.9.2-p0@cosb/gems/activesupport-3.0.0/lib/active_support/cache/file_store.rb:164:in `file_path_key'
    from (irb):2
    from /home/jwilson/.rvm/gems/ruby-1.9.2-p0@cosb/gems/railties-3.0.0/lib/rails/commands/console.rb:44:in `start'
    from /home/jwilson/.rvm/gems/ruby-1.9.2-p0@cosb/gems/railties-3.0.0/lib/rails/commands/console.rb:8:in `start'
    from /home/jwilson/.rvm/gems/ruby-1.9.2-p0@cosb/gems/railties-3.0.0/lib/rails/commands.rb:23:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6:in `<main>'
>>

with potential fix

      UNESCAPE_FILENAME_CHARS = /%([0-9A-F]{2})/     # create a capture group 
      # Translate a file path into a key.
      def file_path_key(path)
        fname = path[cache_path.size, path.size].split(File::SEPARATOR, 4).last
        # e.g., convert %(2F) to the character "/"
        fname.gsub(UNESCAPE_FILENAME_CHARS){|match| $1.to_i(16).chr}
      end

jwilson@osprey:~/wg/bridge$ rails c
Loading development environment (Rails 3.0.0)
>> t = ActionController::Base.cache_store.send(:key_file_path, "views/index")
=> "/home/jwilson/wg/bridge/tmp/cache/4E4/5C0/views%2Findex"
>> ActionController::Base.cache_store.send(:file_path_key, t)
=> "views/index"
>>

Edited by Rohit Arondekar for formating.

Wait... Its still not quite right. Let me debug some more.

Ok, it is working correctly with above patch. I had an incorrect expire_fragment regexp.

I do see that cache fragments are stored in an encoded fashion:

jwilson@osprey:~/wg/bridge$ dir tmp/cache/F88/640/
tmp/cache/F88/640/views%2Flocalhost%3A3000%2Fgames%2Findex%3Fid%3D1

In 2.3.x they would be stored as:

tmp/cache/F88/640/views/localhost:3000/games/index...

To apply the provided patch we need tests to show the error.
Please can one of you make a test case, thank you ;).

Added a test that fails without the patch and passes with it. git diff below

git diff -p
diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb
index 84f6f29..3bfddae 100644
--- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -12,7 +12,7 @@ module ActiveSupport

   DIR_FORMATTER = "%03X"
   ESCAPE_FILENAME_CHARS = /[^a-z0-9_.-]/i

• UNESCAPE_FILENAME_CHARS = /%[0-9A-F]{2}/

• UNESCAPE_FILENAME_CHARS = /%([0-9A-F]{2})/

  def initialize(cache_path, options = nil)

   super(options)
  

  @@ -156,7 +156,7 @@ module ActiveSupport

   # Translate a file path into a key.
   def file_path_key(path)
     fname = path[cache_path.size, path.size].split(File::SEPARATOR, 4).last
  

•    fname.gsub(UNESCAPE_FILENAME_CHARS){|match| $1.ord.to_s(16)}
  

•    fname.gsub(UNESCAPE_FILENAME_CHARS){|match| $1.to_i(16).chr}
   end
  
   # Delete empty directories in the cache.
  

  diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb
  index 28ef695..05a68e5 100644
  --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -509,6 +509,11 @@ class FileStoreTest < ActiveSupport::TestCase assert_nil old_cache.read('foo') end end
• def test_key_transformation
• key = @cache.send(:key_file_path, "views/index?id=1")
• assert_equal "views/index?id=1", @cache.send(:file_path_key, key)
• end end

class MemoryStoreTest < ActiveSupport::TestCase

Lighthouse is messing up the formatting. I am not real familiar with Lighthouse...
Here it is attached.

The patch works fine for me (ruby ee 1.8.7). Like to see this fixed for 3.0.1 as well.. monkey-patched for now.

Jim Wilson great work but can you provide a patch following http://rails.lighthouseapp.com/projects/8994/sending-patches
I need you to do this to give credits to you for the patch.

Oh, looks like this is a duplicate of #5850 ActiveSupport::Cache::FileStore: undefined method `ord' for nil:NilClass where I submitted a patch but nobody has touched in awhile. Should take a look at that one as well Santiago as its done a different way and I think the proper way although nobody commented on it.

Denis yeah I know ;).
I wanted to apply this patch before to give some credit to Jim because he worked a lot I guess :) and after that I will apply yours.
BTW please check #5850 ActiveSupport::Cache::FileStore: undefined method `ord' for nil:NilClass I made a comment on it.

Perfect, makes sense, thanks :)

I don't need any credit. Just take simpliest approach to fix it. However if you need me to do a patch I can... I am not a very active submitter...

(from [4e0477c9b75683372f662a614ae91b158120ebe8]) Test cache.key_file_path with separators ht. Jim Wilson [#5611 active_support/cache/file_store.rb is this correct?] https://github.com/rails/rails/commit/4e0477c9b75683372f662a614ae91...

(from [c7fac8c6ea2590e3cafe50c1e71de78bad9303c1]) Test cache.key_file_path with separators ht. Jim Wilson [#5611 active_support/cache/file_store.rb is this correct?] https://github.com/rails/rails/commit/c7fac8c6ea2590e3cafe50c1e71de...