[PATCH conductor] BZ 811852 Exception when tried to create a new image from wrong URL(rev.)

Jan Provaznik jprovazn at redhat.com
Thu Aug 30 14:15:42 UTC 2012


On 08/23/2012 05:54 PM, jzigmund at redhat.com wrote:
> From: Jozef Zigmund <jzigmund at redhat.com>
>
> Moved the method 'import_xml_from_url' from DeployablesController
> to ApplicationController, with fix for "undefined method `request_uri'".
> ApplicationController's one is applied for ImagesController#edit_xml also.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=811852
> ---
>   src/app/controllers/application_controller.rb | 20 ++++++++++++++++++++
>   src/app/controllers/deployables_controller.rb | 18 ------------------
>   src/app/controllers/images_controller.rb      | 24 ++++++++++++++----------
>   src/app/views/images/new.html.haml            |  7 +++++--
>   src/config/locales/en.yml                     |  8 +++++---
>   5 files changed, 44 insertions(+), 33 deletions(-)
>
> diff --git a/src/app/controllers/application_controller.rb b/src/app/controllers/application_controller.rb
> index 2ce15ab..3fd1884 100644
> --- a/src/app/controllers/application_controller.rb
> +++ b/src/app/controllers/application_controller.rb
> @@ -146,6 +146,26 @@ class ApplicationController < ActionController::Base
>       end
>     end
>
> +  def import_xml_from_url(url)
> +    if url.blank?
> +      flash[:error] = t('application_controller.flash.error.no_url_provided')
> +    elsif not url =~ URI::regexp
> +      flash[:error] = t('application_controller.flash.error.not_valid_url', :url => url)
> +    else
> +      begin
> +        response = RestClient.get(url, :accept => :xml)
> +        if response.code == 200
> +          return response
> +        else
> +          flash[:error] = t('application_controller.flash.error.download_failed')
> +        end
> +      rescue RestClient::Exception, SocketError, URI::InvalidURIError, Errno::ECONNREFUSED, Errno::ETIMEDOUT
> +        flash[:error] = t('application_controller.flash.error.not_valid_or_reachable', :url => url)
> +      end
> +    end
> +    return nil
> +  end
> +
>     private
>     def json_error_hash(msg, status)
>       json = {}
> diff --git a/src/app/controllers/deployables_controller.rb b/src/app/controllers/deployables_controller.rb
> index e07a603..74b5fc3 100644
> --- a/src/app/controllers/deployables_controller.rb
> +++ b/src/app/controllers/deployables_controller.rb
> @@ -302,22 +302,4 @@ class DeployablesController < ApplicationController
>                                         Privilege::CREATE, Deployable).
>         where('pool_family_id' => @pool_family.id)
>     end
> -
> -  def import_xml_from_url(url)
> -    begin
> -      response = RestClient.get(url, :accept => :xml)
> -      if response.code == 200
> -        response
> -      end
> -    rescue RestClient::Exception, SocketError, URI::InvalidURIError, Errno::ECONNREFUSED, Errno::ETIMEDOUT
> -      if url.present?
> -        flash[:error] = t('catalog_entries.flash.warning.not_valid_or_reachable', :url => url)
> -      else
> -        flash[:error] = t('catalog_entries.flash.warning.no_url_provided')
> -      end
> -      nil
> -    end
> -  end
> -
> -

Not caused by your patch but it's a good chance to fix it:
no matter what I will in in URL for deployable, error message is always 
"XML is blank" because if import fails, create action continues in 
saving deployable with nil xml.

>   end
> diff --git a/src/app/controllers/images_controller.rb b/src/app/controllers/images_controller.rb
> index d9ab289..9cfdd2c 100644
> --- a/src/app/controllers/images_controller.rb
> +++ b/src/app/controllers/images_controller.rb
> @@ -183,7 +183,11 @@ class ImagesController < ApplicationController
>           t("images.flash.error.no_provider_accounts_for_import") :
>           t("images.flash.error.no_provider_accounts")
>       end
> -    render 'import' == params[:tab] ? :import : :new
> +    if 'import' == params[:tab]
> +      render :import
> +    else
> +      render :new, :locals => {:active => "file"}
> +    end
>     end
>
>     def import
> @@ -216,21 +220,21 @@ class ImagesController < ApplicationController
>       @environment = PoolFamily.find(params[:environment])
>       check_permissions
>       @name = params[:name]
> -
>       if params.has_key? :image_url
> -      url = params[:image_url]
> -      begin
> -        xml_source = RestClient.get(url, :accept => :xml)
> -      rescue RestClient::Exception, SocketError, URI::InvalidURIError, Errno::ECONNREFUSED, Errno::ETIMEDOUT
> -        flash.now[:error] = t('images.flash.error.invalid_url')
> -        render :new and return
> +      import_xml_from_url(params[:image_url])
> +      if flash.has_key? :error

I get following error:
undefined method `has_key?' for 
#<ActionDispatch::Flash::FlashHash:0x00000007a54770>

You might check the result of import_xml_from_url call instead.

> +        @accounts = @environment.provider_accounts.enabled.
> +                  list_for_user(current_session, current_user, Privilege::USE)
> +        render :new, :locals => {:active => "url"} and return
>         end
>       else
>         file = params[:image_file]
>         xml_source = file && file.read
>         unless xml_source
> -        flash.now[:error] = t('images.flash.error.no_file')
> -        render :new and return
> +        @accounts = @environment.provider_accounts.enabled.
> +            list_for_user(current_session, current_user, Privilege::USE)
> +        flash[:error] = t('images.flash.error.no_file')
> +        render :new, :locals => {:active => "file"} and return
>         end
>       end
>
> diff --git a/src/app/views/images/new.html.haml b/src/app/views/images/new.html.haml
> index 79f8b6c..2ea9cba 100644
> --- a/src/app/views/images/new.html.haml
> +++ b/src/app/views/images/new.html.haml
> @@ -1,4 +1,5 @@
>   = render :partial => 'layouts/admin_nav'
> +
>   %header.page-header
>     .obj_actions
>       .return_to
> @@ -15,9 +16,11 @@
>         %nav#image-upload-tabs.faceted
>           %ul.tabs
>             %li
> -            %a{ :href => '#image-file-form', :class => 'active'}= t('.upload')
> +            %a{ :href => '#image-file-form', :class => "#{'active' if active == "file"}"}
> +              = t('.upload')
>             %li
> -            %a{ :href => '#image-url-form'}= t('.from_url')
> +            %a{ :href => '#image-url-form', :class => "#{'active' if active == "url"}"}
> +              = t('.from_url')
>
>         %section#image-file-form
>           = form_tag(edit_xml_images_path, { :multipart => true, :class => 'generic horizontal' }) do
> diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
> index 8d2fb7d..a5213a6 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -838,8 +838,6 @@ en:
>         catalog_entries_list: Deployables List
>       flash:
>         warning:
> -        not_valid_or_reachable: "Deployable XML file is either invalid or no longer reachable at %{url}"
> -        no_url_provided: "No URL provided for the Deployable XML file."
>           not_valid: "The Deployable XML file contains syntax errors"
>           not_valid_duplicate_assembly_names: "Deployable XML must contain unique Assembly names"
>           not_valid_cyclic_reference: "Contains cyclic reference between following Assemblies or services: %{reference}"
> @@ -963,7 +961,6 @@ en:
>           delete_failed: Unable to Delete Image
>           not_found: Image not found
>         error:
> -        invalid_url: Could not load the provided URL
>           no_file: You must specify the Image Template XML file
>           no_template: "The Image doesn't have an Image Template because it was originally imported, rather than built in Conductor."
>           no_provider_accounts: "Images cannot be built. There are no enabled Provider Accounts associated with this Environment."
> @@ -1478,6 +1475,11 @@ en:
>           record_not_exist: "The record you tried to access does not exist. It may have been deleted"
>           must_be_logged: You must be logged in to access this page
>           must_not_be_logged: You must be logged out to access this page
> +      error:
> +        no_url_provided: No URL is provided for XML import
> +        not_valid_url: "Provided URL is not valid %{url}"
> +        download_failed: Download of XML file failed
> +        not_valid_or_reachable: "XML file is either invalid or no longer reachable at %{url}"
>       admin_tabs:
>         catalogs: Catalogs
>         realms: Realms
>

Hi, this looks good, though found 2 minor problems (inline).

Jan



More information about the aeolus-devel mailing list