From 4144746d3359a30dcb3023401f16256b62ec57de Mon Sep 17 00:00:00 2001 From: eileencodes Date: Mon, 25 Jan 2021 10:52:56 -0500 Subject: [PATCH] Expose `primary_abstract_class` public API Previously Rails was treating `ApplicationRecord` as special in the `primary_class?` check. The reason we need to treat it differtently than other connection classes is that `ActiveRecord::Base` will establish a connection to the primary database on boot. The established connection is to your primary database, or the first database defined in your configuration. We need to do this so that 2 connections aren't opened to the same database since `ActiveRecord::Base` and `AppliationRecord` are different classes, on connection the connection_speicification_name would be different. However, there is no guarantee that an application is using `ApplicationRecord` as it's primary abstract class. This exposes a public method for setting a class to a `primary_abstract_class` like this: ``` class PrimaryApplicationRecord < ActiveRecord::Base self.primary_abstract_class end ``` Calling `primary_abstract_class` will automatically set `self.abstract_class = true`. This change is backwards compatible because we weren't supporting multiple application records previously, and if you had an `ApplicationRecord` we assumed that was the primary class. This change continues to assume that `ApplicationRecord` is your primary class. You only need to set `primary_abstract_class` if your application record is not ApplicationRecord and you're using multiple databases. Co-authored-by: John Crepezzi --- activerecord/CHANGELOG.md | 19 +++ .../lib/active_record/connection_handling.rb | 2 +- activerecord/lib/active_record/core.rb | 13 ++ activerecord/lib/active_record/inheritance.rb | 15 +++ .../connection_swapping_nested_test.rb | 1 + activerecord/test/cases/primary_class_test.rb | 111 ++++++++++++++++++ .../active_record_multiple_databases.md | 14 ++- 7 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 activerecord/test/cases/primary_class_test.rb diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 15926a8f14..b0ec76e71f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,22 @@ +* Expose a way for applications to set a `primary_abstract_class` + + Multiple database applications that use a primary abstract class that is not + named `ApplicationRecord` can now set a specific class to be the `primary_abstract_class`. + + ```ruby + class PrimaryApplicationRecord + self.primary_abstract_class + end + ``` + + When an application boots it automatically connects to the primary or first database in the + database configuration file. In a multiple database application that then call `connects_to` + needs to know that the default connection is the same as the `ApplicationRecord` connection. + However some applications have a differently named `ApplicationRecord`. This prevents Active + Record from opening duplicate connections to the same database. + + *Eileen M. Uchitelle*, *John Crepezzi* + * Support hash config for `structure_dump_flags` and `structure_load_flags` flags Now that Active Record supports multiple databases configuration we need a way to pass specific flags for dump/load databases since diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index 789202415e..3b6fbf9f3d 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -276,7 +276,7 @@ module ActiveRecord end def primary_class? # :nodoc: - self == Base || defined?(ApplicationRecord) && self == ApplicationRecord + self == Base || application_record_class? end # Returns the configuration of the associated connection as a hash: diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index ee47c6ad97..012949aff1 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -155,6 +155,19 @@ module ActiveRecord mattr_accessor :legacy_connection_handling, instance_writer: false, default: true + mattr_accessor :application_record_class, instance_accessor: false, default: nil + + def self.application_record_class? # :nodoc: + if Base.application_record_class + self == Base.application_record_class + else + if defined?(ApplicationRecord) && self == ApplicationRecord + Base.application_record_class = self + true + end + end + end + self.filter_attributes = [] def self.connection_handler diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index d0ef768433..7aaa4cb97b 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -164,6 +164,21 @@ module ActiveRecord defined?(@abstract_class) && @abstract_class == true end + # Sets the application record class for Active Record + # + # This is useful if your application uses a different class than + # ApplicationRecord for your primary abstract class. This class + # will share a database connection with Active Record. It is the class + # that connects to your primary database. + def primary_abstract_class + if Base.application_record_class && Base.application_record_class != self + raise ArgumentError, "The `primary_abstract_class` is already set to #{Base.application_record_class}. There can only be one `primary_abstract_class` in an application." + end + + self.abstract_class = true + Base.application_record_class = self + end + # Returns the value to be stored in the inheritance column for STI. def sti_name store_full_sti_class && store_full_class_name ? name : name.demodulize diff --git a/activerecord/test/cases/connection_adapters/connection_swapping_nested_test.rb b/activerecord/test/cases/connection_adapters/connection_swapping_nested_test.rb index 5aa3318abb..dc14b301c5 100644 --- a/activerecord/test/cases/connection_adapters/connection_swapping_nested_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_swapping_nested_test.rb @@ -448,6 +448,7 @@ module ActiveRecord end end ensure + ActiveRecord::Base.application_record_class = nil Object.send(:remove_const, :ApplicationRecord) ActiveRecord::Base.establish_connection :arunit end diff --git a/activerecord/test/cases/primary_class_test.rb b/activerecord/test/cases/primary_class_test.rb new file mode 100644 index 0000000000..56329ddfe8 --- /dev/null +++ b/activerecord/test/cases/primary_class_test.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require "cases/helper" + +class PrimaryClassTest < ActiveRecord::TestCase + self.use_transactional_tests = false + + def teardown + clean_up_connection_handler + end + + class PrimaryAppRecord < ActiveRecord::Base + end + + class AnotherAppRecord < PrimaryAppRecord + self.abstract_class = true + end + + class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true + end + + def test_application_record_is_used_if_no_primary_class_is_set + Object.const_set(:ApplicationRecord, ApplicationRecord) + + assert_predicate ApplicationRecord, :primary_class? + assert_predicate ApplicationRecord, :application_record_class? + assert_predicate ApplicationRecord, :abstract_class? + ensure + ActiveRecord::Base.application_record_class = nil + Object.send(:remove_const, :ApplicationRecord) + end + + def test_primary_class_and_primary_abstract_class_behavior + PrimaryClassTest::PrimaryAppRecord.primary_abstract_class + + assert_predicate PrimaryClassTest::PrimaryAppRecord, :primary_class? + assert_predicate PrimaryClassTest::PrimaryAppRecord, :application_record_class? + assert_predicate PrimaryClassTest::PrimaryAppRecord, :abstract_class? + + assert_not_predicate AnotherAppRecord, :primary_class? + assert_not_predicate AnotherAppRecord, :application_record_class? + assert_predicate AnotherAppRecord, :abstract_class? + + assert_predicate ActiveRecord::Base, :primary_class? + assert_not_predicate ActiveRecord::Base, :application_record_class? + assert_not_predicate ActiveRecord::Base, :abstract_class? + ensure + ActiveRecord::Base.application_record_class = nil + end + + def test_primary_abstract_class_cannot_be_reset + PrimaryClassTest::PrimaryAppRecord.primary_abstract_class + + assert_raises do + PrimaryClassTest::AnotherAppRecord.primary_abstract_class + end + ensure + ActiveRecord::Base.application_record_class = nil + end + + def test_primary_abstract_class_is_used_over_application_record_if_set + PrimaryClassTest::PrimaryAppRecord.primary_abstract_class + Object.const_set(:ApplicationRecord, ApplicationRecord) + + assert_predicate PrimaryClassTest::PrimaryAppRecord, :primary_class? + assert_predicate PrimaryClassTest::PrimaryAppRecord, :application_record_class? + assert_predicate PrimaryClassTest::PrimaryAppRecord, :abstract_class? + + assert_not_predicate ApplicationRecord, :primary_class? + assert_not_predicate ApplicationRecord, :application_record_class? + assert_predicate ApplicationRecord, :abstract_class? + + assert_predicate ActiveRecord::Base, :primary_class? + assert_not_predicate ActiveRecord::Base, :application_record_class? + assert_not_predicate ActiveRecord::Base, :abstract_class? + ensure + ActiveRecord::Base.application_record_class = nil + Object.send(:remove_const, :ApplicationRecord) + end + + unless in_memory_db? + def test_application_record_shares_a_connection_with_active_record_by_default + Object.const_set(:ApplicationRecord, ApplicationRecord) + + ApplicationRecord.connects_to(database: { writing: :arunit, reading: :arunit }) + + assert_predicate ApplicationRecord, :primary_class? + assert_predicate ApplicationRecord, :application_record_class? + assert_equal ActiveRecord::Base.connection, ApplicationRecord.connection + ensure + ActiveRecord::Base.application_record_class = nil + Object.send(:remove_const, :ApplicationRecord) + ActiveRecord::Base.establish_connection :arunit + end + + def test_application_record_shares_a_connection_with_the_primary_abstract_class_if_set + PrimaryClassTest::PrimaryAppRecord.primary_abstract_class + + PrimaryClassTest::PrimaryAppRecord.connects_to(database: { writing: :arunit, reading: :arunit }) + + assert_predicate PrimaryClassTest::PrimaryAppRecord, :primary_class? + assert_predicate PrimaryClassTest::PrimaryAppRecord, :application_record_class? + assert_predicate PrimaryClassTest::PrimaryAppRecord, :abstract_class? + assert_equal ActiveRecord::Base.connection, PrimaryClassTest::PrimaryAppRecord.connection + ensure + ActiveRecord::Base.application_record_class = nil + ActiveRecord::Base.establish_connection :arunit + end + end +end diff --git a/guides/source/active_record_multiple_databases.md b/guides/source/active_record_multiple_databases.md index 5e8f2283f6..12d61fde56 100644 --- a/guides/source/active_record_multiple_databases.md +++ b/guides/source/active_record_multiple_databases.md @@ -123,8 +123,18 @@ class ApplicationRecord < ActiveRecord::Base end ``` -Classes that connect to primary/primary_replica can inherit from `ApplicationRecord` like -standard Rails applications: +If you use a differently named class for your application record you can need to +set `primary_abstract_class` instead so Rails knowns which class `ActiveRecord::Base` +should share a connection with. + +``` +class PrimaryApplicationRecord < ActiveRecord::Base + self.primary_abstract_class +end +``` + +Classes that connect to primary/primary_replica can inherit from your primary abstract +class like standard Rails applications: ```ruby class Person < ApplicationRecord