From: Martyn Taylor <mtaylor(a)redhat.com&gt; --- .../controllers/provider_accounts_controller.rb | 1 + src/app/models/provider_account.rb | 25 ++++++++++++------- src/spec/models/provider_account_spec.rb | 18 ++++++++++++++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/app/controllers/provider_accounts_controller.rb b/src/app/controllers/provider_accounts_controller.rb index 9b5517d..b78ee52 100644 --- a/src/app/controllers/provider_accounts_controller.rb +++ b/src/app/controllers/provider_accounts_controller.rb @@ -79,6 +79,7 @@ class ProviderAccountsController < ApplicationController params[:provider_account][:provider_id] = @provider.id @providers = Provider.all @provider_account = ProviderAccount.new(params[:provider_account]) + @provider_account.credentials_hash = params[:provider_account][:credentials_hash] @provider_account.quota = @quota = Quota.new limit = params[:quota][:maximum_running_instances] if params[:quota] @provider_account.quota.set_maximum_running_instances(limit) diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb index e772f52..541834e 100644 --- a/src/app/models/provider_account.rb +++ b/src/app/models/provider_account.rb @@ -74,6 +74,11 @@ class ProviderAccount < ActiveRecord::Base scope :enabled, lambda { where(:provider_id => Provider.enabled) } + # We set credentials hash as protected so that it is not set during mass assign on new + # This is to avoid the scenario where the credentials are set before provider which + # will result in an exception. + attr_protected :credentials_hash + def validate_presence_of_credentials provider.provider_type.credential_definitions.each do |cd| errors.add(:base, "#{cd.label} can't be blank") if credentials_hash[cd.name].blank? @@ -207,16 +212,18 @@ class ProviderAccount < ActiveRecord::Base end def credentials_hash=(hash={}) - cred_defs = provider.provider_type.credential_definitions - hash.each do |k,v| - cred_def = cred_defs.detect {|d| d.name == k.to_s} - raise "Key #{k} not found" unless cred_def - unless cred = credentials.detect {|c| c.credential_definition_id == cred_def.id} - cred = Credential.new(:provider_account_id => id, :credential_definition_id => cred_def.id) - credentials << cred + if @provider + cred_defs = provider.provider_type.credential_definitions + hash.each do |k,v| + cred_def = cred_defs.detect {|d| d.name == k.to_s} + raise "Key #{k} not found" unless cred_def + unless cred = credentials.detect {|c| c.credential_definition_id == cred_def.id} + cred = Credential.new(:provider_account_id => id, :credential_definition_id => cred_def.id) + credentials << cred + end + # we need to handle uploaded files: + cred.value = v.respond_to?(:read) ? v.read : v end - # we need to handle uploaded files: - cred.value = v.respond_to?(:read) ? v.read : v end end diff --git a/src/spec/models/provider_account_spec.rb b/src/spec/models/provider_account_spec.rb index 6a52224..04b4b1e 100644 --- a/src/spec/models/provider_account_spec.rb +++ b/src/spec/models/provider_account_spec.rb @@ -153,6 +153,24 @@ describe ProviderAccount do @provider_account.should_not be_valid end + it "should not set credentials in intialise" do + @provider_account = ProviderAccount.new({:credentials_hash => {"username" => "test", "password" => "test"}}) + @provider_account.credentials_hash.should == {} + end + + it "should not set credentials before provider is set" do + @provider_account = ProviderAccount.new + @provider_account.credentials_hash = {"username" => "test", "password" => "test"} + @provider_account.credentials_hash.should == {} + end + + it "should set credentials after provider is set" do + provider = FactoryGirl.create :mock_provider + @provider_account = ProviderAccount.new({:provider => provider}) + @provider_account.credentials_hash = {"username" => "test", "password" => "test"} + @provider_account.credentials_hash.should == {"username" => "test", "password" => "test"} + end + context "validations" do context "priority" do it "can be positive integer" do -- 1.7.6.4