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

Tzu-Mainn Chen tzumainn at redhat.com
Thu Sep 6 15:00:43 UTC 2012


One minor issue; after an incorrect url is entered into the deployables form, the url disappears from the text entry (this doesn't happen with the images form).  Feel free to push after that's fixed.

Mainn

----- Original Message -----
> From: jzigmund at redhat.com
> To: aeolus-devel at lists.fedorahosted.org
> Cc: "Jozef Zigmund" <jzigmund at redhat.com>
> Sent: Thursday, September 6, 2012 10:29:00 AM
> Subject: [PATCH conductor] BZ 811852 Exception when tried to create a new	image from wrong URL(rev.)
> 
> 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 | 21
>  ++-------------------
>  src/app/controllers/images_controller.rb      | 22
>  ++++++++++++++++------
>  src/app/views/images/new.html.haml            |  7 +++++--
>  src/config/locales/en.yml                     |  7 +++++--
>  5 files changed, 48 insertions(+), 29 deletions(-)
> 
> diff --git a/src/app/controllers/application_controller.rb
> b/src/app/controllers/application_controller.rb
> index b0d8fd5..f385e05 100644
> --- a/src/app/controllers/application_controller.rb
> +++ b/src/app/controllers/application_controller.rb
> @@ -151,6 +151,26 @@ class ApplicationController <
> ActionController::Base
>      end
>    end
>  
> +  def import_xml_from_url(url)
> +    if url.blank?
> +      error =
> t('application_controller.flash.error.no_url_provided')
> +    elsif not url =~ URI::regexp
> +      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, nil
> +        else
> +          error =
> t('application_controller.flash.error.download_failed')
> +        end
> +      rescue RestClient::Exception, SocketError,
> URI::InvalidURIError, Errno::ECONNREFUSED, Errno::ETIMEDOUT
> +        error =
> t('application_controller.flash.error.not_valid_or_reachable', :url
> => url)
> +      end
> +    end
> +    return nil, error
> +  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 947f578..c12feda 100644
> --- a/src/app/controllers/deployables_controller.rb
> +++ b/src/app/controllers/deployables_controller.rb
> @@ -137,7 +137,7 @@ class DeployablesController <
> ApplicationController
>      end
>  
>      if params.has_key? :url
> -      xml = import_xml_from_url(params[:url])
> +      xml, error = import_xml_from_url(params[:url])
>        unless xml.nil?
>          #store xml_filename for url (i.e. url ends to: foo ||
>          foo.xml)
>          @deployable.xml_filename =
>           File.basename(URI.parse(params[:url]).path)
> @@ -175,6 +175,7 @@ class DeployablesController <
> ApplicationController
>        end
>  
>      rescue => e
> +      @deployable.errors.add(:url, error) if error
>        if @deployable.errors.empty?
>          logger.error e.message
>          logger.error e.backtrace.join("\n ")
> @@ -302,22 +303,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
> -
> -
>  end
> diff --git a/src/app/controllers/images_controller.rb
> b/src/app/controllers/images_controller.rb
> index 13fe2cf..0cbd2d6 100644
> --- a/src/app/controllers/images_controller.rb
> +++ b/src/app/controllers/images_controller.rb
> @@ -191,7 +191,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
> @@ -231,17 +235,23 @@ class ImagesController < ApplicationController
>      end
>  
>      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
> -        errors << t('images.flash.error.invalid_url')
> +      xml_source, error = import_xml_from_url(params[:image_url])
> +      if error
> +        errors << error
> +        @accounts = @environment.provider_accounts.enabled.
> +                  list_for_user(current_session, current_user,
> Privilege::USE)
> +        flash[:error] = errors
> +        render :new, :locals => {:active => "url"} and return
>        end
>      else
>        file = params[:image_file]
>        xml_source = file && file.read
>        unless xml_source
> +        @accounts = @environment.provider_accounts.enabled.
> +            list_for_user(current_session, current_user,
> Privilege::USE)
>          errors << t('images.flash.error.no_file')
> +        flash[:error] = errors
> +        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 8c966c0..3b10472 100644
> --- a/src/config/locales/en.yml
> +++ b/src/config/locales/en.yml
> @@ -866,8 +866,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: "file contains syntax errors"
>          not_valid_duplicate_assembly_names: "must contain unique
>          Assembly names"
>          not_valid_cyclic_reference: "Contains cyclic reference
>          between following Assemblies or services: %{reference}"
> @@ -1510,6 +1508,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
> --
> 1.7.11.4
> 
> 



More information about the aeolus-devel mailing list