Add thread-safety and idempotency (#48)

* Add thread-safety and idempotency to loader
* Fix CI

Author: djezzzl

activerecord-gemfiles/ar_5_latest.gemfile

@@ -1,3 +1,4 @@
 # frozen_string_literal: true
 
 gem "activerecord", "~> 5"
+gem "concurrent-ruby", "= 1.3.4"

activerecord-gemfiles/ar_6_latest.gemfile

@@ -1,3 +1,6 @@
 # frozen_string_literal: true
 
 gem "activerecord", "~> 6"
+gem "benchmark"
+gem "bigdecimal"
+gem "concurrent-ruby", "= 1.3.4"

activerecord-gemfiles/ar_7_latest.gemfile

@@ -1,3 +1,6 @@
 # frozen_string_literal: true
 
 gem "activerecord", "~> 7"
+gem "benchmark"
+gem "bigdecimal"
+gem "concurrent-ruby", "= 1.3.4"

lib/n1_loader/ar_lazy_preload/loader_patch.rb

@@ -7,12 +7,15 @@ module LoaderPatch
       attr_accessor :context_setup
 
       def loaded
-        return @loaded if @loaded
+        return @loaded if @already_loaded && @already_context
 
         super
 
-        context_setup&.call(preloaded_records)
+        synchronize do
+          context_setup&.call(@loaded.values.flatten) unless @already_context
+        end
 
+        @already_context = true
         @loaded
       end
     end

lib/n1_loader/core/loader.rb

@@ -6,6 +6,8 @@ module N1Loader
   # Subclasses must define +perform+ method that accepts single argument
   # and returns hash where key is the element and value is what we want to load.
   class Loader
+    prepend MonitorMixin
+
     class << self
       attr_reader :arguments
 
@@ -105,7 +107,17 @@ def fulfill(element, value)
     end
 
     def loaded
-      return @loaded if @loaded
+      return @loaded if @already_loaded
+
+      synchronize do
+        non_thread_safe_loaded unless @already_loaded
+      end
+
+      @loaded
+    end
+
+    def non_thread_safe_loaded
+      return @loaded if @already_loaded
 
       check_arguments!
 
@@ -117,6 +129,7 @@ def loaded
         perform(elements)
       end
 
+      @already_loaded = true
       @loaded
     end
   end

n1_loader.gemspec

@@ -21,6 +21,8 @@ Gem::Specification.new do |spec|
   spec.files = Dir["lib/**/*"]
   spec.require_paths = ["lib"]
 
+  spec.add_runtime_dependency "mutex_m"
+
   spec.add_development_dependency "activerecord", ">= 5"
   spec.add_development_dependency "ar_lazy_preload", ">= 0.6"
   spec.add_development_dependency "db-query-matchers", "~> 0.11"

spec/n1_loader_spec.rb

@@ -31,6 +31,13 @@ def count
         elements.each { |element| fulfill(element, [element]) }
       end
 
+      n1_optimized :sleepy do |elements|
+        sleep(0.5)
+
+        elements.first.class.perform!
+        elements.each { |element| fulfill(element, [element]) }
+      end
+
       n1_optimized :custom, custom_loader
 
       n1_optimized :single_optimized do
@@ -138,6 +145,42 @@ def perform(elements)
     end.not_to raise_error(NoMethodError)
   end
 
+  describe "thread-safety" do
+    it "is thread-safe" do
+      N1Loader::Preloader.new(objects).preload(:sleepy)
+
+      threads = []
+
+      10.times do
+        threads << Thread.new do
+          objects.each do |obj|
+            expect(obj.sleepy).to eq([obj])
+          end
+        end
+      end
+      threads.each(&:join)
+
+      expect(klass.count).to eq(1)
+    end
+  end
+
+  describe "error handling" do
+    it "raises the same error on the subsequent calls" do
+      faulty_klass = Class.new do
+        include N1Loader::Loadable
+
+        n1_optimized :faulty do |_|
+          raise StandardError, "Something went wrong"
+        end
+      end
+
+      faulty_object = faulty_klass.new
+
+      expect { faulty_object.faulty }.to raise_error(StandardError, "Something went wrong")
+      expect { faulty_object.faulty }.to raise_error(StandardError, "Something went wrong")
+    end
+  end
+
   describe "loaded comparison" do
     it "compares by value" do
       instance = loader.new(objects)