From 5545f81e951498fb43ccb1d35bdc757f41dfe61d Mon Sep 17 00:00:00 2001 From: Paul Hayes Date: Tue, 19 Sep 2017 11:29:11 +0100 Subject: [PATCH] Create local and network component resolvers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need the option to use local components when using Slimmer in static. This avoids network requests, excessive network stubbing and i18n chain hacks. Because static components have a `.raw.html.erb` filename, we can’t use the default Rails component resolver. eg `govuk_component/title` can’t find the `_title` partial – we need to load `title.raw.html.erb`. * Split ComponentResolver into LocalComponentResolver and NetworkComponentResolver * Create new module for LocalGovukComponents which govuk_publishing_components can use when included on static * LocalGovukComponents don’t need a test version --- lib/slimmer.rb | 3 ++ lib/slimmer/component_resolver.rb | 41 ++----------------- lib/slimmer/govuk_components.rb | 2 +- lib/slimmer/local_component_resolver.rb | 19 +++++++++ lib/slimmer/local_govuk_components.rb | 49 +++++++++++++++++++++++ lib/slimmer/network_component_resolver.rb | 45 +++++++++++++++++++++ test/component_resolver_test.rb | 30 ++------------ test/local_component_resolver_test.rb | 27 +++++++++++++ test/network_component_resolver_test.rb | 41 +++++++++++++++++++ test/test_helper.rb | 2 +- 10 files changed, 193 insertions(+), 66 deletions(-) create mode 100644 lib/slimmer/local_component_resolver.rb create mode 100644 lib/slimmer/local_govuk_components.rb create mode 100644 lib/slimmer/network_component_resolver.rb create mode 100644 test/local_component_resolver_test.rb create mode 100644 test/network_component_resolver_test.rb diff --git a/lib/slimmer.rb b/lib/slimmer.rb index 2384658..55e29f6 100644 --- a/lib/slimmer.rb +++ b/lib/slimmer.rb @@ -28,7 +28,10 @@ def fetch(*) autoload :HTTPClient, 'slimmer/http_client' autoload :GovukComponents, 'slimmer/govuk_components' + autoload :LocalGovukComponents, 'slimmer/local_govuk_components' autoload :ComponentResolver, 'slimmer/component_resolver' + autoload :NetworkComponentResolver, 'slimmer/network_component_resolver' + autoload :LocalComponentResolver, 'slimmer/local_component_resolver' autoload :I18nBackend, 'slimmer/i18n_backend' module Processors diff --git a/lib/slimmer/component_resolver.rb b/lib/slimmer/component_resolver.rb index 257281a..e4121db 100644 --- a/lib/slimmer/component_resolver.rb +++ b/lib/slimmer/component_resolver.rb @@ -1,61 +1,26 @@ -require 'slimmer/govuk_request_id' -require 'active_support/core_ext/string/inflections' - module Slimmer class ComponentResolver < ::ActionView::Resolver TEST_TAG_NAME = 'test-govuk-component' def find_templates(name, prefix, partial, details, outside_app_allowed = false) return [] unless prefix == 'govuk_component' - template_path = [prefix, name].join('/') - if test? - template_body = test_body(template_path) - else - template_body = Slimmer.cache.fetch(template_path, expires_in: Slimmer::CACHE_TTL) do - fetch(template_url(template_path)) - end - end - details = { :format => 'text/html', :updated_at => Time.now, :virtual_path => template_path } - [ActionView::Template.new(template_body, template_path, erb_handler, details)] + [ActionView::Template.new(template_body(template_path), template_path, erb_handler, details)] end private - def test? - defined?(Rails) && Rails.env.test? - end - def erb_handler @erb_handler ||= ActionView::Template.registered_template_handler(:erb) end - def fetch(template_url) - HTTPClient.get(template_url) - rescue RestClient::Exception => e - raise TemplateNotFoundException, "Unable to fetch: '#{template_url}' because #{e}", caller - rescue Errno::ECONNREFUSED => e - raise CouldNotRetrieveTemplate, "Unable to fetch: '#{template_url}' because #{e}", caller - rescue SocketError => e - raise CouldNotRetrieveTemplate, "Unable to fetch: '#{template_url}' because #{e}", caller - end - - def template_url(template_path) - path = template_path.sub(/\.raw(\.html\.erb)?$/, '') - [static_host, "templates", "#{path}.raw.html.erb"].join('/') - end - - def static_host - @static_host ||= Plek.new.find('static') - end - - def test_body(path) - %Q{<#{TEST_TAG_NAME} data-template="#{path.parameterize}"><%= JSON.dump(local_assigns) %>} + def template_body(_template_path) + raise NotImplementedError, "Use NetworkComponentResolver or LocalComponentResolver" end end end diff --git a/lib/slimmer/govuk_components.rb b/lib/slimmer/govuk_components.rb index b891173..77ca457 100644 --- a/lib/slimmer/govuk_components.rb +++ b/lib/slimmer/govuk_components.rb @@ -50,7 +50,7 @@ def resolver @cache_last_reset = Time.now end - @resolver ||= Slimmer::ComponentResolver.new + @resolver ||= Slimmer::NetworkComponentResolver.new end end end diff --git a/lib/slimmer/local_component_resolver.rb b/lib/slimmer/local_component_resolver.rb new file mode 100644 index 0000000..f7a05e6 --- /dev/null +++ b/lib/slimmer/local_component_resolver.rb @@ -0,0 +1,19 @@ +module Slimmer + class LocalComponentResolver < ComponentResolver + private + + def template_body(template_path) + File.read(template_file(template_path)) + end + + def template_file(template_path) + path = template_path.sub(/\.raw(\.html\.erb)?$/, '') + + if defined?(Rails) + Rails.root.join("app", "views", "#{path}.raw.html.erb") + else + "#{path}.raw.html.erb" + end + end + end +end diff --git a/lib/slimmer/local_govuk_components.rb b/lib/slimmer/local_govuk_components.rb new file mode 100644 index 0000000..f967e38 --- /dev/null +++ b/lib/slimmer/local_govuk_components.rb @@ -0,0 +1,49 @@ +module Slimmer + # @api public + # + # Include this module to avoid loading components over the network + # @example + # class ApplicationController < ActionController::Base + # include Slimmer::LocalGovukComponents + # end + # + # # In your views: + # + # <%= render partial: 'govuk_component/example_component' %> + module LocalGovukComponents + def self.included into + into.before_action :add_govuk_components + end + + # @private + def add_govuk_components + append_view_path LocalGovukComponents.expiring_resolver_cache.resolver + end + + # @private + def self.expiring_resolver_cache + @expiring_resolver_cache ||= TimedExpirationResolverCache.new + end + + private + + # Slimmer::ComponentResolver instantiates a lot of large objects and leaks + # memory. This class will cache the resolver so that it doesn't have to + # create new ActionView::Template objects for each request. The cache is + # timed to allow frontends to pick up changes made to components in `static`. + class TimedExpirationResolverCache + def initialize + @cache_last_reset = Time.now + end + + def resolver + if (@cache_last_reset + Slimmer::CACHE_TTL) < Time.now + @resolver = nil + @cache_last_reset = Time.now + end + + @resolver ||= Slimmer::LocalComponentResolver.new + end + end + end +end diff --git a/lib/slimmer/network_component_resolver.rb b/lib/slimmer/network_component_resolver.rb new file mode 100644 index 0000000..91b8032 --- /dev/null +++ b/lib/slimmer/network_component_resolver.rb @@ -0,0 +1,45 @@ +require 'slimmer/govuk_request_id' +require 'active_support/core_ext/string/inflections' + +module Slimmer + class NetworkComponentResolver < ComponentResolver + private + + def template_body(template_path) + if test? + test_body(template_path) + else + Slimmer.cache.fetch(template_path, expires_in: Slimmer::CACHE_TTL) do + fetch(template_url(template_path)) + end + end + end + + def test? + defined?(Rails) && Rails.env.test? + end + + def fetch(template_url) + HTTPClient.get(template_url) + rescue RestClient::Exception => e + raise TemplateNotFoundException, "Unable to fetch: '#{template_url}' because #{e}", caller + rescue Errno::ECONNREFUSED => e + raise CouldNotRetrieveTemplate, "Unable to fetch: '#{template_url}' because #{e}", caller + rescue SocketError => e + raise CouldNotRetrieveTemplate, "Unable to fetch: '#{template_url}' because #{e}", caller + end + + def template_url(template_path) + path = template_path.sub(/\.raw(\.html\.erb)?$/, '') + [static_host, "templates", "#{path}.raw.html.erb"].join('/') + end + + def static_host + @static_host ||= Plek.new.find('static') + end + + def test_body(path) + %{<#{TEST_TAG_NAME} data-template="#{path.parameterize}"><%= JSON.dump(local_assigns) %>} + end + end +end diff --git a/test/component_resolver_test.rb b/test/component_resolver_test.rb index edfbcb5..248c2ad 100644 --- a/test/component_resolver_test.rb +++ b/test/component_resolver_test.rb @@ -10,32 +10,10 @@ assert_equal [], @resolver.find_templates('name', 'prefix', false, {}, false) end - it "should request a valid template from the server" do - assert_valid_template_requested('name', 'name.raw.html.erb') + it "should raise when template_body is called directly from ComponentResolver" do + assert_raises NotImplementedError do + @resolver.find_templates('name', 'govuk_component', false, {}, false) + end end - - it "should request a valid template from the server when a raw template is requested" do - assert_valid_template_requested('name.raw', 'name.raw.html.erb') - end - - it "should request a valid template from the server when the full template filename is requested" do - assert_valid_template_requested('name.raw.html.erb', 'name.raw.html.erb') - end - - it "should return a known template in test mode" do - @resolver.expects(:test?).returns(true) - - templates = @resolver.find_templates('name', 'govuk_component', false, {}, false) - assert_match //, templates.first.args[0] - end - end - - def assert_valid_template_requested(requested, expected) - expected_url = "http://static.dev.gov.uk/templates/govuk_component/#{expected}" - stub_request(:get, expected_url).to_return body: "" - - templates = @resolver.find_templates(requested, 'govuk_component', false, {}, false) - assert_requested :get, expected_url - assert_equal '', templates.first.args[0] end end diff --git a/test/local_component_resolver_test.rb b/test/local_component_resolver_test.rb new file mode 100644 index 0000000..827e91a --- /dev/null +++ b/test/local_component_resolver_test.rb @@ -0,0 +1,27 @@ +require_relative "test_helper" + +describe Slimmer::LocalComponentResolver do + describe "find_templates" do + before do + @resolver = Slimmer::LocalComponentResolver.new + end + + it "should request a valid template" do + assert_valid_template_requested('name', 'name.raw.html.erb') + end + + it "should request a valid template when a raw template is requested" do + assert_valid_template_requested('name.raw', 'name.raw.html.erb') + end + + it "should request a valid template when the full template filename is requested" do + assert_valid_template_requested('name.raw.html.erb', 'name.raw.html.erb') + end + end + + def assert_valid_template_requested(requested, expected) + File.expects(:read).with("govuk_component/#{expected}").returns('') + templates = @resolver.find_templates(requested, 'govuk_component', false, {}, false) + assert_equal '', templates.first.args[0] + end +end diff --git a/test/network_component_resolver_test.rb b/test/network_component_resolver_test.rb new file mode 100644 index 0000000..631e3af --- /dev/null +++ b/test/network_component_resolver_test.rb @@ -0,0 +1,41 @@ +require_relative "test_helper" + +describe Slimmer::NetworkComponentResolver do + describe "find_templates" do + before do + @resolver = Slimmer::NetworkComponentResolver.new + end + + it "should return nothing if the prefix doesn't match 'govuk_component'" do + assert_equal [], @resolver.find_templates('name', 'prefix', false, {}, false) + end + + it "should request a valid template from the server" do + assert_valid_template_requested('name', 'name.raw.html.erb') + end + + it "should request a valid template from the server when a raw template is requested" do + assert_valid_template_requested('name.raw', 'name.raw.html.erb') + end + + it "should request a valid template from the server when the full template filename is requested" do + assert_valid_template_requested('name.raw.html.erb', 'name.raw.html.erb') + end + + it "should return a known template in test mode" do + @resolver.expects(:test?).returns(true) + + templates = @resolver.find_templates('name', 'govuk_component', false, {}, false) + assert_match(//, templates.first.args[0]) + end + end + + def assert_valid_template_requested(requested, expected) + expected_url = "http://static.dev.gov.uk/templates/govuk_component/#{expected}" + stub_request(:get, expected_url).to_return body: "" + + templates = @resolver.find_templates(requested, 'govuk_component', false, {}, false) + assert_requested :get, expected_url + assert_equal '', templates.first.args[0] + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 00de771..17a2d36 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -40,7 +40,7 @@ def teardown require 'webmock/minitest' WebMock.disable_net_connect! -# Including action_view is dificualt because it depends on rails and internal +# Including action_view is difficult because it depends on rails and internal # ~*magic*~. To avoid depending on the whole of rails mock out the method we # need so we can tests the internal implementations which don't depend on rails module ActionView