fix(multidb): fix issue with multiple connection detection (#578)

Author: lostie

app/services/forest_liana/base_getter.rb

@@ -34,10 +34,10 @@ def optimize_record_loading(resource, records)
       instance_dependent_associations = instance_dependent_associations(resource)
 
       preload_loads = @includes.select do |name|
-        targetModelConnection = resource.reflect_on_association(name).inverse_of&.active_record&.connection
+        targetModelConnection = resource.reflect_on_association(name).klass.connection
         targetModelDatabase = targetModelConnection.current_database if targetModelConnection.respond_to? :current_database
         resourceConnection = resource.connection
-        resourceDatabase = resourceConnection if resourceConnection.respond_to? :current_database
+        resourceDatabase = resourceConnection.current_database if resourceConnection.respond_to? :current_database
 
         targetModelDatabase != resourceDatabase
       end + instance_dependent_associations

spec/dummy/app/models/manufacturer.rb

@@ -0,0 +1,3 @@
+class Manufacturer < ApplicationRecord
+  has_many :products
+end

spec/dummy/app/models/product.rb

@@ -1,3 +1,6 @@
 class Product < ApplicationRecord
+  belongs_to :manufacturer
+  belongs_to :driver, optional: true
+
   validates :uri, presence: true, format: { with: URI::DEFAULT_PARSER.make_regexp }
 end

spec/dummy/db/migrate/20220727114450_create_manufacturers.rb

@@ -0,0 +1,7 @@
+class CreateManufacturers < ActiveRecord::Migration[6.0]
+  def change
+    create_table :manufacturers do |t|
+      t.string :name
+    end
+  end
+end

spec/dummy/db/migrate/20220727114930_add_columns_to_products.rb

@@ -0,0 +1,9 @@
+class AddColumnsToProducts < ActiveRecord::Migration[6.0]
+  def change
+    change_table :products do |table|
+      table.string :name
+      table.belongs_to(:manufacturer)
+      table.belongs_to(:driver)
+    end
+  end
+end

spec/dummy/db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_05_26_084712) do
+ActiveRecord::Schema.define(version: 2022_07_27_114930) do
 
   create_table "isle", force: :cascade do |t|
     t.string "name"
@@ -27,13 +27,22 @@
     t.index ["island_id"], name: "index_locations_on_island_id"
   end
 
+  create_table "manufacturers", force: :cascade do |t|
+    t.string "name"
+  end
+
   create_table "owners", force: :cascade do |t|
     t.string "name"
     t.datetime "hired_at"
   end
 
   create_table "products", force: :cascade do |t|
     t.string "uri"
+    t.string "name"
+    t.integer "manufacturer_id"
+    t.integer "driver_id"
+    t.index ["driver_id"], name: "index_products_on_driver_id"
+    t.index ["manufacturer_id"], name: "index_products_on_manufacturer_id"
   end
 
   create_table "references", force: :cascade do |t|

spec/lib/forest_liana/bootstrapper_spec.rb

@@ -25,6 +25,7 @@ module ForestLiana
         [
           Island,
           Location,
+          Manufacturer,
           Owner,
           Product,
           Reference,

spec/services/forest_liana/resources_getter_spec.rb

@@ -4,9 +4,9 @@ module ForestLiana
     let(:pageSize) { 10 }
     let(:pageNumber) { 1 }
     let(:sort) { 'id' }
-    let(:fields) { }
-    let(:filters) { }
-    let(:scopes) { { } }
+    let(:fields) {}
+    let(:filters) {}
+    let(:scopes) { {} }
     let(:rendering_id) { 13 }
     let(:user) { { 'id' => '1', 'rendering_id' => rendering_id } }
 
@@ -22,7 +22,21 @@ def init_scopes
       allow(ForestLiana::ScopeManager).to receive(:fetch_scopes).and_return(scopes)
     end
 
+    def clean_database
+      [
+        Driver,
+        Island,
+        Location,
+        Manufacturer,
+        Product,
+        Tree,
+        User,
+      ].each(&:destroy_all)
+    end
+
     before(:each) do
+      clean_database
+
       users = ['Michel', 'Robert', 'Vince', 'Sandro', 'Olesya', 'Romain', 'Valentin', 'Jason', 'Arnaud', 'Jeff', 'Steve', 'Marc', 'Xavier', 'Paul', 'Mickael', 'Mike', 'Maxime', 'Gertrude', 'Monique', 'Mia', 'Rachid', 'Edouard', 'Sacha', 'Caro', 'Amand', 'Nathan', 'Noémie', 'Robin', 'Gaelle', 'Isabelle']
       .map { |name| User.create(name: name) }
 
@@ -50,15 +64,94 @@ def init_scopes
         { :coordinates => '32154', :island => islands[4] }
       ].map { |location| Location.create(coordinates: location[:coordinates], island: location[:island]) }
 
+      manufacturers = ['Orange', 'Pear'].map { |name| Manufacturer.create!(name: 'name') }
+
+      drivers = ['Baby driver', 'Taxi driver'].map { |firstname| Driver.create!(firstname: firstname) }
+
+      products = [
+        { name: 'Valencia', uri: 'https://valencia.com', manufacturer: manufacturers[0], driver: drivers[0] },
+        { name: 'Blood', uri: 'https://blood.com', manufacturer: manufacturers[0], driver: drivers[1] },
+        { name: 'Conference', uri: 'https://conference.com', manufacturer: manufacturers[1], driver: drivers[0] },
+        { name: 'Concorde', uri: 'https://concorde.com', manufacturer: manufacturers[1], driver: drivers[1] }
+      ].map {|attributes| Product.create!(attributes) }
+
       reference = Reference.create()
       init_scopes
     end
 
-    after(:each) do
-      User.destroy_all
-      Island.destroy_all
-      Location.destroy_all
-      Tree.destroy_all
+    describe 'records eager loading' do
+      let(:resource) { Product }
+      let(:fields) { { resource.name => 'id,name,manufacturer', 'manufacturer' => 'name' } }
+
+      shared_context 'resource current_database' do
+        before do
+          connection = resource.connection
+
+          def connection.current_database
+            'db/test.sqlite3'
+          end
+        end
+
+        after do
+          resource.connection.singleton_class.remove_method(:current_database)
+        end
+      end
+
+      shared_examples 'left outer join' do
+        it 'should perform a left outer join with the association' do
+          expect(getter.perform.to_sql).to match(/LEFT OUTER JOIN "manufacturers"/)
+        end
+      end
+
+      shared_examples 'records' do
+        it 'should get only the expected records' do
+          getter.perform
+
+          records = getter.records
+
+          count = getter.count
+
+          expect(records.count).to eq 4
+          expect(count).to eq 4
+          expect(records.map(&:name)).to match_array(%w[Valencia Blood Conference Concorde])
+        end
+      end
+
+      context 'when the connections do not support current_database' do
+        include_examples 'left outer join'
+        include_examples 'records'
+      end
+
+      context 'when the included association uses a different database connection' do
+        let(:fields) { { resource.name => 'id,name,driver', 'driver' => 'firstname' } }
+
+        before do
+          association_connection = resource.reflect_on_association(:driver).klass.connection
+
+          def association_connection.current_database
+            'db/different_test.sqlite3'
+          end
+        end
+
+        after do
+          resource.reflect_on_association(:driver).klass.connection.singleton_class.remove_method(:current_database)
+        end
+
+        include_context 'resource current_database'
+
+        include_examples 'records'
+
+        it 'does not perform a left outer join with the association' do
+          expect(getter.perform.to_sql).not_to match(/LEFT OUTER JOIN "drivers"/)
+        end
+      end
+
+      context 'when the included association uses the same database connection' do
+        include_context 'resource current_database'
+
+        include_examples 'left outer join'
+        include_examples 'records'
+      end
     end
 
     describe 'when there are more records than the page size' do