commit 46338cd73c542ecc3aa045b45e831434792c710e
Author: Daniel Lobato <elobatocs@gmail.com>
Date:   Tue Dec 17 18:31:31 2013 +0100

    fixes #3515 - API handles not found objects with 404

diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb
index 83afd0fa3..12b345669 100644
--- a/app/controllers/api/base_controller.rb
+++ b/app/controllers/api/base_controller.rb
@@ -15,15 +15,21 @@ module Api
     end
 
     rescue_from StandardError, :with => lambda { |error|
-      Rails.logger.error "#{error.message} (#{error.class})\n#{error.backtrace.join("\n")}"
+      logger.error "#{error.message} (#{error.class})\n#{error.backtrace.join("\n")}"
       render_error 'standard_error', :status => 500, :locals => { :exception => error }
     }
 
     rescue_from Apipie::ParamError, :with => lambda { |error|
-      Rails.logger.info "#{error.message} (#{error.class})"
+      logger.info "#{error.message} (#{error.class})"
       render_error 'param_error', :status => :bad_request, :locals => { :exception => error }
     }
 
+    rescue_from ActiveRecord::RecordNotFound, :with => lambda { |error|
+      logger.info "#{error.message} (#{error.class})"
+      not_found(:message => "#{error.message}", :class => "#{error.class}")
+    }
+
+
     def get_resource
       instance_variable_get :"@#{resource_name}" or raise 'no resource loaded'
     end
@@ -46,8 +52,19 @@ module Api
 
     protected
 
-    def not_found
-      render_error 'not_found', :status => :not_found and return false
+    def not_found(options = nil)
+      not_found_message = {}
+
+      case options
+      when String
+        not_found_message.merge! :message => options
+      when Hash
+        not_found_message.merge! options
+      else
+        render_error 'not_found', :status => :not_found and return false
+      end
+
+      render :json => not_found_message, :status => :not_found and return false
     end
 
     def process_resource_error(options = { })
@@ -215,8 +232,7 @@ module Api
     def not_found_if_nested_id_exists
       allowed_nested_id.each do |obj_id|
         if params[obj_id].present?
-          msg = "#{obj_id.humanize} not found by id '#{params[obj_id]}'"
-          render :json => {:message => msg}, :status => :not_found and return false
+          not_found "#{obj_id.humanize} not found by id '#{params[obj_id]}'"
         end
       end
     end
diff --git a/app/controllers/api/v2/host_classes_controller.rb b/app/controllers/api/v2/host_classes_controller.rb
index 5c162d5d2..aca1081db 100644
--- a/app/controllers/api/v2/host_classes_controller.rb
+++ b/app/controllers/api/v2/host_classes_controller.rb
@@ -41,10 +41,10 @@ module Api
         else
           @host ||= Host::Managed.find_by_name(params[:host_id])
           return @host_id = @host.id if @host
-          render_error 'not_found', :status => :not_found and return false
+          not_found
         end
       end
 
     end
   end
-end
\ No newline at end of file
+end
diff --git a/app/controllers/api/v2/override_values_controller.rb b/app/controllers/api/v2/override_values_controller.rb
index 6620de17d..d66a83298 100644
--- a/app/controllers/api/v2/override_values_controller.rb
+++ b/app/controllers/api/v2/override_values_controller.rb
@@ -77,8 +77,7 @@ module Api
 
       def return_if_override_mismatch
         if (@override_values && @override_value && !@override_values.find_by_id(@override_value.id)) || (@override_values && !@override_value) || !@override_values
-          msg = "Override value not found by id '#{params[:id]}'"
-          render :json => {:message => msg}, :status => :not_found and return false
+          not_found "Override value not found by id '#{params[:id]}'"
         end
       end
 
@@ -89,4 +88,4 @@ module Api
 
     end
   end
-end
\ No newline at end of file
+end
diff --git a/app/controllers/api/v2/parameters_controller.rb b/app/controllers/api/v2/parameters_controller.rb
index bdc395631..136419a8d 100644
--- a/app/controllers/api/v2/parameters_controller.rb
+++ b/app/controllers/api/v2/parameters_controller.rb
@@ -149,7 +149,7 @@ module Api
         @parameter   = @parameters.find_by_id(params[:id].to_i) if params[:id].to_i > 0
         @parameter ||= @parameters.find_by_name(params[:id])
         return @parameter if @parameter
-        render_error 'not_found', :status => :not_found and return false
+        not_found
       end
 
     end
diff --git a/app/controllers/api/v2/template_combinations_controller.rb b/app/controllers/api/v2/template_combinations_controller.rb
index 652a99abb..2672543fe 100644
--- a/app/controllers/api/v2/template_combinations_controller.rb
+++ b/app/controllers/api/v2/template_combinations_controller.rb
@@ -36,9 +36,7 @@ module Api
 
       def find_parent_config_template
         @config_template = ConfigTemplate.find(params[:config_template_id])
-        unless @config_template
-          render_error 'not_found', :status => :not_found and return false
-        end
+        not_found unless @config_template
         @config_template
       end
     end
diff --git a/app/controllers/concerns/api/import_puppetclasses_common_controller.rb b/app/controllers/concerns/api/import_puppetclasses_common_controller.rb
index 5653bb89a..c84693cd3 100644
--- a/app/controllers/concerns/api/import_puppetclasses_common_controller.rb
+++ b/app/controllers/concerns/api/import_puppetclasses_common_controller.rb
@@ -82,8 +82,7 @@ module Api::ImportPuppetclassesCommonController
     @smart_proxy   = SmartProxy.find_by_id(id.to_i) if id.to_i > 0
     @smart_proxy ||= SmartProxy.find_by_name(id)
     unless @smart_proxy && SmartProxy.puppet_proxies.pluck("smart_proxies.id").include?(@smart_proxy.id)
-      msg = 'We did not find a foreman proxy that can provide the information, ensure that this proxy has the puppet feature turned on.'
-      render :json => {:message => msg}, :status => :not_found and return false
+      not_found 'We did not find a foreman proxy that can provide the information, ensure that this proxy has the puppet feature turned on.'
     end
     @smart_proxy
   end
diff --git a/app/controllers/concerns/api/v2/lookup_keys_common_controller.rb b/app/controllers/concerns/api/v2/lookup_keys_common_controller.rb
index c74cb43b1..159a45a1c 100644
--- a/app/controllers/concerns/api/v2/lookup_keys_common_controller.rb
+++ b/app/controllers/concerns/api/v2/lookup_keys_common_controller.rb
@@ -137,8 +137,7 @@ module Api::V2::LookupKeysCommonController
              else
                params.keys.include?('smart_class_parameter_id') ? params['smart_variable_id'] : params['id']
              end
-        msg = "#{obj} not found by id '#{id}'"
-        render :json => {:message => msg}, :status => :not_found and return false
+        not_found "#{obj} not found by id '#{id}'"
       end
     end
 
diff --git a/test/functional/api/v1/reports_controller_test.rb b/test/functional/api/v1/reports_controller_test.rb
index 2ccdde24e..71c0b1570 100644
--- a/test/functional/api/v1/reports_controller_test.rb
+++ b/test/functional/api/v1/reports_controller_test.rb
@@ -60,7 +60,7 @@ class Api::V1::ReportsControllerTest < ActionController::TestCase
 
   test "should give error if no last report for given host" do
     get :last, {:host_id => hosts(:two).to_param }
-    assert_response 500
+    assert_response :not_found
   end
 
 end
diff --git a/test/functional/api/v1/subnets_controller_test.rb b/test/functional/api/v1/subnets_controller_test.rb
index 812c3c887..6ab31ab62 100644
--- a/test/functional/api/v1/subnets_controller_test.rb
+++ b/test/functional/api/v1/subnets_controller_test.rb
@@ -27,6 +27,11 @@ class Api::V1::SubnetsControllerTest < ActionController::TestCase
     assert_response :success
   end
 
+  test "does not create subnet with non-existent domain" do
+    post :create, { :subnet => valid_attrs.merge( :domain_ids => [1, 2] ) }
+    assert_response :not_found
+  end
+
   test "should update subnet" do
     put :update, { :id => subnets(:one).to_param, :subnet => { } }
     assert_response :success
diff --git a/test/functional/api/v2/reports_controller_test.rb b/test/functional/api/v2/reports_controller_test.rb
index 91d57a27d..5971885e0 100644
--- a/test/functional/api/v2/reports_controller_test.rb
+++ b/test/functional/api/v2/reports_controller_test.rb
@@ -175,7 +175,7 @@ class Api::V2::ReportsControllerTest < ActionController::TestCase
 
   test "should give error if no last report for given host" do
     get :last, {:host_id => hosts(:two).to_param }
-    assert_response 500
+    assert_response :not_found
   end
 
 end
diff --git a/test/functional/api/v2/subnets_controller_test.rb b/test/functional/api/v2/subnets_controller_test.rb
index dd1022f46..c4be463d3 100644
--- a/test/functional/api/v2/subnets_controller_test.rb
+++ b/test/functional/api/v2/subnets_controller_test.rb
@@ -4,13 +4,12 @@ class Api::V2::SubnetsControllerTest < ActionController::TestCase
 
   valid_attrs = { :name => 'QA2', :network => '10.35.2.27', :mask => '255.255.255.0' }
 
-  def test_index
+  test "index content is a JSON array" do
     get :index
     subnets = ActiveSupport::JSON.decode(@response.body)
     assert subnets['results'].is_a?(Array)
     assert_response :success
     assert !subnets.empty?
-
   end
 
   test "should show individual record" do
@@ -27,6 +26,11 @@ class Api::V2::SubnetsControllerTest < ActionController::TestCase
     assert_response :success
   end
 
+  test "does not create subnet with non-existent domain" do
+    post :create, { :subnet => valid_attrs.merge( :domain_ids => [1, 2] ) }
+    assert_response :not_found
+  end
+
   test "should update subnet" do
     put :update, { :id => subnets(:one).to_param, :subnet => { } }
     assert_response :success
@@ -46,7 +50,7 @@ class Api::V2::SubnetsControllerTest < ActionController::TestCase
     assert_response :unprocessable_entity
   end
 
-  def test_destroy_json
+  test "delete destroys subnet not in use" do
     subnet = Subnet.first
     subnet.hosts.clear
     subnet.interfaces.clear
