From f413fb4fe681f573e91baf0d8b57bcee57a8a348 Mon Sep 17 00:00:00 2001
From: Justin Coyne <jcoyne@justincoyne.com>
Date: Fri, 5 Oct 2018 17:00:24 -0500
Subject: [PATCH] Avoid unnecessary lookup of field config

---
 app/presenters/blacklight/index_presenter.rb  |  5 +--
 app/presenters/blacklight/show_presenter.rb   |  4 +-
 app/views/catalog/_field.json.jbuilder        |  2 +-
 app/views/catalog/_index.html.erb             |  2 +-
 app/views/catalog/_show.html.erb              |  2 +-
 .../blacklight/index_presenter_spec.rb        | 32 +++++++---------
 .../blacklight/show_presenter_spec.rb         | 38 ++++++++-----------
 7 files changed, 36 insertions(+), 49 deletions(-)

diff --git a/app/presenters/blacklight/index_presenter.rb b/app/presenters/blacklight/index_presenter.rb
index 5351309b18..35effbfbbc 100644
--- a/app/presenters/blacklight/index_presenter.rb
+++ b/app/presenters/blacklight/index_presenter.rb
@@ -43,12 +43,11 @@ def label(field_or_string_or_proc, opts = {})
     #
     # Allow an extention point where information in the document
     # may drive the value of the field
-    # @param [String] field
+    # @param [Configuration::Field] field
     # @param [Hash] options
     # @option options [String] :value
     def field_value field, options = {}
-      field_config = field_config(field)
-      field_values(field_config, options)
+      field_values(field, options)
     end
 
     def thumbnail
diff --git a/app/presenters/blacklight/show_presenter.rb b/app/presenters/blacklight/show_presenter.rb
index 7e70725e91..90f6939aa9 100644
--- a/app/presenters/blacklight/show_presenter.rb
+++ b/app/presenters/blacklight/show_presenter.rb
@@ -56,11 +56,11 @@ def heading
     #
     # Allow an extention point where information in the document
     # may drive the value of the field
-    # @param [String] field
+    # @param [Configuration::Field] field
     # @param [Hash] options
     # @option options [String] :value
     def field_value field, options = {}
-      field_values(field_config(field), options)
+      field_values(field, options)
     end
 
     private
diff --git a/app/views/catalog/_field.json.jbuilder b/app/views/catalog/_field.json.jbuilder
index 432cd97cd6..b7bfa1da6d 100644
--- a/app/views/catalog/_field.json.jbuilder
+++ b/app/views/catalog/_field.json.jbuilder
@@ -4,7 +4,7 @@ json.set!(field_name) do
   json.id "#{document_url}##{field_name}"
   json.type 'document_value'
   json.attributes do
-    json.value doc_presenter.field_value(field_name)
+    json.value doc_presenter.field_value(field)
     json.label field.label
   end
 end
diff --git a/app/views/catalog/_index.html.erb b/app/views/catalog/_index.html.erb
index 230c9eb40c..545cd24c77 100644
--- a/app/views/catalog/_index.html.erb
+++ b/app/views/catalog/_index.html.erb
@@ -4,7 +4,7 @@
 
   <% doc_presenter.fields_to_render.each do |field_name, field| -%>
     <dt class="blacklight-<%= field_name.parameterize %> col-md-3"><%= render_index_field_label document, field: field_name %></dt>
-    <dd class="blacklight-<%= field_name.parameterize %> col-md-9"><%= doc_presenter.field_value field_name %></dd>
+    <dd class="blacklight-<%= field_name.parameterize %> col-md-9"><%= doc_presenter.field_value field %></dd>
   <% end -%>
 
 </dl>
diff --git a/app/views/catalog/_show.html.erb b/app/views/catalog/_show.html.erb
index 926dcbfb50..34eb819485 100644
--- a/app/views/catalog/_show.html.erb
+++ b/app/views/catalog/_show.html.erb
@@ -3,6 +3,6 @@
 <dl class="row dl-invert document-metadata">
   <% doc_presenter.fields_to_render.each do |field_name, field| -%>
     <dt class="blacklight-<%= field_name.parameterize %> col-md-3"><%= render_document_show_field_label document, field: field_name %></dt>
-    <dd class="blacklight-<%= field_name.parameterize %> col-md-9"><%= doc_presenter.field_value field_name %></dd>
+    <dd class="blacklight-<%= field_name.parameterize %> col-md-9"><%= doc_presenter.field_value field %></dd>
   <% end -%>
 </dl>
diff --git a/spec/presenters/blacklight/index_presenter_spec.rb b/spec/presenters/blacklight/index_presenter_spec.rb
index 46a064411d..fa303d03fa 100644
--- a/spec/presenters/blacklight/index_presenter_spec.rb
+++ b/spec/presenters/blacklight/index_presenter_spec.rb
@@ -17,8 +17,7 @@
     SolrDocument.new(id: 1,
                      'link_to_facet_true' => 'x',
                      'link_to_facet_named' => 'x',
-                     'qwer' => 'document qwer value',
-                     'mnbv' => 'document mnbv value')
+                     'qwer' => 'document qwer value')
   end
 
   before do
@@ -42,78 +41,73 @@
     end
 
     it "checks for an explicit value" do
-      value = subject.field_value 'asdf', value: 'asdf'
+      value = subject.field_value config.index_fields['asdf'], value: 'asdf'
       expect(value).to eq 'asdf'
     end
 
     it "checks for a helper method to call" do
       allow(request_context).to receive(:render_asdf_index_field).and_return('custom asdf value')
-      value = subject.field_value 'asdf'
+      value = subject.field_value config.index_fields['asdf']
       expect(value).to eq 'custom asdf value'
     end
 
     it "checks for a link_to_facet" do
       allow(request_context).to receive(:search_action_path).with('f' => { 'link_to_facet_true' => ['x'] }).and_return('/foo')
       allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar')
-      value = subject.field_value 'link_to_facet_true'
+      value = subject.field_value config.index_fields['link_to_facet_true']
       expect(value).to eq 'bar'
     end
 
     it "checks for a link_to_facet with a field name" do
       allow(request_context).to receive(:search_action_path).with('f' => { 'some_field' => ['x'] }).and_return('/foo')
       allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar')
-      value = subject.field_value 'link_to_facet_named'
+      value = subject.field_value config.index_fields['link_to_facet_named']
       expect(value).to eq 'bar'
     end
 
     it "gracefully handles when no highlight field is available" do
       allow(document).to receive(:has_highlight_field?).and_return(false)
-      value = subject.field_value 'highlight'
+      value = subject.field_value config.index_fields['highlight']
       expect(value).to be_blank
     end
 
     it "checks for a highlighted field" do
       allow(document).to receive(:has_highlight_field?).and_return(true)
       allow(document).to receive(:highlight_field).with('highlight').and_return(['<em>highlight</em>'.html_safe])
-      value = subject.field_value 'highlight'
+      value = subject.field_value config.index_fields['highlight']
       expect(value).to eq '<em>highlight</em>'
     end
 
     it "checks the document field value" do
-      value = subject.field_value 'qwer'
+      value = subject.field_value config.index_fields['qwer']
       expect(value).to eq 'document qwer value'
     end
 
-    it "works with index fields that aren't explicitly defined" do
-      value = subject.field_value 'mnbv'
-      expect(value).to eq 'document mnbv value'
-    end
-
     it "calls an accessor on the solr document" do
       allow(document).to receive_messages(solr_doc_accessor: "123")
-      value = subject.field_value 'solr_doc_accessor'
+      value = subject.field_value config.index_fields['solr_doc_accessor']
       expect(value).to eq "123"
     end
 
     it "calls an explicit accessor on the solr document" do
       allow(document).to receive_messages(solr_doc_accessor: "123")
-      value = subject.field_value 'explicit_accessor'
+      value = subject.field_value config.index_fields['explicit_accessor']
       expect(value).to eq "123"
     end
 
     it "calls an accessor on the solr document with the field as an argument" do
       allow(document).to receive(:solr_doc_accessor_with_arg).with('explicit_accessor_with_arg').and_return("123")
-      value = subject.field_value 'explicit_accessor_with_arg'
+      value = subject.field_value config.index_fields['explicit_accessor_with_arg']
       expect(value).to eq "123"
     end
 
     it "supports solr field configuration" do
-      value = subject.field_value 'alias'
+      value = subject.field_value config.index_fields['alias']
       expect(value).to eq "document qwer value"
     end
 
     it "supports default values in the field configuration" do
-      value = subject.field_value 'with_default'
+      value = subject.field_value config.index_fields['with_default']
       expect(value).to eq "value"
     end
   end
diff --git a/spec/presenters/blacklight/show_presenter_spec.rb b/spec/presenters/blacklight/show_presenter_spec.rb
index 895eac800e..161bbcb08f 100644
--- a/spec/presenters/blacklight/show_presenter_spec.rb
+++ b/spec/presenters/blacklight/show_presenter_spec.rb
@@ -17,8 +17,7 @@
     SolrDocument.new(id: 1,
                      'link_to_facet_true' => 'x',
                      'link_to_facet_named' => 'x',
-                     'qwer' => 'document qwer value',
-                     'mnbv' => 'document mnbv value')
+                     'qwer' => 'document qwer value')
   end
 
   before do
@@ -124,43 +123,43 @@ def mock_document_app_helper_url *args
     end
 
     it 'html-escapes values' do
-      value = subject.field_value 'asdf', value: '<b>val1</b>'
+      value = subject.field_value config.show_fields['asdf'], value: '<b>val1</b>'
       expect(value).to eq '&lt;b&gt;val1&lt;/b&gt;'
     end
 
     it 'joins multivalued valued fields' do
-      value = subject.field_value 'asdf', value: ['<a', 'b']
+      value = subject.field_value config.show_fields['asdf'], value: ['<a', 'b']
       expect(value).to eq '&lt;a and b'
     end
 
     it 'joins multivalued valued fields' do
-      value = subject.field_value 'asdf', value: %w[a b c]
+      value = subject.field_value config.show_fields['asdf'], value: %w[a b c]
       expect(value).to eq 'a, b, and c'
     end
 
     it "checks for an explicit value" do
       expect(request_context).not_to receive(:render_asdf_document_show_field)
-      value = subject.field_value 'asdf', value: 'val1'
+      value = subject.field_value config.show_fields['asdf'], value: 'val1'
       expect(value).to eq 'val1'
     end
 
     it "checks for a helper method to call" do
       allow(request_context).to receive(:render_asdf_document_show_field).and_return('custom asdf value')
-      value = subject.field_value 'asdf'
+      value = subject.field_value config.show_fields['asdf']
       expect(value).to eq 'custom asdf value'
     end
 
     it "checks for a link_to_facet" do
       allow(request_context).to receive(:search_action_path).and_return('/foo')
       allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar')
-      value = subject.field_value 'link_to_facet_true'
+      value = subject.field_value config.show_fields['link_to_facet_true']
       expect(value).to eq 'bar'
     end
 
     it "checks for a link_to_facet with a field name" do
       allow(request_context).to receive(:search_action_path).and_return('/foo')
       allow(request_context).to receive(:link_to).with("x", '/foo').and_return('bar')
-      value = subject.field_value 'link_to_facet_named'
+      value = subject.field_value config.show_fields['link_to_facet_named']
       expect(value).to eq 'bar'
     end
 
@@ -169,7 +168,7 @@ def mock_document_app_helper_url *args
         allow(document).to receive(:has_highlight_field?).and_return(false)
       end
 
-      let(:value) { subject.field_value 'highlight' }
+      let(:value) { subject.field_value config.show_fields['highlight'] }
 
       it "is blank" do
         expect(value).to be_blank
@@ -179,48 +178,43 @@ def mock_document_app_helper_url *args
     it "checks for a highlighted field" do
       allow(document).to receive(:has_highlight_field?).and_return(true)
       allow(document).to receive(:highlight_field).with('highlight').and_return(['<em>highlight</em>'.html_safe])
-      value = subject.field_value 'highlight'
+      value = subject.field_value config.show_fields['highlight']
       expect(value).to eq '<em>highlight</em>'
     end
 
     it 'respects the HTML-safeness of multivalued highlight fields' do
       allow(document).to receive(:has_highlight_field?).and_return(true)
       allow(document).to receive(:highlight_field).with('highlight').and_return(['<em>highlight</em>'.html_safe, '<em>other highlight</em>'.html_safe])
-      value = subject.field_value 'highlight'
+      value = subject.field_value config.show_fields['highlight']
       expect(value).to eq '<em>highlight</em> and <em>other highlight</em>'
     end
 
     it "checks the document field value" do
-      value = subject.field_value 'qwer'
+      value = subject.field_value config.show_fields['qwer']
       expect(value).to eq 'document qwer value'
     end
 
-    it "works with show fields that aren't explicitly defined" do
-      value = subject.field_value 'mnbv'
-      expect(value).to eq 'document mnbv value'
-    end
-
     it "calls an accessor on the solr document" do
       allow(document).to receive_messages(solr_doc_accessor: "123")
-      value = subject.field_value 'solr_doc_accessor'
+      value = subject.field_value config.show_fields['solr_doc_accessor']
       expect(value).to eq "123"
     end
 
     it "calls an explicit accessor on the solr document" do
       allow(document).to receive_messages(solr_doc_accessor: "123")
-      value = subject.field_value 'explicit_accessor'
+      value = subject.field_value config.show_fields['explicit_accessor']
       expect(value).to eq "123"
     end
 
     it "calls an explicit array-style accessor on the solr document" do
       allow(document).to receive_message_chain(:solr_doc_accessor, some_method: "123")
-      value = subject.field_value 'explicit_array_accessor'
+      value = subject.field_value config.show_fields['explicit_array_accessor']
       expect(value).to eq "123"
     end
 
     it "calls an accessor on the solr document with the field as an argument" do
       allow(document).to receive(:solr_doc_accessor_with_arg).with('explicit_accessor_with_arg').and_return("123")
-      value = subject.field_value 'explicit_accessor_with_arg'
+      value = subject.field_value config.show_fields['explicit_accessor_with_arg']
       expect(value).to eq "123"
     end
   end