Skip to content

Commit

Permalink
Avoid unnecessary lookup of field config
Browse files Browse the repository at this point in the history
  • Loading branch information
jcoyne committed Oct 6, 2018
1 parent 8de6ad0 commit f413fb4
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 49 deletions.
5 changes: 2 additions & 3 deletions app/presenters/blacklight/index_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions app/presenters/blacklight/show_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/views/catalog/_field.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/views/catalog/_index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
2 changes: 1 addition & 1 deletion app/views/catalog/_show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
32 changes: 13 additions & 19 deletions spec/presenters/blacklight/index_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
38 changes: 16 additions & 22 deletions spec/presenters/blacklight/show_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit f413fb4

Please sign in to comment.