Refactor generators with helper classes and improvements (#183)

Extract generator logic into focused helper classes following Single
Responsibility Principle, and improve code organization and readability.

New Helper Classes:
- `Fx::Generators::MigrationHelper`, handles migration creation logic
- `Fx::Generators::NameHelper`, handles name formatting for migrations
- `Fx::Generators::VersionHelper`, manages version tracking and file
  discovery

Generator Improvements:
- Simplify `FunctionGenerator` and `TriggerGenerator` by delegating to
  helpers
- Remove inline version regex patterns
- Remove inline path definitions
- Remove duplicate logic for checking if creating/updating
- Remove unused Dir.entries stub in function generator spec
- Change SQL query to heredoc format in `TriggerGenerator`

Naming Consistency:
- s/activerecord_migration_class/active_record_migration_class

Author: teoljungberg

lib/generators/fx/function/function_generator.rb

@@ -1,5 +1,8 @@
 require "rails/generators"
 require "rails/generators/active_record"
+require "generators/fx/version_helper"
+require "generators/fx/migration_helper"
+require "generators/fx/name_helper"
 
 module Fx
   module Generators
@@ -9,35 +12,38 @@ class FunctionGenerator < Rails::Generators::NamedBase
 
       source_root File.expand_path("../templates", __FILE__)
 
+      DEFINITION_PATH = %w[db functions].freeze
+
       class_option :migration, type: :boolean
 
       def create_functions_directory
-        unless function_definition_path.exist?
-          empty_directory(function_definition_path)
-        end
+        return if function_definition_path.exist?
+
+        empty_directory(function_definition_path)
       end
 
       def create_function_definition
-        if creating_new_function?
-          create_file definition.path
+        if version_helper.creating_new?
+          create_file(definition.path)
         else
-          copy_file previous_definition.full_path, definition.full_path
+          copy_file(previous_definition.full_path, definition.full_path)
         end
       end
 
       def create_migration_file
-        return if skip_migration_creation?
-        if updating_existing_function?
-          migration_template(
-            "db/migrate/update_function.erb",
-            "db/migrate/update_function_#{file_name}_to_version_#{version}.rb"
-          )
-        else
-          migration_template(
-            "db/migrate/create_function.erb",
-            "db/migrate/create_function_#{file_name}.rb"
-          )
-        end
+        return if migration_helper.skip_creation?
+
+        template_info = migration_helper.migration_template_info(
+          object_type: :function,
+          file_name: file_name,
+          updating_existing: version_helper.updating_existing?,
+          version: version_helper.current_version
+        )
+
+        migration_template(
+          template_info.fetch(:template),
+          template_info.fetch(:filename)
+        )
       end
 
       def self.next_migration_number(dir)
@@ -46,75 +52,65 @@ def self.next_migration_number(dir)
 
       no_tasks do
         def previous_version
-          @_previous_version ||= Dir.entries(function_definition_path)
-            .map { |name| version_regex.match(name).try(:[], "version").to_i }
-            .max
+          version_helper.previous_version
         end
 
         def version
-          @_version ||= previous_version.next
+          version_helper.current_version
         end
 
         def migration_class_name
-          if updating_existing_function?
-            "UpdateFunction#{class_name}ToVersion#{version}"
+          if version_helper.updating_existing?
+            migration_helper.update_migration_class_name(
+              object_type: :function,
+              class_name: class_name,
+              version: version
+            )
           else
             super
           end
         end
 
-        def activerecord_migration_class
-          if ActiveRecord::Migration.respond_to?(:current_version)
-            "ActiveRecord::Migration[#{ActiveRecord::Migration.current_version}]"
-          else
-            "ActiveRecord::Migration"
-          end
+        def active_record_migration_class
+          migration_helper.active_record_migration_class
         end
 
         def formatted_name
-          if singular_name.include?(".")
-            "\"#{singular_name}\""
-          else
-            ":#{singular_name}"
-          end
+          NameHelper.format_for_migration(singular_name)
         end
       end
 
       private
 
       def function_definition_path
-        @_function_definition_path ||= Rails.root.join(*%w[db functions])
+        @_function_definition_path ||= Rails.root.join(*DEFINITION_PATH)
       end
 
-      def version_regex
-        /\A#{file_name}_v(?<version>\d+)\.sql\z/
+      def version_helper
+        @_version_helper ||= Fx::Generators::VersionHelper.new(
+          file_name: file_name,
+          definition_path: function_definition_path
+        )
       end
 
-      def updating_existing_function?
-        previous_version > 0
-      end
-
-      def creating_new_function?
-        previous_version == 0
+      def migration_helper
+        @_migration_helper ||= Fx::Generators::MigrationHelper.new(options)
       end
 
       def definition
-        Fx::Definition.function(name: file_name, version: version)
+        version_helper.definition_for_version(version: version, type: :function)
       end
 
       def previous_definition
-        Fx::Definition.function(name: file_name, version: previous_version)
+        version_helper.definition_for_version(version: previous_version, type: :function)
       end
 
-      # Skip creating migration file if:
-      #   - migrations option is nil or false
-      def skip_migration_creation?
-        !migration
+      def updating_existing_function?
+        version_helper.updating_existing?
       end
 
-      # True unless explicitly false
-      def migration
-        options[:migration] != false
+      def creating_new_function?
+        version_helper.creating_new?
       end
     end
   end

lib/generators/fx/function/templates/db/migrate/create_function.erb

@@ -1,4 +1,4 @@
-class <%= migration_class_name %> < <%= activerecord_migration_class %>
+class <%= migration_class_name %> < <%= active_record_migration_class %>
   def change
     create_function <%= formatted_name %>
   end

lib/generators/fx/function/templates/db/migrate/update_function.erb

@@ -1,4 +1,4 @@
-class <%= migration_class_name %> < <%= activerecord_migration_class %>
+class <%= migration_class_name %> < <%= active_record_migration_class %>
   def change
     update_function <%= formatted_name %>, version: <%= version %>, revert_to_version: <%= previous_version %>
   end

lib/generators/fx/migration_helper.rb

@@ -0,0 +1,53 @@
+module Fx
+  module Generators
+    # @api private
+    class MigrationHelper
+      def initialize(options)
+        @options = options
+      end
+
+      def skip_creation?
+        !should_create_migration?
+      end
+
+      def active_record_migration_class
+        if ActiveRecord::Migration.respond_to?(:current_version)
+          "ActiveRecord::Migration[#{ActiveRecord::Migration.current_version}]"
+        else
+          "ActiveRecord::Migration"
+        end
+      end
+
+      def update_migration_class_name(object_type:, class_name:, version:)
+        "Update#{object_type.capitalize}#{class_name}ToVersion#{version}"
+      end
+
+      def migration_template_info(
+        object_type:,
+        file_name:,
+        updating_existing:,
+        version:
+      )
+        if updating_existing
+          {
+            template: "db/migrate/update_#{object_type}.erb",
+            filename: "db/migrate/update_#{object_type}_#{file_name}_to_version_#{version}.rb"
+          }
+        else
+          {
+            template: "db/migrate/create_#{object_type}.erb",
+            filename: "db/migrate/create_#{object_type}_#{file_name}.rb"
+          }
+        end
+      end
+
+      private
+
+      attr_reader :options
+
+      def should_create_migration?
+        options.fetch(:migration, true)
+      end
+    end
+  end
+end

lib/generators/fx/name_helper.rb

@@ -0,0 +1,33 @@
+module Fx
+  module Generators
+    # @api private
+    class NameHelper
+      def self.format_for_migration(name)
+        if name.include?(".")
+          "\"#{name}\""
+        else
+          ":#{name}"
+        end
+      end
+
+      def self.format_table_name_from_hash(table_hash)
+        name = table_hash["table_name"] || table_hash["on"]
+
+        if name.nil?
+          raise(
+            ArgumentError,
+            "Either `table_name:NAME` or `on:NAME` must be specified"
+          )
+        end
+
+        format_for_migration(name)
+      end
+
+      def self.validate_and_format(name)
+        raise ArgumentError, "Name cannot be blank" if name.blank?
+
+        format_for_migration(name)
+      end
+    end
+  end
+end

lib/generators/fx/trigger/templates/db/migrate/create_trigger.erb

@@ -1,4 +1,4 @@
-class <%= migration_class_name %> < <%= activerecord_migration_class %>
+class <%= migration_class_name %> < <%= active_record_migration_class %>
   def change
     create_trigger <%= formatted_name %>, on: <%= formatted_table_name %>
   end

lib/generators/fx/trigger/templates/db/migrate/update_trigger.erb

@@ -1,4 +1,4 @@
-class <%= migration_class_name %> < <%= activerecord_migration_class %>
+class <%= migration_class_name %> < <%= active_record_migration_class %>
   def change
     update_trigger <%= formatted_name %>, on: <%= formatted_table_name %>, version: <%= version %>, revert_to_version: <%= previous_version %>
   end