Fix conflicting annotations for multiple-database scenario

Given a Rails application with multiple databases, tables in two
databases with the same table name, and corresponding models with
different names, some files get annotated with the wrong table's schema.

The issue is that some of the patterns include `%TABLE_NAME%`, which
will be identical between the two tables, even though they have
different model names.

This fixes the issue by eliminating the use of the table name when the
model is not connected to the primary database.

Fixes #891.

Author: afn

lib/annotate/annotate_models.rb

@@ -480,7 +480,7 @@ def annotate(klass, file, header, options = {})
         klass.reset_column_information
         info = get_schema_info(klass, header, options)
         model_name = klass.name.underscore
-        table_name = klass.table_name
+        table_name = klass.table_name if klass.connection_specification_name == ActiveRecord::Base.name
         model_file_name = File.join(file)
         annotated = []
 
@@ -702,7 +702,7 @@ def remove_annotations(options = {})
           klass = get_model_class(file)
           if klass < ActiveRecord::Base && !klass.abstract_class?
             model_name = klass.name.underscore
-            table_name = klass.table_name
+            table_name = klass.table_name if klass.connection_specification_name == ActiveRecord::Base.name
             model_file_name = file
             deannotated_klass = true if remove_annotation_of_file(model_file_name, options)
 

spec/integration/rails_6.0.2.1/app/models/secondary_record.rb

@@ -0,0 +1,5 @@
+class SecondaryRecord < ActiveRecord::Base
+  self.abstract_class = true
+
+  connects_to database: { reading: :secondary, writing: :secondary }
+end

spec/integration/rails_6.0.2.1/app/models/secondary_task.rb

@@ -0,0 +1,3 @@
+class SecondaryTask < SecondaryRecord
+  self.table_name = 'tasks'
+end

spec/integration/rails_6.0.2.1/config/multi-database.yml

@@ -16,6 +16,7 @@ development:
   secondary:
     <<: *default
     database: db/development-secondary.sqlite3
+    migrations_paths: db/secondary_migrate
 
 # Warning: The database defined as "test" will be erased and
 # re-generated from your development database when you run "rake".
@@ -27,6 +28,7 @@ test:
   secondary:
     <<: *default
     database: db/test-secondary.sqlite3
+    migrations_paths: db/secondary_migrate
 
 production:
   primary:
@@ -35,3 +37,4 @@ production:
   secondary:
     <<: *default
     database: db/production-secondary.sqlite3
+    migrations_paths: db/secondary_migrate

spec/integration/rails_6.0.2.1/db/secondary_migrate/20200201204456_create_tasks.rb

@@ -0,0 +1,10 @@
+class CreateTasks < ActiveRecord::Migration[6.0]
+  def change
+    create_table :tasks do |t|
+      t.boolean :completed, null: false
+      t.string :description
+
+      t.timestamps
+    end
+  end
+end

spec/integration/rails_6.0.2.1/db/secondary_schema.rb

@@ -0,0 +1,22 @@
+# This file is auto-generated from the current state of the database. Instead
+# of editing this file, please use the migrations feature of Active Record to
+# incrementally modify your database, and then regenerate this schema definition.
+#
+# This file is the source Rails uses to define your schema when running `rails
+# db:schema:load`. When creating a new database, `rails db:schema:load` tends to
+# be faster and is potentially less error prone than running all of your
+# migrations from scratch. Old migrations may fail to apply correctly if those
+# migrations use external dependencies or application code.
+#
+# It's strongly recommended that you check this file into your version control system.
+
+ActiveRecord::Schema.define(version: 2020_02_01_204456) do
+
+  create_table "tasks", force: :cascade do |t|
+    t.boolean "completed", null: false
+    t.string "description"
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
+  end
+
+end

spec/integration/rails_6.0.2.1/test/fixtures/secondary_tasks.yml

@@ -0,0 +1,9 @@
+# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
+
+one:
+  description: MyString
+  completed: false
+
+two:
+  description: MyString
+  completed: false

spec/integration/rails_6.0.2.1_spec.rb

@@ -72,6 +72,46 @@
         patch: include(patch)
     }
   end
+  let(:secondary_task_model) do
+    patch = <<~PATCH
+      +# == Schema Information
+      +#
+      +# Table name: tasks
+      +#
+      +#  id          :integer          not null, primary key
+      +#  completed   :boolean          not null
+      +#  description :string
+      +#  created_at  :datetime         not null
+      +#  updated_at  :datetime         not null
+      +#
+    PATCH
+
+    path = 'app/models/secondary_task.rb'
+    {
+        path: include(path),
+        patch: include(patch)
+    }
+  end
+  let(:secondary_task_fixture) do
+    patch = <<~PATCH
+      +# == Schema Information
+      +#
+      +# Table name: tasks
+      +#
+      +#  id          :integer          not null, primary key
+      +#  completed   :boolean          not null
+      +#  description :string
+      +#  created_at  :datetime         not null
+      +#  updated_at  :datetime         not null
+      +#
+    PATCH
+
+    path = 'test/fixtures/secondary_tasks.yml'
+    {
+        path: include(path),
+        patch: include(patch)
+    }
+  end
 
   before(:all) do
     Bundler.with_clean_env do
@@ -108,6 +148,21 @@
                                       an_object_having_attributes(task_fixture)
                                   )
     end
+
+    context 'with multi-db environment' do
+      let(:command) { 'bundle exec annotate --models app/models/secondary_task.rb' }
+
+      it 'does not annotate files based on table name when the model is from a secondary database' do
+        expect(git.diff.any?).to be_falsy
+
+        system({ 'MULTI_DB' => 'true' }, command)
+
+        expect(git.diff.entries).to contain_exactly(
+                                        an_object_having_attributes(secondary_task_model),
+                                        an_object_having_attributes(secondary_task_fixture)
+                                    )
+      end
+    end
   end
 
   describe 'annotate --routes' do
@@ -181,7 +236,7 @@
     end
 
     context 'with multi-db environment' do
-      let(:migrate_command) { 'bin/rails db:migrate:primary' }
+      let(:migrate_command) { 'bin/rails db:migrate' }
 
       it 'hooks database-specific commands and annotates models' do
         expect(git.diff.any?).to be_falsy
@@ -192,7 +247,9 @@
         expect(git.diff.entries).to contain_exactly(
                                         an_object_having_attributes(task_model),
                                         an_object_having_attributes(task_test),
-                                        an_object_having_attributes(task_fixture)
+                                        an_object_having_attributes(task_fixture),
+                                        an_object_having_attributes(secondary_task_model),
+                                        an_object_having_attributes(secondary_task_fixture)
                                     )
       end
     end