Add new option. Review.

Author: omeH

lib/table_sync/receiving/config.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative "hooks/once"
+
 module TableSync::Receiving
   class Config
     attr_reader :model, :events
@@ -53,19 +55,23 @@ def add_option(name, value_setter_wrapper:, value_as_proc_setter_wrapper:, defau
         end
       end
 
-      def add_simple_option(name)
+      def add_hook_option(name, hook_class:)
         ivar = :"@#{name}"
 
-        @default_values_for_options ||= {}
-        @default_values_for_options[ivar] = proc { [nil, proc {}] }
-
-        define_method(name) do |options = nil, &block|
-          old_options, old_block = instance_variable_get(ivar)
+        default_conditions = { columns: %i[] }
+        default_handler = proc { |**_| }
 
-          new_options = options || old_options
-          new_block = block || old_block
+        @default_values_for_options ||= {}
+        @default_values_for_options[ivar] = proc do
+          hook_class.new(
+            conditions: default_conditions,
+            handler: default_handler,
+          )
+        end
 
-          instance_variable_set(ivar, [new_options, new_block])
+        define_method(name) do |conditions, &handler|
+          hook = hook_class.new(conditions:, handler:)
+          instance_variable_set(ivar, hook)
         end
       end
     end
@@ -218,7 +224,10 @@ def option(name)
   value_as_proc_setter_wrapper: any_value,
   default: proc { proc { |&block| block.call } }
 
-TableSync::Receiving::Config.add_simple_option :on_first_sync
+TableSync::Receiving::Config.add_hook_option(
+  :on_first_sync,
+  hook_class: TableSync::Receiving::Hooks::Once,
+)
 
 %i[
   before_update

lib/table_sync/receiving/handler.rb

@@ -161,15 +161,8 @@ def perform(config, params) # rubocop:disable Metrics/MethodLength
         model.after_commit do
           config.option(:after_commit_on_update, **params, results:)
 
-          # In case the `on_first_sync` option is not configured
-          # we'll have default value:
-          #   [nil, proc {}]
-          # Default value is treated as the option is disabled.
-          conditions, handler = config.option(:on_first_sync)
-          if conditions.present?
-            hook = ::TableSync::Receiving::Hooks::Once.new(conditions:, config:)
-            hook.perform(targets: results, &handler)
-          end
+          hook = config.option(:on_first_sync)
+          hook.perform(config:, targets: results) if hook.enabled?
         end
       else
         model.after_commit { config.option(:after_commit_on_destroy, **params, results:) }

lib/table_sync/receiving/hooks/once.rb

@@ -2,14 +2,19 @@
 
 module TableSync::Receiving::Hooks
   class Once
-    attr_reader :conditions, :config
+    attr_reader :conditions, :handler, :lookup_code
 
-    def initialize(conditions:, config:)
+    def initialize(conditions:, handler:)
       @conditions = conditions
-      @config = config
+      @handler = handler
+      init_lookup_code
     end
 
-    def perform(targets:, &)
+    def enabled?
+      conditions[:columns].any?
+    end
+
+    def perform(config:, targets:)
       target_keys = config.option(:target_keys)
       model = config.model
 
@@ -21,8 +26,8 @@ def perform(targets:, &)
             next unless allow?(entry)
 
             entry.hooks ||= []
-            entry.hooks << hook_lookup_code
-            model.after_commit { yield(entry:) }
+            entry.hooks << lookup_code
+            model.after_commit { handler.call(entry:) }
           end
         end
       end
@@ -31,11 +36,11 @@ def perform(targets:, &)
     private
 
     def allow?(entry)
-      Array(entry.hooks).exclude?(hook_lookup_code)
+      Array(entry.hooks).exclude?(lookup_code)
     end
 
-    def hook_lookup_code
-      @hook_lookup_code ||= conditions[:columns].map do |column|
+    def init_lookup_code
+      @lookup_code = conditions[:columns].map do |column|
         "#{column}-#{conditions[column]}"
       end.join(":")
     end

lib/table_sync/receiving/model/active_record.rb

@@ -8,7 +8,6 @@ class ActiveRecord
       repeatable: :repeatable_read,
       serializable: :serializable,
     }.freeze
-    private_constant :ISOLATION_LEVELS
 
     class AfterCommitWrap
       def initialize(&block)

lib/table_sync/receiving/model/sequel.rb

@@ -10,7 +10,6 @@ class Sequel
       repeatable: :repeatable,
       serializable: :serializable,
     }.freeze
-    private_constant :ISOLATION_LEVELS
 
     def initialize(table_name)
       @raw_model = Class.new(::Sequel::Model(table_name)).tap(&:unrestrict_primary_key)

spec/receiving/config_spec.rb

@@ -351,33 +351,23 @@
 
     describe "#on_first_sync" do
       it "returns correct default value" do
-        value, callable = config.option(:on_first_sync)
-        expect(value).to be_nil
-        expect(callable).to be_a(Proc)
+        hook = config.option(:on_first_sync)
+        expect(hook).not_to be_nil
+        expect(hook).not_to be_enabled
+        expect(hook.conditions[:columns]).to be_empty
+        expect(hook.lookup_code).to eq("")
       end
 
-      it "processes a single value" do
-        config.on_first_sync :test
-
-        value, callable = config.option(:on_first_sync)
-        expect(value).to eq(:test)
-        expect(callable).to be_a(Proc)
-      end
-
-      it "processes a single block" do
-        config.on_first_sync { :test }
-
-        value, callable = config.option(:on_first_sync)
-        expect(value).to be_nil
-        expect(callable.call).to eq(:test)
-      end
-
-      it "processes a value and a block" do
-        config.on_first_sync(:spam) { :test }
+      it "processes a value" do
+        config.on_first_sync(columns: %i[test], test: "value") do |**_|
+          # Some hook work here
+        end
 
-        value, callable = config.option(:on_first_sync)
-        expect(value).to eq(:spam)
-        expect(callable.call).to eq(:test)
+        hook = config.option(:on_first_sync)
+        expect(hook).not_to be_nil
+        expect(hook).to be_enabled
+        expect(hook.conditions[:columns]).not_to be_empty
+        expect(hook.lookup_code).to eq("test-value")
       end
     end