From b362c22e07b31ab54f6294729b6315c402aedcb0 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Wed, 17 Jan 2024 22:10:09 +0100 Subject: [PATCH 1/8] chore: add ApplicationRecord for test --- spec/support/models.rb | 79 +++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/spec/support/models.rb b/spec/support/models.rb index 9927af41..e845ea44 100644 --- a/spec/support/models.rb +++ b/spec/support/models.rb @@ -1,5 +1,11 @@ -class Tag < ActiveRecord::Base - has_closure_tree :dependent => :destroy, :order => :name +# frozen_string_literal: true + +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true +end + +class Tag < ApplicationRecord + has_closure_tree dependent: :destroy, order: :name before_destroy :add_destroyed_tag def to_s @@ -8,11 +14,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:) 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 +34,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:) 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 +75,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 +105,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 +120,7 @@ def to_s end end -class CuisineType < ActiveRecord::Base +class CuisineType < ApplicationRecord acts_as_tree end @@ -122,12 +128,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 +146,6 @@ class Adamantium < Metal class Unobtanium < Metal end -class MenuItem < ActiveRecord::Base +class MenuItem < ApplicationRecord has_closure_tree touch: true, with_advisory_lock: false end From 3af1cf82cc00f8cb5fa0dd07819f4f70ee94c67d Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Wed, 17 Jan 2024 23:18:38 +0100 Subject: [PATCH 2/8] feat: add multi database support --- .github/workflows/ci.yml | 7 ++-- .github/workflows/ci_jruby.yml | 7 ++-- .github/workflows/ci_truffleruby.yml | 7 ++-- lib/closure_tree/support_attributes.rb | 2 +- spec/spec_helper.rb | 7 +++- spec/support/exceed_query_limit.rb | 4 ++- spec/support/helpers.rb | 4 ++- spec/support/models.rb | 6 +--- spec/support/query_counter.rb | 6 ++-- spec/support/schema.rb | 46 +++++++++++++++++--------- test/closure_tree/model_test.rb | 15 +++++++++ test/test_helper.rb | 19 +++++------ 12 files changed, 83 insertions(+), 47 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77046851..76fff215 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,7 @@ jobs: - activerecord_6.1 - activerecord_edge adapter: - - 'sqlite3:///:memory:' + - '' # SQLite3 - mysql2://root:root@0/closure_tree_test - postgres://closure_tree:closure_tree@0/closure_tree_test exclude: @@ -59,7 +59,7 @@ jobs: 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 }} 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 c7d0156a..b6569a3d 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 - mysql2://root:root@0/closure_tree_test - postgres://closure_tree:closure_tree@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 }} 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 b61630fd..33f79567 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 - mysql2://root:root@0/closure_tree_test - postgres://closure_tree:closure_tree@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 }} BUNDLE_GEMFILE: gemfiles/${{ matrix.rails }}.gemfile WITH_ADVISORY_LOCK_PREFIX: ${{ github.run_id }} run: bin/rake diff --git a/lib/closure_tree/support_attributes.rb b/lib/closure_tree/support_attributes.rb index 38879ade..5ec8ef4e 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 eba943f7..28311c02 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -20,9 +20,14 @@ end end +database_file = SecureRandom.hex ActiveRecord::Base.configurations = { default_env: { - url: ENV.fetch('DATABASE_URL', "sqlite3::memory:"), + url: ENV['DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}.sqlite3", + properties: { allowPublicKeyRetrieval: true } # for JRuby madness + }, + secondary_env: { + url: ENV['SECONDARY_DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}-s.sqlite3", properties: { allowPublicKeyRetrieval: true } # for JRuby madness } } diff --git a/spec/support/exceed_query_limit.rb b/spec/support/exceed_query_limit.rb index b167a826..a8d7cf9a 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 14047149..d303eba6 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 e845ea44..077a7b7d 100644 --- a/spec/support/models.rb +++ b/spec/support/models.rb @@ -1,9 +1,5 @@ # frozen_string_literal: true -class ApplicationRecord < ActiveRecord::Base - self.abstract_class = true -end - class Tag < ApplicationRecord has_closure_tree dependent: :destroy, order: :name before_destroy :add_destroyed_tag @@ -146,6 +142,6 @@ class Adamantium < Metal class Unobtanium < Metal end -class MenuItem < ApplicationRecord +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 f36cfc09..44c9864a 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 30a4a992..7d92f401 100644 --- a/spec/support/schema.rb +++ b/spec/support/schema.rb @@ -1,7 +1,17 @@ # frozen_string_literal: true +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true +end + +class SecondDatabaseRecord < ActiveRecord::Base + self.abstract_class = true + + establish_connection :secondary_env +end + ActiveRecord::Schema.define(version: 0) do - create_table 'tags', force: :cascade do |t| + connection.create_table 'tags', force: :cascade do |t| t.string 'name' t.string 'title' t.references 'parent' @@ -126,18 +136,6 @@ 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 - - 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 - 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' @@ -149,9 +147,25 @@ 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 + +SecondDatabaseRecord.connection_pool.with_connection do |connection| + ActiveRecord::Schema.define(version: 0) do + connection.create_table 'menu_items', force: :cascade do |t| + t.string 'name' + t.references 'parent' + t.timestamps null: false + end + + connection.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 + end + connection.add_foreign_key(:menu_items, :menu_items, column: 'parent_id', on_delete: :cascade) + connection.add_foreign_key(:menu_item_hierarchies, :menu_items, column: 'ancestor_id', on_delete: :cascade) + connection.add_foreign_key(:menu_item_hierarchies, :menu_items, column: 'descendant_id', on_delete: :cascade) + end +end diff --git a/test/closure_tree/model_test.rb b/test/closure_tree/model_test.rb index f05783ad..52020718 100644 --- a/test/closure_tree/model_test.rb +++ b/test/closure_tree/model_test.rb @@ -7,3 +7,18 @@ 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 \ No newline at end of file diff --git a/test/test_helper.rb b/test/test_helper.rb index c4ff05b1..cab9ceb2 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,19 +1,20 @@ # 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' +database_file = SecureRandom.hex ActiveRecord::Base.configurations = { default_env: { - url: ENV.fetch('DATABASE_URL', "sqlite3://#{Dir.tmpdir}/#{SecureRandom.hex}.sqlite3"), + url: ENV['DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}.sqlite3", + properties: { allowPublicKeyRetrieval: true } # for JRuby madness + }, + secondary_env: { + url: ENV['SECONDARY_DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}-s.sqlite3", properties: { allowPublicKeyRetrieval: true } # for JRuby madness } } @@ -23,11 +24,7 @@ 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 @@ -40,7 +37,6 @@ 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 @@ -67,3 +63,4 @@ class Spec require 'closure_tree' require_relative '../spec/support/schema' require_relative '../spec/support/models' +ActiveRecord::Base.connection.recreate_database('closure_tree_test') unless sqlite? From f0f0cc0f5740de93e9723db02e5c0933dda7fd61 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Wed, 17 Jan 2024 23:42:10 +0100 Subject: [PATCH 3/8] itest --- spec/spec_helper.rb | 8 +++----- test/closure_tree/model_test.rb | 5 ++--- test/test_helper.rb | 3 ++- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 28311c02..7dcae0ee 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -32,6 +32,8 @@ } } +puts "Testing with #{ActiveRecord::Base.configurations}" + # Configure ActiveRecord ActiveRecord::Migration.verbose = false ActiveRecord::Base.table_name_prefix = ENV['DB_PREFIX'].to_s @@ -39,11 +41,7 @@ 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 diff --git a/test/closure_tree/model_test.rb b/test/closure_tree/model_test.rb index 52020718..a17dc0a1 100644 --- a/test/closure_tree/model_test.rb +++ b/test/closure_tree/model_test.rb @@ -8,7 +8,7 @@ end end -describe "multi database support" do +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 @@ -20,5 +20,4 @@ # The hierarchy table is in the same database assert_equal MenuItem.connection, MenuItemHierarchy.connection end - -end \ No newline at end of file +end diff --git a/test/test_helper.rb b/test/test_helper.rb index cab9ceb2..f0e18472 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -19,9 +19,10 @@ } } +puts "Testing with #{ActiveRecord::Base.configurations}" + ENV['WITH_ADVISORY_LOCK_PREFIX'] ||= SecureRandom.hex -ActiveRecord::Base.establish_connection def env_db @env_db ||= ActiveRecord::Base.connection_db_config.adapter.to_sym From 14afa86bbe3e55606c6404f38f93316467fda13b Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Wed, 17 Jan 2024 23:46:03 +0100 Subject: [PATCH 4/8] debug --- spec/spec_helper.rb | 4 ++-- test/test_helper.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7dcae0ee..5c79ccd1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -21,7 +21,7 @@ end database_file = SecureRandom.hex -ActiveRecord::Base.configurations = { +ActiveRecord::Base.configurations = debug = { default_env: { url: ENV['DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}.sqlite3", properties: { allowPublicKeyRetrieval: true } # for JRuby madness @@ -32,7 +32,7 @@ } } -puts "Testing with #{ActiveRecord::Base.configurations}" +puts "Testing with #{debug}" # Configure ActiveRecord ActiveRecord::Migration.verbose = false diff --git a/test/test_helper.rb b/test/test_helper.rb index f0e18472..d5d7b7a6 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -8,7 +8,7 @@ require 'parallel' database_file = SecureRandom.hex -ActiveRecord::Base.configurations = { +ActiveRecord::Base.configurations = debug = { default_env: { url: ENV['DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}.sqlite3", properties: { allowPublicKeyRetrieval: true } # for JRuby madness @@ -19,7 +19,7 @@ } } -puts "Testing with #{ActiveRecord::Base.configurations}" +puts "Testing with #{debug}" ENV['WITH_ADVISORY_LOCK_PREFIX'] ||= SecureRandom.hex From 1837fd0b2d596b141c3a764f6893dd7704eda56a Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Thu, 18 Jan 2024 19:47:05 +0100 Subject: [PATCH 5/8] debug --- .github/workflows/ci.yml | 2 +- .github/workflows/ci_jruby.yml | 2 +- .github/workflows/ci_truffleruby.yml | 2 +- spec/spec_helper.rb | 1 - test/test_helper.rb | 1 - 5 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 76fff215..777e9fa0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,7 +75,7 @@ jobs: env: RAILS_VERSION: ${{ matrix.rails }} DATABASE_URL: ${{ matrix.adapter }} - SECONDARY_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 b6569a3d..04f1d5af 100644 --- a/.github/workflows/ci_jruby.yml +++ b/.github/workflows/ci_jruby.yml @@ -64,7 +64,7 @@ jobs: env: RAILS_VERSION: ${{ matrix.rails }} DATABASE_URL: ${{ matrix.adapter }} - SECONDARY_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 33f79567..7dc07ab9 100644 --- a/.github/workflows/ci_truffleruby.yml +++ b/.github/workflows/ci_truffleruby.yml @@ -67,7 +67,7 @@ jobs: env: RAILS_VERSION: ${{ matrix.rails }} DATABASE_URL: ${{ matrix.adapter }} - SECONDARY_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/spec/spec_helper.rb b/spec/spec_helper.rb index 5c79ccd1..2466790a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -38,7 +38,6 @@ 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 ||= ActiveRecord::Base.connection_db_config.adapter.to_sym diff --git a/test/test_helper.rb b/test/test_helper.rb index d5d7b7a6..cdb6c6bc 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -31,7 +31,6 @@ def env_db 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? From 6ecd0c3b50d4edda4fa8163f277c985a01951ce3 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Thu, 18 Jan 2024 22:51:07 +0100 Subject: [PATCH 6/8] remove debug --- .github/workflows/ci.yml | 4 +-- .github/workflows/ci_jruby.yml | 4 +-- .github/workflows/ci_truffleruby.yml | 4 +-- .gitignore | 1 + spec/spec_helper.rb | 54 +++++++++++++++++----------- spec/support/models.rb | 4 +-- spec/support/schema.rb | 2 +- test/test_helper.rb | 45 ++++++++++++++++------- 8 files changed, 75 insertions(+), 43 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 777e9fa0..8a7c089d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,9 +50,9 @@ jobs: - activerecord_6.1 - activerecord_edge adapter: - - '' # SQLite3 + - 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 diff --git a/.github/workflows/ci_jruby.yml b/.github/workflows/ci_jruby.yml index 04f1d5af..558fbcc7 100644 --- a/.github/workflows/ci_jruby.yml +++ b/.github/workflows/ci_jruby.yml @@ -43,9 +43,9 @@ jobs: - activerecord_7.0 - activerecord_6.1 adapter: - - '' # SQLite3 + - 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@v4 diff --git a/.github/workflows/ci_truffleruby.yml b/.github/workflows/ci_truffleruby.yml index 7dc07ab9..77bf270e 100644 --- a/.github/workflows/ci_truffleruby.yml +++ b/.github/workflows/ci_truffleruby.yml @@ -45,9 +45,9 @@ jobs: - activerecord_7.0 - activerecord_6.1 adapter: - - '' # SQLite3 + - 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 diff --git a/.gitignore b/.gitignore index 2f6900cc..e9f555fc 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ tmp/ .ruby-* *.iml coverage/ +.env diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2466790a..aabdd6f4 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,20 +20,25 @@ end end -database_file = SecureRandom.hex -ActiveRecord::Base.configurations = debug = { +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['DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}.sqlite3", - properties: { allowPublicKeyRetrieval: true } # for JRuby madness - }, - secondary_env: { - url: ENV['SECONDARY_DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}-s.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 + } } } -puts "Testing with #{debug}" - # Configure ActiveRecord ActiveRecord::Migration.verbose = false ActiveRecord::Base.table_name_prefix = ENV['DB_PREFIX'].to_s @@ -75,15 +80,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 @@ -94,10 +96,19 @@ 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' +begin + ActiveRecord::Base.establish_connection(:primary) +rescue + ActiveRecord::Tasks::DatabaseTasks.create_current('primary') +end + +begin + ActiveRecord::Base.establish_connection(:secondary) +rescue + ActiveRecord::Tasks::DatabaseTasks.create_current('secondary') +end # Load test helpers require_relative 'support/schema' @@ -105,3 +116,4 @@ def sqlite? 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/models.rb b/spec/support/models.rb index 077a7b7d..eee68410 100644 --- a/spec/support/models.rb +++ b/spec/support/models.rb @@ -10,7 +10,7 @@ def to_s def add_destroyed_tag # Proof for the tests that the destroy rather than the delete method was called: - DestroyedTag.create(name:) + DestroyedTag.create(name: to_s) end end @@ -30,7 +30,7 @@ def to_s def add_destroyed_tag # Proof for the tests that the destroy rather than the delete method was called: - DestroyedTag.create(name:) + DestroyedTag.create(name: to_s) end end diff --git a/spec/support/schema.rb b/spec/support/schema.rb index 7d92f401..0626605f 100644 --- a/spec/support/schema.rb +++ b/spec/support/schema.rb @@ -7,7 +7,7 @@ class ApplicationRecord < ActiveRecord::Base class SecondDatabaseRecord < ActiveRecord::Base self.abstract_class = true - establish_connection :secondary_env + establish_connection :secondary end ActiveRecord::Schema.define(version: 0) do diff --git a/test/test_helper.rb b/test/test_helper.rb index cdb6c6bc..8a870a67 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -7,23 +7,29 @@ require 'support/query_counter' require 'parallel' -database_file = SecureRandom.hex -ActiveRecord::Base.configurations = debug = { +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['DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}.sqlite3", - properties: { allowPublicKeyRetrieval: true } # for JRuby madness - }, - secondary_env: { - url: ENV['SECONDARY_DATABASE_URL'].presence || "sqlite3://#{Dir.tmpdir}/#{database_file}-s.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 + } } } -puts "Testing with #{debug}" - ENV['WITH_ADVISORY_LOCK_PREFIX'] ||= SecureRandom.hex - def env_db @env_db ||= ActiveRecord::Base.connection_db_config.adapter.to_sym end @@ -37,9 +43,9 @@ def sqlite? env_db == :sqlite3 end -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 @@ -61,6 +67,19 @@ class Spec Thread.abort_on_exception = true require 'closure_tree' +begin + ActiveRecord::Base.establish_connection(:primary) +rescue + ActiveRecord::Tasks::DatabaseTasks.create_current('primary') +end + +begin + ActiveRecord::Base.establish_connection(:secondary) +rescue + ActiveRecord::Tasks::DatabaseTasks.create_current('secondary') +end + require_relative '../spec/support/schema' require_relative '../spec/support/models' -ActiveRecord::Base.connection.recreate_database('closure_tree_test') unless sqlite? + +puts "Testing with #{env_db} database" From dee7b571310642d3d88acbb4026639c232db1509 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Fri, 19 Jan 2024 00:18:39 +0100 Subject: [PATCH 7/8] fix --- spec/spec_helper.rb | 14 ++++---------- spec/support/schema.rb | 5 ++++- test/test_helper.rb | 15 ++++----------- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index aabdd6f4..1c02f255 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -98,17 +98,11 @@ def sqlite? # Require our gem require 'closure_tree' -begin - ActiveRecord::Base.establish_connection(:primary) -rescue - ActiveRecord::Tasks::DatabaseTasks.create_current('primary') -end -begin - ActiveRecord::Base.establish_connection(:secondary) -rescue - ActiveRecord::Tasks::DatabaseTasks.create_current('secondary') -end +ActiveRecord::Tasks::DatabaseTasks.drop_current(:primary) +ActiveRecord::Tasks::DatabaseTasks.create_current(:primary) +ActiveRecord::Tasks::DatabaseTasks.drop_current(:secondary) +ActiveRecord::Tasks::DatabaseTasks.create_current(:secondary) # Load test helpers require_relative 'support/schema' diff --git a/spec/support/schema.rb b/spec/support/schema.rb index 0626605f..665f46fb 100644 --- a/spec/support/schema.rb +++ b/spec/support/schema.rb @@ -2,12 +2,14 @@ class ApplicationRecord < ActiveRecord::Base self.abstract_class = true + + connects_to database: { writing: :primary, reading: :primary } end class SecondDatabaseRecord < ActiveRecord::Base self.abstract_class = true - establish_connection :secondary + connects_to database: { writing: :secondary, reading: :secondary } end ActiveRecord::Schema.define(version: 0) do @@ -151,6 +153,7 @@ class SecondDatabaseRecord < ActiveRecord::Base add_foreign_key(:tag_hierarchies, :tags, column: 'descendant_id', on_delete: :cascade) end +SecondDatabaseRecord.establish_connection SecondDatabaseRecord.connection_pool.with_connection do |connection| ActiveRecord::Schema.define(version: 0) do connection.create_table 'menu_items', force: :cascade do |t| diff --git a/test/test_helper.rb b/test/test_helper.rb index 8a870a67..f1c547a3 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -67,17 +67,10 @@ class Spec Thread.abort_on_exception = true require 'closure_tree' -begin - ActiveRecord::Base.establish_connection(:primary) -rescue - ActiveRecord::Tasks::DatabaseTasks.create_current('primary') -end - -begin - ActiveRecord::Base.establish_connection(:secondary) -rescue - ActiveRecord::Tasks::DatabaseTasks.create_current('secondary') -end +ActiveRecord::Tasks::DatabaseTasks.drop_current(:primary) +ActiveRecord::Tasks::DatabaseTasks.create_current(:primary) +ActiveRecord::Tasks::DatabaseTasks.drop_current(:secondary) +ActiveRecord::Tasks::DatabaseTasks.create_current(:secondary) require_relative '../spec/support/schema' require_relative '../spec/support/models' From d147b956ebcf32b65e80e990a14332630c87c31b Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Fri, 19 Jan 2024 00:48:30 +0100 Subject: [PATCH 8/8] fix --- lib/closure_tree/has_closure_tree.rb | 4 - lib/closure_tree/has_closure_tree_root.rb | 2 - lib/closure_tree/numeric_order_support.rb | 78 +++++++------- lib/closure_tree/support.rb | 7 +- spec/spec_helper.rb | 7 +- spec/support/models.rb | 13 +++ spec/support/schema.rb | 121 +++++++++++----------- test/closure_tree/pool_test.rb | 7 +- test/test_helper.rb | 6 +- 9 files changed, 125 insertions(+), 120 deletions(-) diff --git a/lib/closure_tree/has_closure_tree.rb b/lib/closure_tree/has_closure_tree.rb index b0bc5b1a..a45462cc 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 70e327d1..26f9d535 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 0223a415..351720a7 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 e887090e..0e406bd9 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/spec/spec_helper.rb b/spec/spec_helper.rb index 1c02f255..1803df2d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -29,6 +29,7 @@ ActiveRecord::Base.configurations = { default_env: { primary: { + primary: true, url: primary_database_url, properties: { allowPublicKeyRetrieval: true } # for JRuby madness }, @@ -99,14 +100,8 @@ def sqlite? # Require our gem require 'closure_tree' -ActiveRecord::Tasks::DatabaseTasks.drop_current(:primary) -ActiveRecord::Tasks::DatabaseTasks.create_current(:primary) -ActiveRecord::Tasks::DatabaseTasks.drop_current(:secondary) -ActiveRecord::Tasks::DatabaseTasks.create_current(:secondary) - # 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' diff --git a/spec/support/models.rb b/spec/support/models.rb index eee68410..cb937a15 100644 --- a/spec/support/models.rb +++ b/spec/support/models.rb @@ -1,5 +1,18 @@ # 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 diff --git a/spec/support/schema.rb b/spec/support/schema.rb index 665f46fb..95cbdf19 100644 --- a/spec/support/schema.rb +++ b/spec/support/schema.rb @@ -1,19 +1,21 @@ # 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 } +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 - connection.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' @@ -21,13 +23,17 @@ class SecondDatabaseRecord < ActiveRecord::Base 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' @@ -36,95 +42,99 @@ class SecondDatabaseRecord < ActiveRecord::Base 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' @@ -132,43 +142,38 @@ class SecondDatabaseRecord < ActiveRecord::Base 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 - 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(:tag_hierarchies, :tags, column: 'ancestor_id', on_delete: :cascade) - add_foreign_key(:tag_hierarchies, :tags, column: 'descendant_id', on_delete: :cascade) + 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 SecondDatabaseRecord.establish_connection SecondDatabaseRecord.connection_pool.with_connection do |connection| ActiveRecord::Schema.define(version: 0) do - connection.create_table 'menu_items', force: :cascade do |t| + connection.create_table MenuItem.table_name, force: :cascade do |t| t.string 'name' t.references 'parent' t.timestamps null: false end - connection.create_table 'menu_item_hierarchies', id: false, force: :cascade do |t| + 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(:menu_items, :menu_items, column: 'parent_id', on_delete: :cascade) - connection.add_foreign_key(:menu_item_hierarchies, :menu_items, column: 'ancestor_id', on_delete: :cascade) - connection.add_foreign_key(:menu_item_hierarchies, :menu_items, column: 'descendant_id', on_delete: :cascade) + 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 end diff --git a/test/closure_tree/pool_test.rb b/test/closure_tree/pool_test.rb index db5dfc61..0250cf7e 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 f1c547a3..06053b9f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -67,12 +67,8 @@ class Spec Thread.abort_on_exception = true require 'closure_tree' -ActiveRecord::Tasks::DatabaseTasks.drop_current(:primary) -ActiveRecord::Tasks::DatabaseTasks.create_current(:primary) -ActiveRecord::Tasks::DatabaseTasks.drop_current(:secondary) -ActiveRecord::Tasks::DatabaseTasks.create_current(:secondary) + require_relative '../spec/support/schema' -require_relative '../spec/support/models' puts "Testing with #{env_db} database"