[PATCH aeolus-conductor] BZ786452 Correct inconsistency in returning errors at API controllers

Tomas Sedovic tsedovic at redhat.com
Wed Feb 1 14:55:18 UTC 2012


On 02/01/2012 02:38 PM, Petr Blaho wrote:
> Changes returning error from 500 to 404 of API::ImagesController#destroy
> when looked up Image does not exist
>
> This mimics behaviour and code in API::ProviderImagesController#destroy
> and API::BuildsController#destroy.
>
> The same correction for API::TargetImagesController#destroy method.
> ---
>   src/app/controllers/api/images_controller.rb       |    2 ++
>   .../controllers/api/target_images_controller.rb    |    2 ++
>   src/spec/controllers/api/images_controller_spec.rb |   19 ++++++++++++++-----
>   .../api/target_images_controller_spec.rb           |   19 ++++++++++++++-----
>   4 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/src/app/controllers/api/images_controller.rb b/src/app/controllers/api/images_controller.rb
> index 6261fab..18870f2 100644
> --- a/src/app/controllers/api/images_controller.rb
> +++ b/src/app/controllers/api/images_controller.rb
> @@ -93,6 +93,8 @@ module Api
>           else
>             raise(Aeolus::Conductor::API::ImageNotFound.new(404, t("api.error_messages.image_not_found", :image =>  params[:id])))
>           end
> +      rescue Aeolus::Conductor::API::ImageNotFound =>  e
> +        raise(e)
>         rescue =>  e
>           raise(Aeolus::Conductor::API::ImageDeleteFailure.new(500, e.message))
>         end
> diff --git a/src/app/controllers/api/target_images_controller.rb b/src/app/controllers/api/target_images_controller.rb
> index 4ba64c5..03f7521 100644
> --- a/src/app/controllers/api/target_images_controller.rb
> +++ b/src/app/controllers/api/target_images_controller.rb
> @@ -65,6 +65,8 @@ module Api
>           else
>             raise(Aeolus::Conductor::API::TargetImageNotFound.new(404, t("api.error_messages.target_image_not_found", :targetimage =>  params[:id])))
>           end
> +      rescue Aeolus::Conductor::API::TargetImageNotFound =>  e
> +        raise(e)
>         rescue =>  e
>           raise(Aeolus::Conductor::API::TargetImageDeleteFailure.new(500, e.message))
>         end
> diff --git a/src/spec/controllers/api/images_controller_spec.rb b/src/spec/controllers/api/images_controller_spec.rb
> index 67ddf15..ccce985 100644
> --- a/src/spec/controllers/api/images_controller_spec.rb
> +++ b/src/spec/controllers/api/images_controller_spec.rb
> @@ -336,11 +336,20 @@ describe Api::ImagesController do
>               get :destroy, :id =>  '3'
>             end
>
> -          it { response.headers['Content-Type'].should include("application/xml") }
> -          it "should have error" do
> -            resp = Hash.from_xml(response.body)
> -            resp['error']['code'].should == "ImageDeleteFailure"
> -            resp['error']['message'].should == "Could not find Image 3"
> +          context "response code" do
> +            subject { response.code.to_i }
> +            it { should == 404 }
> +          end
> +          context "response's Content-Type header" do
> +            subject { response.headers['Content-Type'] }
> +            it { should include "application/xml" }
> +          end
> +          context "response's body" do
> +            subject { Hash.from_xml response.body }
> +            it "should have error" do
> +              subject['error']['code'].should == "ImageNotFound"
> +              subject['error']['message'].should == "Could not find Image 3"
> +            end
>             end
>           end
>
> diff --git a/src/spec/controllers/api/target_images_controller_spec.rb b/src/spec/controllers/api/target_images_controller_spec.rb
> index e7b03ca..28b9d23 100644
> --- a/src/spec/controllers/api/target_images_controller_spec.rb
> +++ b/src/spec/controllers/api/target_images_controller_spec.rb
> @@ -212,11 +212,20 @@ describe Api::TargetImagesController do
>               get :destroy, :id =>  '99'
>             end
>
> -          it { response.headers['Content-Type'].should include("application/xml") }
> -          it "should have error" do
> -            resp = Hash.from_xml(response.body)
> -            resp['error']['code'].should == "TargetImageDeleteFailure"
> -            resp['error']['message'].should == "Could not find TargetImage 99"
> +          context "response code" do
> +            subject { response.code.to_i }
> +            it { should == 404 }
> +          end
> +          context "response's Content-Type header" do
> +            subject { response.headers['Content-Type'] }
> +            it { should include "application/xml" }
> +          end
> +          context "response's body" do
> +            subject { Hash.from_xml response.body }
> +            it "should have error" do
> +              subject['error']['code'].should == "TargetImageNotFound"
> +              subject['error']['message'].should == "Could not find TargetImage 99"
> +            end
>             end
>           end
>         end

ACK



More information about the aeolus-devel mailing list