use new with_advisory_lock (#450)

seuros · web-flow · commit 1b07d5bd486d · 2025-05-28T21:27:20.000+01:00
diff --git a/Gemfile b/Gemfile
@@ -3,3 +3,5 @@
 source 'https://rubygems.org'
 
 gemspec
+
+gem 'with_advisory_lock', github: 'closuretree/with_advisory_lock'
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
