refactor: extract logic for determining if a request is for UI or API

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

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

spec/lib/rack/pact_broker/ui_request_filter_spec.rb → 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