From 2463b555188b5c4fee436ff6f96f3144a66c16fa Mon Sep 17 00:00:00 2001 From: Matteo Rossi Date: Wed, 29 Jan 2025 11:13:24 +0100 Subject: [PATCH] refactor: remove configurable routes (#12) Co-authored-by: Matteo Rossi --- README.md | 23 +--------- lib/rack/idempotency_key.rb | 11 +++-- lib/rack/idempotency_key/request.rb | 45 ++------------------ spec/rack/idempotency_key/request_spec.rb | 52 +---------------------- spec/rack/idempotency_key_spec.rb | 7 +-- 5 files changed, 13 insertions(+), 125 deletions(-) diff --git a/README.md b/README.md index 23159f8..3f476bf 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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. diff --git a/lib/rack/idempotency_key.rb b/lib/rack/idempotency_key.rb index 47cc756..6d1cea7 100644 --- a/lib/rack/idempotency_key.rb +++ b/lib/rack/idempotency_key.rb @@ -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) @@ -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 diff --git a/lib/rack/idempotency_key/request.rb b/lib/rack/idempotency_key/request.rb index 9e9b5ea..ac64b1a 100644 --- a/lib/rack/idempotency_key/request.rb +++ b/lib/rack/idempotency_key/request.rb @@ -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 @@ -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] @@ -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 diff --git a/spec/rack/idempotency_key/request_spec.rb b/spec/rack/idempotency_key/request_spec.rb index c2df61a..4a0b993 100644 --- a/spec/rack/idempotency_key/request_spec.rb +++ b/spec/rack/idempotency_key/request_spec.rb @@ -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 } @@ -21,20 +20,16 @@ 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 @@ -42,19 +37,6 @@ 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 @@ -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" diff --git a/spec/rack/idempotency_key_spec.rb b/spec/rack/idempotency_key_spec.rb index ceca6eb..a14f66a 100644 --- a/spec/rack/idempotency_key_spec.rb +++ b/spec/rack/idempotency_key_spec.rb @@ -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) @@ -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 "/", {}, {} }