From 28c6f4d3c338265cb22b711529d8ee8122e576f9 Mon Sep 17 00:00:00 2001 From: Philippe Vaucher Date: Mon, 8 Oct 2018 09:55:48 +0200 Subject: [PATCH 1/6] Drop support for ActiveRecord less than 4.2 --- .travis.yml | 10 ---------- Appraisals | 20 -------------------- acts_as_list.gemspec | 2 +- gemfiles/rails_3_2.gemfile | 34 ---------------------------------- gemfiles/rails_4_1.gemfile | 34 ---------------------------------- 5 files changed, 1 insertion(+), 99 deletions(-) delete mode 100644 gemfiles/rails_3_2.gemfile delete mode 100644 gemfiles/rails_4_1.gemfile diff --git a/.travis.yml b/.travis.yml index 59323b9c..62d71bcc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,8 +22,6 @@ env: - DB=mysql - DB=postgresql gemfile: - - gemfiles/rails_3_2.gemfile - - gemfiles/rails_4_1.gemfile - gemfiles/rails_4_2.gemfile - gemfiles/rails_5_0.gemfile - gemfiles/rails_5_1.gemfile @@ -48,11 +46,3 @@ matrix: gemfile: gemfiles/rails_5_1.gemfile - rvm: 2.1.10 gemfile: gemfiles/rails_5_2.gemfile - - rvm: 2.4.3 - gemfile: gemfiles/rails_3_2.gemfile - - rvm: 2.4.3 - gemfile: gemfiles/rails_4_1.gemfile - - rvm: 2.5.0 - gemfile: gemfiles/rails_3_2.gemfile - - rvm: 2.5.0 - gemfile: gemfiles/rails_4_1.gemfile diff --git a/Appraisals b/Appraisals index e4913930..d26b6996 100644 --- a/Appraisals +++ b/Appraisals @@ -1,23 +1,3 @@ -appraise "rails-3-2" do - group :mysql do - gem "mysql2", "~> 0.3.21", platforms: [:ruby] - end - gem "activerecord", "~> 3.2.22.2" - group :test do - gem "after_commit_exception_notification" - end -end - -appraise "rails-4-1" do - group :mysql do - gem "mysql2", "~> 0.3.21", platforms: [:ruby] - end - gem "activerecord", "~> 4.1.16" - group :test do - gem "after_commit_exception_notification" - end -end - appraise "rails-4-2" do group :mysql do gem "mysql2", "~> 0.4.10", platforms: [:ruby] diff --git a/acts_as_list.gemspec b/acts_as_list.gemspec index 4e9ae8bf..0c730c75 100644 --- a/acts_as_list.gemspec +++ b/acts_as_list.gemspec @@ -24,6 +24,6 @@ Gem::Specification.new do |s| # Dependencies (installed via "bundle install") - s.add_dependency "activerecord", ">= 3.0" + s.add_dependency "activerecord", ">= 4.2" s.add_development_dependency "bundler", ">= 1.0.0" end diff --git a/gemfiles/rails_3_2.gemfile b/gemfiles/rails_3_2.gemfile deleted file mode 100644 index 708ce48b..00000000 --- a/gemfiles/rails_3_2.gemfile +++ /dev/null @@ -1,34 +0,0 @@ -# This file was generated by Appraisal - -source "http://rubygems.org" - -gem "rack", "~> 1", platforms: [:ruby_19, :ruby_20, :ruby_21] -gem "rake" -gem "appraisal" -gem "activerecord", "~> 3.2.22.2" - -group :development do - gem "github_changelog_generator", "1.9.0" -end - -group :test do - gem "minitest", "~> 5.0" - gem "test_after_commit", "~> 0.4.2" - gem "timecop" - gem "mocha" - gem "after_commit_exception_notification" -end - -group :sqlite do - gem "sqlite3", platforms: [:ruby] -end - -group :postgresql do - gem "pg", "~> 0.18.0", platforms: [:ruby] -end - -group :mysql do - gem "mysql2", "~> 0.3.21", platforms: [:ruby] -end - -gemspec path: "../" diff --git a/gemfiles/rails_4_1.gemfile b/gemfiles/rails_4_1.gemfile deleted file mode 100644 index 35f1f9fb..00000000 --- a/gemfiles/rails_4_1.gemfile +++ /dev/null @@ -1,34 +0,0 @@ -# This file was generated by Appraisal - -source "http://rubygems.org" - -gem "rack", "~> 1", platforms: [:ruby_19, :ruby_20, :ruby_21] -gem "rake" -gem "appraisal" -gem "activerecord", "~> 4.1.16" - -group :development do - gem "github_changelog_generator", "1.9.0" -end - -group :test do - gem "minitest", "~> 5.0" - gem "test_after_commit", "~> 0.4.2" - gem "timecop" - gem "mocha" - gem "after_commit_exception_notification" -end - -group :sqlite do - gem "sqlite3", platforms: [:ruby] -end - -group :postgresql do - gem "pg", "~> 0.18.0", platforms: [:ruby] -end - -group :mysql do - gem "mysql2", "~> 0.3.21", platforms: [:ruby] -end - -gemspec path: "../" From e48508af757d92d9fe9bb2b471025adda64fcd1d Mon Sep 17 00:00:00 2001 From: Philippe Vaucher Date: Tue, 9 Oct 2018 13:18:49 +0200 Subject: [PATCH 2/6] Drop support for Ruby less than 2.3 --- .travis.yml | 24 ------------------------ Gemfile | 2 -- acts_as_list.gemspec | 2 +- gemfiles/rails_4_2.gemfile | 1 - gemfiles/rails_5_0.gemfile | 1 - gemfiles/rails_5_1.gemfile | 1 - gemfiles/rails_5_2.gemfile | 1 - 7 files changed, 1 insertion(+), 31 deletions(-) diff --git a/.travis.yml b/.travis.yml index 62d71bcc..a52d59a4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,10 +10,6 @@ before_script: - mysql -e 'create database acts_as_list;' - psql -c 'create database acts_as_list;' -U postgres rvm: - - 1.9.3 - - 2.0.0 - - 2.1.10 - - 2.2.6 - 2.3.6 - 2.4.3 - 2.5.0 @@ -26,23 +22,3 @@ gemfile: - gemfiles/rails_5_0.gemfile - gemfiles/rails_5_1.gemfile - gemfiles/rails_5_2.gemfile -matrix: - exclude: - - rvm: 1.9.3 - gemfile: gemfiles/rails_5_0.gemfile - - rvm: 1.9.3 - gemfile: gemfiles/rails_5_1.gemfile - - rvm: 1.9.3 - gemfile: gemfiles/rails_5_2.gemfile - - rvm: 2.0.0 - gemfile: gemfiles/rails_5_0.gemfile - - rvm: 2.0.0 - gemfile: gemfiles/rails_5_1.gemfile - - rvm: 2.0.0 - gemfile: gemfiles/rails_5_2.gemfile - - rvm: 2.1.10 - gemfile: gemfiles/rails_5_0.gemfile - - rvm: 2.1.10 - gemfile: gemfiles/rails_5_1.gemfile - - rvm: 2.1.10 - gemfile: gemfiles/rails_5_2.gemfile diff --git a/Gemfile b/Gemfile index 142a1c46..bd499cf8 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,5 @@ source "http://rubygems.org" -gem "rack", "~> 1", platforms: [:ruby_19, :ruby_20, :ruby_21] - gemspec gem "rake" diff --git a/acts_as_list.gemspec b/acts_as_list.gemspec index 0c730c75..4b58162f 100644 --- a/acts_as_list.gemspec +++ b/acts_as_list.gemspec @@ -14,7 +14,7 @@ Gem::Specification.new do |s| s.description = 'This "acts_as" extension provides the capabilities for sorting and reordering a number of objects in a list. The class that has this specified needs to have a "position" column defined as an integer on the mapped database table.' s.license = "MIT" s.rubyforge_project = "acts_as_list" - s.required_ruby_version = ">= 1.9.2" + s.required_ruby_version = ">= 2.3.0" # Load Paths... s.files = `git ls-files`.split("\n") diff --git a/gemfiles/rails_4_2.gemfile b/gemfiles/rails_4_2.gemfile index c6529c04..27277ef6 100644 --- a/gemfiles/rails_4_2.gemfile +++ b/gemfiles/rails_4_2.gemfile @@ -2,7 +2,6 @@ source "http://rubygems.org" -gem "rack", "~> 1", platforms: [:ruby_19, :ruby_20, :ruby_21] gem "rake" gem "appraisal" gem "activerecord", "~> 4.2.10" diff --git a/gemfiles/rails_5_0.gemfile b/gemfiles/rails_5_0.gemfile index c97f10c7..79a914ce 100644 --- a/gemfiles/rails_5_0.gemfile +++ b/gemfiles/rails_5_0.gemfile @@ -2,7 +2,6 @@ source "http://rubygems.org" -gem "rack", "~> 1", platforms: [:ruby_19, :ruby_20, :ruby_21] gem "rake" gem "appraisal" gem "activerecord", "~> 5.0.6" diff --git a/gemfiles/rails_5_1.gemfile b/gemfiles/rails_5_1.gemfile index 1e788426..caf62083 100644 --- a/gemfiles/rails_5_1.gemfile +++ b/gemfiles/rails_5_1.gemfile @@ -2,7 +2,6 @@ source "http://rubygems.org" -gem "rack", "~> 1", platforms: [:ruby_19, :ruby_20, :ruby_21] gem "rake" gem "appraisal" gem "activerecord", "~> 5.1.4" diff --git a/gemfiles/rails_5_2.gemfile b/gemfiles/rails_5_2.gemfile index da76a799..17b0968a 100644 --- a/gemfiles/rails_5_2.gemfile +++ b/gemfiles/rails_5_2.gemfile @@ -2,7 +2,6 @@ source "http://rubygems.org" -gem "rack", "~> 1", platforms: [:ruby_19, :ruby_20, :ruby_21] gem "rake" gem "appraisal" gem "activerecord", "~> 5.2.0.rc1" From 4d2cef4291fa810b795099904f1f5c00b3d026ba Mon Sep 17 00:00:00 2001 From: Philippe Vaucher Date: Mon, 29 Oct 2018 17:14:00 +0100 Subject: [PATCH 3/6] Fix minitest warning --- test/helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helper.rb b/test/helper.rb index cd8374b8..0a239195 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -13,7 +13,7 @@ end require "active_record" require "minitest/autorun" -require "mocha/mini_test" +require "mocha/minitest" require "#{File.dirname(__FILE__)}/../init" if defined?(ActiveRecord::VERSION) && From 28b2db6e6e88660013c875a30d45243f9f1c967c Mon Sep 17 00:00:00 2001 From: Philippe Vaucher Date: Tue, 30 Oct 2018 14:52:56 +0100 Subject: [PATCH 4/6] Update rails 5.2 version --- gemfiles/rails_5_2.gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gemfiles/rails_5_2.gemfile b/gemfiles/rails_5_2.gemfile index 17b0968a..f54d7ee6 100644 --- a/gemfiles/rails_5_2.gemfile +++ b/gemfiles/rails_5_2.gemfile @@ -4,7 +4,7 @@ source "http://rubygems.org" gem "rake" gem "appraisal" -gem "activerecord", "~> 5.2.0.rc1" +gem "activerecord", "~> 5.2.1" group :development do gem "github_changelog_generator", "1.9.0" From 83ed1366b5a01588bbdb4a6de91fb66c904f745c Mon Sep 17 00:00:00 2001 From: Philippe Vaucher Date: Mon, 29 Oct 2018 11:53:23 +0100 Subject: [PATCH 5/6] Add AdvisoryLock module --- acts_as_list.gemspec | 1 + lib/acts_as_list.rb | 1 + .../active_record/acts/advisory_lock.rb | 43 +++++++++++++++++++ lib/acts_as_list/active_record/acts/list.rb | 1 + 4 files changed, 46 insertions(+) create mode 100644 lib/acts_as_list/active_record/acts/advisory_lock.rb diff --git a/acts_as_list.gemspec b/acts_as_list.gemspec index 4b58162f..891cb408 100644 --- a/acts_as_list.gemspec +++ b/acts_as_list.gemspec @@ -25,5 +25,6 @@ Gem::Specification.new do |s| # Dependencies (installed via "bundle install") s.add_dependency "activerecord", ">= 4.2" + s.add_dependency "with_advisory_lock", "~> 4.0" s.add_development_dependency "bundler", ">= 1.0.0" end diff --git a/lib/acts_as_list.rb b/lib/acts_as_list.rb index aef6f448..771e6c73 100644 --- a/lib/acts_as_list.rb +++ b/lib/acts_as_list.rb @@ -10,3 +10,4 @@ require "acts_as_list/active_record/acts/no_update" require "acts_as_list/active_record/acts/sequential_updates_method_definer" require "acts_as_list/active_record/acts/active_record" +require "acts_as_list/active_record/acts/advisory_lock" diff --git a/lib/acts_as_list/active_record/acts/advisory_lock.rb b/lib/acts_as_list/active_record/acts/advisory_lock.rb new file mode 100644 index 00000000..8ed8e74a --- /dev/null +++ b/lib/acts_as_list/active_record/acts/advisory_lock.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'with_advisory_lock' + +module ActiveRecord + module Acts #:nodoc: + module List #:nodoc: + module AdvisoryLock #:nodoc: + def self.included(base) + base.prepend(InstanceMethods) + end + + def self.acts_as_list_methods + ActiveRecord::Acts::List::InstanceMethods.public_instance_methods + end + + def self.acts_as_list_lockable_methods + acts_as_list_methods.grep(/^(insert_|move_|remove_|set_|(dec|inc)rement_)/) + end + + module InstanceMethods + AdvisoryLock.acts_as_list_lockable_methods.each do |m| + define_method(m) do |*args| + with_table_lock do + super(*args) + end + end + end + + def advisory_lock_name + format('lock-%s', acts_as_list_class.to_s.downcase) + end + + def with_table_lock + acts_as_list_class.with_advisory_lock(advisory_lock_name) do + yield + end + end + end + end + end + end +end diff --git a/lib/acts_as_list/active_record/acts/list.rb b/lib/acts_as_list/active_record/acts/list.rb index 3fb3a247..5bd9be2f 100644 --- a/lib/acts_as_list/active_record/acts/list.rb +++ b/lib/acts_as_list/active_record/acts/list.rb @@ -37,6 +37,7 @@ def acts_as_list(options = {}) include ActiveRecord::Acts::List::InstanceMethods include ActiveRecord::Acts::List::NoUpdate + include ActiveRecord::Acts::List::AdvisoryLock if configuration[:advisory_lock] end # This +acts_as+ extension provides the capabilities for sorting and reordering a number of objects in a list. From 6a6e5e6b993886c7090d4d7cf6f2af4380a19d43 Mon Sep 17 00:00:00 2001 From: Philippe Vaucher Date: Mon, 29 Oct 2018 17:14:21 +0100 Subject: [PATCH 6/6] Add test showing the race condition --- test/test_list.rb | 61 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/test_list.rb b/test/test_list.rb index dff99a9e..05857323 100644 --- a/test/test_list.rb +++ b/test/test_list.rb @@ -63,6 +63,26 @@ class ListMixin < Mixin acts_as_list column: "pos", scope: :parent end +class SlowListMixin < Mixin + acts_as_list column: "pos", scope: :parent + + before_create :slow_operation + + def slow_operation + sleep 0.1 + end +end + +class SlowListMixinWithAdvisoryLock < Mixin + acts_as_list column: "pos", scope: :parent, advisory_lock: true + + before_create :slow_operation + + def slow_operation + sleep 0.1 + end +end + class ListMixinSub1 < ListMixin end @@ -256,6 +276,47 @@ def test_insert_race_condition end end +class ListAdvisoryLockTest < ActsAsListTestCase + def setup + setup_db + end + + def test_errors_without_lock + range = 1..10 + threads = range.map do + Thread.new do + begin + SlowListMixin.create!(parent_id: 5) + rescue + # Retry ActiveRecord::ConnectionTimeoutError, etc + retry + end + end + end + threads.each(&:join) + + assert(range.to_a != SlowListMixin.order(:pos).pluck(:pos)) + end + + + def test_no_errors_with_lock + range = 1..10 + threads = range.map do + Thread.new do + begin + SlowListMixinWithAdvisoryLock.create!(parent_id: 5) + rescue + # Retry ActiveRecord::ConnectionTimeoutError, etc + retry + end + end + end + threads.each(&:join) + + assert_equal(range.to_a, SlowListMixinWithAdvisoryLock.order(:pos).pluck(:pos)) + end +end + class ListWithCallbackTest < ActsAsListTestCase include Shared::List