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

jzigmund at redhat.com jzigmund at redhat.com
Thu Sep 6 14:29:00 UTC 2012


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