Skip to content

Commit

Permalink
refactor: remove configurable routes (#12)
Browse files Browse the repository at this point in the history
Co-authored-by: Matteo Rossi <[email protected]>
  • Loading branch information
matteoredz and Matteo Rossi authored Jan 29, 2025
1 parent e0333c8 commit 2463b55
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 125 deletions.
23 changes: 1 addition & 22 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,7 @@ module MyApp

config.middleware.use(
Rack::IdempotencyKey,
store: Rack::IdempotencyKey::MemoryStore.new,
routes: [
{ path: "/posts", method: "POST" },
{ path: "/posts/*", method: "PATCH" },
{ path: "/posts/*/comments", method: "POST" }
]
store: Rack::IdempotencyKey::MemoryStore.new
)
end
end
Expand Down Expand Up @@ -118,22 +113,6 @@ The Array returned must conform to the [Rack Specification](https://github.com/r
]
```

## Idempotent Routes

To declare the routes where you want to enable idempotency, you only need to pass a `route` keyword parameter when the Middleware gets mounted.

Each route entry must be compliant with what follows:

```ruby
routes: [
{ path: "/posts", method: "POST" },
{ path: "/posts/*", method: "PATCH" },
{ path: "/posts/*/comments", method: "POST" }
]
```

The `*` char is a placeholder representing a named parameter that will get converted to an any-chars regex.

## Development

After checking out the repo, run `bin/setup` to install dependencies.
Expand Down
11 changes: 5 additions & 6 deletions lib/rack/idempotency_key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@

module Rack
class IdempotencyKey
def initialize(app, routes: [], store: MemoryStore.new)
@app = app
@routes = routes
@store = store
def initialize(app, store: MemoryStore.new)
@app = app
@store = store
end

def call(env)
request = Request.new(Rack::Request.new(env), routes, store)
request = Request.new(Rack::Request.new(env), store)
return app.call(env) unless request.allowed?

handle_request!(request, env)
Expand All @@ -31,7 +30,7 @@ def call(env)

private

attr_reader :app, :store, :routes
attr_reader :app, :store

def handle_request!(request, env)
request.with_lock! do
Expand Down
45 changes: 4 additions & 41 deletions lib/rack/idempotency_key/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,17 @@ module Rack
class IdempotencyKey
class Request
# @param request [Rack::Request]
# @param routes [Array]
# @param store [Store]
def initialize(request, routes, store)
def initialize(request, store)
@request = request
@routes = routes
@store = store
end

# Checks if the `Idempotency-Key` header is present, if the HTTP request method is
# allowed and if there is any matching route whitelisted in the `routes` array.
# Checks if the `Idempotency-Key` header is present, if the HTTP request method is allowed.
#
# @return [Boolean]
def allowed?
idempotency_key? && allowed_method? && any_matching_route?
idempotency_key? && allowed_method?
end

# TODO
Expand Down Expand Up @@ -48,14 +45,6 @@ def allowed_method?
%w[POST PATCH CONNECT].include? request.request_method
end

# Checks if there is any matching route from the `routes` input array against
# the currently requested path.
#
# @return [Boolean]
def any_matching_route?
routes.any? { |route| matching_route?(route[:path]) && matching_method?(route[:method]) }
end

# Checks if the given request has the Idempotency-Key header
#
# @return [Boolean]
Expand All @@ -76,33 +65,7 @@ def cache_key

private

attr_reader :request, :routes, :store

def matching_route?(route_path)
route_segments = segments route_path
path_segments.size == route_segments.size && same_segments?(route_segments)
end

def matching_method?(route_method)
request.request_method.casecmp(route_method).zero?
end

def path_segments
@path_segments ||= segments(request.path_info)
end

def segments(path)
path.split("/").reject(&:empty?)
end

def same_segments?(route_segments)
path_segments.each_with_index do |path_segment, index|
route_segment = Regexp.new route_segments[index].gsub("*", '\w+'), Regexp::IGNORECASE
return false unless path_segment.match?(route_segment)
end

true
end
attr_reader :request, :store
end
end
end
52 changes: 2 additions & 50 deletions spec/rack/idempotency_key/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@
RSpec.describe Rack::IdempotencyKey::Request do
include Rack::Test::Methods

subject(:request) { described_class.new(rack_request, routes, store) }
subject(:request) { described_class.new(rack_request, store) }

let(:rack_request) { Rack::Request.new(env) }
let(:env) { Rack::MockRequest.env_for(env_uri, env_opts) }
let(:env_uri) { "/" }
let(:env_opts) { {} }
let(:routes) { [] }
let(:idempotency_key) { "123456789" }
let(:store) { Rack::IdempotencyKey::MemoryStore.new }

Expand All @@ -21,40 +20,23 @@
end

describe "#allowed?" do
context "with idempotency key over an allowed method and a matching route" do
context "with idempotency key over an allowed method" do
include_context "with idempotency key in place"

let(:env_opts) { { method: "POST" } }
let(:env_uri) { "/posts" }
let(:routes) { [{ path: "/posts", method: "POST" }] }

it { is_expected.to be_allowed }
end

context "without the idempotency key" do
let(:env_opts) { { method: "POST" } }
let(:env_uri) { "/posts" }
let(:routes) { [{ path: "/posts", method: "POST" }] }

it { is_expected.not_to be_allowed }
end

context "with a not allowed request method" do
include_context "with idempotency key in place"

let(:env_uri) { "/posts" }
let(:routes) { [{ path: "/posts", method: "GET" }] }

it { is_expected.not_to be_allowed }
end

context "without a matching route" do
include_context "with idempotency key in place"

let(:env_opts) { { method: "POST" } }
let(:env_uri) { "/posts/1/authors" }
let(:routes) { [{ path: "/posts/*", method: "POST" }] }

it { is_expected.not_to be_allowed }
end
end
Expand All @@ -77,36 +59,6 @@
end
end

describe "#any_matching_route?" do
context "without any declared route" do
it { is_expected.not_to be_any_matching_route }
end

context "when the route ends with a placeholder" do
let(:env_opts) { { method: "PATCH" } }
let(:env_uri) { "/posts/1" }
let(:routes) { [{ path: "/posts/*", method: "PATCH" }] }

it { is_expected.to be_any_matching_route }
end

context "when the route ends with a defined character" do
let(:env_opts) { { method: "POST" } }
let(:env_uri) { "/posts" }
let(:routes) { [{ path: "/posts", method: "POST" }] }

it { is_expected.to be_any_matching_route }
end

context "with declared but no matching routes" do
let(:env_opts) { { method: "POST" } }
let(:env_uri) { "/authors" }
let(:routes) { [{ path: "/posts/*/authors", method: "POST" }] }

it { is_expected.not_to be_any_matching_route }
end
end

describe "#idempotency_key?" do
context "with the idempotency key" do
include_context "with idempotency key in place"
Expand Down
7 changes: 1 addition & 6 deletions spec/rack/idempotency_key_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
RSpec.describe Rack::IdempotencyKey do
include Rack::Test::Methods

let(:app) { described_class.new(next_app, store: store, routes: idempotent_routes) }
let(:app) { described_class.new(next_app, store: store) }
let(:app_with_default_store) { described_class.new(next_app) }
let(:next_app) { ->(_env = {}) { [200, { "Content-Type" => "text/plain" }, ["OK"]] } }
let(:store) { described_class::MemoryStore.new }
let(:idempotent_routes) { [{ path: "/", method: "POST" }] }

it "has a VERSION" do
expect(Rack::IdempotencyKey::VERSION).to be_a(String)
Expand All @@ -20,10 +19,6 @@
expect(app_with_default_store.send(:store)).to be_a(Rack::IdempotencyKey::MemoryStore)
end

it "has a default empty routes array" do
expect(app_with_default_store.send(:routes)).to be_empty
end

context "without Idempotency-Key header" do
context "with an idempotent method" do
before { get "/", {}, {} }
Expand Down

0 comments on commit 2463b55

Please sign in to comment.