Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Model should have more priority over connection_hash for connection_class #157

Merged
merged 1 commit into from

2 participants

@pftg

When connection should get from model through :model attribute, load_config loaded and set connection_hash. After that for connection_class instead of using model value it use connection_hash. All this two cases are bugs. I added fix changes to pull request:

  • Skip load_config for ActiveRecord if model set.
  • If model and connection_hash are set, then should return model value as connection_class instead of connection_hash builder
@pftg pftg Model has more priority over connection_hash
* Skip load_config for ActiveRecord if model set for strategy.
* If model and connection_hash are set, then should return model as connection_class instead of connection_hash builder
21fb66f
@bmabey bmabey merged commit 8aa8526 into DatabaseCleaner:master
@bmabey
Owner

Makes sense, and the patch looks good (travis build is in disrepair so you can ignore that). Merged in, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 2, 2012
  1. @pftg

    Model has more priority over connection_hash

    pftg authored
    * Skip load_config for ActiveRecord if model set for strategy.
    * If model and connection_hash are set, then should return model as connection_class instead of connection_hash builder
This page is out of date. Refresh to see the latest.
View
18 lib/database_cleaner/active_record/base.rb
@@ -32,9 +32,9 @@ def db
end
def load_config
- if self.db != :default && File.file?(ActiveRecord.config_file_location)
- connection_details = YAML::load(ERB.new(IO.read(ActiveRecord.config_file_location)).result)
- @connection_hash = connection_details[self.db.to_s]
+ if self.db != :default && self.db.is_a?(Symbol) && File.file?(ActiveRecord.config_file_location)
+ connection_details = YAML::load(ERB.new(IO.read(ActiveRecord.config_file_location)).result)
+ @connection_hash = connection_details[self.db.to_s]
end
end
@@ -43,12 +43,12 @@ def create_connection_class
end
def connection_class
- @connection_class ||= if @db == :default || (@db.nil? && connection_hash.nil?)
- ::ActiveRecord::Base
+ @connection_class ||= if @db && !@db.is_a?(Symbol)
+ @db
elsif connection_hash
lookup_from_connection_pool || establish_connection
else
- @db # allows for an actual class to be passed in
+ ::ActiveRecord::Base
end
end
@@ -57,8 +57,8 @@ def connection_class
def lookup_from_connection_pool
if ::ActiveRecord::Base.respond_to?(:descendants)
database_name = connection_hash["database"] || connection_hash[:database]
- models = ::ActiveRecord::Base.descendants
- models.detect {|m| m.connection_pool.spec.config[:database] == database_name}
+ models = ::ActiveRecord::Base.descendants
+ models.detect { |m| m.connection_pool.spec.config[:database] == database_name }
end
end
@@ -67,7 +67,7 @@ def establish_connection
strategy_class.send :establish_connection, connection_hash
strategy_class
end
-
+
end
end
end
View
43 spec/database_cleaner/active_record/base_spec.rb
@@ -62,7 +62,7 @@ class ExampleStrategy
before do
subject.db = :my_db
- yaml = <<-Y
+ yaml = <<-Y
my_db:
database: <%= "ONE".downcase %>
Y
@@ -71,7 +71,7 @@ class ExampleStrategy
end
it "should parse the config" do
- YAML.should_receive(:load).and_return( {:nil => nil} )
+ YAML.should_receive(:load).and_return({ :nil => nil })
subject.load_config
end
@@ -80,19 +80,26 @@ class ExampleStrategy
my_db:
database: one
Y
- YAML.should_receive(:load).with(transformed).and_return({ "my_db" => {"database" => "one"} })
+ YAML.should_receive(:load).with(transformed).and_return({ "my_db" => { "database" => "one" } })
subject.load_config
end
it "should store the relevant config in connection_hash" do
subject.load_config
- subject.connection_hash.should == {"database" => "one"}
+ subject.connection_hash.should == { "database" => "one" }
end
it "should skip config if config file is not available" do
File.should_receive(:file?).with(config_location).and_return(false)
subject.load_config
- subject.connection_hash.should be_blank
+ subject.connection_hash.should_not be
+ end
+
+ it "skips the file when the model is set" do
+ subject.db = FakeModel
+ YAML.should_not_receive(:load)
+ subject.load_config
+ subject.connection_hash.should_not be
end
it "skips the file when the db is set to :default" do
@@ -100,6 +107,7 @@ class ExampleStrategy
subject.db = :default
YAML.should_not_receive(:load)
subject.load_config
+ subject.connection_hash.should_not be
end
end
@@ -122,19 +130,32 @@ class ExampleStrategy
end
describe "connection_class" do
- it { expect{ subject.connection_class }.to_not raise_error }
+ it { expect { subject.connection_class }.to_not raise_error }
it "should default to ActiveRecord::Base" do
subject.connection_class.should == ::ActiveRecord::Base
end
- it "allows for database models to be passed in" do
- subject.db = FakeModel
- subject.connection_class.should == FakeModel
+ context "with database models" do
+ context "connection_hash is set" do
+ it "allows for database models to be passed in" do
+ subject.db = FakeModel
+ subject.connection_hash = { }
+ subject.load_config
+ subject.connection_class.should == FakeModel
+ end
+ end
+
+ context "connection_hash is not set" do
+ it "allows for database models to be passed in" do
+ subject.db = FakeModel
+ subject.connection_class.should == FakeModel
+ end
+ end
end
context "when connection_hash is set" do
let(:hash) { mock("hash") }
- before { ::ActiveRecord::Base.stub!(:respond_to?).and_return(false)}
+ before { ::ActiveRecord::Base.stub!(:respond_to?).and_return(false) }
before { subject.stub(:connection_hash).and_return(hash) }
it "should create connection_class if it doesnt exist if connection_hash is set" do
@@ -142,7 +163,7 @@ class ExampleStrategy
subject.connection_class
end
- it "should configure the class from create_connection_class if connection_hash is set" do
+ it "should configure the class from create_connection_class if connection_hash is set" do
strategy_class = mock('strategy_class')
strategy_class.should_receive(:establish_connection).with(hash)
Something went wrong with that request. Please try again.