From dae4cd51de7bdc0b971613b540e82b7942526d4d Mon Sep 17 00:00:00 2001 From: Tsutomu Katsube Date: Thu, 1 Oct 2026 05:51:54 +0900 Subject: [PATCH 1/3] Extract cache location to RemoteInput::CacheEntry GitHub: GH-5 This is a preparation for the `cache_path:` option that overrides the cache location. It does not change behavior. --- lib/remote_input.rb | 29 +++---------------- lib/remote_input/cache-entry.rb | 50 +++++++++++++++++++++++++++++++++ test/test-cache-entry.rb | 30 ++++++++++++++++++++ test/test-remote-input.rb | 16 ----------- 4 files changed, 84 insertions(+), 41 deletions(-) create mode 100644 lib/remote_input/cache-entry.rb create mode 100644 test/test-cache-entry.rb diff --git a/lib/remote_input.rb b/lib/remote_input.rb index 122d597..7e6465c 100644 --- a/lib/remote_input.rb +++ b/lib/remote_input.rb @@ -1,3 +1,4 @@ +require_relative "remote_input/cache-entry" require_relative "remote_input/cache-path" require_relative "remote_input/downloader" require_relative "remote_input/zip-extractor" @@ -24,12 +25,11 @@ def initialize(url, internal_encoding: nil, external_encoding: nil, **http_options) - @url = URI(url) @encoding = encoding @internal_encoding = internal_encoding @external_encoding = external_encoding @downloader = Downloader.new(url, *fallback_urls, **http_options) - @cache_path = nil + @cache_entry = CacheEntry.from_url(url) @local_file = nil @closed = false end @@ -44,34 +44,13 @@ def close end def clear_cache - cache_path.remove + @cache_entry.remove end private def path - cache_path.base_dir + File.basename(normalize_path) - end - - def cache_path - return @cache_path if @cache_path - dirname = File.dirname(normalize_path).delete_suffix("/") - cache_id = to_cache_id("#{@url.host}#{dirname}") - query = @url.query - cache_id += "+#{to_cache_id(query)}" if query and not query.empty? - @cache_path = CachePath.new(cache_id) - end - - def to_cache_id(s) - allow_list = "0-9A-Za-z._~=-" - s.tr("^#{allow_list}", "-") - end - - def normalize_path - url_path = @url.path - url_path = "/" if url_path.empty? - url_path += "data" if url_path.end_with?("/") - url_path + @cache_entry.path end def local_file diff --git a/lib/remote_input/cache-entry.rb b/lib/remote_input/cache-entry.rb new file mode 100644 index 0000000..1f20ee1 --- /dev/null +++ b/lib/remote_input/cache-entry.rb @@ -0,0 +1,50 @@ +require_relative "cache-path" + +class RemoteInput + # @api private + class CacheEntry + class << self + def from_url(url) + url = URI(url) + path = url.path + path = "/" if path.empty? + path += "data" if path.end_with?("/") + dirname = File.dirname(path).delete_suffix("/") + id = to_id("#{url.host}#{dirname}") + query = url.query + id += "+#{to_id(query)}" if query and not query.empty? + new(id, File.basename(path)) + end + + private + def to_id(s) + allow_list = "0-9A-Za-z._~=-" + s.tr("^#{allow_list}", "-") + end + end + + def initialize(id, path_in_cache) + @id = id + @path_in_cache = path_in_cache + @cache_path = CachePath.new(@id) + end + + def path + @cache_path.base_dir + @path_in_cache + end + + def remove + @cache_path.remove + end + + def ==(other) + other.is_a?(self.class) and + @id == other.id and + @path_in_cache == other.path_in_cache + end + + protected + attr_reader :id + attr_reader :path_in_cache + end +end diff --git a/test/test-cache-entry.rb b/test/test-cache-entry.rb new file mode 100644 index 0000000..370cac7 --- /dev/null +++ b/test/test-cache-entry.rb @@ -0,0 +1,30 @@ +require_relative "helper" + +class CacheEntryTest < Test::Unit::TestCase + data("no path", ["example.com", "data", "https://example.com"]) + data("root", ["example.com", "data", "https://example.com/"]) + data("file", ["example.com", "file", "https://example.com/file"]) + data("query", ["example.com+a=-", "file", "https://example.com/file?a=+"]) + data("directory", ["example.com-a", "data", "https://example.com/a/"]) + data("nested file", ["example.com-a", "file", "https://example.com/a/file"]) + data("deeply nested", ["example.com-a-b", "file", "https://example.com/a/b/file"]) + def test_from_url(data) + id, path_in_cache, url = data + cache_entry = RemoteInput::CacheEntry.from_url(URI(url)) + assert_equal(RemoteInput::CacheEntry.new(id, path_in_cache), + cache_entry) + end + + def test_equal_same_path_different_id + cache_entry1 = RemoteInput::CacheEntry.new("example.com/a", "file") + cache_entry2 = RemoteInput::CacheEntry.new("example.com", "a/file") + assert_equal([ + true, + false + ], + [ + cache_entry1.path == cache_entry2.path, + cache_entry1 == cache_entry2, + ]) + end +end diff --git a/test/test-remote-input.rb b/test/test-remote-input.rb index d115aa7..c2cd816 100644 --- a/test/test-remote-input.rb +++ b/test/test-remote-input.rb @@ -58,22 +58,6 @@ def test_open_with_block_raised end end - data("no path", ["/example.com/data", "https://example.com"]) - data("root", ["/example.com/data", "https://example.com/"]) - data("file", ["/example.com/file", "https://example.com/file"]) - data("query", ["/example.com+a=-/file", "https://example.com/file?a=+"]) - data("directory", ["/example.com-a/data", "https://example.com/a/"]) - data("nested file", ["/example.com-a/file", "https://example.com/a/file"]) - data("deeply nested", ["/example.com-a-b/file", "https://example.com/a/b/file"]) - def test_path(data) - expected, url = data - RemoteInput.open(url) do |input| - assert do - input.send(:path).to_s.end_with?(expected) - end - end - end - def test_clear_cache open_input do |input| input.send(:path).parent.mkpath From 0b0ee3bd16f9ddc405210be2866be022427583cf Mon Sep 17 00:00:00 2001 From: Tsutomu Katsube Date: Fri, 2 Oct 2026 06:07:32 +0900 Subject: [PATCH 2/3] Add cache_path: option to RemoteInput.open GitHub: GH-5 `cache_path:` overrides the cache location computed from the URL. The first entry is the cache ID and the rest is path in the cache: ```ruby RemoteInput.open("https://example.com/sample/data.csv", cache_path: "example/sub-directory/data.csv") do |input| # Cached at: # /red-remote-input/example/sub-directory/data.csv end ``` --- lib/remote_input.rb | 7 ++++++- lib/remote_input/cache-entry.rb | 19 +++++++++++++++++++ test/test-cache-entry.rb | 21 ++++++++++++++++++++- test/test-remote-input.rb | 9 +++++++++ 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/lib/remote_input.rb b/lib/remote_input.rb index 7e6465c..36fd99c 100644 --- a/lib/remote_input.rb +++ b/lib/remote_input.rb @@ -21,6 +21,7 @@ def open(...) def initialize(url, *fallback_urls, + cache_path: nil, encoding: nil, internal_encoding: nil, external_encoding: nil, @@ -29,7 +30,11 @@ def initialize(url, @internal_encoding = internal_encoding @external_encoding = external_encoding @downloader = Downloader.new(url, *fallback_urls, **http_options) - @cache_entry = CacheEntry.from_url(url) + if cache_path + @cache_entry = CacheEntry.from_path(cache_path) + else + @cache_entry = CacheEntry.from_url(url) + end @local_file = nil @closed = false end diff --git a/lib/remote_input/cache-entry.rb b/lib/remote_input/cache-entry.rb index 1f20ee1..bab6dcd 100644 --- a/lib/remote_input/cache-entry.rb +++ b/lib/remote_input/cache-entry.rb @@ -16,6 +16,25 @@ def from_url(url) new(id, File.basename(path)) end + def from_path(cache_path) + path = Pathname(cache_path) + if path.absolute? + raise ArgumentError, + "cache_path must be relative: #{cache_path.inspect}" + end + id, *rest_filenames = path.each_filename.to_a + if [id, *rest_filenames].intersect?([".", ".."]) + raise ArgumentError, + "cache_path must not include '.' or '..': #{cache_path.inspect}" + end + if rest_filenames.empty? + raise ArgumentError, + "cache_path must be /: " + + cache_path.inspect + end + new(id, File.join(*rest_filenames)) + end + private def to_id(s) allow_list = "0-9A-Za-z._~=-" diff --git a/test/test-cache-entry.rb b/test/test-cache-entry.rb index 370cac7..7061771 100644 --- a/test/test-cache-entry.rb +++ b/test/test-cache-entry.rb @@ -15,12 +15,31 @@ def test_from_url(data) cache_entry) end + def test_from_valid_path + cache_path = "example/sub-directory/data.csv" + cache_entry = RemoteInput::CacheEntry.from_path(cache_path) + assert_equal(RemoteInput::CacheEntry.new("example", + "sub-directory/data.csv"), + cache_entry) + end + + data("absolute", "/data.csv") + data("no cache ID", "data.csv") + data("no path in cache", "example/") + data("root cache", "./data.csv") + data("path traversal", "../example/data.csv") + def test_from_invalid_path(cache_path) + assert_raise(ArgumentError) do + RemoteInput::CacheEntry.from_path(cache_path) + end + end + def test_equal_same_path_different_id cache_entry1 = RemoteInput::CacheEntry.new("example.com/a", "file") cache_entry2 = RemoteInput::CacheEntry.new("example.com", "a/file") assert_equal([ true, - false + false, ], [ cache_entry1.path == cache_entry2.path, diff --git a/test/test-remote-input.rb b/test/test-remote-input.rb index c2cd816..f62838b 100644 --- a/test/test-remote-input.rb +++ b/test/test-remote-input.rb @@ -58,6 +58,15 @@ def test_open_with_block_raised end end + def test_cache_path + RemoteInput.open("https://example.com/file", + cache_path: "example/sub-directory/data.csv") do |input| + assert do + input.send(:path).to_s.end_with?("/example/sub-directory/data.csv") + end + end + end + def test_clear_cache open_input do |input| input.send(:path).parent.mkpath From 2089b64b8935edf894bdc6bb7232d583b4a4abcc Mon Sep 17 00:00:00 2001 From: Tsutomu Katsube Date: Sat, 10 Oct 2026 07:33:44 +0900 Subject: [PATCH 3/3] Accept URI or Pathname by CacheEntry.new Co-authored-by: Sutou Kouhei --- lib/remote_input.rb | 4 +- lib/remote_input/cache-entry.rb | 84 +++++++++++++++++---------------- test/test-cache-entry.rb | 53 +++++++++------------ 3 files changed, 67 insertions(+), 74 deletions(-) diff --git a/lib/remote_input.rb b/lib/remote_input.rb index 36fd99c..e5ac0e4 100644 --- a/lib/remote_input.rb +++ b/lib/remote_input.rb @@ -31,9 +31,9 @@ def initialize(url, @external_encoding = external_encoding @downloader = Downloader.new(url, *fallback_urls, **http_options) if cache_path - @cache_entry = CacheEntry.from_path(cache_path) + @cache_entry = CacheEntry.new(Pathname(cache_path)) else - @cache_entry = CacheEntry.from_url(url) + @cache_entry = CacheEntry.new(URI(url)) end @local_file = nil @closed = false diff --git a/lib/remote_input/cache-entry.rb b/lib/remote_input/cache-entry.rb index bab6dcd..8a16bf6 100644 --- a/lib/remote_input/cache-entry.rb +++ b/lib/remote_input/cache-entry.rb @@ -3,48 +3,15 @@ class RemoteInput # @api private class CacheEntry - class << self - def from_url(url) - url = URI(url) - path = url.path - path = "/" if path.empty? - path += "data" if path.end_with?("/") - dirname = File.dirname(path).delete_suffix("/") - id = to_id("#{url.host}#{dirname}") - query = url.query - id += "+#{to_id(query)}" if query and not query.empty? - new(id, File.basename(path)) + def initialize(target) + if target.is_a?(URI) + initialize_url(target) + elsif target.is_a?(Pathname) + initialize_path(target) + else + raise ArgumentError, + "target must be URI or Pathname: #{target.inspect}" end - - def from_path(cache_path) - path = Pathname(cache_path) - if path.absolute? - raise ArgumentError, - "cache_path must be relative: #{cache_path.inspect}" - end - id, *rest_filenames = path.each_filename.to_a - if [id, *rest_filenames].intersect?([".", ".."]) - raise ArgumentError, - "cache_path must not include '.' or '..': #{cache_path.inspect}" - end - if rest_filenames.empty? - raise ArgumentError, - "cache_path must be /: " + - cache_path.inspect - end - new(id, File.join(*rest_filenames)) - end - - private - def to_id(s) - allow_list = "0-9A-Za-z._~=-" - s.tr("^#{allow_list}", "-") - end - end - - def initialize(id, path_in_cache) - @id = id - @path_in_cache = path_in_cache @cache_path = CachePath.new(@id) end @@ -65,5 +32,40 @@ def ==(other) protected attr_reader :id attr_reader :path_in_cache + + private + def initialize_url(url) + url_path = url.path + url_path = "/" if url_path.empty? + url_path += "data" if url_path.end_with?("/") + dirname = File.dirname(url_path).delete_suffix("/") + @id = to_id("#{url.host}#{dirname}") + query = url.query + @id += "+#{to_id(query)}" if query and not query.empty? + @path_in_cache = File.basename(url_path) + end + + def initialize_path(path) + if path.absolute? + raise ArgumentError, + "cache path must be relative: #{path.inspect}" + end + @id, *rest_filenames = path.each_filename.to_a + if [@id, *rest_filenames].intersect?([".", ".."]) + raise ArgumentError, + "cache path must not include '.' or '..': #{path.inspect}" + end + if rest_filenames.empty? + raise ArgumentError, + "cache path must be /: " + + path.inspect + end + @path_in_cache = File.join(*rest_filenames) + end + + def to_id(s) + allow_list = "0-9A-Za-z._~=-" + s.tr("^#{allow_list}", "-") + end end end diff --git a/test/test-cache-entry.rb b/test/test-cache-entry.rb index 7061771..d111fb3 100644 --- a/test/test-cache-entry.rb +++ b/test/test-cache-entry.rb @@ -1,26 +1,24 @@ require_relative "helper" class CacheEntryTest < Test::Unit::TestCase - data("no path", ["example.com", "data", "https://example.com"]) - data("root", ["example.com", "data", "https://example.com/"]) - data("file", ["example.com", "file", "https://example.com/file"]) - data("query", ["example.com+a=-", "file", "https://example.com/file?a=+"]) - data("directory", ["example.com-a", "data", "https://example.com/a/"]) - data("nested file", ["example.com-a", "file", "https://example.com/a/file"]) - data("deeply nested", ["example.com-a-b", "file", "https://example.com/a/b/file"]) - def test_from_url(data) - id, path_in_cache, url = data - cache_entry = RemoteInput::CacheEntry.from_url(URI(url)) - assert_equal(RemoteInput::CacheEntry.new(id, path_in_cache), - cache_entry) + data("no path", ["example.com/data", "https://example.com"]) + data("root", ["example.com/data", "https://example.com/"]) + data("file", ["example.com/file", "https://example.com/file"]) + data("query", ["example.com+a=-/file", "https://example.com/file?a=+"]) + data("directory", ["example.com-a/data", "https://example.com/a/"]) + data("nested file", ["example.com-a/file", "https://example.com/a/file"]) + data("nested dir", ["example.com-a-b/file", "https://example.com/a/b/file"]) + def test_url_equal_path(data) + cache_path, url = data + assert_equal(RemoteInput::CacheEntry.new(Pathname(cache_path)), + RemoteInput::CacheEntry.new(URI(url))) end - def test_from_valid_path - cache_path = "example/sub-directory/data.csv" - cache_entry = RemoteInput::CacheEntry.from_path(cache_path) - assert_equal(RemoteInput::CacheEntry.new("example", - "sub-directory/data.csv"), - cache_entry) + def test_path_split + cache_path = Pathname("example/sub-directory/data.csv") + cache_entry = RemoteInput::CacheEntry.new(cache_path) + assert_equal(["example", "sub-directory/data.csv"], + [cache_entry.send(:id), cache_entry.send(:path_in_cache)]) end data("absolute", "/data.csv") @@ -28,22 +26,15 @@ def test_from_valid_path data("no path in cache", "example/") data("root cache", "./data.csv") data("path traversal", "../example/data.csv") - def test_from_invalid_path(cache_path) + def test_invalid_path(cache_path) assert_raise(ArgumentError) do - RemoteInput::CacheEntry.from_path(cache_path) + RemoteInput::CacheEntry.new(Pathname(cache_path)) end end - def test_equal_same_path_different_id - cache_entry1 = RemoteInput::CacheEntry.new("example.com/a", "file") - cache_entry2 = RemoteInput::CacheEntry.new("example.com", "a/file") - assert_equal([ - true, - false, - ], - [ - cache_entry1.path == cache_entry2.path, - cache_entry1 == cache_entry2, - ]) + def test_invalid_target + assert_raise(ArgumentError) do + RemoteInput::CacheEntry.new("https://example.com/file") + end end end