[PATCH conductor] Redmine #3210: Conductor should validate the provider, not just deltacloud-core instance.

Jason Guiditta jguiditt at redhat.com
Wed May 2 20:45:33 UTC 2012


https://www.aeolusproject.org/redmine/issues/3210

This patch adds a new method to the provider validation to verify
that the named provider is valid for a given deltacloud instance. In
addition, it moves this check out of the controller and down to the
model where it belongs on create action. It also adds errors to the
activerecord/en.yml, running tests on that value rather than a string.
---
 src/app/controllers/providers_controller.rb     |   17 ++++--------
 src/app/models/provider.rb                      |   11 +++++++-
 src/config/locales/activerecord/en.yml          |    4 +++
 src/features/provider.feature                   |   22 +++------------
 src/features/step_definitions/provider_steps.rb |   31 ++++++++++++++++++++--
 src/features/support/custom.rb                  |    4 +++
 src/spec/factories/provider.rb                  |    1 -
 src/spec/models/provider_spec.rb                |   18 ++++++++-----
 src/spec/spec_helper.rb                         |    4 +++
 9 files changed, 72 insertions(+), 40 deletions(-)

diff --git a/src/app/controllers/providers_controller.rb b/src/app/controllers/providers_controller.rb
index 1335292..41cf5d5 100644
--- a/src/app/controllers/providers_controller.rb
+++ b/src/app/controllers/providers_controller.rb
@@ -104,18 +104,13 @@ class ProvidersController < ApplicationController
       params[:provider][:provider_type_id] = provider_type.id
     end
     @provider = Provider.new(params[:provider])
-    if !@provider.connect
-      flash[:warning] = t"providers.flash.warning.connect_failed"
-      render :action => "new"
+    if @provider.save
+      @provider.assign_owner_roles(current_user)
+      flash[:notice] = t"providers.flash.notice.added"
+      redirect_to edit_provider_path(@provider)
     else
-      if @provider.save
-        @provider.assign_owner_roles(current_user)
-        flash[:notice] = t"providers.flash.notice.added"
-        redirect_to edit_provider_path(@provider)
-      else
-        flash[:warning] = t"providers.flash.error.not_added"
-        render :action => "new"
-      end
+      flash[:warning] = t"providers.flash.error.not_added"
+      render :action => "new"
     end
   end
 
diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
index 2033a04..fc3040e 100644
--- a/src/app/models/provider.rb
+++ b/src/app/models/provider.rb
@@ -251,7 +251,8 @@ class Provider < ActiveRecord::Base
 
   def validate_provider
     if !nil_or_empty(url)
-      errors.add("url", "must be a valid provider url") unless valid_framework?
+      errors.add("url", I18n.t("activerecord.errors.provider.url")) unless valid_framework?
+      errors.add("deltacloud_provider", I18n.t("activerecord.errors.provider.deltacloud_provider")) unless valid_provider?
     end
   end
 
@@ -261,4 +262,12 @@ class Provider < ActiveRecord::Base
     connect.nil? ? false : true
   end
 
+  def valid_provider?
+    if !nil_or_empty(deltacloud_provider)
+      client = connect
+      return false if client.nil?
+      return false unless client.valid_provider? deltacloud_provider
+    end
+    true
+  end
 end
diff --git a/src/config/locales/activerecord/en.yml b/src/config/locales/activerecord/en.yml
index 3ac086f..86b250f 100644
--- a/src/config/locales/activerecord/en.yml
+++ b/src/config/locales/activerecord/en.yml
@@ -343,3 +343,7 @@ en:
         state: "State"
         created_at: "Created At"
         updated_at: "Updated At"
+    errors:
+      provider:
+        url: "Must be a valid provider uri"
+        deltacloud_provider: "Must be a valid provider identifier"
diff --git a/src/features/provider.feature b/src/features/provider.feature
index b1df22e..2ba7081 100644
--- a/src/features/provider.feature
+++ b/src/features/provider.feature
@@ -46,27 +46,15 @@ Feature: Manage Providers
     And I should see "Provider URL"
 
   Scenario: Create a new Provider
-    Given I am on the providers page
-    And there is not a provider named "testprovider"
-    When I follow "create_new_provider"
-    Then I should be on the new provider page
-    When I fill in "provider[name]" with "testprovider"
-    And I fill in "provider[url]" with "http://localhost:3002/api"
-    And I select "Amazon EC2" from "provider_provider_type_id"
-    And I press "save"
+    Given I am on the new provider page
+    And I attempt to add a valid provider
     Then I should be on the testprovider's edit provider page
-    And I should see "Provider added"
+    And I should see a confirmation message
     And I should have a provider named "testprovider"
 
   Scenario: Create a new Provider failure when using wrong url
-    Given I am on the providers page
-    And there is not a provider named "testprovider"
-    When I follow "create_new_provider"
-    Then I should be on the new provider page
-    When I fill in "provider[name]" with "testprovider"
-    And I fill in "provider[url]" with "http://localhost:3010/api"
-    And I select "Amazon EC2" from "provider_provider_type_id"
-    And I press "save"
+    Given I am on the new provider page
+    And I attempt to add a provider with an invalid url
     Then I should be on the providers page
     And I should see a warning message
 
diff --git a/src/features/step_definitions/provider_steps.rb b/src/features/step_definitions/provider_steps.rb
index fc25f87..605f49c 100644
--- a/src/features/step_definitions/provider_steps.rb
+++ b/src/features/step_definitions/provider_steps.rb
@@ -13,13 +13,30 @@
 #   See the License for the specific language governing permissions and
 #   limitations under the License.
 #
+def new_provider(port,bool)
+  destroy_provider("testprovider")
+  stub_framework(bool)
+  fill_in "provider[name]", :with => "testprovider"
+  fill_in "provider[url]", :with => "http://localhost:#{port}/api"
+  select("Amazon EC2", :from => "provider_provider_type_id")
+  click_button "save"
+end
+
+def destroy_provider(name)
+  provider = Provider.find_by_name(name)
+  if provider then provider.destroy end
+end
+
+def stub_framework(bool)
+  Provider.any_instance.stub(:valid_framework?).and_return(bool)
+end
+
 Given /^there should not exist a provider named "([^\"]*)"$/ do |name|
   Provider.find_by_name(name).should be_nil
 end
 
 Given /^there is not a provider named "([^"]*)"$/ do |name|
-  provider = Provider.find_by_name(name)
-  if provider then provider.destroy end
+  destroy_provider(name)
 end
 
 Given /^there is a provider named "([^\"]*)"$/ do |name|
@@ -27,7 +44,15 @@ Given /^there is a provider named "([^\"]*)"$/ do |name|
 end
 
 Given /^provider "([^"]*)" is not accessible$/ do |arg1|
-  Provider.any_instance.stub(:valid_framework?).and_return(false)
+  stub_framework(false)
+end
+
+Given /^I attempt to add a valid provider$/ do
+  new_provider(3002,true)
+end
+
+Given /^I attempt to add a provider with an invalid url$/ do
+  new_provider(3010,false)
 end
 
 Then /^I should have a provider named "([^\"]*)"$/ do |name|
diff --git a/src/features/support/custom.rb b/src/features/support/custom.rb
index a02427c..e529073 100644
--- a/src/features/support/custom.rb
+++ b/src/features/support/custom.rb
@@ -23,6 +23,10 @@ Provider.class_eval do
     true
   end
 
+  def valid_provider?
+    true
+  end
+
   def populate_realms
     true
   end
diff --git a/src/spec/factories/provider.rb b/src/spec/factories/provider.rb
index de6092d..a85b73b 100644
--- a/src/spec/factories/provider.rb
+++ b/src/spec/factories/provider.rb
@@ -33,7 +33,6 @@ FactoryGirl.define do
     name 'mock2'
     provider_type { ProviderType.find_by_deltacloud_driver("mock") }
     url 'http://localhost:3002/api'
-    deltacloud_provider 'mock'
     after_create { |p| p.realms << FactoryGirl.create(:realm3, :provider => p) }
   end
 
diff --git a/src/spec/models/provider_spec.rb b/src/spec/models/provider_spec.rb
index 11cf71e..4d07f9d 100644
--- a/src/spec/models/provider_spec.rb
+++ b/src/spec/models/provider_spec.rb
@@ -56,17 +56,21 @@ describe Provider do
       end
     end
 
-    it "should be able to connect to the specified framework" do
-      @provider.should be_valid
-      @provider.connect.should_not be_nil
-
+    it "should set error if unable to connect to specified deltacloud instance" do
+      @provider.stub(:valid_provider?).and_return(true)
       @provider.url = "http://invalid.provider/url"
       @provider.stub(:connect).and_return(nil)
-      deltacloud = @provider.connect
       @provider.should have(1).error_on(:url)
-      @provider.errors[:url].first.should eql("must be a valid provider url")
+      @provider.errors[:url].first.should eql(I18n.t("activerecord.errors.provider.url"))
+      @provider.should_not be_valid
+    end
+
+    it "should set error if unable to connect to specified deltacloud_provider" do
+      @provider.stub(:valid_provider?).and_return(false)
+      @provider.stub(:valid_framework?).and_return(true)
+      @provider.should have(1).error_on(:deltacloud_provider)
+      @provider.errors[:deltacloud_provider].first.should eql(I18n.t("activerecord.errors.provider.deltacloud_provider"))
       @provider.should_not be_valid
-      deltacloud.should be_nil
     end
 
     it "should require unique name" do
diff --git a/src/spec/spec_helper.rb b/src/spec/spec_helper.rb
index 09bb9f3..00cb1fd 100644
--- a/src/spec/spec_helper.rb
+++ b/src/spec/spec_helper.rb
@@ -81,6 +81,10 @@ RSpec.configure do |config|
   #  activate_authlogic
   #end
 
+  config.before(:each) do
+    Provider.any_instance.stub(:valid_provider?).and_return(true)
+    Provider.any_instance.stub(:valid_famework?).and_return(true)
+  end
   #config.after(:each, :type => :controller) do
   #  current_user_session.destroy
   #end
-- 
1.7.7.6




More information about the aeolus-devel mailing list