mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Generate abstract class when generating scaffold in another database
This PR ensures that when you're generating a scaffold or model and that model should belong to another database it will create an abstract class if it doesn't already exist. The new abstract class will ensure that the new model inherits from that class, but will not be deleted if the scaffold is deleted. This is because Rails can't know if you have other models inheriting from that class so we don't want to revoke that if the scaffold is destroyed. If the abstract class already exists it won't be created twice. If the options for `parent` are set, the generator will use that as the abstract class instead of creating one. The generated abstract class will add the writing connection automatically, users need to add the reading connection themselves as Rails doesn't know which is the reading connection. Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
This commit is contained in:
parent
4cc438a1df
commit
261cbcd2a8
6 changed files with 169 additions and 3 deletions
|
@ -18,12 +18,13 @@ module ActiveRecord
|
||||||
|
|
||||||
# creates the migration file for the model.
|
# creates the migration file for the model.
|
||||||
def create_migration_file
|
def create_migration_file
|
||||||
return unless options[:migration] && options[:parent].nil?
|
return if skip_migration_creation?
|
||||||
attributes.each { |a| a.attr_options.delete(:index) if a.reference? && !a.has_index? } if options[:indexes] == false
|
attributes.each { |a| a.attr_options.delete(:index) if a.reference? && !a.has_index? } if options[:indexes] == false
|
||||||
migration_template "../../migration/templates/create_table_migration.rb", File.join(db_migrate_path, "create_#{table_name}.rb")
|
migration_template "../../migration/templates/create_table_migration.rb", File.join(db_migrate_path, "create_#{table_name}.rb")
|
||||||
end
|
end
|
||||||
|
|
||||||
def create_model_file
|
def create_model_file
|
||||||
|
generate_abstract_class if database && !parent
|
||||||
template "model.rb", File.join("app/models", class_path, "#{file_name}.rb")
|
template "model.rb", File.join("app/models", class_path, "#{file_name}.rb")
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -35,13 +36,49 @@ module ActiveRecord
|
||||||
hook_for :test_framework
|
hook_for :test_framework
|
||||||
|
|
||||||
private
|
private
|
||||||
|
# Skip creating migration file if:
|
||||||
|
# - options parent is present and database option is not present
|
||||||
|
# - migrations option is nil or false
|
||||||
|
def skip_migration_creation?
|
||||||
|
parent && !database || !migration
|
||||||
|
end
|
||||||
|
|
||||||
def attributes_with_index
|
def attributes_with_index
|
||||||
attributes.select { |a| !a.reference? && a.has_index? }
|
attributes.select { |a| !a.reference? && a.has_index? }
|
||||||
end
|
end
|
||||||
|
|
||||||
# Used by the migration template to determine the parent name of the model
|
# Used by the migration template to determine the parent name of the model
|
||||||
def parent_class_name
|
def parent_class_name
|
||||||
options[:parent] || "ApplicationRecord"
|
if parent
|
||||||
|
parent
|
||||||
|
elsif database
|
||||||
|
abstract_class_name
|
||||||
|
else
|
||||||
|
"ApplicationRecord"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def generate_abstract_class
|
||||||
|
path = File.join("app/models", "#{database.underscore}_record.rb")
|
||||||
|
return if File.exist?(path)
|
||||||
|
|
||||||
|
template "abstract_base_class.rb", path
|
||||||
|
end
|
||||||
|
|
||||||
|
def abstract_class_name
|
||||||
|
"#{database.classify}Record"
|
||||||
|
end
|
||||||
|
|
||||||
|
def database
|
||||||
|
options[:database]
|
||||||
|
end
|
||||||
|
|
||||||
|
def parent
|
||||||
|
options[:parent]
|
||||||
|
end
|
||||||
|
|
||||||
|
def migration
|
||||||
|
options[:migration]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
<% module_namespacing do -%>
|
||||||
|
class <%= abstract_class_name %> < ApplicationRecord
|
||||||
|
self.abstract_class = true
|
||||||
|
|
||||||
|
connects_to database: { <%= ActiveRecord::Base.writing_role %>: :<%= database -%> }
|
||||||
|
end
|
||||||
|
<% end -%>
|
|
@ -1,3 +1,46 @@
|
||||||
|
* Automatically generate abstract class when using multiple databases.
|
||||||
|
|
||||||
|
When generating a scaffold for a multiple database application, Rails will now automatically generate the abstract class for the database when the database argument is passed. This abstract class will include the connection information for the writing configuration and any models generated for that database will automatically inherit from the abstract class.
|
||||||
|
|
||||||
|
Usage:
|
||||||
|
|
||||||
|
```
|
||||||
|
rails generate scaffold Pet name:string --database=animals
|
||||||
|
```
|
||||||
|
|
||||||
|
Will create an abstract class for the animals connection.
|
||||||
|
|
||||||
|
```ruby
|
||||||
|
class AnimalsRecord < ApplicationRecord
|
||||||
|
self.abstract_class = true
|
||||||
|
|
||||||
|
connects_to database: { writing: :animals }
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
And generate a `Pet` model that inherits from the new `AnimalsRecord`:
|
||||||
|
|
||||||
|
```ruby
|
||||||
|
class Pet < AnimalsRecord
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
If you already have an abstract class and it follows a different pattern than Rails defaults, you can pass a parent class with the database argument.
|
||||||
|
|
||||||
|
```
|
||||||
|
rails generate scaffold Pet name:string --database=animals --parent=SecondaryBase
|
||||||
|
```
|
||||||
|
|
||||||
|
This will ensure the model inherits from the `SecondaryBase` parent instead of `AnimalsRecrd`
|
||||||
|
|
||||||
|
```ruby
|
||||||
|
class Pet < SecondaryBase
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
*Eileen M. Uchitelle*, *John Crepezzi*
|
||||||
|
|
||||||
|
|
||||||
* Accept params from url to prepopulate the Inbound Emails form in Rails conductor.
|
* Accept params from url to prepopulate the Inbound Emails form in Rails conductor.
|
||||||
|
|
||||||
*Chris Oliver*
|
*Chris Oliver*
|
||||||
|
|
35
railties/test/application/multi_db_rake_test.rb
Normal file
35
railties/test/application/multi_db_rake_test.rb
Normal file
|
@ -0,0 +1,35 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
require "isolation/abstract_unit"
|
||||||
|
require "env_helpers"
|
||||||
|
|
||||||
|
module ApplicationTests
|
||||||
|
class MultiDbRakeTest < ActiveSupport::TestCase
|
||||||
|
include ActiveSupport::Testing::Isolation, EnvHelpers
|
||||||
|
|
||||||
|
def setup
|
||||||
|
build_app(multi_db: true)
|
||||||
|
@output = rails("generate", "scaffold", "Pet", "name:string", "--database=animals")
|
||||||
|
end
|
||||||
|
|
||||||
|
def teardown
|
||||||
|
teardown_app
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_generate_scaffold_creates_abstract_model
|
||||||
|
assert_match %r{app/models/pet\.rb}, @output
|
||||||
|
assert_match %r{app/models/animals_record\.rb}, @output
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_destroy_scaffold_doesnt_remove_abstract_model
|
||||||
|
output = rails("destroy", "scaffold", "Pet", "--database=animals")
|
||||||
|
|
||||||
|
assert_match %r{app/models/pet\.rb}, output
|
||||||
|
assert_no_match %r{app/models/animals_record\.rb}, output
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_creates_a_directory_for_migrations
|
||||||
|
assert_match %r{db/animals_migrate/}, @output
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -49,6 +49,45 @@ class ModelGeneratorTest < Rails::Generators::TestCase
|
||||||
assert_no_migration "db/migrate/create_accounts.rb"
|
assert_no_migration "db/migrate/create_accounts.rb"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_model_with_database_option
|
||||||
|
with_secondary_database_configuration do
|
||||||
|
run_generator ["account", "--database", "secondary"]
|
||||||
|
assert_file "app/models/secondary_record.rb", /class SecondaryRecord < ApplicationRecord/
|
||||||
|
assert_file "app/models/account.rb", /class Account < SecondaryRecord/
|
||||||
|
assert_migration "db/secondary_migrate/create_accounts.rb", /class CreateAccounts < ActiveRecord::Migration\[[0-9.]+\]/
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_model_with_parent_and_database_option
|
||||||
|
with_secondary_database_configuration do
|
||||||
|
run_generator ["account", "--parent", "Admin::Account", "--database", "secondary"]
|
||||||
|
assert_file "app/models/account.rb", /class Account < Admin::Account/
|
||||||
|
assert_migration "db/secondary_migrate/create_accounts.rb", /class CreateAccounts < ActiveRecord::Migration\[[0-9.]+\]/
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_model_with_no_migration_and_database_option
|
||||||
|
with_secondary_database_configuration do
|
||||||
|
run_generator ["account", "--migration", "false", "--database", "secondary"]
|
||||||
|
assert_file "app/models/account.rb", /class Account < SecondaryRecord/
|
||||||
|
assert_no_migration "db/secondary_migrate/create_accounts.rb"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_model_with_no_migration_option
|
||||||
|
run_generator ["account", "--migration", "false"]
|
||||||
|
assert_file "app/models/account.rb", /class Account < ApplicationRecord/
|
||||||
|
assert_no_migration "db/migrate/create_accounts.rb"
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_model_with_parent_option_database_option_and_no_migration_option
|
||||||
|
with_secondary_database_configuration do
|
||||||
|
run_generator ["account", "--migration", "false", "--database", "secondary", "--migration", "false", "--parent", "Admin::Account"]
|
||||||
|
assert_file "app/models/account.rb", /class Account < Admin::Account/
|
||||||
|
assert_no_migration "db/secondary_migrate/create_accounts.rb"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def test_plural_names_are_singularized
|
def test_plural_names_are_singularized
|
||||||
content = run_generator ["accounts"]
|
content = run_generator ["accounts"]
|
||||||
assert_file "app/models/account.rb", /class Account < ApplicationRecord/
|
assert_file "app/models/account.rb", /class Account < ApplicationRecord/
|
||||||
|
|
|
@ -519,11 +519,16 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_scaffold_generator_database
|
def test_scaffold_generator_multi_db_abstract_class
|
||||||
with_secondary_database_configuration do
|
with_secondary_database_configuration do
|
||||||
run_generator ["posts", "--database=secondary"]
|
run_generator ["posts", "--database=secondary"]
|
||||||
|
|
||||||
assert_migration "db/secondary_migrate/create_posts.rb"
|
assert_migration "db/secondary_migrate/create_posts.rb"
|
||||||
|
assert_file "app/models/secondary_record.rb" do |content|
|
||||||
|
assert_match(/class SecondaryRecord < ApplicationRecord/, content)
|
||||||
|
assert_match(/connects_to database: { writing: :secondary }/, content)
|
||||||
|
assert_match(/self.abstract_class = true/, content)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue