From f18362a609b1c1cf0f124a1a6f0efc803029f04d Mon Sep 17 00:00:00 2001
From: Chris Beer <cabeer@stanford.edu>
Date: Wed, 26 Mar 2014 15:56:39 -0400
Subject: [PATCH] Extract SolrDocument references from controllers and models
 and define the solr document class to use in the blacklight configuration.

- add blacklight_config.solr_document_model configuration (defaults to
  SolrDocument)
- make Bookmarks polymorphic and refactor Bookmarks to use objects
  instead of passing IDs around
---
 app/controllers/bookmarks_controller.rb       | 14 +++---
 .../blacklight/catalog_helper_behavior.rb     |  5 +++
 .../configuration_helper_behavior.rb          |  2 +-
 app/models/bookmark.rb                        | 17 +++++--
 app/models/solr_document.rb                   |  5 +++
 app/views/catalog/_bookmark_control.html.erb  |  4 +-
 ...00000_add_polymorphic_type_to_bookmarks.rb |  8 ++++
 lib/blacklight/catalog.rb                     |  8 ++--
 lib/blacklight/configuration.rb               | 11 +++--
 lib/blacklight/solr/document.rb               | 15 +++++++
 lib/blacklight/solr_helper.rb                 | 11 +++--
 lib/blacklight/solr_response.rb               | 44 +++++--------------
 lib/blacklight/solr_response/response.rb      | 18 ++++++++
 lib/blacklight/user.rb                        | 21 +++++++--
 spec/controllers/bookmarks_controller_spec.rb | 10 ++++-
 spec/controllers/catalog_controller_spec.rb   | 36 +++++++--------
 spec/helpers/blacklight_helper_spec.rb        |  1 +
 spec/lib/blacklight/configuration_spec.rb     |  5 ---
 spec/lib/blacklight/solr_helper_spec.rb       |  4 +-
 spec/lib/blacklight/user_spec.rb              | 42 ++++++++++++++++--
 spec/models/bookmark_spec.rb                  | 39 +++++++++-------
 21 files changed, 211 insertions(+), 109 deletions(-)
 create mode 100644 app/models/solr_document.rb
 create mode 100644 db/migrate/20140320000000_add_polymorphic_type_to_bookmarks.rb
 create mode 100644 lib/blacklight/solr_response/response.rb

diff --git a/app/controllers/bookmarks_controller.rb b/app/controllers/bookmarks_controller.rb
index 6b96e8875a..4d2f36dfa4 100644
--- a/app/controllers/bookmarks_controller.rb
+++ b/app/controllers/bookmarks_controller.rb
@@ -22,7 +22,7 @@ def index
     @bookmarks = current_or_guest_user.bookmarks
     bookmark_ids = @bookmarks.collect { |b| b.document_id.to_s }
   
-    @response, @document_list = get_solr_response_for_field_values(SolrDocument.unique_key, bookmark_ids)
+    @response, @document_list = get_solr_response_for_document_ids(bookmark_ids)
   end
 
   def update
@@ -41,13 +41,13 @@ def create
     if params[:bookmarks]
       @bookmarks = params[:bookmarks]
     else
-      @bookmarks = [{ :document_id => params[:id] }]
+      @bookmarks = [{ document_id: params[:id], document_type: blacklight_config.solr_document_model.to_s }]
     end
 
     current_or_guest_user.save! unless current_or_guest_user.persisted?
 
     success = @bookmarks.all? do |bookmark|
-      current_or_guest_user.bookmarks.create(bookmark) unless current_or_guest_user.existing_bookmark_for(bookmark[:document_id])
+      current_or_guest_user.bookmarks.create(bookmark) unless current_or_guest_user.bookmarks.where(bookmark).exists?
     end
 
     if request.xhr?
@@ -66,10 +66,10 @@ def create
   # Beware, :id is the Solr document_id, not the actual Bookmark id.
   # idempotent, as DELETE is supposed to be. 
   def destroy
-    bookmark = current_or_guest_user.existing_bookmark_for(params[:id])
-    
-    success = (!bookmark) || current_or_guest_user.bookmarks.delete(bookmark)
-    
+    bookmark = current_or_guest_user.bookmarks.where(document_id: params[:id], document_type: blacklight_config.solr_document_model).first
+
+    success = bookmark && bookmark.delete && bookmark.destroyed?
+
     unless request.xhr?
       if success
         flash[:notice] =  I18n.t('blacklight.bookmarks.remove.success')
diff --git a/app/helpers/blacklight/catalog_helper_behavior.rb b/app/helpers/blacklight/catalog_helper_behavior.rb
index 2eede55869..162d4bdecc 100644
--- a/app/helpers/blacklight/catalog_helper_behavior.rb
+++ b/app/helpers/blacklight/catalog_helper_behavior.rb
@@ -205,4 +205,9 @@ def render_view_type_group_icon view
   def default_view_type_group_icon_classes view
     "glyphicon-#{view.to_s.parameterize } view-icon-#{view.to_s.parameterize}"
   end
+  
+  def current_bookmarks response = nil
+    response ||= @response
+    @current_bookmarks ||= current_or_guest_user.bookmarks_for_documents(response.documents).to_a
+  end
 end
diff --git a/app/helpers/blacklight/configuration_helper_behavior.rb b/app/helpers/blacklight/configuration_helper_behavior.rb
index 139d0d7684..96071bad83 100644
--- a/app/helpers/blacklight/configuration_helper_behavior.rb
+++ b/app/helpers/blacklight/configuration_helper_behavior.rb
@@ -104,7 +104,7 @@ def spell_check_max
 
   # Used in the document list partial (search view) for creating a link to the document show action
   def document_show_link_field document=nil
-    blacklight_config.view_config(document_index_view_type).title_field.to_sym
+    blacklight_config.view_config(document_index_view_type).title_field.try(:to_sym) || document.id
   end
 
   ##
diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb
index 32383773da..1e3216ab0f 100644
--- a/app/models/bookmark.rb
+++ b/app/models/bookmark.rb
@@ -1,13 +1,22 @@
 # -*- encoding : utf-8 -*-
 class Bookmark < ActiveRecord::Base
   
-  belongs_to :user
-  validates_presence_of :user_id, :scope=>:document_id
-  attr_accessible :id, :document_id, :title if Rails::VERSION::MAJOR < 4
+  belongs_to :user, polymorphic: true
+  belongs_to :document, polymorphic: true
 
+  validates_presence_of :user_id, :scope=>:document_id
+  attr_accessible :id, :document_id, :document_type, :title if Rails::VERSION::MAJOR < 4
 
   def document
-    SolrDocument.new SolrDocument.unique_key => document_id
+    document_type.new document_type.unique_key => document_id
+  end
+  
+  def document_type
+    (super.constantize if defined?(super)) || default_document_type
+  end
+  
+  def default_document_type
+    SolrDocument
   end
   
 end
diff --git a/app/models/solr_document.rb b/app/models/solr_document.rb
new file mode 100644
index 0000000000..14405c855e
--- /dev/null
+++ b/app/models/solr_document.rb
@@ -0,0 +1,5 @@
+class SolrDocument
+
+  include Blacklight::Solr::Document
+
+end
diff --git a/app/views/catalog/_bookmark_control.html.erb b/app/views/catalog/_bookmark_control.html.erb
index 8dd14abaec..68ee1ba40a 100644
--- a/app/views/catalog/_bookmark_control.html.erb
+++ b/app/views/catalog/_bookmark_control.html.erb
@@ -3,8 +3,8 @@
   # Note these two forms are pretty similar but for different :methods, classes, and labels. 
   # but it was simpler to leave them seperate instead of DRYing them, got confusing trying that.
   # the data-doc-id attribute is used by our JS that converts to a checkbox/label.
-  -%>  
-  <% unless current_or_guest_user.document_is_bookmarked? document.id %>  
+  -%>
+  <% if current_bookmarks.find { |x| x.document_id == document.id and x.document_type == document.class }.blank? %>  
 
       <%= form_tag( bookmark_path( document ), :method => :put, :class => "bookmark_toggle", "data-doc-id" => document.id, :'data-present' => t('blacklight.search.bookmarks.present'), :'data-absent' => t('blacklight.search.bookmarks.absent'), :'data-inprogress' => t('blacklight.search.bookmarks.inprogress')) do %>   
         <%= submit_tag(t('blacklight.bookmarks.add.button'), :id => "bookmark_toggle_#{document.id.to_s.parameterize}", :class => "bookmark_add") %>
diff --git a/db/migrate/20140320000000_add_polymorphic_type_to_bookmarks.rb b/db/migrate/20140320000000_add_polymorphic_type_to_bookmarks.rb
new file mode 100644
index 0000000000..00b482bd89
--- /dev/null
+++ b/db/migrate/20140320000000_add_polymorphic_type_to_bookmarks.rb
@@ -0,0 +1,8 @@
+# -*- encoding : utf-8 -*-
+class AddPolymorphicTypeToBookmarks < ActiveRecord::Migration
+  def change
+    add_column(:bookmarks, :document_type, :string)
+    
+    add_index :bookmarks, :user_id
+  end
+end
diff --git a/lib/blacklight/catalog.rb b/lib/blacklight/catalog.rb
index c336f93e25..01c6d06f32 100644
--- a/lib/blacklight/catalog.rb
+++ b/lib/blacklight/catalog.rb
@@ -38,7 +38,7 @@ def index
     
     # get single document from the solr index
     def show
-      @response, @document = get_solr_response_for_doc_id    
+      @response, @document = get_solr_response_for_doc_id   
 
       respond_to do |format|
         format.html {setup_next_and_previous_documents}
@@ -103,7 +103,7 @@ def opensearch
     
     # citation action
     def citation
-      @response, @documents = get_solr_response_for_field_values(SolrDocument.unique_key,params[:id])
+      @response, @documents = get_solr_response_for_document_ids(params[:id])
       respond_to do |format|
         format.html
         format.js { render :layout => false }
@@ -113,7 +113,7 @@ def citation
     
     # Email Action (this will render the appropriate view on GET requests and process the form and send the email on POST requests)
     def email
-      @response, @documents = get_solr_response_for_field_values(SolrDocument.unique_key,params[:id])
+      @response, @documents = get_solr_response_for_document_ids(params[:id])
       
       if request.post? and validate_email_params
         email = RecordMailer.email_record(@documents, {:to => params[:to], :message => params[:message]}, url_options)
@@ -136,7 +136,7 @@ def email
     
     # SMS action (this will render the appropriate view on GET requests and process the form and send the email on POST requests)
     def sms 
-      @response, @documents = get_solr_response_for_field_values(SolrDocument.unique_key,params[:id])
+      @response, @documents = get_solr_response_for_document_ids(params[:id])
       
       if request.post? and validate_sms_params
         to = "#{params[:to].gsub(/[^\d]/, '')}@#{sms_mappings[params[:carrier]]}"
diff --git a/lib/blacklight/configuration.rb b/lib/blacklight/configuration.rb
index 506f672194..1fac26245d 100644
--- a/lib/blacklight/configuration.rb
+++ b/lib/blacklight/configuration.rb
@@ -1,3 +1,5 @@
+require 'solr_document'
+
 module Blacklight
   ##
   # Blacklight::Configuration holds the configuration for a Blacklight::Controller, including
@@ -18,8 +20,6 @@ class Configuration < OpenStructWithHashAccess
     class << self
       def default_values
         @default_values ||= begin
-          unique_key = ((SolrDocument.unique_key if defined?(SolrDocument)) || 'id')
-
           {
           # HTTP method to use when making requests to solr; valid
           # values are :get and :post.
@@ -30,6 +30,7 @@ def default_values
           :solr_path => 'select',
           # Default values of parameters to send with every search request
           :default_solr_params => {},
+          :solr_document_model => SolrDocument,
           # The solr rqeuest handler to use when requesting only a single document 
           :document_solr_request_handler => 'document',
           # THe path to send single document requests to solr
@@ -50,7 +51,7 @@ def default_values
           # General configuration for all views
           :index => ViewConfig::Index.new(
             # solr field to use to render a document title
-            :title_field => unique_key,
+            :title_field => nil,
             # solr field to use to render format-specific partials
             :display_type_field => 'format',
             # partials to render for each document(see #render_document_partials)
@@ -160,6 +161,10 @@ def default_sort_field
 
       field
     end
+    
+    def default_title_field
+      solr_document_model.unique_key || 'id'
+    end
 
     ##
     # Add any configured facet fields to the default solr parameters hash
diff --git a/lib/blacklight/solr/document.rb b/lib/blacklight/solr/document.rb
index 545d03e104..f8324d7c36 100644
--- a/lib/blacklight/solr/document.rb
+++ b/lib/blacklight/solr/document.rb
@@ -151,7 +151,14 @@ def to_semantic_values
     return @semantic_value_hash
   end
   
+  def destroyed?
+    false
+  end
   
+  def new_record?
+    false
+  end
+
   # Certain class-level methods needed for the document-specific
   # extendability architecture
   module ClassMethods
@@ -160,6 +167,14 @@ module ClassMethods
     def unique_key
       @unique_key ||= 'id' 
     end
+    
+    def primary_key
+      'id'
+    end
+    
+    def base_class
+      self
+    end
 
     # Returns array of hashes of registered extensions. Each hash
     # has a :module_obj key and a :condition_proc key. Usually this
diff --git a/lib/blacklight/solr_helper.rb b/lib/blacklight/solr_helper.rb
index bc455dc439..1a4f136ad9 100644
--- a/lib/blacklight/solr_helper.rb
+++ b/lib/blacklight/solr_helper.rb
@@ -91,7 +91,7 @@ def find(*args)
       key = blacklight_config.http_method == :post ? :data : :params
       res = blacklight_solr.send_and_receive(path, {key=>solr_params.to_hash, method:blacklight_config.http_method})
       
-      solr_response = Blacklight::SolrResponse.new(force_to_utf8(res), solr_params)
+      solr_response = Blacklight::SolrResponse.new(force_to_utf8(res), solr_params, solr_document_model: blacklight_config.solr_document_model)
 
       Rails.logger.debug("Solr query: #{solr_params.inspect}")
       Rails.logger.debug("Solr response: #{solr_response.inspect}") if defined?(::BLACKLIGHT_VERBOSE_LOGGING) and ::BLACKLIGHT_VERBOSE_LOGGING
@@ -169,10 +169,14 @@ def solr_doc_params(id=nil)
   def get_solr_response_for_doc_id(id=nil, extra_controller_params={})
     solr_params = solr_doc_params(id).merge(extra_controller_params)
     solr_response = find(blacklight_config.document_solr_path, solr_params)
-    raise Blacklight::Exceptions::InvalidSolrID.new if solr_response.docs.empty?
+    raise Blacklight::Exceptions::InvalidSolrID.new if solr_response.documents.empty?
     [solr_response, solr_response.documents.first]
   end
   
+  def get_solr_response_for_document_ids(ids=[], extra_solr_params = {})
+    get_solr_response_for_field_values(blacklight_config.solr_document_model.unique_key, ids, extra_solr_params)
+  end
+  
   # given a field name and array of values, get the matching SOLR documents
   def get_solr_response_for_field_values(field, values, extra_solr_params = {})
     q = if Array(values).empty?
@@ -323,7 +327,7 @@ def get_opensearch_response(field=nil, extra_controller_params={})
     solr_params = solr_opensearch_params().merge(extra_controller_params)
     response = find(solr_params)
     a = [solr_params[:q]]
-    a << response.docs.map {|doc| doc[solr_params[:fl]].to_s }
+    a << response.documents.map {|doc| doc[solr_params[:fl]].to_s }
   end
   
   
@@ -372,4 +376,5 @@ def blacklight_solr_config
   def should_add_to_solr field_name, field
     field.include_in_request || (field.include_in_request.nil? && blacklight_config.add_field_configuration_to_solr_request)
   end
+
 end
diff --git a/lib/blacklight/solr_response.rb b/lib/blacklight/solr_response.rb
index 196307e42e..aebfd803fa 100644
--- a/lib/blacklight/solr_response.rb
+++ b/lib/blacklight/solr_response.rb
@@ -2,22 +2,25 @@ class Blacklight::SolrResponse < HashWithIndifferentAccess
 
   require  'blacklight/solr_response/pagination_methods'
 
-  autoload :Spelling, 'blacklight/solr_response/spelling'
-  autoload :Facets, 'blacklight/solr_response/facets'
-  autoload :MoreLikeThis, 'blacklight/solr_response/more_like_this'
+  require 'blacklight/solr_response/response'
+  require 'blacklight/solr_response/spelling'
+  require 'blacklight/solr_response/facets'
+  require 'blacklight/solr_response/more_like_this'
   autoload :GroupResponse, 'blacklight/solr_response/group_response'
   autoload :Group, 'blacklight/solr_response/group'
 
   include PaginationMethods
+  include Spelling
+  include Facets
+  include Response
+  include MoreLikeThis
 
   attr_reader :request_params
-  def initialize(data, request_params)
+  attr_accessor :solr_document_model
+  def initialize(data, request_params, options = {})
     super(data)
     @request_params = request_params
-    extend Spelling
-    extend Facets
-    extend Response
-    extend MoreLikeThis
+    self.solr_document_model = options[:solr_document_model] || SolrDocument
   end
 
   def header
@@ -44,11 +47,7 @@ def docs
   end
   
   def documents
-    docs.collect{|doc| SolrDocument.new(doc, self) }
-  end
-
-  def spelling
-    self['spelling']
+    docs.collect{|doc| solr_document_model.new(doc, self) }
   end
 
   def grouped
@@ -76,23 +75,4 @@ def grouped?
     self.has_key? "grouped"
   end
 
-  module Response
-    def response
-      self[:response] || {}
-    end
-    
-    # short cut to response['numFound']
-    def total
-      response[:numFound].to_s.to_i
-    end
-    
-    def start
-      response[:start].to_s.to_i
-    end
-
-    def empty?
-      total == 0
-    end
-    
-  end
 end
diff --git a/lib/blacklight/solr_response/response.rb b/lib/blacklight/solr_response/response.rb
new file mode 100644
index 0000000000..0fa8c90eda
--- /dev/null
+++ b/lib/blacklight/solr_response/response.rb
@@ -0,0 +1,18 @@
+module Blacklight::SolrResponse::Response
+  def response
+    self[:response] || {}
+  end
+  
+  # short cut to response['numFound']
+  def total
+    response[:numFound].to_s.to_i
+  end
+  
+  def start
+    response[:start].to_s.to_i
+  end
+
+  def empty?
+    total == 0
+  end
+end
diff --git a/lib/blacklight/user.rb b/lib/blacklight/user.rb
index 27c6b797ef..1faccacb70 100644
--- a/lib/blacklight/user.rb
+++ b/lib/blacklight/user.rb
@@ -1,5 +1,8 @@
 # -*- encoding : utf-8 -*-
 module Blacklight::User
+  
+  extend Deprecation
+  self.deprecation_horizon = 'blacklight 6.0'
 
   # This gives us an is_blacklight_user method that can be included in
   # the containing applications models. 
@@ -12,17 +15,27 @@ def self.included(base)
     end
   end
 
+  def bookmarks_for_documents documents = []
+    if documents.length > 0
+      bookmarks.where(document_type: documents.first.class.base_class, document_id: documents.map { |x| x.id})
+    else
+      []
+    end
+  end
+
   def bookmarked_document_ids
+    Deprecation.warn self, "The User#bookmarked_document_ids method is deprecated and will be removed in Blacklight 6.0"
+
     self.bookmarks.pluck(:document_id)
   end
 
-  def document_is_bookmarked?(document_id)
-    bookmarked_document_ids.include? document_id.to_s
+  def document_is_bookmarked?(document)
+    bookmarks_for_documents([document]).any?
   end
     
   # returns a Bookmark object if there is one for document_id, else
   # nil. 
-  def existing_bookmark_for(document_id)
-    self.bookmarks.where(:document_id => document_id).first
+  def existing_bookmark_for(document)
+    bookmarks_for_documents([document]).first
   end
 end
diff --git a/spec/controllers/bookmarks_controller_spec.rb b/spec/controllers/bookmarks_controller_spec.rb
index c4de5166d9..05b8085a96 100644
--- a/spec/controllers/bookmarks_controller_spec.rb
+++ b/spec/controllers/bookmarks_controller_spec.rb
@@ -14,6 +14,7 @@
     it "has a 500 status code when fails is success" do
       @controller.stub_chain(:current_or_guest_user, :existing_bookmark_for).and_return(false)
       @controller.stub_chain(:current_or_guest_user, :persisted?).and_return(true)
+      @controller.stub_chain(:current_or_guest_user, :bookmarks, :where, :exists?).and_return(false)  
       @controller.stub_chain(:current_or_guest_user, :bookmarks, :create).and_return(false)  
       xhr :put, :update, :id => 'iamabooboo', :format => :js
       expect(response.code).to eq "500"
@@ -21,6 +22,11 @@
   end
   
   describe "delete" do
+    before do
+      @controller.send(:current_or_guest_user).save
+      @controller.send(:current_or_guest_user).bookmarks.create! document_id: '2007020969', document_type: "SolrDocument"
+    end
+    
     it "has a 204 status code when delete is success" do
       xhr :delete, :destroy, :id => '2007020969', :format => :js
       expect(response).to be_success
@@ -30,7 +36,7 @@
    it "has a 500 status code when delete is not success" do
       bm = double(Bookmark)
       @controller.stub_chain(:current_or_guest_user, :existing_bookmark_for).and_return(bm)
-      @controller.stub_chain(:current_or_guest_user, :bookmarks, :delete).and_return(false)
+      @controller.stub_chain(:current_or_guest_user, :bookmarks, :where, :first).and_return(double('bookmark', delete: nil, destroyed?: false)) 
      
       xhr :delete, :destroy, :id => 'pleasekillme', :format => :js
 
@@ -38,4 +44,4 @@
     end
   end
   
-end
\ No newline at end of file
+end
diff --git a/spec/controllers/catalog_controller_spec.rb b/spec/controllers/catalog_controller_spec.rb
index c5873a2651..7ce1d56acb 100644
--- a/spec/controllers/catalog_controller_spec.rb
+++ b/spec/controllers/catalog_controller_spec.rb
@@ -304,7 +304,7 @@ def assigns_response
     describe "@document" do
       before do
         @mock_response = double()
-        @mock_response.stub(:docs => [{ :id => 'my_fake_doc' }])
+        @mock_response.stub(documents: [SolrDocument.new(id: 'my_fake_doc')])
         @mock_document = double()
         controller.stub(:find => @mock_response, 
                         :get_single_doc_via_search => @mock_document)
@@ -329,18 +329,7 @@ def export_as_mock
           "mock_export"
         end
       end
-      before do
-        @mock_response = double()
-        @mock_response.stub(:docs => [{ :id => 'my_fake_doc' }])
-        @mock_document = double()
-        controller.stub(:find => @mock_response, 
-                        :get_single_doc_via_search => @mock_document)
-
-        controller.stub(:find => @mock_response, 
-                        :get_single_doc_via_search => @mock_document)
-      end
-
-       before(:each) do
+      before(:each) do
 
         # Rails3 needs this to propertly setup a new mime type and
         # render the results. 
@@ -352,8 +341,19 @@ def export_as_mock
         SolrDocument.use_extension(FakeExtension)
       end
       
+      before do
+        @mock_response = double()
+        @mock_response.stub(:documents => [SolrDocument.new(id: 'my_fake_doc')])
+        @mock_document = double()
+        controller.stub(:find => @mock_response, 
+                        :get_single_doc_via_search => @mock_document)
+
+        controller.stub(:find => @mock_response, 
+                        :get_single_doc_via_search => @mock_document)
+      end
+
       it "should respond to an extension-registered format properly" do
-        get :show, :id => doc_id, :format => "mock" # This no longer works: :format => "mock"
+        get :show, :id => doc_id, :format => "mock"
         expect(response).to be_success
         expect(response.body).to match /mock_export/
       end
@@ -371,7 +371,7 @@ def export_as_mock
     before do
       @mock_response = double()
       @mock_document = double()
-      @mock_response.stub(:docs => [{ :id => 'my_fake_doc' }, { :id => 'my_other_doc'}])
+      @mock_response.stub(documents:  [SolrDocument.new(id: 'my_fake_doc'), SolrDocument.new(id: 'my_other_doc')])
       @mock_document = double()
       controller.stub(:find => @mock_response, 
                       :get_single_doc_via_search => @mock_document)
@@ -391,7 +391,7 @@ def export_as_mock
       before do
         @mock_response = double()
         @mock_document = double()
-        @mock_response.stub(:docs => [{ :id => 'my_fake_doc' }, { :id => 'my_other_doc'}])
+        @mock_response.stub(documents: [SolrDocument.new(id: 'my_fake_doc'), SolrDocument.new(id: 'my_other_doc')])
         @mock_document = double()
         controller.stub(:find => @mock_response, 
                         :get_single_doc_via_search => @mock_document)
@@ -469,7 +469,7 @@ def export_as_mock
   describe "errors" do
     it "should return status 404 for a record that doesn't exist" do
         @mock_response = double()
-        @mock_response.stub(:docs => [])
+        @mock_response.stub(:documents => [])
         @mock_document = double()
         controller.stub(:find => @mock_response, 
                         :get_single_doc_via_search => @mock_document)
@@ -479,7 +479,7 @@ def export_as_mock
     end
     it "should return status 404 for a record that doesn't exist even for non-html format" do
       @mock_response = double()
-      @mock_response.stub(:docs => [])
+      @mock_response.stub(:documents => [])
       @mock_document = double()
       controller.stub(:find => @mock_response,
                       :get_single_doc_via_search => @mock_document)
diff --git a/spec/helpers/blacklight_helper_spec.rb b/spec/helpers/blacklight_helper_spec.rb
index a31a95310a..1284742501 100644
--- a/spec/helpers/blacklight_helper_spec.rb
+++ b/spec/helpers/blacklight_helper_spec.rb
@@ -122,6 +122,7 @@ def mock_document_app_helper_url *args
       helper.stub(:blacklight_config).and_return(@config)
       helper.stub(:has_user_authentication_provider?).and_return(true)
       helper.stub(:current_or_guest_user).and_return(User.new)
+      helper.stub(current_bookmarks: [])
     end
     describe "render_index_doc_actions" do
       it "should render partials" do
diff --git a/spec/lib/blacklight/configuration_spec.rb b/spec/lib/blacklight/configuration_spec.rb
index 23fdf5b838..a6925ee9ba 100644
--- a/spec/lib/blacklight/configuration_spec.rb
+++ b/spec/lib/blacklight/configuration_spec.rb
@@ -39,11 +39,6 @@
       expect(@config.index).to be_a_kind_of OpenStruct
     end
 
-    it "should introspect SolrDocument for sensible defaults  for show + index" do
-      expect(@config.view_config(:show).title_field).to eq 'id'
-      expect(@config.index.title_field).to eq 'id'
-    end
-
     it "should have ordered hashes for field configuration" do
       expect(@config.facet_fields).to be_a_kind_of ActiveSupport::OrderedHash
       expect(@config.index_fields).to be_a_kind_of ActiveSupport::OrderedHash
diff --git a/spec/lib/blacklight/solr_helper_spec.rb b/spec/lib/blacklight/solr_helper_spec.rb
index d8b22862ff..70d80530e0 100644
--- a/spec/lib/blacklight/solr_helper_spec.rb
+++ b/spec/lib/blacklight/solr_helper_spec.rb
@@ -1047,7 +1047,6 @@ def logger
       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).title_field)).not_to be_nil
       expect(@document.get(blacklight_config.view_config(:show).display_type_field)).not_to be_nil
     end
   end
@@ -1125,7 +1124,6 @@ def logger
     end
 
     it 'should have non-nil values for required fields set in initializer' do
-      expect(@doc[blacklight_config.view_config(:show).title_field]).not_to be_nil
       expect(@doc[blacklight_config.view_config(:show).display_type_field]).not_to be_nil
     end
 
@@ -1244,7 +1242,7 @@ def logger
     describe "#get_solr_response_for_field_values" do
       before do
         @mock_response = double()
-        @mock_response.stub(:docs => [])
+        @mock_response.stub(documents: [])
       end
       it "should contruct a solr query based on the field and value pair" do
         subject.should_receive(:find).with(hash_including(:q => "field_name:(value)")).and_return(@mock_response)
diff --git a/spec/lib/blacklight/user_spec.rb b/spec/lib/blacklight/user_spec.rb
index a11597820c..c059391883 100644
--- a/spec/lib/blacklight/user_spec.rb
+++ b/spec/lib/blacklight/user_spec.rb
@@ -5,12 +5,46 @@
   subject { User.create! :email => 'xyz@example.com', :password => 'xyz12345' }
 
   def mock_bookmark document_id
-    Bookmark.new :document_id => document_id
+    Bookmark.new document_id: document_id, document_type: SolrDocument.to_s
+  end
+  
+  describe "#bookmarks_for_documents" do
+    before do
+      subject.bookmarks << mock_bookmark(1)
+      subject.bookmarks << mock_bookmark(2)
+      subject.bookmarks << mock_bookmark(3)
+    end
+    
+    it "should return all the bookmarks that match the given documents" do
+      bookmarks = subject.bookmarks_for_documents([SolrDocument.new(id: 1), SolrDocument.new(id: 2)])
+      expect(bookmarks).to have(2).items
+      expect(bookmarks.first.document_id).to eq "1"
+      expect(bookmarks.last.document_id).to eq "2"
+    end
+  end
+  
+  describe "#document_is_bookmarked?" do
+    before do
+      subject.bookmarks << mock_bookmark(1)
+    end
+    
+    it "should  be true if the document is bookmarked" do
+      expect(subject).to be_document_is_bookmarked(SolrDocument.new(id: 1))
+    end
+
+    it "should be false if the document is not bookmarked" do
+      expect(subject).to_not be_document_is_bookmarked(SolrDocument.new(id: 2))
+    end
   end
 
-  it "should know if it has a bookmarked document" do
-    subject.bookmarks << mock_bookmark(1)
-    expect(subject.document_is_bookmarked?(1)).to be_true
+  describe "#existing_bookmark_for" do
+    before do
+      subject.bookmarks << mock_bookmark(1)
+    end
+    
+    it "should return the bookmark for that document id" do
+      expect(subject.existing_bookmark_for(SolrDocument.new(id: 1))).to eq subject.bookmarks.first
+    end
   end
 
 end    
diff --git a/spec/models/bookmark_spec.rb b/spec/models/bookmark_spec.rb
index 400e915645..0949bf0f6f 100644
--- a/spec/models/bookmark_spec.rb
+++ b/spec/models/bookmark_spec.rb
@@ -1,20 +1,15 @@
-# -*- encoding : utf-8 -*-
-require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+require 'spec_helper'
 
 describe Bookmark do
-  before(:each) do
-    @bookmark = Bookmark.new
+  subject do
+    b = Bookmark.new 
+    b.user_id = 1
+    b.document = SolrDocument.new(id: 'u001')
+    b
   end
   
   it "should be valid" do
-    @bookmark.id = 1
-    @bookmark.user_id = 1
-    @bookmark.document_id = 'u001'
-    expect(@bookmark).to be_valid
-  end
-   
-  it "should require user_id" do
-    expect(@bookmark).to have(1).error_on(:user_id)
+    expect(subject).to be_valid
   end
 
   it "should belong to user" do
@@ -22,10 +17,20 @@
   end
 
   it "should be valid after saving" do
-    @bookmark.id = 1
-    @bookmark.user_id = 1
-    @bookmark.document_id = 'u001'
-    @bookmark.save
-    expect(@bookmark).to be_valid
+    subject.save
+    expect(subject).to be_valid
+  end
+  
+  describe "#document_type" do
+    it "should be the class of the solr document" do
+      expect(subject.document_type).to eq SolrDocument
+    end
+  end
+  
+  describe "#document" do
+    it "should be a SolrDocument with just an id field" do
+      expect(subject.document).to be_a_kind_of SolrDocument
+      expect(subject.document.id).to eq 'u001'
+    end
   end
 end