Update routing constraints to use ContentItemLoader

- Now that ContentItemLoader is handling the caching, we can remove that layer of code / responsibility from the constraints.
- ContentItemLoader either returns the adapter response or the error that it caught, so querying for those classes gives us a "was it or wasn't it an error" check.
- We also simplify the spec tests slightly to use more idiomatic RSpec.

lib/api_error_routing_constraint.rb

@@ -1,5 +1,5 @@
 class ApiErrorRoutingConstraint
   def matches?(request)
-    request.env[:content_item_error].present?
+    ContentItemLoader.load(request.path).is_a?(StandardError)
   end
 end

lib/format_routing_constraint.rb

@@ -4,27 +4,12 @@ def initialize(format)
   end
 
   def matches?(request)
-    content_item = set_content_item(request)
-    @format == content_item&.[]("schema_name")
-  end
-
-  def set_content_item(request)
-    return request.env[:content_item] if already_cached?(request)
-
-    begin
-      request.env[:content_item] = GdsApi.content_store.content_item(key(request))
-    rescue GdsApi::HTTPErrorResponse, GdsApi::InvalidUrl => e
-      request.env[:content_item_error] = e
-      nil
-    end
+    content_item = ContentItemLoader.load(key(request))
+    content_item.is_a?(GdsApi::Response) && content_item["schema_name"] == @format
   end
 
 private
 
-  def already_cached?(request)
-    request.env.include?(:content_item) || request.env.include?(:content_item_error)
-  end
-
   def key(request)
     "/#{request.params.fetch(:slug)}"
   end

lib/full_path_format_routing_constraint.rb

@@ -1,10 +1,6 @@
 class FullPathFormatRoutingConstraint < FormatRoutingConstraint
 private
 
-  def already_cached?(_request)
-    false
-  end
-
   def key(request)
     request.path
   end

spec/unit/api_error_routing_constraint_spec.rb

@@ -1,21 +1,18 @@
-require "api_error_routing_constraint"
-
 RSpec.describe ApiErrorRoutingConstraint do
+  include ContentStoreHelpers
+
+  let(:subject) { described_class.new }
+  let(:request) { double(path: "/slug") }
+
   it "returns true if there's a cached error" do
-    request = double(env: { content_item_error: StandardError.new })
+    stub_content_store_does_not_have_item("/slug")
 
     expect(subject.matches?(request)).to be true
   end
 
   it "returns false if there was no error in API calls" do
-    request = double(env: {})
+    stub_content_store_has_item("/slug")
 
     expect(subject.matches?(request)).to be false
   end
-
-private
-
-  def subject
-    ApiErrorRoutingConstraint.new
-  end
 end

spec/unit/format_routing_constraint_spec.rb

@@ -2,60 +2,30 @@
   include ContentStoreHelpers
 
   describe "#matches?" do
-    context "when the content_store returns a document" do
-      let(:format) { "foo" }
-      let(:request) { double(params: { slug: "test_slug" }, env: {}) }
+    let(:request) { double(params: { slug: "slug" }, env: {}) }
 
+    context "when the content_store returns a document" do
       before do
-        stub_content_store_has_item("/test_slug", schema_name: format)
+        stub_content_store_has_item("/slug", schema_name: "foo")
       end
 
       it "returns true if format matches" do
-        expect(described_class.new(format).matches?(request)).to be true
+        expect(described_class.new("foo").matches?(request)).to be true
       end
 
       it "returns false if format doesn't match" do
         expect(described_class.new("not_the_format").matches?(request)).to be false
       end
-
-      it "sets the content item on the request object" do
-        described_class.new(format).matches?(request)
-
-        expect(request.env[:content_item].present?).to be true
-      end
     end
 
     context "when the content_store API call throws an error" do
-      let(:request) { double(params: { slug: "test_slug" }, env: {}) }
-
       before do
-        stub_content_store_does_not_have_item("/test_slug")
+        stub_content_store_does_not_have_item("/slug")
       end
 
       it "returns false" do
         expect(described_class.new("any_format").matches?(request)).to be false
       end
-
-      it "sets an error on the request object" do
-        described_class.new("any_format").matches?(request)
-        expect(request.env[:content_item_error].present?).to be true
-      end
-    end
-  end
-
-  context "content items are memoized and used across multiple requests" do
-    let(:subject) { described_class.new("foo") }
-    let(:request) { double(params: { slug: "test_slug" }, env: {}) }
-
-    before do
-      @stub = stub_content_store_has_item("/test_slug", schema_name: "foo")
-    end
-
-    it "only makes one API call" do
-      subject.matches?(request)
-      subject.matches?(request)
-
-      assert_requested(@stub, times: 1)
     end
   end
 end

spec/unit/full_path_format_routing_constraint_spec.rb

@@ -2,44 +2,30 @@
   include ContentStoreHelpers
 
   describe "#matches?" do
-    context "when the content_store returns a document" do
-      let(:format) { "foo" }
-      let(:request) { double(path: "/format/routing/test", env: {}) }
+    let(:request) { double(path: "/format/routing/test", env: {}) }
 
+    context "when the content_store returns a document" do
       before do
-        stub_content_store_has_item("/format/routing/test", schema_name: format)
+        stub_content_store_has_item("/format/routing/test", schema_name: "foo")
       end
 
       it "returns true if format matches" do
-        expect(described_class.new(format).matches?(request)).to be true
+        expect(described_class.new("foo").matches?(request)).to be true
       end
 
       it "returns false if format doesn't match" do
         expect(described_class.new("not_the_format").matches?(request)).to be false
       end
-
-      it "sets the content item on the request object" do
-        described_class.new(format).matches?(request)
-
-        expect(request.env[:content_item].present?).to be true
-      end
     end
 
     context "when the content_store API call throws an error" do
-      let(:request) { double(path: "/format/routing/test", env: {}) }
-
       before do
         stub_content_store_does_not_have_item("/format/routing/test")
       end
 
       it "returns false" do
         expect(described_class.new("any_format").matches?(request)).to be false
       end
-
-      it "sets an error on the request object" do
-        described_class.new("any_format").matches?(request)
-        expect(request.env[:content_item_error].present?).to be true
-      end
     end
   end
 end