default invalid_solr_id_error just a default 404 much like ActiveReco…

…rd::RecordNotFound

For #756

lib/blacklight/catalog.rb

@@ -245,12 +245,31 @@ def validate_email_params
       flash[:error].blank?
     end
 
-    # when a request for /catalog/BAD_SOLR_ID is made, this method is executed...
-    def invalid_solr_id_error
-      flash[:notice] = I18n.t('blacklight.search.errors.invalid_solr_id')
-      params.delete(:id)
-      index
-      render "index", :status => 404
+    # when a request for /catalog/BAD_SOLR_ID is made, this method is executed.
+    # Just returns a 404 response, but you can override locally in your own
+    # CatalogController to do something else -- older BL displayed a Catalog#inde
+    # page with a flash message and a 404 status.
+    def invalid_solr_id_error(exception)
+      error_info = {
+        "status" => "404",
+        "error"  => "#{exception.class}: #{exception.message}"
+      }
+
+      respond_to do |format|
+        format.xml  { render :xml  => error_info, :status => 404 }
+        format.json { render :json => error_info, :status => 404 }
+
+        # default to HTML response, even for other non-HTML formats we don't
+        # neccesarily know about, seems to be consistent with what Rails4 does
+        # by default with uncaught ActiveRecord::RecordNotFound in production
+        format.any do
+          # use standard, possibly locally overridden, 404.html file. Even for
+          # possibly non-html formats, this is consistent with what Rails does
+          # on raising an ActiveRecord::RecordNotFound. Rails.root IS needed
+          # for it to work under testing, without worrying about CWD.
+          render :file => "#{Rails.root}/public/404.html", :status => 404, :layout => false, :content_type => 'text/html'
+        end
+      end
     end
 
     def start_new_search_session?

spec/controllers/catalog_controller_spec.rb

@@ -464,10 +464,21 @@ def export_as_mock
         controller.stub(:find => @mock_response, 
                         :get_single_doc_via_search => @mock_document)
       get :show, :id=>"987654321"
-      expect(request.flash[:notice]).to eq "Sorry, you have requested a record that doesn't exist."
-      expect(response).to render_template('index')
       expect(response.status).to eq 404
+      expect(response.content_type).to eq Mime::HTML
     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_document = double()
+      controller.stub(:find => @mock_response,
+                      :get_single_doc_via_search => @mock_document)
+
+      get :show, :id=>"987654321", :format => "xml"
+      expect(response.status).to eq 404
+      expect(response.content_type).to eq Mime::XML
+    end
+
     it "should redirect the user to the root url for a bad search" do
       req = {}
       res = {}
@@ -479,7 +490,6 @@ def export_as_mock
       expect(response.redirect_url).to eq root_url
       expect(request.flash[:notice]).to eq "Sorry, I don't understand your search."
       expect(response).to_not be_success
-      expect(response.status).to eq 302
     end
     it "should return status 500 if the catalog path is raising an exception" do
 

spec/features/record_view_spec.rb

@@ -35,6 +35,6 @@
   it "should not display 404" do
     visit catalog_path('this_id_does_not_exist')
     page.driver.status_code.should == 404
-    expect(page).to have_content "Sorry, you have requested a record that doesn't exist." 
+    expect(page).to have_content "The page you were looking for doesn't exist." 
   end
 end