From c1bc7ee82e77ab17916eaeb62d20d0e18a0cccc0 Mon Sep 17 00:00:00 2001 From: Andrew Novoselac Date: Wed, 8 May 2024 07:40:53 -0400 Subject: [PATCH] Refactoring Database generators Extract all the DB information (gems, dockerfile packages, devcontainer etc.) into an object. This let's us remove the growing number of case statements in this code. --- railties/lib/rails/generators/app_base.rb | 13 +- railties/lib/rails/generators/database.rb | 308 +++++++++++++++--- railties/lib/rails/generators/devcontainer.rb | 130 +------- .../templates/config/databases/mysql.yml.tt | 6 +- .../templates/config/databases/trilogy.yml.tt | 6 +- .../rails/app/templates/github/ci.yml.tt | 2 +- .../db/system/change/change_generator.rb | 52 +-- .../test/generators/app_generator_test.rb | 14 +- 8 files changed, 317 insertions(+), 214 deletions(-) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 0daea182bde..5c451f028ca 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -12,7 +12,6 @@ require "active_support/core_ext/array/extract_options" module Rails module Generators class AppBase < Base # :nodoc: - include Database include Devcontainer include AppName @@ -40,7 +39,7 @@ module Rails desc: "Path to some #{name} template (can be a filesystem path or URL)" class_option :database, type: :string, aliases: "-d", default: "sqlite3", - enum: DATABASES, + enum: Database::DATABASES, desc: "Preconfigure for selected database" class_option :skip_git, type: :boolean, aliases: "-G", default: nil, @@ -279,7 +278,7 @@ module Rails def database_gemfile_entry # :doc: return if options[:skip_active_record] - gem_name, gem_version = gem_for_database + gem_name, gem_version = database.gem GemfileEntry.version gem_name, gem_version, "Use #{options[:database]} as the database for Active Record" end @@ -574,7 +573,7 @@ module Rails packages = ["curl"] # ActiveRecord databases - packages << base_package_for_database unless skip_active_record? + packages << database.base_package unless skip_active_record? # ActiveStorage preview support packages << "libvips" unless skip_active_storage? @@ -590,7 +589,7 @@ module Rails packages = %w(build-essential git pkg-config) # add database support - packages << build_package_for_database unless skip_active_record? + packages << database.build_package unless skip_active_record? packages << "unzip" if using_bun? @@ -772,6 +771,10 @@ module Rails directories.sort end + + def database + @database ||= Database.build(options[:database]) + end end end end diff --git a/railties/lib/rails/generators/database.rb b/railties/lib/rails/generators/database.rb index 970e24dd0b8..6be3aa92d10 100644 --- a/railties/lib/rails/generators/database.rb +++ b/railties/lib/rails/generators/database.rb @@ -2,59 +2,84 @@ module Rails module Generators - module Database # :nodoc: + class Database DATABASES = %w( mysql trilogy postgresql sqlite3 ) - def gem_for_database(database = options[:database]) - case database - when "mysql" then ["mysql2", ["~> 0.5"]] - when "trilogy" then ["trilogy", ["~> 2.7"]] - when "postgresql" then ["pg", ["~> 1.1"]] - when "sqlite3" then ["sqlite3", [">= 1.4"]] - else [database, nil] + class << self + def build(database_name) + case database_name + when "mysql" then MySQL.new + when "postgresql" then PostgreSQL.new + when "trilogy" then MariaDB.new + when "sqlite3" then SQLite3.new + else Null.new + end + end + + def all + @all ||= [ + MySQL.new, + PostgreSQL.new, + MariaDB.new, + SQLite3.new, + ] end end - def docker_for_database_base(database = options[:database]) - case database - when "mysql" then "curl default-mysql-client libvips" - when "trilogy" then "curl libvips" - when "postgresql" then "curl libvips postgresql-client" - when "sqlite3" then "curl libsqlite3-0 libvips" - else nil - end + def name + raise NotImplementedError end - def docker_for_database_build(database = options[:database]) - case database - when "mysql" then "build-essential default-libmysqlclient-dev git" - when "trilogy" then "build-essential git" - when "postgresql" then "build-essential git libpq-dev" - when "sqlite3" then "build-essential git" - else nil - end + def service + raise NotImplementedError end - def base_package_for_database(database = options[:database]) - case database - when "mysql" then "default-mysql-client" - when "postgresql" then "postgresql-client" - when "sqlite3" then "libsqlite3-0" - else nil - end + def port + raise NotImplementedError end - def build_package_for_database(database = options[:database]) - case database - when "mysql" then "default-libmysqlclient-dev" - when "postgresql" then "libpq-dev" - else nil - end + def feature_name + raise NotImplementedError end - private - def mysql_socket - @mysql_socket ||= [ + def gem + raise NotImplementedError + end + + def docker_base + raise NotImplementedError + end + + def docker_build + raise NotImplementedError + end + + def base_package + raise NotImplementedError + end + + def build_package + raise NotImplementedError + end + + def socket; end + def host; end + + def feature + return unless feature_name + + { feature_name => {} } + end + + def volume + return unless service + + "#{name}-data" + end + + module MySqlSocket + def socket + @socket ||= [ "/tmp/mysql.sock", # default "/var/run/mysqld/mysqld.sock", # debian/gentoo "/var/tmp/mysql.sock", # freebsd @@ -67,13 +92,204 @@ module Rails ].find { |f| File.exist?(f) } unless Gem.win_platform? end - def mysql_database_host - if options[:skip_devcontainer] - "localhost" - else - "<%= ENV.fetch(\"DB_HOST\") { \"localhost\" } %>" - end + def host + "localhost" end + end + + class MySQL < Database + include MySqlSocket + + def name + "mysql" + end + + def service + { + "image" => "mysql/mysql-server:8.0", + "restart" => "unless-stopped", + "environment" => { + "MYSQL_ALLOW_EMPTY_PASSWORD" => "true", + "MYSQL_ROOT_HOST" => "%" + }, + "volumes" => ["mysql-data:/var/lib/mysql"], + "networks" => ["default"], + } + end + + def port + 3306 + end + + def gem + ["mysql2", ["~> 0.5"]] + end + + def docker_base + "curl default-mysql-client libvips" + end + + def docker_build + "build-essential default-libmysqlclient-dev git" + end + + def base_package + "default-mysql-client" + end + + def build_package + "default-libmysqlclient-dev" + end + + def feature_name + "ghcr.io/rails/devcontainer/features/mysql-client" + end + end + + class PostgreSQL < Database + def name + "postgres" + end + + def service + { + "image" => "postgres:16.1", + "restart" => "unless-stopped", + "networks" => ["default"], + "volumes" => ["postgres-data:/var/lib/postgresql/data"], + "environment" => { + "POSTGRES_USER" => "postgres", + "POSTGRES_PASSWORD" => "postgres" + } + } + end + + def port + 5432 + end + + def gem + ["pg", ["~> 1.1"]] + end + + def docker_base + "curl libvips postgresql-client" + end + + def docker_build + "build-essential git libpq-dev" + end + + def base_package + "postgresql-client" + end + + def build_package + "libpq-dev" + end + + def feature_name + "ghcr.io/rails/devcontainer/features/postgres-client" + end + end + + class MariaDB < Database + include MySqlSocket + + def name + "mariadb" + end + + def service + { + "image" => "mariadb:10.5", + "restart" => "unless-stopped", + "networks" => ["default"], + "volumes" => ["mariadb-data:/var/lib/mysql"], + "environment" => { + "MARIADB_ALLOW_EMPTY_ROOT_PASSWORD" => "true", + }, + } + end + + def port + 3306 + end + + def gem + ["trilogy", ["~> 2.7"]] + end + + def docker_base + "curl libvips" + end + + def docker_build + "build-essential git" + end + + def base_package + nil + end + + def build_package + nil + end + + def feature_name + nil + end + end + + class SQLite3 < Database + def name + "sqlite3" + end + + def service + nil + end + + def port + nil + end + + def gem + ["sqlite3", [">= 1.4"]] + end + + def docker_base + "curl libsqlite3-0 libvips" + end + + def docker_build + "build-essential git" + end + + def base_package + "libsqlite3-0" + end + + def build_package + nil + end + + def feature_name + "ghcr.io/rails/devcontainer/features/sqlite3" + end + end + + class Null < Database + def name; end + def service; end + def port; end + def volume; end + def docker_base; end + def docker_build; end + def base_package; end + def build_package; end + def feature_name; end + end end end end diff --git a/railties/lib/rails/generators/devcontainer.rb b/railties/lib/rails/generators/devcontainer.rb index 73295264bee..1166ff32e7d 100644 --- a/railties/lib/rails/generators/devcontainer.rb +++ b/railties/lib/rails/generators/devcontainer.rb @@ -3,12 +3,6 @@ module Rails module Generators module Devcontainer - DB_FEATURES = { - "mysql" => "ghcr.io/rails/devcontainer/features/mysql-client", - "postgresql" => "ghcr.io/rails/devcontainer/features/postgres-client", - "sqlite3" => "ghcr.io/rails/devcontainer/features/sqlite3" - } - private def devcontainer_dependencies return @devcontainer_dependencies if @devcontainer_dependencies @@ -17,7 +11,7 @@ module Rails @devcontainer_dependencies << "selenium" if depends_on_system_test? @devcontainer_dependencies << "redis" if devcontainer_needs_redis? - @devcontainer_dependencies << db_name_for_devcontainer if db_name_for_devcontainer + @devcontainer_dependencies << database.name if database.service @devcontainer_dependencies end @@ -29,7 +23,7 @@ module Rails @devcontainer_variables["CAPYBARA_SERVER_PORT"] = "45678" if depends_on_system_test? @devcontainer_variables["SELENIUM_HOST"] = "selenium" if depends_on_system_test? @devcontainer_variables["REDIS_URL"] = "redis://redis:6379/1" if devcontainer_needs_redis? - @devcontainer_variables["DB_HOST"] = db_name_for_devcontainer if db_name_for_devcontainer + @devcontainer_variables["DB_HOST"] = database.name if database.service @devcontainer_variables end @@ -40,7 +34,7 @@ module Rails @devcontainer_volumes = [] @devcontainer_volumes << "redis-data" if devcontainer_needs_redis? - @devcontainer_volumes << db_volume_name_for_devcontainer if db_volume_name_for_devcontainer + @devcontainer_volumes << database.volume if database.volume @devcontainer_volumes end @@ -55,7 +49,7 @@ module Rails @devcontainer_features["ghcr.io/rails/devcontainer/features/activestorage"] = {} unless options[:skip_active_storage] @devcontainer_features["ghcr.io/devcontainers/features/node:1"] = {} if using_node? - @devcontainer_features.merge!(db_feature_for_devcontainer) if db_feature_for_devcontainer + @devcontainer_features.merge!(database.feature) if database.feature @devcontainer_features end @@ -74,7 +68,7 @@ module Rails return @devcontainer_forward_ports if @devcontainer_forward_ports @devcontainer_forward_ports = [3000] - @devcontainer_forward_ports << db_port_for_devcontainer if db_port_for_devcontainer + @devcontainer_forward_ports << database.port if database.port @devcontainer_forward_ports << 6379 if devcontainer_needs_redis? @devcontainer_forward_ports @@ -84,120 +78,10 @@ module Rails !(options.skip_action_cable? && options.skip_active_job?) end - def db_port_for_devcontainer(database = options[:database]) - case database - when "mysql", "trilogy" then 3306 - when "postgresql" then 5432 - end - end - - def db_name_for_devcontainer(database = options[:database]) - case database - when "mysql" then "mysql" - when "trilogy" then "mariadb" - when "postgresql" then "postgres" - end - end - - def db_volume_name_for_devcontainer(database = options[:database]) - case database - when "mysql" then "mysql-data" - when "trilogy" then "mariadb-data" - when "postgresql" then "postgres-data" - end - end - - def db_package_for_dockerfile(database = options[:database]) - case database - when "mysql" then "default-libmysqlclient-dev" - when "postgresql" then "libpq-dev" - end - end - def devcontainer_db_service_yaml(**options) - return unless service = db_service_for_devcontainer + return unless service = database.service - service.to_yaml(**options)[4..-1] - end - - def db_service_for_devcontainer(database = options[:database]) - case database - when "mysql" then mysql_service - when "trilogy" then mariadb_service - when "postgresql" then postgres_service - end - end - - def db_feature_for_devcontainer(database = options[:database]) - case database - when "sqlite3" then sqlite3_feature - when "mysql" then mysql_feature - when "postgresql" then postgres_feature - end - end - - def postgres_service - { - "postgres" => { - "image" => "postgres:16.1", - "restart" => "unless-stopped", - "networks" => ["default"], - "volumes" => ["postgres-data:/var/lib/postgresql/data"], - "environment" => { - "POSTGRES_USER" => "postgres", - "POSTGRES_PASSWORD" => "postgres" - } - } - } - end - - def mysql_service - { - "mysql" => { - "image" => "mysql/mysql-server:8.0", - "restart" => "unless-stopped", - "environment" => { - "MYSQL_ALLOW_EMPTY_PASSWORD" => "true", - "MYSQL_ROOT_HOST" => "%" - }, - "volumes" => ["mysql-data:/var/lib/mysql"], - "networks" => ["default"], - } - } - end - - def mariadb_service - { - "mariadb" => { - "image" => "mariadb:10.5", - "restart" => "unless-stopped", - "networks" => ["default"], - "volumes" => ["mariadb-data:/var/lib/mysql"], - "environment" => { - "MARIADB_ALLOW_EMPTY_ROOT_PASSWORD" => "true", - }, - } - } - end - - def db_service_names - ["mysql", "mariadb", "postgres"] - end - - def mysql_feature - { DB_FEATURES["mysql"] => {} } - end - - def postgres_feature - { DB_FEATURES["postgresql"] => {} } - end - - def sqlite3_feature - { DB_FEATURES["sqlite3"] => {} } - end - - def db_features - @db_features ||= DB_FEATURES.values + { database.name => service }.to_yaml(**options)[4..-1] end def local_rails_mount diff --git a/railties/lib/rails/generators/rails/app/templates/config/databases/mysql.yml.tt b/railties/lib/rails/generators/rails/app/templates/config/databases/mysql.yml.tt index ee694081071..e90be60d2f6 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/databases/mysql.yml.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/databases/mysql.yml.tt @@ -15,10 +15,10 @@ default: &default pool: <%%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> username: root password: -<% if mysql_socket -%> - socket: <%= mysql_socket %> +<% if database.socket -%> + socket: <%= database.socket %> <% else -%> - host: <%= mysql_database_host %> + host: <%%= ENV.fetch("DB_HOST") { "<%= database.host %>" } %> <% end -%> development: diff --git a/railties/lib/rails/generators/rails/app/templates/config/databases/trilogy.yml.tt b/railties/lib/rails/generators/rails/app/templates/config/databases/trilogy.yml.tt index b0cd1e73f9c..73b9000f4cc 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/databases/trilogy.yml.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/databases/trilogy.yml.tt @@ -15,10 +15,10 @@ default: &default pool: <%%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> username: root password: -<% if mysql_socket -%> - socket: <%= mysql_socket %> +<% if database.socket -%> + socket: <%= database.socket %> <% else -%> - host: <%= mysql_database_host %> + host: <%%= ENV.fetch("DB_HOST") { "<%= database.host %>" } %> <% end -%> development: diff --git a/railties/lib/rails/generators/rails/app/templates/github/ci.yml.tt b/railties/lib/rails/generators/rails/app/templates/github/ci.yml.tt index 28f75230d63..8649298753c 100644 --- a/railties/lib/rails/generators/rails/app/templates/github/ci.yml.tt +++ b/railties/lib/rails/generators/rails/app/templates/github/ci.yml.tt @@ -100,7 +100,7 @@ jobs: <%- end -%> steps: - name: Install packages - run: sudo apt-get update && sudo apt-get install --no-install-recommends -y google-chrome-stable <%= (dockerfile_base_packages + [build_package_for_database]).join(" ") %> + run: sudo apt-get update && sudo apt-get install --no-install-recommends -y google-chrome-stable <%= (dockerfile_base_packages + [database.base_package]).join(" ") %> - name: Checkout code uses: actions/checkout@v4 diff --git a/railties/lib/rails/generators/rails/db/system/change/change_generator.rb b/railties/lib/rails/generators/rails/db/system/change/change_generator.rb index b9b76cc38a2..6ef6201e0da 100644 --- a/railties/lib/rails/generators/rails/db/system/change/change_generator.rb +++ b/railties/lib/rails/generators/rails/db/system/change/change_generator.rb @@ -9,8 +9,6 @@ module Rails module Db module System class ChangeGenerator < Base # :nodoc: - include Database - include Devcontainer include AppName class_option :to, required: true, @@ -24,8 +22,8 @@ module Rails def initialize(*) super - unless DATABASES.include?(options[:to]) - raise Error, "Invalid value for --to option. Supported preconfigurations are: #{DATABASES.join(", ")}." + unless Database::DATABASES.include?(options[:to]) + raise Error, "Invalid value for --to option. Supported preconfigurations are: #{Database::DATABASES.join(", ")}." end opt = options.dup @@ -38,7 +36,7 @@ module Rails end def edit_gemfile - name, version = gem_for_database + name, version = database.gem gsub_file("Gemfile", all_database_gems_regex, name) gsub_file("Gemfile", gem_entry_regex_for(name), gem_entry_for(name, *version)) end @@ -47,8 +45,8 @@ module Rails dockerfile_path = File.expand_path("Dockerfile", destination_root) return unless File.exist?(dockerfile_path) - base_name = docker_for_database_base - build_name = docker_for_database_build + base_name = database.docker_base + build_name = database.docker_build if base_name gsub_file("Dockerfile", all_docker_bases_regex, base_name) end @@ -67,15 +65,15 @@ module Rails private def all_database_gems - DATABASES.map { |database| gem_for_database(database) } + Database.all.map { |database| database.gem } end def all_docker_bases - DATABASES.filter_map { |database| docker_for_database_base(database) } + Database.all.filter_map { |database| database.docker_base } end def all_docker_builds - DATABASES.filter_map { |database| docker_for_database_build(database) } + Database.all.filter_map { |database| database.docker_build } end def all_database_gems_regex @@ -113,19 +111,17 @@ module Rails compose_config = YAML.load_file(compose_yaml_path) - db_service_names.each do |db_service_name| - compose_config["services"].delete(db_service_name) - compose_config["volumes"]&.delete("#{db_service_name}-data") - compose_config["services"]["rails-app"]["depends_on"]&.delete(db_service_name) + Database.all.each do |database| + compose_config["services"].delete(database.name) + compose_config["volumes"]&.delete(database.volume) + compose_config["services"]["rails-app"]["depends_on"]&.delete(database.name) end - db_service = db_service_for_devcontainer - - if db_service - compose_config["services"].merge!(db_service) - compose_config["volumes"] = { db_volume_name_for_devcontainer => nil }.merge(compose_config["volumes"] || {}) + if database.service + compose_config["services"][database.name] = database.service + compose_config["volumes"] = { database.volume => nil }.merge(compose_config["volumes"] || {}) compose_config["services"]["rails-app"]["depends_on"] = [ - db_name_for_devcontainer, + database.name, compose_config["services"]["rails-app"]["depends_on"] ].flatten.compact end @@ -138,16 +134,16 @@ module Rails def update_devcontainer_db_host container_env = devcontainer_json["containerEnv"] - db_name = db_name_for_devcontainer + db_name = database.name if container_env["DB_HOST"] - if db_name + if database.service container_env["DB_HOST"] = db_name else container_env.delete("DB_HOST") end else - if db_name + if database.service container_env["DB_HOST"] = db_name end end @@ -159,10 +155,10 @@ module Rails def update_devcontainer_db_feature features = devcontainer_json["features"] - db_feature = db_feature_for_devcontainer + db_feature = database.feature - db_features.each do |feature| - features.delete(feature) + Database.all.each do |database| + features.delete(database.feature_name) end features.merge!(db_feature) if db_feature @@ -181,6 +177,10 @@ module Rails def devcontainer_json_path File.expand_path(".devcontainer/devcontainer.json", destination_root) end + + def database + @database ||= Database.build(options[:database]) + end end end end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 321b1f97f86..177bcbde7c1 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -1240,9 +1240,9 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_equal "redis://redis:6379/1", content["containerEnv"]["REDIS_URL"] assert_equal "45678", content["containerEnv"]["CAPYBARA_SERVER_PORT"] assert_equal "selenium", content["containerEnv"]["SELENIUM_HOST"] - assert_equal({}, content["features"]["ghcr.io/rails/devcontainer/features/activestorage"]) - assert_equal({}, content["features"]["ghcr.io/devcontainers/features/github-cli:1"]) - assert_equal({}, content["features"]["ghcr.io/rails/devcontainer/features/sqlite3"]) + assert_includes content["features"].keys, "ghcr.io/rails/devcontainer/features/activestorage" + assert_includes content["features"].keys, "ghcr.io/devcontainers/features/github-cli:1" + assert_includes content["features"].keys, "ghcr.io/rails/devcontainer/features/sqlite3" assert_includes(content["forwardPorts"], 3000) assert_includes(content["forwardPorts"], 6379) end @@ -1320,8 +1320,8 @@ class AppGeneratorTest < Rails::Generators::TestCase end assert_devcontainer_json_file do |content| assert_equal "postgres", content["containerEnv"]["DB_HOST"] - assert_equal({}, content["features"]["ghcr.io/rails/devcontainer/features/postgres-client"]) - assert_includes(content["forwardPorts"], 5432) + assert_includes content["features"].keys, "ghcr.io/rails/devcontainer/features/postgres-client" + assert_includes content["forwardPorts"], 5432 end assert_file("config/database.yml") do |content| assert_match(/host: <%= ENV\["DB_HOST"\] %>/, content) @@ -1350,8 +1350,8 @@ class AppGeneratorTest < Rails::Generators::TestCase end assert_devcontainer_json_file do |content| assert_equal "mysql", content["containerEnv"]["DB_HOST"] - assert_equal({}, content["features"]["ghcr.io/rails/devcontainer/features/mysql-client"]) - assert_includes(content["forwardPorts"], 3306) + assert_includes content["features"].keys, "ghcr.io/rails/devcontainer/features/mysql-client" + assert_includes content["forwardPorts"], 3306 end assert_file("config/database.yml") do |content| assert_match(/host: <%= ENV.fetch\("DB_HOST"\) \{ "localhost" } %>/, content)