diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7704685..8a7c089 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,16 +50,16 @@ jobs: - activerecord_6.1 - activerecord_edge adapter: - - 'sqlite3:///:memory:' + - sqlite3:///tmp/closure_tree_test - mysql2://root:root@0/closure_tree_test - - postgres://closure_tree:closure_tree@0/closure_tree_test + - postgres://postgres:postgres@0/closure_tree_test exclude: - ruby: '3.0' rails: activerecord_edge steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup Ruby uses: ruby/setup-ruby@v1 @@ -74,7 +74,8 @@ jobs: - name: RSpec env: RAILS_VERSION: ${{ matrix.rails }} - DB_ADAPTER: ${{ matrix.adapter }} + DATABASE_URL: ${{ matrix.adapter }} + SECONDARY_DATABASE_URL: ${{ matrix.adapter }}_secondary BUNDLE_GEMFILE: gemfiles/${{ matrix.rails }}.gemfile WITH_ADVISORY_LOCK_PREFIX: ${{ github.run_id }} run: bin/rake diff --git a/.github/workflows/ci_jruby.yml b/.github/workflows/ci_jruby.yml index c7d0156..558fbcc 100644 --- a/.github/workflows/ci_jruby.yml +++ b/.github/workflows/ci_jruby.yml @@ -43,12 +43,12 @@ jobs: - activerecord_7.0 - activerecord_6.1 adapter: - - 'sqlite3:///:memory:' + - sqlite3:///tmp/closure_tree_test - mysql2://root:root@0/closure_tree_test - - postgres://closure_tree:closure_tree@0/closure_tree_test + - postgres://postgres:postgres@0/closure_tree_test steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup Ruby uses: ruby/setup-ruby@v1 @@ -63,7 +63,8 @@ jobs: - name: RSpec env: RAILS_VERSION: ${{ matrix.rails }} - DB_ADAPTER: ${{ matrix.adapter }} + DATABASE_URL: ${{ matrix.adapter }} + SECONDARY_DATABASE_URL: ${{ matrix.adapter }}_secondary BUNDLE_GEMFILE: gemfiles/${{ matrix.rails }}.gemfile WITH_ADVISORY_LOCK_PREFIX: ${{ github.run_id }} run: bin/rake diff --git a/.github/workflows/ci_truffleruby.yml b/.github/workflows/ci_truffleruby.yml index b61630f..77bf270 100644 --- a/.github/workflows/ci_truffleruby.yml +++ b/.github/workflows/ci_truffleruby.yml @@ -45,13 +45,13 @@ jobs: - activerecord_7.0 - activerecord_6.1 adapter: - - 'sqlite3:///:memory:' + - sqlite3:///tmp/closure_tree_test - mysql2://root:root@0/closure_tree_test - - postgres://closure_tree:closure_tree@0/closure_tree_test + - postgres://postgres:postgres@0/closure_tree_test steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup Ruby uses: ruby/setup-ruby@v1 @@ -66,7 +66,8 @@ jobs: - name: RSpec env: RAILS_VERSION: ${{ matrix.rails }} - DB_ADAPTER: ${{ matrix.adapter }} + DATABASE_URL: ${{ matrix.adapter }} + SECONDARY_DATABASE_URL: ${{ matrix.adapter }}_secondary BUNDLE_GEMFILE: gemfiles/${{ matrix.rails }}.gemfile WITH_ADVISORY_LOCK_PREFIX: ${{ github.run_id }} run: bin/rake diff --git a/.gitignore b/.gitignore index 2f6900c..e9f555f 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ tmp/ .ruby-* *.iml coverage/ +.env diff --git a/lib/closure_tree/has_closure_tree.rb b/lib/closure_tree/has_closure_tree.rb index b0bc5b1..a45462c 100644 --- a/lib/closure_tree/has_closure_tree.rb +++ b/lib/closure_tree/has_closure_tree.rb @@ -31,10 +31,6 @@ def has_closure_tree(options = {}) include ClosureTree::DeterministicOrdering if _ct.order_option? include ClosureTree::NumericDeterministicOrdering if _ct.order_is_numeric? - - connection_pool.release_connection - rescue StandardError => e - raise e unless ClosureTree.configuration.database_less end alias_method :acts_as_tree, :has_closure_tree diff --git a/lib/closure_tree/has_closure_tree_root.rb b/lib/closure_tree/has_closure_tree_root.rb index 70e327d..26f9d53 100644 --- a/lib/closure_tree/has_closure_tree_root.rb +++ b/lib/closure_tree/has_closure_tree_root.rb @@ -77,8 +77,6 @@ def has_closure_tree_root(assoc_name, options = {}) @closure_tree_roots[assoc_name][assoc_map] = root end - - connection_pool.release_connection end end end diff --git a/lib/closure_tree/numeric_order_support.rb b/lib/closure_tree/numeric_order_support.rb index 0223a41..351720a 100644 --- a/lib/closure_tree/numeric_order_support.rb +++ b/lib/closure_tree/numeric_order_support.rb @@ -1,53 +1,42 @@ module ClosureTree module NumericOrderSupport - - def self.adapter_for_connection(connection) - das = WithAdvisoryLock::DatabaseAdapterSupport.new(connection) - if das.postgresql? - ::ClosureTree::NumericOrderSupport::PostgreSQLAdapter - elsif das.mysql? - ::ClosureTree::NumericOrderSupport::MysqlAdapter - else - ::ClosureTree::NumericOrderSupport::GenericAdapter - end - end - module MysqlAdapter - def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil) - return if parent_id.nil? && dont_order_roots + module_function def reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil) + return if parent_id.nil? && ct.dont_order_roots + min_where = if minimum_sort_order_value - "AND #{quoted_order_column} >= #{minimum_sort_order_value}" + "AND #{ct.quoted_order_column} >= #{minimum_sort_order_value}" else "" end - connection.execute 'SET @i = 0' - connection.execute <<-SQL.squish - UPDATE #{quoted_table_name} - SET #{quoted_order_column} = (@i := @i + 1) + #{minimum_sort_order_value.to_i - 1} - WHERE #{where_eq(parent_column_name, parent_id)} #{min_where} - ORDER BY #{nulls_last_order_by} + ct.connection.execute 'SET @i = 0' + ct.connection.execute <<-SQL.squish + UPDATE #{ct.quoted_table_name} + SET #{ct.quoted_order_column} = (@i := @i + 1) + #{minimum_sort_order_value.to_i - 1} + WHERE #{ct.where_eq(ct.parent_column_name, parent_id)} #{min_where} + ORDER BY #{ct.nulls_last_order_by} SQL end end module PostgreSQLAdapter - def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil) - return if parent_id.nil? && dont_order_roots + module_function def reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil) + return if parent_id.nil? && ct.dont_order_roots min_where = if minimum_sort_order_value - "AND #{quoted_order_column} >= #{minimum_sort_order_value}" + "AND #{ct.quoted_order_column} >= #{minimum_sort_order_value}" else "" end - connection.execute <<-SQL.squish - UPDATE #{quoted_table_name} - SET #{quoted_order_column(false)} = t.seq + #{minimum_sort_order_value.to_i - 1} + ct.connection.execute <<-SQL.squish + UPDATE #{ct.quoted_table_name} + SET #{ct.quoted_order_column(false)} = t.seq + #{minimum_sort_order_value.to_i - 1} FROM ( - SELECT #{quoted_id_column_name} AS id, row_number() OVER(ORDER BY #{order_by}) AS seq - FROM #{quoted_table_name} - WHERE #{where_eq(parent_column_name, parent_id)} #{min_where} + SELECT #{ct.quoted_id_column_name} AS id, row_number() OVER(ORDER BY #{ct.order_by}) AS seq + FROM #{ct.quoted_table_name} + WHERE #{ct.where_eq(ct.parent_column_name, parent_id)} #{min_where} ) AS t - WHERE #{quoted_table_name}.#{quoted_id_column_name} = t.id and - #{quoted_table_name}.#{quoted_order_column(false)} is distinct from t.seq + #{minimum_sort_order_value.to_i - 1} + WHERE #{ct.quoted_table_name}.#{ct.quoted_id_column_name} = t.id and + #{ct.quoted_table_name}.#{ct.quoted_order_column(false)} is distinct from t.seq + #{minimum_sort_order_value.to_i - 1} SQL end @@ -57,18 +46,31 @@ def rows_updated(result) end module GenericAdapter - def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil) - return if parent_id.nil? && dont_order_roots - scope = model_class. - where(parent_column_sym => parent_id). - order(nulls_last_order_by) + module_function def reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil) + return if parent_id.nil? && ct.dont_order_roots + binding.irb + scope = ct. + where(ct.parent_column_sym => parent_id). + order(ct.nulls_last_order_by) if minimum_sort_order_value - scope = scope.where("#{quoted_order_column} >= #{minimum_sort_order_value}") + scope = scope.where("#{ct.quoted_order_column} >= #{minimum_sort_order_value}") end scope.each_with_index do |ea, idx| ea.update_order_value(idx + minimum_sort_order_value.to_i) end end end + + + module_function def adapter_for_connection(ct, parent_id, minimum_sort_order_value = nil) + das = WithAdvisoryLock::DatabaseAdapterSupport.new(ct.connection) + if das.postgresql? + PostgreSQLAdapter.reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil) + elsif das.mysql? + MysqlAdapter.reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil) + else + GenericAdapter.reorder_with_parent_id(ct, parent_id, minimum_sort_order_value = nil) + end + end end end diff --git a/lib/closure_tree/support.rb b/lib/closure_tree/support.rb index e887090..0e406bd 100644 --- a/lib/closure_tree/support.rb +++ b/lib/closure_tree/support.rb @@ -26,9 +26,6 @@ def initialize(model_class, options) :numeric_order => false }.merge(options) raise ArgumentError, "name_column can't be 'path'" if options[:name_column] == 'path' - if order_is_numeric? - extend NumericOrderSupport.adapter_for_connection(connection) - end end def hierarchy_class_for_model @@ -54,6 +51,10 @@ def hash hierarchy_class end + def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil) + NumericOrderSupport.adapter_for_connection(self, parent_id, minimum_sort_order_value) + end + def hierarchy_table_name # We need to use the table_name, not something like ct_class.to_s.demodulize + "_hierarchies", # because they may have overridden the table name, which is what we want to be consistent with diff --git a/lib/closure_tree/support_attributes.rb b/lib/closure_tree/support_attributes.rb index 38879ad..5ec8ef4 100644 --- a/lib/closure_tree/support_attributes.rb +++ b/lib/closure_tree/support_attributes.rb @@ -117,7 +117,7 @@ def quoted_order_column(include_table_name = true) # table_name alias keyword , like "AS". When used on table name alias, Oracle Database don't support used 'AS' def t_alias_keyword - (ActiveRecord::Base.connection.adapter_name.to_sym == :OracleEnhanced) ? "" : "AS" + (connection.adapter_name.to_sym == :OracleEnhanced) ? "" : "AS" end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index eba943f..1803df2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true - require 'database_cleaner' require 'closure_tree/test/matcher' require 'tmpdir' @@ -11,6 +10,7 @@ require 'active_record' require 'active_support/core_ext/array' +puts "Using ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}" # Start Simplecov if RUBY_ENGINE == 'ruby' @@ -20,10 +20,23 @@ end end +primary_database_url = ENV['DATABASE_URL'].presence || "sqlite3:///tmp/closure_tree_test" +secondary_database_url = ENV['SECONDARY_DATABASE_URL'].presence || "sqlite3:///tmp/closure_tree_test-s" + +puts "Using primary database #{primary_database_url}" +puts "Using secondary database #{secondary_database_url}" + ActiveRecord::Base.configurations = { default_env: { - url: ENV.fetch('DATABASE_URL', "sqlite3::memory:"), - properties: { allowPublicKeyRetrieval: true } # for JRuby madness + primary: { + primary: true, + url: primary_database_url, + properties: { allowPublicKeyRetrieval: true } # for JRuby madness + }, + secondary: { + url: secondary_database_url, + properties: { allowPublicKeyRetrieval: true } # for JRuby madness + } } } @@ -31,14 +44,9 @@ ActiveRecord::Migration.verbose = false ActiveRecord::Base.table_name_prefix = ENV['DB_PREFIX'].to_s ActiveRecord::Base.table_name_suffix = ENV['DB_SUFFIX'].to_s -ActiveRecord::Base.establish_connection def env_db - @env_db ||= if ActiveRecord::Base.respond_to?(:connection_db_config) - ActiveRecord::Base.connection_db_config.adapter - else - ActiveRecord::Base.connection_config[:adapter] - end.to_sym + @env_db ||= ActiveRecord::Base.connection_db_config.adapter.to_sym end # Use in specs to skip some tests @@ -73,15 +81,12 @@ def sqlite? # disable monkey patching # see: https://relishapp.com/rspec/rspec-core/v/3-8/docs/configuration/zero-monkey-patching-mode config.disable_monkey_patching! + config.before(:suite) do + ENV['FLOCK_DIR'] = Dir.mktmpdir if sqlite? + end - if sqlite? - config.before(:suite) do - ENV['FLOCK_DIR'] = Dir.mktmpdir - end - - config.after(:suite) do - FileUtils.remove_entry_secure ENV['FLOCK_DIR'] - end + config.after(:suite) do + FileUtils.remove_entry_secure(ENV['FLOCK_DIR']) if sqlite? end end @@ -92,14 +97,12 @@ def sqlite? # See: https://github.com/ClosureTree/with_advisory_lock ENV['WITH_ADVISORY_LOCK_PREFIX'] ||= SecureRandom.hex -ActiveRecord::Base.connection.recreate_database("closure_tree_test") unless sqlite? -puts "Testing with #{env_db} database, ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}" # Require our gem require 'closure_tree' # Load test helpers require_relative 'support/schema' -require_relative 'support/models' require_relative 'support/helpers' require_relative 'support/exceed_query_limit' require_relative 'support/query_counter' +puts "Testing with #{env_db} database" diff --git a/spec/support/exceed_query_limit.rb b/spec/support/exceed_query_limit.rb index b167a82..a8d7cf9 100644 --- a/spec/support/exceed_query_limit.rb +++ b/spec/support/exceed_query_limit.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Derived from http://stackoverflow.com/a/13423584/153896. Updated for RSpec 3. RSpec::Matchers.define :exceed_query_limit do |expected| supports_block_expectations @@ -6,7 +8,7 @@ query_count(&block) > expected end - failure_message_when_negated do |actual| + failure_message_when_negated do |_actual| "Expected to run maximum #{expected} queries, got #{@counter.query_count}" end diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 1404714..d303eba 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -1,7 +1,9 @@ +# frozen_string_literal: true + # See http://stackoverflow.com/a/22388177/1268016 def count_queries(&block) count = 0 - counter_fn = ->(name, started, finished, unique_id, payload) do + counter_fn = lambda do |_name, _started, _finished, _unique_id, payload| count += 1 unless %w[CACHE SCHEMA].include? payload[:name] end ActiveSupport::Notifications.subscribed(counter_fn, 'sql.active_record', &block) diff --git a/spec/support/models.rb b/spec/support/models.rb index 9927af4..cb937a1 100644 --- a/spec/support/models.rb +++ b/spec/support/models.rb @@ -1,5 +1,20 @@ -class Tag < ActiveRecord::Base - has_closure_tree :dependent => :destroy, :order => :name +# frozen_string_literal: true + + + +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true + + connects_to database: { writing: :primary, reading: :primary } +end + +class SecondDatabaseRecord < ActiveRecord::Base + self.abstract_class = true + + connects_to database: { writing: :secondary, reading: :secondary } +end +class Tag < ApplicationRecord + has_closure_tree dependent: :destroy, order: :name before_destroy :add_destroyed_tag def to_s @@ -8,11 +23,11 @@ def to_s def add_destroyed_tag # Proof for the tests that the destroy rather than the delete method was called: - DestroyedTag.create(:name => name) + DestroyedTag.create(name: to_s) end end -class UUIDTag < ActiveRecord::Base +class UUIDTag < ApplicationRecord self.primary_key = :uuid before_create :set_uuid has_closure_tree dependent: :destroy, order: 'name', parent_column_name: 'parent_uuid' @@ -28,40 +43,40 @@ def to_s def add_destroyed_tag # Proof for the tests that the destroy rather than the delete method was called: - DestroyedTag.create(:name => name) + DestroyedTag.create(name: to_s) end end -class DestroyedTag < ActiveRecord::Base +class DestroyedTag < ApplicationRecord end -class Group < ActiveRecord::Base +class Group < ApplicationRecord has_closure_tree_root :root_user end -class Grouping < ActiveRecord::Base - has_closure_tree_root :root_person, class_name: "User", foreign_key: :group_id +class Grouping < ApplicationRecord + has_closure_tree_root :root_person, class_name: 'User', foreign_key: :group_id end -class UserSet < ActiveRecord::Base - has_closure_tree_root :root_user, class_name: "Useur" +class UserSet < ApplicationRecord + has_closure_tree_root :root_user, class_name: 'Useur' end -class Team < ActiveRecord::Base - has_closure_tree_root :root_user, class_name: "User", foreign_key: :grp_id +class Team < ApplicationRecord + has_closure_tree_root :root_user, class_name: 'User', foreign_key: :grp_id end -class User < ActiveRecord::Base - acts_as_tree :parent_column_name => "referrer_id", - :name_column => 'email', - :hierarchy_class_name => 'ReferralHierarchy', - :hierarchy_table_name => 'referral_hierarchies' +class User < ApplicationRecord + acts_as_tree parent_column_name: 'referrer_id', + name_column: 'email', + hierarchy_class_name: 'ReferralHierarchy', + hierarchy_table_name: 'referral_hierarchies' has_many :contracts, inverse_of: :user belongs_to :group # Can't use and don't need inverse_of here when using has_closure_tree_root. def indirect_contracts - Contract.where(:user_id => descendant_ids) + Contract.where(user_id: descendant_ids) end def to_s @@ -69,21 +84,21 @@ def to_s end end -class Contract < ActiveRecord::Base +class Contract < ApplicationRecord belongs_to :user, inverse_of: :contracts belongs_to :contract_type, inverse_of: :contracts end -class ContractType < ActiveRecord::Base +class ContractType < ApplicationRecord has_many :contracts, inverse_of: :contract_type end -class Label < ActiveRecord::Base +class Label < ApplicationRecord # make sure order doesn't matter - acts_as_tree :order => :column_whereby_ordering_is_inferred, # <- symbol, and not "sort_order" - :numeric_order => true, - :parent_column_name => "mother_id", - :dependent => :destroy + acts_as_tree order: :column_whereby_ordering_is_inferred, # <- symbol, and not "sort_order" + numeric_order: true, + parent_column_name: 'mother_id', + dependent: :destroy def to_s "#{self.class}: #{name}" @@ -99,13 +114,13 @@ class DateLabel < Label class DirectoryLabel < Label end -class LabelWithoutRootOrdering < ActiveRecord::Base +class LabelWithoutRootOrdering < ApplicationRecord # make sure order doesn't matter - acts_as_tree :order => :column_whereby_ordering_is_inferred, # <- symbol, and not "sort_order" - :numeric_order => true, - :dont_order_roots => true, - :parent_column_name => "mother_id", - :hierarchy_table_name => "label_hierarchies" + acts_as_tree order: :column_whereby_ordering_is_inferred, # <- symbol, and not "sort_order" + numeric_order: true, + dont_order_roots: true, + parent_column_name: 'mother_id', + hierarchy_table_name: 'label_hierarchies' self.table_name = "#{table_name_prefix}labels#{table_name_suffix}" @@ -114,7 +129,7 @@ def to_s end end -class CuisineType < ActiveRecord::Base +class CuisineType < ApplicationRecord acts_as_tree end @@ -122,12 +137,13 @@ module Namespace def self.table_name_prefix 'namespace_' end - class Type < ActiveRecord::Base + + class Type < ApplicationRecord has_closure_tree dependent: :destroy end end -class Metal < ActiveRecord::Base +class Metal < ApplicationRecord self.table_name = "#{table_name_prefix}metal#{table_name_suffix}" has_closure_tree order: 'sort_order', name_column: 'value' self.inheritance_column = 'metal_type' @@ -139,6 +155,6 @@ class Adamantium < Metal class Unobtanium < Metal end -class MenuItem < ActiveRecord::Base +class MenuItem < SecondDatabaseRecord has_closure_tree touch: true, with_advisory_lock: false end diff --git a/spec/support/query_counter.rb b/spec/support/query_counter.rb index f36cfc0..44c9864 100644 --- a/spec/support/query_counter.rb +++ b/spec/support/query_counter.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # From http://stackoverflow.com/a/13423584/153896 module ActiveRecord class QueryCounter @@ -11,8 +13,8 @@ def to_proc lambda(&method(:callback)) end - def callback(name, start, finish, message_id, values) - @query_count += 1 unless %w(CACHE SCHEMA).include?(values[:name]) + def callback(_name, _start, _finish, _message_id, values) + @query_count += 1 unless %w[CACHE SCHEMA].include?(values[:name]) end end end diff --git a/spec/support/schema.rb b/spec/support/schema.rb index 30a4a99..95cbdf1 100644 --- a/spec/support/schema.rb +++ b/spec/support/schema.rb @@ -1,7 +1,21 @@ # frozen_string_literal: true +require_relative 'models' +ApplicationRecord.establish_connection +if sqlite? + ActiveRecord::Tasks::DatabaseTasks.drop(:primary, 'test') + ActiveRecord::Tasks::DatabaseTasks.create(:primary, 'test') + ActiveRecord::Tasks::DatabaseTasks.drop(:secondary, 'test') + ActiveRecord::Tasks::DatabaseTasks.create(:secondary, 'test') +else + ActiveRecord::Tasks::DatabaseTasks.drop(:primary) + ActiveRecord::Tasks::DatabaseTasks.create(:primary) + ActiveRecord::Tasks::DatabaseTasks.drop(:secondary) + ActiveRecord::Tasks::DatabaseTasks.create(:secondary) +end + ActiveRecord::Schema.define(version: 0) do - create_table 'tags', force: :cascade do |t| + connection.create_table Tag.table_name, force: :cascade do |t| t.string 'name' t.string 'title' t.references 'parent' @@ -9,13 +23,17 @@ t.timestamps null: false end - create_table 'tag_hierarchies', id: false, force: :cascade do |t| + create_table Tag.hierarchy_class.table_name, id: false, force: :cascade do |t| t.references 'ancestor', null: false t.references 'descendant', null: false t.integer 'generations', null: false end - create_table 'uuid_tags', id: false, force: :cascade do |t| + add_index Tag.hierarchy_class.table_name, %i[ancestor_id descendant_id generations], unique: true, + name: 'tag_anc_desc_idx' + add_index Tag.hierarchy_class.table_name, [:descendant_id], name: 'tag_desc_idx' + + create_table UUIDTag.table_name, id: false, force: :cascade do |t| t.string 'uuid', primary_key: true t.string 'name' t.string 'title' @@ -24,95 +42,99 @@ t.timestamps null: false end - create_table 'uuid_tag_hierarchies', id: false, force: :cascade do |t| + create_table UUIDTag.hierarchy_class.table_name, id: false, force: :cascade do |t| t.string 'ancestor_id', null: false t.string 'descendant_id', null: false t.integer 'generations', null: false end - create_table 'destroyed_tags', force: :cascade do |t| + create_table DestroyedTag.table_name, force: :cascade do |t| t.string 'name' end - add_index 'tag_hierarchies', %i[ancestor_id descendant_id generations], unique: true, - name: 'tag_anc_desc_idx' - add_index 'tag_hierarchies', [:descendant_id], name: 'tag_desc_idx' - - create_table 'groups', force: :cascade do |t| + create_table Group.table_name, force: :cascade do |t| t.string 'name', null: false end - create_table 'groupings', force: :cascade do |t| + create_table Grouping.table_name, force: :cascade do |t| t.string 'name', null: false end - create_table 'user_sets', force: :cascade do |t| + create_table UserSet.table_name, force: :cascade do |t| t.string 'name', null: false end - create_table 'teams', force: :cascade do |t| + create_table Team.table_name, force: :cascade do |t| t.string 'name', null: false end - create_table 'users', force: :cascade do |t| + create_table User.table_name, force: :cascade do |t| t.string 'email' t.references 'referrer' t.integer 'group_id' t.timestamps null: false end - create_table 'contracts', force: :cascade do |t| + create_table User.hierarchy_class.table_name, id: false, force: :cascade do |t| + t.references 'ancestor', null: false + t.references 'descendant', null: false + t.integer 'generations', null: false + end + + add_index User.hierarchy_class.table_name, %i[ancestor_id descendant_id generations], unique: true, + name: 'ref_anc_desc_idx' + add_index User.hierarchy_class.table_name, [:descendant_id], name: 'ref_desc_idx' + + create_table Contract.table_name, force: :cascade do |t| t.references 'user', null: false t.references 'contract_type' t.string 'title' end - create_table 'contract_types', force: :cascade do |t| + create_table ContractType.table_name, force: :cascade do |t| t.string 'name', null: false end - create_table 'referral_hierarchies', id: false, force: :cascade do |t| - t.references 'ancestor', null: false - t.references 'descendant', null: false - t.integer 'generations', null: false - end - - create_table 'labels', force: :cascade do |t| + create_table Label.table_name, force: :cascade do |t| t.string 'name' t.string 'type' t.integer 'column_whereby_ordering_is_inferred' t.references 'mother' end - create_table 'label_hierarchies', id: false, force: :cascade do |t| + create_table Label.hierarchy_class.table_name, id: false, force: :cascade do |t| t.references 'ancestor', null: false t.references 'descendant', null: false t.integer 'generations', null: false end - create_table 'cuisine_types', force: :cascade do |t| + add_index Label.hierarchy_class.table_name, %i[ancestor_id descendant_id generations], unique: true, + name: 'lh_anc_desc_idx' + add_index Label.hierarchy_class.table_name, [:descendant_id], name: 'lh_desc_idx' + + create_table CuisineType.table_name, force: :cascade do |t| t.string 'name' t.references 'parent' end - create_table 'cuisine_type_hierarchies', id: false, force: :cascade do |t| + create_table CuisineType.hierarchy_class.table_name, id: false, force: :cascade do |t| t.references 'ancestor', null: false t.references 'descendant', null: false t.integer 'generations', null: false end - create_table 'namespace_types', force: :cascade do |t| + create_table Namespace::Type.table_name, force: :cascade do |t| t.string 'name' t.references 'parent' end - create_table 'namespace_type_hierarchies', id: false, force: :cascade do |t| + create_table Namespace::Type.hierarchy_class.table_name, id: false, force: :cascade do |t| t.references 'ancestor', null: false t.references 'descendant', null: false t.integer 'generations', null: false end - create_table 'metal', force: :cascade do |t| + create_table Metal.table_name, force: :cascade do |t| t.references 'parent' t.string 'metal_type' t.string 'value' @@ -120,38 +142,38 @@ t.integer 'sort_order' end - create_table 'metal_hierarchies', id: false, force: :cascade do |t| + create_table Metal.hierarchy_class.table_name, id: false, force: :cascade do |t| t.references 'ancestor', null: false t.references 'descendant', null: false t.integer 'generations', null: false end - create_table 'menu_items', force: :cascade do |t| - t.string 'name' - t.references 'parent' - t.timestamps null: false - end + add_foreign_key(Tag.table_name, Tag.table_name, column: 'parent_id', on_delete: :cascade) + add_foreign_key(User.table_name, User.table_name, column: 'referrer_id', on_delete: :cascade) + add_foreign_key(Label.table_name, Label.table_name, column: 'mother_id', on_delete: :cascade) + add_foreign_key(Metal.table_name, Metal.table_name, column: 'parent_id', on_delete: :cascade) + add_foreign_key(Tag.hierarchy_class.table_name, Tag.table_name, column: 'ancestor_id', on_delete: :cascade) + add_foreign_key(Tag.hierarchy_class.table_name, Tag.table_name, column: 'descendant_id', on_delete: :cascade) +end - create_table 'menu_item_hierarchies', id: false, force: :cascade do |t| - t.references 'ancestor', null: false - t.references 'descendant', null: false - t.integer 'generations', null: false +SecondDatabaseRecord.establish_connection +SecondDatabaseRecord.connection_pool.with_connection do |connection| + ActiveRecord::Schema.define(version: 0) do + connection.create_table MenuItem.table_name, force: :cascade do |t| + t.string 'name' + t.references 'parent' + t.timestamps null: false + end + + connection.create_table MenuItem.hierarchy_class.table_name, id: false, force: :cascade do |t| + t.references 'ancestor', null: false + t.references 'descendant', null: false + t.integer 'generations', null: false + end + connection.add_foreign_key(MenuItem.table_name, MenuItem.table_name, column: 'parent_id', on_delete: :cascade) + connection.add_foreign_key(MenuItem.hierarchy_class.table_name, MenuItem.table_name, column: 'ancestor_id', + on_delete: :cascade) + connection.add_foreign_key(MenuItem.hierarchy_class.table_name, MenuItem.table_name, column: 'descendant_id', + on_delete: :cascade) end - - add_index 'label_hierarchies', %i[ancestor_id descendant_id generations], unique: true, - name: 'lh_anc_desc_idx' - add_index 'label_hierarchies', [:descendant_id], name: 'lh_desc_idx' - add_index 'referral_hierarchies', %i[ancestor_id descendant_id generations], unique: true, - name: 'ref_anc_desc_idx' - add_index 'referral_hierarchies', [:descendant_id], name: 'ref_desc_idx' - - add_foreign_key(:tags, :tags, column: 'parent_id', on_delete: :cascade) - add_foreign_key(:users, :users, column: 'referrer_id', on_delete: :cascade) - add_foreign_key(:labels, :labels, column: 'mother_id', on_delete: :cascade) - add_foreign_key(:metal, :metal, column: 'parent_id', on_delete: :cascade) - add_foreign_key(:menu_items, :menu_items, column: 'parent_id', on_delete: :cascade) - add_foreign_key(:menu_item_hierarchies, :menu_items, column: 'ancestor_id', on_delete: :cascade) - add_foreign_key(:menu_item_hierarchies, :menu_items, column: 'descendant_id', on_delete: :cascade) - add_foreign_key(:tag_hierarchies, :tags, column: 'ancestor_id', on_delete: :cascade) - add_foreign_key(:tag_hierarchies, :tags, column: 'descendant_id', on_delete: :cascade) end diff --git a/test/closure_tree/model_test.rb b/test/closure_tree/model_test.rb index f05783a..a17dc0a 100644 --- a/test/closure_tree/model_test.rb +++ b/test/closure_tree/model_test.rb @@ -7,3 +7,17 @@ assert_equal Tag._ct, Tag.new._ct end end + +describe 'multi database support' do + it 'should have a different connection for menu items' do + # These 2 models are in the same database + assert_equal Tag.connection, Metal.connection + # The hierarchy table is in the same database + assert_equal Tag.connection, TagHierarchy.connection + + # However, these 2 models are in different databases + refute_equal MenuItem.connection, Tag.connection + # The hierarchy table is in the same database + assert_equal MenuItem.connection, MenuItemHierarchy.connection + end +end diff --git a/test/closure_tree/pool_test.rb b/test/closure_tree/pool_test.rb index db5dfc6..0250cf7 100644 --- a/test/closure_tree/pool_test.rb +++ b/test/closure_tree/pool_test.rb @@ -9,8 +9,7 @@ class TypeDuplicate < ActiveRecord::Base has_closure_tree end - refute ActiveRecord::Base.connection_pool.active_connection? - # +false+ in AR 4, +nil+ in AR 5 + assert_nil TypeDuplicate.connection_pool.active_connection? end it 'returns connection to the pool after has_closure_tree setup with order' do @@ -19,7 +18,7 @@ class MetalDuplicate < ActiveRecord::Base has_closure_tree order: 'sort_order', name_column: 'value' end - refute ActiveRecord::Base.connection_pool.active_connection? + refute MetalDuplicate.connection_pool.active_connection? end it 'returns connection to the pool after has_closure_tree_root setup' do @@ -28,6 +27,6 @@ class GroupDuplicate < ActiveRecord::Base has_closure_tree_root :root_user end - refute ActiveRecord::Base.connection_pool.active_connection? + refute GroupDuplicate.connection_pool.active_connection? end end diff --git a/test/test_helper.rb b/test/test_helper.rb index c4ff05b..06053b9 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,49 +1,51 @@ # frozen_string_literal: true -require 'erb' require 'active_record' -require 'with_advisory_lock' -require 'tmpdir' -require 'securerandom' require 'minitest' require 'minitest/autorun' require 'database_cleaner' require 'support/query_counter' require 'parallel' +puts "Using ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}" + +primary_database_url = ENV['DATABASE_URL'].presence || "sqlite3:///tmp/closure_tree_test" +secondary_database_url = ENV['SECONDARY_DATABASE_URL'].presence || "sqlite3:///tmp/closure_tree_test-s" + +puts "Using primary database #{primary_database_url}" +puts "Using secondary database #{secondary_database_url}" + ActiveRecord::Base.configurations = { default_env: { - url: ENV.fetch('DATABASE_URL', "sqlite3://#{Dir.tmpdir}/#{SecureRandom.hex}.sqlite3"), - properties: { allowPublicKeyRetrieval: true } # for JRuby madness + primary: { + url: primary_database_url, + properties: { allowPublicKeyRetrieval: true } # for JRuby madness + }, + secondary: { + url: secondary_database_url, + properties: { allowPublicKeyRetrieval: true } # for JRuby madness + } } } ENV['WITH_ADVISORY_LOCK_PREFIX'] ||= SecureRandom.hex -ActiveRecord::Base.establish_connection - def env_db - @env_db ||= if ActiveRecord::Base.respond_to?(:connection_db_config) - ActiveRecord::Base.connection_db_config.adapter - else - ActiveRecord::Base.connection_config[:adapter] - end.to_sym + @env_db ||= ActiveRecord::Base.connection_db_config.adapter.to_sym end ActiveRecord::Migration.verbose = false ActiveRecord::Base.table_name_prefix = ENV['DB_PREFIX'].to_s ActiveRecord::Base.table_name_suffix = ENV['DB_SUFFIX'].to_s -ActiveRecord::Base.establish_connection # Use in specs to skip some tests def sqlite? env_db == :sqlite3 end -ActiveRecord::Base.connection.recreate_database('closure_tree_test') unless sqlite? -puts "Testing with #{env_db} database, ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}" DatabaseCleaner.strategy = :truncation +DatabaseCleaner.allow_remote_database_url = true module Minitest class Spec @@ -65,5 +67,8 @@ class Spec Thread.abort_on_exception = true require 'closure_tree' + + require_relative '../spec/support/schema' -require_relative '../spec/support/models' + +puts "Testing with #{env_db} database"