Compare database config with include? instead of expanding path

[rzane]

I use Firebird database (not by preference). It works similarly to sqlite3 in that it creates database files.

Because of the explicit check for sqlite3, I was having some trouble. Instead of expanding the path to the database, I figure you could probably just check if the expanded file path includes the un-expanded path.

The other change I made downcases matched table names. The firebird adapter capitalizes table names, so the merge in Cleaner#clean was returning an empty array. I think all of the supported adapters return table names in lower case, anyway. This would save you if you explicitly used connection.execute 'INSERT INTO TABLE'

[deeeki]

Thanks, LGTM :smiley: Please take a look. @amatsuda

[amatsuda]

@rzane I understand you situation, but I'm uncertain as to whether or not this patch keeps the current behaviour.

For instance, what if we had two databases foo and foo_bar configured, and a query was made via foo_bar database. Your code cleaners.detect {|c| config[:database].include?(c.db)} would possibly detect a wrong database foo.

The other change in the patch looks good. Bonus point :moneybag: if the commits where separated.

[deeeki]

would possibly detect a wrong database foo

Ah, exactly. Thank you for your comment :bow:

[rzane]

@amatsuda Done. This should prevent the foo_bar problem. I also split it into multiple commits and added specs.

[amatsuda]

@rzane Thanks, but changing include? to end_with? wouldn't essentially solve the problem. Let me show you another one called the barfoo problem...

diff --git a/spec/database_rewinder_spec.rb b/spec/database_rewinder_spec.rb
index e015b55..be2a5ec 100644
--- a/spec/database_rewinder_spec.rb
+++ b/spec/database_rewinder_spec.rb
@@ -108,16 +108,16 @@ describe DatabaseRewinder do

     context 'multiple databases' do
       before do
-        DatabaseRewinder.database_configuration = {'foo' => {'database' => 'foo'}, 'foobar' => {'database' => 'foobar'}}
+        DatabaseRewinder.database_configuration = {'foo' => {'database' => 'foo'}, 'barfoo' => {'database' => 'barfoo'}}
       end
-      let(:database) { 'foo' }
-      let!(:foobar)  { DatabaseRewinder.create_cleaner 'foobar' }
+      let(:database) { 'barfoo' }
       let!(:foo)     { DatabaseRewinder.create_cleaner 'foo' }
+      let!(:barfoo)  { DatabaseRewinder.create_cleaner 'barfoo' }

       it 'does not add tables to the wrong database' do
         DatabaseRewinder.record_inserted_table(connection, 'INSERT INTO "something" ("name") VALUES (?)')
-        expect(foo.inserted_tables).to eq(['something'])
-        expect(foobar.inserted_tables).to be_empty
+        expect(barfoo.inserted_tables).to eq(['something'])
+        expect(foo.inserted_tables).to be_empty
       end
     end
   end

[rzane]

Hmm... how about only comparing the expanded path to the file if the file exists?

cleaner = cleaners.detect do |c|
  if File.exist? full_path
    File.expand_path(c.db, root_dir) == full_path
  else
    c.db == config[:database]
  end
end or return

Essentially, file based databases (SQLite/Firebird) would compare full paths, and PG/MySql/SqlServer would just compare database names.