From 74544634fbd321e4ec3c5fb19f0cb1341e29689b Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Fri, 20 Sep 2024 12:51:01 +0200 Subject: [PATCH] Support Rack response bodies that allow to_ary (#22) Newer Rack versions support bodies that respond to `to_ary` and in newer versions of Rails the Rails response body does as well. It seems previously we would not treat such response bodies correctly, meaning they would not get cached by Idempo (providing locking but no caching). This should be rectified with this PR - when running on Rack 3 (and newer Rails) `to_ary` will be called on the body, materializing it into an Array of Strings. --- .github/workflows/ci.yml | 12 +++++++++--- idempo.gemspec | 2 +- lib/idempo.rb | 12 ++++++++++++ spec/idempo_spec.rb | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b3966a..f6e9878 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: ruby: - - '2.7' + - "2.7" steps: - name: Checkout uses: actions/checkout@v4 @@ -37,11 +37,16 @@ jobs: name: Specs runs-on: ubuntu-22.04 if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository + env: + IDEMPO_RACK_VERSION: ${{ matrix.rack }} strategy: matrix: ruby: - - '2.7' - - '3.2' + - "2.7" + - "3.2" + rack: + - "2.0" + - "3.0" services: mysql: image: mysql:5.7 @@ -65,6 +70,7 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + - name: Setup Ruby uses: ruby/setup-ruby@v1 with: diff --git a/idempo.gemspec b/idempo.gemspec index 278433e..7bbb8bb 100644 --- a/idempo.gemspec +++ b/idempo.gemspec @@ -36,7 +36,7 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] # Uncomment to register a new dependency of your gem - spec.add_dependency "rack" + spec.add_dependency "rack", "~> #{ENV.fetch("IDEMPO_RACK_VERSION", "3.0")}" spec.add_dependency "msgpack" spec.add_dependency "measurometer", "~> 1.3" diff --git a/lib/idempo.rb b/lib/idempo.rb index ad08bf1..cce6985 100644 --- a/lib/idempo.rb +++ b/lib/idempo.rb @@ -7,6 +7,7 @@ require "msgpack" require "zlib" require "set" +require "rack" require "idempo/version" @@ -57,6 +58,12 @@ def call(env) status, headers, body = @app.call(env) expires_in_seconds = (headers.delete("X-Idempo-Persist-For-Seconds") || @persist_for_seconds).to_i + + # In some cases `body` could respond to to_ary. In this case, we don't need to call .close on body afterwards. + # + # @see https://github.com/rack/rack/blob/main/SPEC.rdoc#the-body- + body = body.to_ary if rack_v3? && body.respond_to?(:to_ary) + if response_may_be_persisted?(status, headers, body) # Body is replaced with a cached version since a Rack response body is not rewindable marshaled_response, body = serialize_response(status, headers, body) @@ -76,6 +83,10 @@ def call(env) private + def rack_v3? + Gem::Version.new(Rack.release) >= Gem::Version.new("3.0") + end + def from_persisted_response(marshaled_response) if marshaled_response[-2..] != ":1" raise Error, "Unknown serialization of the marshaled response" @@ -104,6 +115,7 @@ def serialize_response(status, headers, rack_response_body) # does not [deflated_message_packed_str, body_chunks] ensure + # This will not be applied to response bodies of Array type. rack_response_body.close if rack_response_body.respond_to?(:close) end diff --git a/spec/idempo_spec.rb b/spec/idempo_spec.rb index 75446c8..259e81b 100644 --- a/spec/idempo_spec.rb +++ b/spec/idempo_spec.rb @@ -60,7 +60,7 @@ describe "when no double requests are in progress" do let(:app) do the_app = ->(env) { - [200, {"X-Foo" => "bar"}, [Random.new.bytes(15), env["rack.input"].read]] + [200, {"X-Foo" => "bar"}, [Random.new.bytes(15), env["rack.input"]&.read]] } Idempo.new(the_app, backend: Idempo::MemoryBackend.new) end