From 1f87c8a736ec84b011b4b06925df030304b6c0b8 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Wed, 28 May 2025 21:11:40 +0100 Subject: [PATCH] use new with_advisory_lock --- Gemfile | 2 ++ closure_tree.gemspec | 2 +- gemfiles/activerecord_7.1.gemfile | 1 + gemfiles/activerecord_7.2.gemfile | 1 + gemfiles/activerecord_8.0.gemfile | 1 + gemfiles/activerecord_edge.gemfile | 1 + lib/closure_tree/numeric_order_support.rb | 6 ++--- lib/closure_tree/support.rb | 10 ++++---- lib/closure_tree/support_flags.rb | 10 -------- test/closure_tree/matcher_test.rb | 11 ++++++--- test/support/tag_examples.rb | 5 ---- test/test_helper.rb | 29 ++++++++++++++++++++++- 12 files changed, 51 insertions(+), 28 deletions(-) diff --git a/Gemfile b/Gemfile index 7f4f5e95..8753f543 100644 --- a/Gemfile +++ b/Gemfile @@ -3,3 +3,5 @@ source 'https://rubygems.org' gemspec + +gem 'with_advisory_lock', github: 'closuretree/with_advisory_lock' \ No newline at end of file diff --git a/closure_tree.gemspec b/closure_tree.gemspec index df5101ad..d014d301 100644 --- a/closure_tree.gemspec +++ b/closure_tree.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |gem| gem.required_ruby_version = '>= 3.3.0' gem.add_runtime_dependency 'activerecord', '>= 7.1.0' - gem.add_runtime_dependency 'with_advisory_lock', '>= 5.0.0', '< 6.0.0' + gem.add_runtime_dependency 'with_advisory_lock', '>= 6.0.0' gem.add_development_dependency 'appraisal' gem.add_development_dependency 'database_cleaner' diff --git a/gemfiles/activerecord_7.1.gemfile b/gemfiles/activerecord_7.1.gemfile index 244ee766..433f5e74 100644 --- a/gemfiles/activerecord_7.1.gemfile +++ b/gemfiles/activerecord_7.1.gemfile @@ -2,6 +2,7 @@ source "https://rubygems.org" +gem "with_advisory_lock", github: "closuretree/with_advisory_lock" gem "activerecord", "~> 7.1.0" gem "railties" diff --git a/gemfiles/activerecord_7.2.gemfile b/gemfiles/activerecord_7.2.gemfile index 1aeae35c..db97d653 100644 --- a/gemfiles/activerecord_7.2.gemfile +++ b/gemfiles/activerecord_7.2.gemfile @@ -2,6 +2,7 @@ source "https://rubygems.org" +gem "with_advisory_lock", github: "closuretree/with_advisory_lock" gem "activerecord", "~> 7.2.0" gem "railties" diff --git a/gemfiles/activerecord_8.0.gemfile b/gemfiles/activerecord_8.0.gemfile index 6293355e..a2a4411a 100644 --- a/gemfiles/activerecord_8.0.gemfile +++ b/gemfiles/activerecord_8.0.gemfile @@ -2,6 +2,7 @@ source "https://rubygems.org" +gem "with_advisory_lock", github: "closuretree/with_advisory_lock" gem "activerecord", "~> 8.0.0" gem "railties" diff --git a/gemfiles/activerecord_edge.gemfile b/gemfiles/activerecord_edge.gemfile index 6220b84f..c30e4bfc 100644 --- a/gemfiles/activerecord_edge.gemfile +++ b/gemfiles/activerecord_edge.gemfile @@ -2,6 +2,7 @@ source "https://rubygems.org" +gem "with_advisory_lock", github: "closuretree/with_advisory_lock" gem "activerecord", github: "rails/rails" gem "railties", github: "rails/rails" diff --git a/lib/closure_tree/numeric_order_support.rb b/lib/closure_tree/numeric_order_support.rb index 0223a415..faeec052 100644 --- a/lib/closure_tree/numeric_order_support.rb +++ b/lib/closure_tree/numeric_order_support.rb @@ -2,10 +2,10 @@ module ClosureTree module NumericOrderSupport def self.adapter_for_connection(connection) - das = WithAdvisoryLock::DatabaseAdapterSupport.new(connection) - if das.postgresql? + adapter_name = connection.adapter_name.downcase + if adapter_name.include?('postgresql') || adapter_name.include?('postgis') ::ClosureTree::NumericOrderSupport::PostgreSQLAdapter - elsif das.mysql? + elsif adapter_name.include?('mysql') || adapter_name.include?('trilogy') ::ClosureTree::NumericOrderSupport::MysqlAdapter else ::ClosureTree::NumericOrderSupport::GenericAdapter diff --git a/lib/closure_tree/support.rb b/lib/closure_tree/support.rb index 2125541c..31013886 100644 --- a/lib/closure_tree/support.rb +++ b/lib/closure_tree/support.rb @@ -18,11 +18,15 @@ class Support def initialize(model_class, options) @model_class = model_class + + # Detect if we're using SQLite and disable advisory locks + default_with_advisory_lock = !connection.adapter_name.downcase.include?('sqlite') + @options = { :parent_column_name => 'parent_id', :dependent => :nullify, # or :destroy or :delete_all -- see the README :name_column => 'name', - :with_advisory_lock => true, + :with_advisory_lock => default_with_advisory_lock, :numeric_order => false }.merge(options) raise ArgumentError, "name_column can't be 'path'" if options[:name_column] == 'path' @@ -34,14 +38,10 @@ def initialize(model_class, options) def hierarchy_class_for_model parent_class = model_class.module_parent hierarchy_class = parent_class.const_set(short_hierarchy_class_name, Class.new(model_class.superclass)) - use_attr_accessible = use_attr_accessible? - include_forbidden_attributes_protection = include_forbidden_attributes_protection? model_class_name = model_class.to_s hierarchy_class.class_eval do - include ActiveModel::ForbiddenAttributesProtection if include_forbidden_attributes_protection belongs_to :ancestor, class_name: model_class_name belongs_to :descendant, class_name: model_class_name - attr_accessible :ancestor, :descendant, :generations if use_attr_accessible def ==(other) self.class == other.class && ancestor_id == other.ancestor_id && descendant_id == other.descendant_id end diff --git a/lib/closure_tree/support_flags.rb b/lib/closure_tree/support_flags.rb index cdf7fd8b..ec8586cb 100644 --- a/lib/closure_tree/support_flags.rb +++ b/lib/closure_tree/support_flags.rb @@ -1,16 +1,6 @@ module ClosureTree module SupportFlags - def use_attr_accessible? - defined?(ActiveModel::MassAssignmentSecurity) && - model_class.respond_to?(:accessible_attributes) && - ! model_class.accessible_attributes.empty? - end - - def include_forbidden_attributes_protection? - defined?(ActiveModel::ForbiddenAttributesProtection) && - model_class.ancestors.include?(ActiveModel::ForbiddenAttributesProtection) - end def order_option? order_by.present? diff --git a/test/closure_tree/matcher_test.rb b/test/closure_tree/matcher_test.rb index 35306590..ec11ea71 100644 --- a/test/closure_tree/matcher_test.rb +++ b/test/closure_tree/matcher_test.rb @@ -28,9 +28,14 @@ class MatcherTest < ActiveSupport::TestCase end test "advisory_lock option" do - assert_closure_tree User, with_advisory_lock: true - assert_closure_tree Label, ordered: true, with_advisory_lock: true - assert_closure_tree Metal, ordered: :sort_order, with_advisory_lock: true + # SQLite doesn't support advisory locks, so skip these tests when using SQLite + if ActiveRecord::Base.connection.adapter_name.downcase.include?('sqlite') + skip "SQLite doesn't support advisory locks" + else + assert_closure_tree User, with_advisory_lock: true + assert_closure_tree Label, ordered: true, with_advisory_lock: true + assert_closure_tree Metal, ordered: :sort_order, with_advisory_lock: true + end end test "without_advisory_lock option" do diff --git a/test/support/tag_examples.rb b/test/support/tag_examples.rb index ba196120..74a312bc 100644 --- a/test/support/tag_examples.rb +++ b/test/support/tag_examples.rb @@ -12,11 +12,6 @@ def self.included(mod) end describe 'class setup' do - it 'has correct accessible_attributes' do - if @tag_class._ct.use_attr_accessible? - assert_equal(%w[parent name title].sort, @tag_class.accessible_attributes.to_a.sort) - end - end it 'should build hierarchy classname correctly' do assert_equal @tag_hierarchy_class, @tag_class.hierarchy_class diff --git a/test/test_helper.rb b/test/test_helper.rb index a3b4bf2d..f3ae3fc1 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -78,10 +78,37 @@ def sqlite? env_db == :sqlite3 end -ActiveRecord::Base.connection.recreate_database('closure_tree_test') unless sqlite? +# For PostgreSQL and MySQL, we need to create/reset the database structure +unless sqlite? + begin + if ActiveRecord::Base.connection.adapter_name.downcase.include?('postgresql') + # PostgreSQL requires disconnecting before dropping the database + ActiveRecord::Base.connection.disconnect! + # Connect to postgres database to drop/create closure_tree_test + if connection_config.is_a?(String) + # Parse the DATABASE_URL and change database to postgres + postgres_url = connection_config.gsub(/\/closure_tree_test/, '/postgres') + ActiveRecord::Base.establish_connection(postgres_url) + else + ActiveRecord::Base.establish_connection(connection_config.merge(database: 'postgres')) + end + ActiveRecord::Base.connection.drop_database('closure_tree_test') rescue nil + ActiveRecord::Base.connection.create_database('closure_tree_test') + ActiveRecord::Base.connection.disconnect! + ActiveRecord::Base.establish_connection(connection_config) + else + # MySQL can recreate directly + ActiveRecord::Base.connection.recreate_database('closure_tree_test') + end + rescue => e + puts "Warning: Could not recreate database: #{e.message}" + end +end puts "Testing with #{env_db} database, ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}" DatabaseCleaner.strategy = :truncation +# Allow DatabaseCleaner to work with our test database +DatabaseCleaner.allow_remote_database_url = true module Minitest class Spec