From b68601dbd4aafe65c4cc2ba5f5f37ad743a3bb58 Mon Sep 17 00:00:00 2001 From: Mark Meeus Date: Mon, 30 Sep 2013 20:57:06 +0200 Subject: [PATCH 1/3] disable observers without cross-thread side effects --- .../observers/active_model/observer_array.rb | 18 +++++++++++++++--- test/observer_array_test.rb | 8 +++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/rails/observers/active_model/observer_array.rb b/lib/rails/observers/active_model/observer_array.rb index 77bc0f7..37b0f14 100644 --- a/lib/rails/observers/active_model/observer_array.rb +++ b/lib/rails/observers/active_model/observer_array.rb @@ -73,7 +73,11 @@ def enable(*observers, &block) protected def disabled_observers #:nodoc: - @disabled_observers ||= Set.new + Thread.current[disabled_observers_thread_key] ||= Set.new + end + + def disabled_observers= dis_observers #:nodoc: + Thread.current[disabled_observers_thread_key] = dis_observers end def observer_class_for(observer) #:nodoc: @@ -95,11 +99,11 @@ def start_transaction #:nodoc: end def disabled_observer_stack #:nodoc: - @disabled_observer_stack ||= [] + Thread.current[disabled_observer_stack_thread_key] ||= [] end def end_transaction #:nodoc: - @disabled_observers = disabled_observer_stack.pop + self.disabled_observers = disabled_observer_stack.pop each_subclass_array do |array| array.end_transaction end @@ -148,5 +152,13 @@ def set_enablement(enabled, observers) #:nodoc: end end end + + def disabled_observers_thread_key + "observer_array:#{model_class}::disabled_observers" + end + + def disabled_observer_stack_thread_key + "observer_array:#{model_class}::disabled_observer_stack" + end end end diff --git a/test/observer_array_test.rb b/test/observer_array_test.rb index 16fdf53..d103103 100644 --- a/test/observer_array_test.rb +++ b/test/observer_array_test.rb @@ -109,7 +109,6 @@ def assert_observer_not_notified(model_class, observer_class) test "can disable observers on individual models without affecting those observers on other models" do Widget.observers.disable :all - assert_observer_not_notified Widget, WidgetObserver assert_observer_notified Budget, BudgetObserver assert_observer_not_notified Widget, AuditTrail @@ -162,6 +161,13 @@ def assert_observer_not_notified(model_class, observer_class) assert_observer_notified Budget, AuditTrail end + test "can enable observer without side effects on other threads" do + Thread.new do + Widget.observers.disable :audit_trail + end.join() + assert_observer_notified Widget, AuditTrail + end + test "raises an appropriate error when a developer accidentally enables or disables the wrong class (i.e. Widget instead of WidgetObserver)" do assert_raise ArgumentError do ORM.observers.enable :widget From b03616ce6fb18d16e0a655a96c182ac252553ec4 Mon Sep 17 00:00:00 2001 From: Mark Meeus Date: Wed, 2 Oct 2013 09:46:00 +0200 Subject: [PATCH 2/3] use ActiveSupport::PerThreadRegistry --- .../active_model/disabled_observers_registry.rb | 13 +++++++++++++ lib/rails/observers/active_model/observer_array.rb | 8 ++++---- lib/rails/observers/active_model/observing.rb | 1 + 3 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 lib/rails/observers/active_model/disabled_observers_registry.rb diff --git a/lib/rails/observers/active_model/disabled_observers_registry.rb b/lib/rails/observers/active_model/disabled_observers_registry.rb new file mode 100644 index 0000000..f69cb5c --- /dev/null +++ b/lib/rails/observers/active_model/disabled_observers_registry.rb @@ -0,0 +1,13 @@ +module ActiveModel + class DisabledObserversRegistry + extend ActiveSupport::PerThreadRegistry + + attr_accessor :disabled_observers_per_class + attr_accessor :disabled_observers_stacks_per_class + + def initialize + @disabled_observers_per_class = {} + @disabled_observers_stacks_per_class = {} + end + end +end \ No newline at end of file diff --git a/lib/rails/observers/active_model/observer_array.rb b/lib/rails/observers/active_model/observer_array.rb index 37b0f14..cee0291 100644 --- a/lib/rails/observers/active_model/observer_array.rb +++ b/lib/rails/observers/active_model/observer_array.rb @@ -73,11 +73,11 @@ def enable(*observers, &block) protected def disabled_observers #:nodoc: - Thread.current[disabled_observers_thread_key] ||= Set.new + DisabledObserversRegistry.disabled_observers_per_class[model_class] ||= Set.new end - def disabled_observers= dis_observers #:nodoc: - Thread.current[disabled_observers_thread_key] = dis_observers + def disabled_observers= observers #:nodoc: + DisabledObserversRegistry.disabled_observers_per_class[model_class] = observers end def observer_class_for(observer) #:nodoc: @@ -99,7 +99,7 @@ def start_transaction #:nodoc: end def disabled_observer_stack #:nodoc: - Thread.current[disabled_observer_stack_thread_key] ||= [] + DisabledObserversRegistry.disabled_observers_stacks_per_class[model_class] ||= [] end def end_transaction #:nodoc: diff --git a/lib/rails/observers/active_model/observing.rb b/lib/rails/observers/active_model/observing.rb index 3f01a90..228cc08 100644 --- a/lib/rails/observers/active_model/observing.rb +++ b/lib/rails/observers/active_model/observing.rb @@ -1,4 +1,5 @@ require 'singleton' +require 'rails/observers/active_model/disabled_observers_registry' require 'rails/observers/active_model/observer_array' require 'active_support/core_ext/module/aliasing' require 'active_support/core_ext/module/remove_method' From b12cc323443f581785f38b6bbcc0fc732c4ec38b Mon Sep 17 00:00:00 2001 From: Mark Meeus Date: Wed, 2 Oct 2013 09:51:19 +0200 Subject: [PATCH 3/3] cleanup code --- lib/rails/observers/active_model/observer_array.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/rails/observers/active_model/observer_array.rb b/lib/rails/observers/active_model/observer_array.rb index cee0291..e9aa826 100644 --- a/lib/rails/observers/active_model/observer_array.rb +++ b/lib/rails/observers/active_model/observer_array.rb @@ -152,13 +152,5 @@ def set_enablement(enabled, observers) #:nodoc: end end end - - def disabled_observers_thread_key - "observer_array:#{model_class}::disabled_observers" - end - - def disabled_observer_stack_thread_key - "observer_array:#{model_class}::disabled_observer_stack" - end end end