From 0c9587821985f91860fb5ec2ef9768a5079f7143 Mon Sep 17 00:00:00 2001 From: Beth Skurrie Date: Tue, 17 Dec 2019 20:51:05 +1100 Subject: [PATCH] refactor: extract logic for determining if a request is for UI or API --- lib/rack/pact_broker/request_target.rb | 58 +++++++++++++++++++ lib/rack/pact_broker/ui_request_filter.rb | 49 ++-------------- ..._filter_spec.rb => request_target_spec.rb} | 51 +++++++--------- 3 files changed, 86 insertions(+), 72 deletions(-) create mode 100644 lib/rack/pact_broker/request_target.rb rename spec/lib/rack/pact_broker/{ui_request_filter_spec.rb => request_target_spec.rb} (50%) diff --git a/lib/rack/pact_broker/request_target.rb b/lib/rack/pact_broker/request_target.rb new file mode 100644 index 000000000..edf3a82d0 --- /dev/null +++ b/lib/rack/pact_broker/request_target.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Rack + module PactBroker + module RequestTarget + extend self + + WEB_ASSET_EXTENSIONS = %w[.js .woff .woff2 .css .png .html .map .ttf .ico].freeze + API_CONTENT_TYPES = %w[application/hal+json application/json text/csv application/yaml].freeze + + def request_for_ui?(env) + !(request_for_api?(env)) + end + + def request_for_api?(env) + explicit_request_for_api(env) || no_accept_header(env) || (accept_all(env) && !is_web_extension(env)) + end + + private + + def body_is_json(env) + env['CONTENT_TYPE']&.include?("json") + end + + def explicit_request_for_api(env) + accepts_api_content_type(env) || body_is_api_content_type(env) + end + + def accepts_api_content_type(env) + is_api_content_type((env['HTTP_ACCEPT']&.downcase) || "") + end + + def body_is_api_content_type(env) + is_api_content_type((env['CONTENT_TYPE']&.downcase) || "") + end + + def is_api_content_type(header) + API_CONTENT_TYPES.any?{ |content_type| header.include?(content_type) } + end + + # default curl Accept header + # Also used by browsers to request various web assets like woff files + def accept_all(env) + env['HTTP_ACCEPT'] == "*/*" + end + + # No browser ever makes a request without an accept header, so it must be an API + # request if there is no Accept header + def no_accept_header(env) + env['HTTP_ACCEPT'] == nil || env['HTTP_ACCEPT'] == "" + end + + def is_web_extension(env) + env['PATH_INFO'].end_with?(*WEB_ASSET_EXTENSIONS) + end + end + end +end diff --git a/lib/rack/pact_broker/ui_request_filter.rb b/lib/rack/pact_broker/ui_request_filter.rb index 17dd2674d..7f866437b 100644 --- a/lib/rack/pact_broker/ui_request_filter.rb +++ b/lib/rack/pact_broker/ui_request_filter.rb @@ -4,62 +4,25 @@ # to actually handle the request, as it would short circuit the cascade # logic. +require 'rack/pact_broker/request_target' + module Rack module PactBroker class UIRequestFilter - WEB_ASSET_EXTENSIONS = %w[.js .woff .woff2 .css .png .html .map .ttf .ico].freeze - API_CONTENT_TYPES = %w[application/hal+json application/json text/csv application/yaml].freeze + include RequestTarget def initialize app @app = app end def call env - if request_for_api(env) || no_accept_header(env) || (accept_all(env) && !is_web_extension(env)) + if request_for_ui?(env) + @app.call(env) + else # send the request on to the next app in the Rack::Cascade [404, {},[]] - else - @app.call(env) end end - - private - - def body_is_json(env) - env['CONTENT_TYPE'] && env['CONTENT_TYPE'].include?("json") - end - - def request_for_api(env) - accepts_api_content_type(env) || body_is_api_content_type(env) - end - - def accepts_api_content_type(env) - is_api_content_type((env['HTTP_ACCEPT'] && env['HTTP_ACCEPT'].downcase) || "") - end - - def body_is_api_content_type(env) - is_api_content_type((env['CONTENT_TYPE'] && env['CONTENT_TYPE'].downcase) || "") - end - - def is_api_content_type(header) - API_CONTENT_TYPES.any?{ |content_type| header.include?(content_type) } - end - - # default curl Accept header - # Also used by browsers to request various web assets like woff files - def accept_all(env) - env['HTTP_ACCEPT'] == "*/*" - end - - # No browser ever makes a request without an accept header, so it must be an API - # request if there is no Accept header - def no_accept_header(env) - env['HTTP_ACCEPT'] == nil || env['HTTP_ACCEPT'] == "" - end - - def is_web_extension(env) - env['PATH_INFO'].end_with?(*WEB_ASSET_EXTENSIONS) - end end end end diff --git a/spec/lib/rack/pact_broker/ui_request_filter_spec.rb b/spec/lib/rack/pact_broker/request_target_spec.rb similarity index 50% rename from spec/lib/rack/pact_broker/ui_request_filter_spec.rb rename to spec/lib/rack/pact_broker/request_target_spec.rb index 6649814a0..7ba62dfd5 100644 --- a/spec/lib/rack/pact_broker/ui_request_filter_spec.rb +++ b/spec/lib/rack/pact_broker/request_target_spec.rb @@ -1,67 +1,60 @@ -require 'rack/pact_broker/ui_request_filter' -require 'rack/test' +require 'rack/pact_broker/request_target' module Rack module PactBroker - describe UIRequestFilter do - include Rack::Test::Methods + describe RequestTarget do + let(:rack_env) do + { + 'CONTENT_TYPE' => content_type, + 'HTTP_ACCEPT' => accept, + 'PATH_INFO' => path + } + end + let(:content_type) { nil } + let(:accept) { nil } + let(:path) { '' } - describe "#call" do - let(:target_app) { double('target_app', call: [200, {}, []]) } - let(:app) { UIRequestFilter.new(target_app) } + describe "#request_for_ui?" do let(:path) { "/" } - let(:accept) { "text/html" } - subject { get path, nil, { "HTTP_ACCEPT" => accept } } + subject { RequestTarget.request_for_ui?(rack_env) } context "when the Accept header includes text/html" do - it "forwards the request to the target app" do - expect(target_app).to receive(:call) - subject - end + let(:accept) { "text/html" } + + it { is_expected.to be true } end context "when the request is for a web asset with an Accept header of */*" do let(:path) { "/blah/foo.woff" } let(:accept) { "*/*" } - it "forwards the request to the target app" do - expect(target_app).to receive(:call) - subject - end + it { is_expected.to be true } end context "when the request is for a content type served by the API (HAL browser request)" do let(:accept) { "application/hal+json, application/json, */*; q=0.01" } - it "returns a 404" do - expect(subject.status).to eq 404 - end + it { is_expected.to be false } end context "when the request is not for a web asset and the Accept headers is */* (default Accept header from curl request)" do let(:accept) { "*/*" } - it "returns a 404" do - expect(subject.status).to eq 404 - end + it { is_expected.to be false } end context "when the request is not for a web asset and no Accept header is specified" do let(:accept) { nil } - it "returns a 404" do - expect(subject.status).to eq 404 - end + it { is_expected.to be false } end context "when the request ends in a web asset extension but has Accept application/hal+json" do let(:accept) { "application/hal+json" } let(:path) { "/blah/foo.woff" } - it "returns a 404" do - expect(subject.status).to eq 404 - end + it { is_expected.to be false } end end end