Add Document#fetch and deprecate Document#get

app/helpers/blacklight/blacklight_helper_behavior.rb

@@ -183,7 +183,7 @@ def render_index_field_value *args
     document = args.shift || options[:document]
 
     field = args.shift || options[:field]
-    presenter(document).render_index_field_value field, options
+    presenter(document).render_index_field_value field, options.except(:document, :field)
   end
 
   ##
@@ -236,7 +236,7 @@ def render_document_show_field_value *args
     document = args.shift || options[:document]
 
     field = args.shift || options[:field]
-    presenter(document).render_document_show_field_value field, options
+    presenter(document).render_document_show_field_value field, options.except(:document, :field)
   end
 
   ##

lib/blacklight/document.rb

@@ -26,6 +26,8 @@ module Blacklight::Document
   include Blacklight::Document::SemanticFields
   include Blacklight::Document::Export
 
+  extend Deprecation
+
   included do
     extend ActiveModel::Naming
     include Blacklight::Document::Extensions
@@ -52,15 +54,15 @@ def persisted?
   # If a method is missing, it gets sent to @_source
   # with all of the original params and block
   def method_missing(m, *args, &b)
-    if _source and _source.respond_to? m
+    if _source_responds_to?(m)
       _source.send(m, *args, &b)
     else
       super
     end
   end
 
   def respond_to_missing? *args
-    (_source && _source.respond_to?(*args)) || super
+    _source_responds_to?(*args) || super
   end
 
   def [] *args
@@ -111,11 +113,24 @@ def key? k
   #  - :default - a value to return when the key doesn't exist
   # if :sep is nil and the field is a multivalued field, the array is returned
   def get(key, opts={:sep=>', ', :default=>nil})
+    val = fetch(key, opts[:default])
+
+    if val.is_a?(Array) and opts[:sep]
+      Deprecation.warn(Blacklight::Solr::Document, "#{self.class}#get with a :sep option is deprecated; use #[] or #fetch and join the values using e.g. Array#to_sentence") unless opts[:sep].nil?
+      val.join(opts[:sep])
+    else
+      val
+    end
+  end
+  deprecation_deprecate get: "Use #[] or #fetch instead"
+
+  def fetch key, *default
     if key? key
-      val = self[key]
-      (val.is_a?(Array) and opts[:sep]) ? val.join(opts[:sep]) : val
+      self[key]
+    elsif default.empty? and !block_given?
+      raise KeyError.new("key not found \"#{key}\"")
     else
-      opts[:default]
+      (yield(self) if block_given?) || default.first
     end
   end
 
@@ -179,4 +194,11 @@ def base_class
       self
     end  
   end
+
+  private
+
+  def _source_responds_to? *args
+    _source && self != _source && _source.respond_to?(*args)
+  end
+
 end

lib/blacklight/document_presenter.rb

@@ -82,7 +82,7 @@ def render_document_index_label field, opts ={}
       end
       label = case field
       when Symbol
-        @document.get(field, :sep => nil)
+        @document[field]
       when Proc
         field.call(@document, opts)
       when String
@@ -156,10 +156,14 @@ def get_field_values field, field_config, options = {}
             end
           end
         when field_config
-          # regular solr
-          @document.get(field_config.field, sep: nil)
+          # regular document field
+          if field_config.default and field_config.default.is_a? Proc
+            @document.fetch(field_config.field, &field_config.default)
+          else
+            @document.fetch(field_config.field, field_config.default)
+          end
         when field
-          @document.get(field, sep: nil)
+          @document[field]
       end
 
       # rendering values

spec/helpers/blacklight_helper_spec.rb

@@ -207,203 +207,53 @@ def mock_document_app_helper_url *args
     end
   end
 
-  describe "render_index_field_value" do
+  describe "#render_index_field_value" do
+    let(:presenter) { double }
     before do
-      @config = Blacklight::Configuration.new.configure do |config|
-        config.add_index_field 'qwer'
-        config.add_index_field 'asdf', :helper_method => :render_asdf_index_field
-        config.add_index_field 'link_to_search_true', :link_to_search => true
-        config.add_index_field 'link_to_search_named', :link_to_search => :some_field
-        config.add_index_field 'highlight', :highlight => true
-        config.add_index_field 'solr_doc_accessor', :accessor => true
-        config.add_index_field 'explicit_accessor', :accessor => :solr_doc_accessor
-        config.add_index_field 'explicit_accessor_with_arg', :accessor => :solr_doc_accessor_with_arg
-      end
-      allow(helper).to receive(:blacklight_config).and_return(@config)
-    end
-
-    it "should check for an explicit value" do
-      doc = double()
-      expect(doc).to_not receive(:get).with('asdf', :sep => nil)
-      value = helper.render_index_field_value :value => 'asdf', :document => doc, :field => 'asdf'
-      expect(value).to eq 'asdf'
+      allow(helper).to receive(:presenter).with(doc).and_return(presenter)
     end
 
-    it "should check for a helper method to call" do
-      doc = double()
-      allow(doc).to receive(:get).with('asdf', :sep => nil)
-      allow(helper).to receive(:render_asdf_index_field).and_return('custom asdf value')
-      value = helper.render_index_field_value :document => doc, :field => 'asdf'
-      expect(value).to eq 'custom asdf value'
-    end
-
-    it "should check for a link_to_search" do
-      doc = double()
-      allow(doc).to receive(:get).with('link_to_search_true', :sep => nil).and_return('x')
-      value = helper.render_index_field_value :document => doc, :field => 'link_to_search_true'
-      expect(value).to eq helper.link_to("x", helper.search_action_path(:f => { :link_to_search_true => ['x'] }))
-    end
-
-    it "should check for a link_to_search with a field name" do
-      doc = double()
-      allow(doc).to receive(:get).with('link_to_search_named', :sep => nil).and_return('x')
-      value = helper.render_index_field_value :document => doc, :field => 'link_to_search_named'
-      expect(value).to eq helper.link_to("x", helper.search_action_path(:f => { :some_field => ['x'] }))
-    end
-
-    it "should gracefully handle when no highlight field is available" do
-      doc = double()
-      expect(doc).to_not receive(:get)
-      allow(doc).to receive(:has_highlight_field?).and_return(false)
-      value = helper.render_index_field_value :document => doc, :field => 'highlight'
-      expect(value).to be_blank
-    end
-
-    it "should check for a highlighted field" do
-      doc = double()
-      expect(doc).to_not receive(:get)
-      allow(doc).to receive(:has_highlight_field?).and_return(true)
-      allow(doc).to receive(:highlight_field).with('highlight').and_return(['<em>highlight</em>'.html_safe])
-      value = helper.render_index_field_value :document => doc, :field => 'highlight'
-      expect(value).to eq '<em>highlight</em>'
-    end
-
-    it "should check the document field value" do
-      doc = double()
-      allow(doc).to receive(:get).with('qwer', :sep => nil).and_return('document qwer value')
-      value = helper.render_index_field_value :document => doc, :field => 'qwer'
-      expect(value).to eq 'document qwer value'
-    end
-
-    it "should work with index fields that aren't explicitly defined" do
-      doc = double()
-      allow(doc).to receive(:get).with('mnbv', :sep => nil).and_return('document mnbv value')
-      value = helper.render_index_field_value :document => doc, :field => 'mnbv'
-      expect(value).to eq 'document mnbv value'
-    end
+    let(:doc) { double }
+    let(:field) { "some_field" }
 
-    it "should call an accessor on the solr document" do
-      doc = double(:solr_doc_accessor => "123")
-      value = helper.render_index_field_value :document => doc, :field => 'solr_doc_accessor'
-      expect(value).to eq "123"
+    it "should pass the document and field through to the presenter" do
+      expect(presenter).to receive(:render_index_field_value).with(field, {})
+      helper.render_index_field_value(doc, field)
     end
 
-    it "should call an explicit accessor on the solr document" do
-      doc = double(:solr_doc_accessor => "123")
-      value = helper.render_index_field_value :document => doc, :field => 'explicit_accessor'
-      expect(value).to eq "123"
+    it "should allow the document and field to be passed as hash arguments" do
+      expect(presenter).to receive(:render_index_field_value).with(field, {})
+      helper.render_index_field_value(document: doc, field: field)
     end
 
-    it "should call an implicit accessor on the solr document" do
-      doc = double()
-      expect(doc).to receive(:solr_doc_accessor_with_arg).with('explicit_accessor_with_arg').and_return("123")
-      value = helper.render_index_field_value :document => doc, :field => 'explicit_accessor_with_arg'
-      expect(value).to eq "123"
+    it "should allow additional options to be passed to the presenter" do
+      expect(presenter).to receive(:render_index_field_value).with(field, x: 1)
+      helper.render_index_field_value(document: doc, field: field, x: 1)
     end
   end
-
-  describe "render_document_show_field_value" do
+
+  describe "#render_document_show_field_value" do
+    let(:presenter) { double }
     before do
-      @config = Blacklight::Configuration.new.configure do |config|
-        config.add_show_field 'qwer'
-        config.add_show_field 'asdf', :helper_method => :render_asdf_document_show_field
-        config.add_show_field 'link_to_search_true', :link_to_search => true
-        config.add_show_field 'link_to_search_named', :link_to_search => :some_field
-        config.add_show_field 'highlight', :highlight => true
-        config.add_show_field 'solr_doc_accessor', :accessor => true
-        config.add_show_field 'explicit_accessor', :accessor => :solr_doc_accessor
-        config.add_show_field 'explicit_array_accessor', :accessor => [:solr_doc_accessor, :some_method]
-        config.add_show_field 'explicit_accessor_with_arg', :accessor => :solr_doc_accessor_with_arg
-      end
-
-      allow(helper).to receive(:blacklight_config).and_return(@config)
-    end
-
-    it "should check for an explicit value" do
-      doc = double()
-      expect(doc).to_not receive(:get).with('asdf', :sep => nil)
-      expect(helper).to_not receive(:render_asdf_document_show_field)
-      value = helper.render_document_show_field_value :value => 'asdf', :document => doc, :field => 'asdf'
-      expect(value).to eq 'asdf'
+      allow(helper).to receive(:presenter).with(doc).and_return(presenter)
     end
 
-    it "should check for a helper method to call" do
-      doc = double()
-      allow(doc).to receive(:get).with('asdf', :sep => nil)
-      allow(helper).to receive(:render_asdf_document_show_field).and_return('custom asdf value')
-      value = helper.render_document_show_field_value :document => doc, :field => 'asdf'
-      expect(value).to eq 'custom asdf value'
-    end
-
-    it "should check for a link_to_search" do
-      doc = double()
-      allow(doc).to receive(:get).with('link_to_search_true', :sep => nil).and_return('x')
-      value = helper.render_document_show_field_value :document => doc, :field => 'link_to_search_true'
-      expect(value).to eq helper.link_to("x", helper.search_action_path(:f => { :link_to_search_true => ['x'] }))
-    end
-
-    it "should check for a link_to_search with a field name" do
-      doc = double()
-      allow(doc).to receive(:get).with('link_to_search_named', :sep => nil).and_return('x')
-      value = helper.render_document_show_field_value :document => doc, :field => 'link_to_search_named'
-      expect(value).to eq helper.link_to("x", helper.search_action_path(:f => { :some_field => ['x'] }))
-    end
-
-    it "should gracefully handle when no highlight field is available" do
-      doc = double()
-      expect(doc).to_not receive(:get)
-      allow(doc).to receive(:has_highlight_field?).and_return(false)
-      value = helper.render_document_show_field_value :document => doc, :field => 'highlight'
-      expect(value).to be_blank
-    end
-
-    it "should check for a highlighted field" do
-      doc = double()
-      expect(doc).to_not receive(:get)
-      allow(doc).to receive(:has_highlight_field?).and_return(true)
-      allow(doc).to receive(:highlight_field).with('highlight').and_return(['<em>highlight</em>'.html_safe])
-      value = helper.render_document_show_field_value :document => doc, :field => 'highlight'
-      expect(value).to eq '<em>highlight</em>'
-    end
-
-
-    it "should check the document field value" do
-      doc = double()
-      allow(doc).to receive(:get).with('qwer', :sep => nil).and_return('document qwer value')
-      value = helper.render_document_show_field_value :document => doc, :field => 'qwer'
-      expect(value).to eq 'document qwer value'
-    end
-
-    it "should work with show fields that aren't explicitly defined" do
-      doc = double()
-      allow(doc).to receive(:get).with('mnbv', :sep => nil).and_return('document mnbv value')
-      value = helper.render_document_show_field_value :document => doc, :field => 'mnbv'
-      expect(value).to eq 'document mnbv value'
-    end
-
-    it "should call an accessor on the solr document" do
-      doc = double(:solr_doc_accessor => "123")
-      value = helper.render_document_show_field_value :document => doc, :field => 'solr_doc_accessor'
-      expect(value).to eq "123"
-    end
+    let(:doc) { double }
+    let(:field) { "some_field" }
 
-    it "should call an explicit accessor on the solr document" do
-      doc = double(:solr_doc_accessor => "123")
-      value = helper.render_document_show_field_value :document => doc, :field => 'explicit_accessor'
-      expect(value).to eq "123"
+    it "should pass the document and field through to the presenter" do
+      expect(presenter).to receive(:render_document_show_field_value).with(field, {})
+      helper.render_document_show_field_value(doc, field)
     end
 
-    it "should call an explicit array-style accessor on the solr document" do
-      doc = double(:solr_doc_accessor => double(:some_method => "123"))
-      value = helper.render_document_show_field_value :document => doc, :field => 'explicit_array_accessor'
-      expect(value).to eq "123"
+    it "should allow the document and field to be passed as hash arguments" do
+      expect(presenter).to receive(:render_document_show_field_value).with(field, {})
+      helper.render_document_show_field_value(document: doc, field: field)
     end
 
-    it "should call an accessor on the solr document with the field as an argument" do
-      doc = double()
-      expect(doc).to receive(:solr_doc_accessor_with_arg).with('explicit_accessor_with_arg').and_return("123")
-      value = helper.render_document_show_field_value :document => doc, :field => 'explicit_accessor_with_arg'
-      expect(value).to eq "123"
+    it "should allow additional options to be passed to the presenter" do
+      expect(presenter).to receive(:render_document_show_field_value).with(field, x: 1)
+      helper.render_document_show_field_value(document: doc, field: field, x: 1)
     end
   end
 

spec/helpers/url_helper_spec.rb

@@ -273,7 +273,7 @@
     it "should accept and return a Proc" do
       data = {'id'=>'123456','title_display'=>['654321'] }
       @document = SolrDocument.new(data)
-      expect(helper.link_to_document(@document, Proc.new { |doc, opts| doc.get(:id) + ": " + doc.get(:title_display) })).to have_selector("a", :text => '123456: 654321', :count => 1)
+      expect(helper.link_to_document(@document, Proc.new { |doc, opts| doc[:id] + ": " + doc.first(:title_display) })).to have_selector("a", :text => '123456: 654321', :count => 1)
     end
 
     it "should return id when label is missing" do

spec/lib/blacklight/search_helper_spec.rb

@@ -195,8 +195,8 @@ def params
         (solr_response, document_list) = subject.get_search_results(q: @all_docs_query)
         result_docs = document_list
         document = result_docs.first
-        expect(document.get(blacklight_config.index.title_field)).not_to be_nil
-        expect(document.get(blacklight_config.index.display_type_field)).not_to be_nil
+        expect(document.fetch(blacklight_config.index.title_field)).not_to be_nil
+        expect(document.fetch(blacklight_config.index.display_type_field)).not_to be_nil
       end
     end
 
@@ -252,8 +252,8 @@ def params
         (solr_response, document_list) = subject.search_results({ q: @all_docs_query }, default_method_chain)
         result_docs = document_list
         document = result_docs.first
-        expect(document.get(blacklight_config.index.title_field)).not_to be_nil
-        expect(document.get(blacklight_config.index.display_type_field)).not_to be_nil
+        expect(document.fetch(blacklight_config.index.title_field)).not_to be_nil
+        expect(document.fetch(blacklight_config.index.display_type_field)).not_to be_nil
       end
     end
 
@@ -503,7 +503,7 @@ def params
       expect(@document.id).to eq @doc_id
     end
     it 'should have non-nil values for required fields set in initializer' do
-      expect(@document.get(blacklight_config.view_config(:show).display_type_field)).not_to be_nil
+      expect(@document.fetch(blacklight_config.view_config(:show).display_type_field)).not_to be_nil
     end
   end
 

spec/lib/blacklight/solr/document_spec.rb

@@ -25,9 +25,11 @@ def my_extension_method
         expect(doc.has?(:cat, 'elec')).not_to eq true
         expect(doc.has?(:cat, 'electronics')).to eq true
 
-        expect(doc.get(:cat)).to eq 'electronics, hard drive'
-        expect(doc.get(:xyz)).to be_nil
-        expect(doc.get(:xyz, :default=>'def')).to eq 'def'
+        expect(doc.fetch(:cat)).to eq ['electronics', 'hard drive']
+        expect(doc.fetch(:xyz, nil)).to be_nil
+        expect(doc.fetch(:xyz, 'def')).to eq 'def'
+        expect(doc.fetch(:xyz) { |el| 'def' }).to eq 'def'
+        expect { doc.fetch(:xyz) }.to raise_exception(KeyError)
       end
     end