Skip to content

Commit

Permalink
Support Rack response bodies that allow to_ary (#22)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Stanislav (Stas) Katkov authored Sep 20, 2024
1 parent 8140994 commit 7454463
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 5 deletions.
12 changes: 9 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
strategy:
matrix:
ruby:
- '2.7'
- "2.7"
steps:
- name: Checkout
uses: actions/checkout@v4
Expand All @@ -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
Expand All @@ -65,6 +70,7 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Setup Ruby
uses: ruby/setup-ruby@v1
with:
Expand Down
2 changes: 1 addition & 1 deletion idempo.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
12 changes: 12 additions & 0 deletions lib/idempo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require "msgpack"
require "zlib"
require "set"
require "rack"

require "idempo/version"

Expand Down Expand Up @@ -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)
Expand All @@ -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"
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion spec/idempo_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7454463

Please sign in to comment.