fixup! Extract compostable objects between Trigger and FunctionGenerator

Author: teoljungberg

lib/fx/generators/name_formatter.rb lib/fx/generators/name_helper.rblib/fx/generators/name_formatter.rb renamed to lib/fx/generators/name_helper.rb

@@ -1,7 +1,7 @@
 module Fx
   module Generators
     # @api private
-    class NameFormatter
+    class NameHelper
       def self.format_for_migration(name)
         if name.include?(".")
           "\"#{name}\""
@@ -22,6 +22,7 @@ def self.format_table_name_from_hash(table_hash)
 
       def self.validate_and_format(name)
         raise ArgumentError, "Name cannot be blank" if name.blank?
+
         format_for_migration(name)
       end
     end

lib/fx/generators/path_helper.rb

@@ -1,18 +1,18 @@
 module Fx
   module Generators
     # @api private
-    class VersionCalculator
+    class VersionHelper
       def initialize(file_name, definition_path)
         @file_name = file_name
         @definition_path = definition_path
       end
 
       def previous_version
-        @_previous_version ||= calculate_previous_version
+        @previous_version ||= existing_versions.max || 0
       end
 
       def current_version
-        @_current_version ||= previous_version.next
+        previous_version.next
       end
 
       def updating_existing?
@@ -23,10 +23,6 @@ def creating_new?
         previous_version == 0
       end
 
-      def version_glob_pattern
-        "#{file_name}_v*.sql"
-      end
-
       def definition_for_version(version, type)
         case type
         when :function
@@ -40,22 +36,15 @@ def definition_for_version(version, type)
 
       private
 
+      VERSION_PATTERN = /v(\d+)/
+
       attr_reader :file_name, :definition_path
 
-      def calculate_previous_version
-        Dir.glob(version_glob_pattern, base: definition_path)
-          .map { |filename| extract_version_from_filename(filename) }
+      def existing_versions
+        Dir
+          .glob("#{file_name}_v*.sql", base: definition_path)
+          .map { |f| f[VERSION_PATTERN, 1].to_i }
           .compact
-          .max || 0
-      end
-
-      def extract_version_from_filename(filename)
-        match = filename.match(version_regex)
-        match ? match["version"].to_i : nil
-      end
-
-      def version_regex
-        /\A#{Regexp.escape(file_name)}_v(?<version>\d+)\.sql\z/
       end
     end
   end

lib/fx/generators/version_calculator.rb lib/fx/generators/version_helper.rblib/fx/generators/version_calculator.rb renamed to lib/fx/generators/version_helper.rb

@@ -1,9 +1,8 @@
 require "rails/generators"
 require "rails/generators/active_record"
-require "fx/generators/version_calculator"
+require "fx/generators/version_helper"
 require "fx/generators/migration_helper"
-require "fx/generators/name_formatter"
-require "fx/generators/path_helper"
+require "fx/generators/name_helper"
 
 module Fx
   module Generators
@@ -12,14 +11,16 @@ class FunctionGenerator < Rails::Generators::NamedBase
       include Rails::Generators::Migration
       source_root File.expand_path("../templates", __FILE__)
 
+      DEFINITION_PATH = %w[db functions].freeze
+
       class_option :migration, type: :boolean
 
       def create_functions_directory
-        PathHelper.ensure_directory_exists(self, function_definition_path)
+        empty_directory(function_definition_path) unless function_definition_path.exist?
       end
 
       def create_function_definition
-        if version_calculator.creating_new?
+        if version_helper.creating_new?
           create_file definition.path
         else
           copy_file previous_definition.full_path, definition.full_path
@@ -32,8 +33,8 @@ def create_migration_file
         template_info = migration_helper.migration_template_info(
           :function,
           file_name,
-          version_calculator.updating_existing?,
-          version_calculator.current_version
+          version_helper.updating_existing?,
+          version_helper.current_version
         )
 
         migration_template(template_info[:template], template_info[:filename])
@@ -45,15 +46,15 @@ def self.next_migration_number(dir)
 
       no_tasks do
         def previous_version
-          version_calculator.previous_version
+          version_helper.previous_version
         end
 
         def version
-          version_calculator.current_version
+          version_helper.current_version
         end
 
         def migration_class_name
-          if version_calculator.updating_existing?
+          if version_helper.updating_existing?
             migration_helper.update_migration_class_name(:function, class_name, version)
           else
             super
@@ -65,38 +66,38 @@ def activerecord_migration_class
         end
 
         def formatted_name
-          NameFormatter.format_for_migration(singular_name)
+          NameHelper.format_for_migration(singular_name)
         end
       end
 
       private
 
       def function_definition_path
-        @_function_definition_path ||= PathHelper.definition_path_for(:function)
+        @_function_definition_path ||= Rails.root.join(*DEFINITION_PATH)
       end
 
-      def version_calculator
-        @_version_calculator ||= VersionCalculator.new(file_name, function_definition_path)
+      def version_helper
+        @_version_helper ||= VersionHelper.new(file_name, function_definition_path)
       end
 
       def migration_helper
         @_migration_helper ||= MigrationHelper.new(options)
       end
 
       def definition
-        version_calculator.definition_for_version(version, :function)
+        version_helper.definition_for_version(version, :function)
       end
 
       def previous_definition
-        version_calculator.definition_for_version(previous_version, :function)
+        version_helper.definition_for_version(previous_version, :function)
       end
 
       def updating_existing_function?
-        version_calculator.updating_existing?
+        version_helper.updating_existing?
       end
 
       def creating_new_function?
-        version_calculator.creating_new?
+        version_helper.creating_new?
       end
     end
   end

lib/generators/fx/function/function_generator.rb

@@ -1,9 +1,8 @@
 require "rails/generators"
 require "rails/generators/active_record"
-require "fx/generators/version_calculator"
+require "fx/generators/version_helper"
 require "fx/generators/migration_helper"
-require "fx/generators/name_formatter"
-require "fx/generators/path_helper"
+require "fx/generators/name_helper"
 
 module Fx
   module Generators
@@ -13,10 +12,12 @@ class TriggerGenerator < Rails::Generators::NamedBase
       source_root File.expand_path("../templates", __FILE__)
       argument :table_name, type: :hash, required: true
 
+      DEFINITION_PATH = %w[db triggers].freeze
+
       class_option :migration, type: :boolean
 
       def create_triggers_directory
-        PathHelper.ensure_directory_exists(self, trigger_definition_path)
+        empty_directory(trigger_definition_path) unless trigger_definition_path.exist?
       end
 
       def create_trigger_definition
@@ -29,8 +30,8 @@ def create_migration_file
         template_info = migration_helper.migration_template_info(
           :trigger,
           file_name,
-          version_calculator.updating_existing?,
-          version_calculator.current_version
+          version_helper.updating_existing?,
+          version_helper.current_version
         )
 
         migration_template(template_info[:template], template_info[:filename])
@@ -42,15 +43,15 @@ def self.next_migration_number(dir)
 
       no_tasks do
         def previous_version
-          version_calculator.previous_version
+          version_helper.previous_version
         end
 
         def version
-          version_calculator.current_version
+          version_helper.current_version
         end
 
         def migration_class_name
-          if version_calculator.updating_existing?
+          if version_helper.updating_existing?
             migration_helper.update_migration_class_name(:trigger, class_name, version)
           else
             super
@@ -62,34 +63,34 @@ def activerecord_migration_class
         end
 
         def formatted_name
-          NameFormatter.format_for_migration(singular_name)
+          NameHelper.format_for_migration(singular_name)
         end
 
         def formatted_table_name
-          NameFormatter.format_table_name_from_hash(table_name)
+          NameHelper.format_table_name_from_hash(table_name)
         end
       end
 
       private
 
       def trigger_definition_path
-        @_trigger_definition_path ||= PathHelper.definition_path_for(:trigger)
+        @_trigger_definition_path ||= Rails.root.join(*DEFINITION_PATH)
       end
 
-      def version_calculator
-        @_version_calculator ||= VersionCalculator.new(file_name, trigger_definition_path)
+      def version_helper
+        @_version_helper ||= VersionHelper.new(file_name, trigger_definition_path)
       end
 
       def migration_helper
         @_migration_helper ||= MigrationHelper.new(options)
       end
 
       def definition
-        version_calculator.definition_for_version(version, :trigger)
+        version_helper.definition_for_version(version, :trigger)
       end
 
       def updating_existing_trigger?
-        version_calculator.updating_existing?
+        version_helper.updating_existing?
       end
     end
   end

lib/generators/fx/trigger/trigger_generator.rb

@@ -58,22 +58,6 @@
 
       expect(helper.activerecord_migration_class).to eq("ActiveRecord::Migration")
     end
-
-    it "memoizes the result" do
-      allow(ActiveRecord::Migration).to receive(:respond_to?)
-        .with(:current_version)
-        .and_return(true)
-      allow(ActiveRecord::Migration).to receive(:current_version)
-        .and_return(6.1)
-
-      helper = described_class.new({})
-
-      first_call = helper.activerecord_migration_class
-      second_call = helper.activerecord_migration_class
-
-      expect(first_call).to eq(second_call)
-      expect(ActiveRecord::Migration).to have_received(:respond_to?).once
-    end
   end
 
   describe "#update_migration_class_name" do

spec/fx/generators/migration_helper_spec.rb

@@ -1,7 +1,7 @@
 require "spec_helper"
-require "fx/generators/name_formatter"
+require "fx/generators/name_helper"
 
-RSpec.describe Fx::Generators::NameFormatter do
+RSpec.describe Fx::Generators::NameHelper do
   describe ".format_for_migration" do
     it "returns symbol format for simple names" do
       result = described_class.format_for_migration("simple_name")
@@ -104,17 +104,5 @@
         described_class.validate_and_format("")
       }.to raise_error(ArgumentError, "Name cannot be blank")
     end
-
-    it "raises error for nil names" do
-      expect {
-        described_class.validate_and_format(nil)
-      }.to raise_error(ArgumentError, "Name cannot be blank")
-    end
-
-    it "raises error for whitespace-only names" do
-      expect {
-        described_class.validate_and_format("   ")
-      }.to raise_error(ArgumentError, "Name cannot be blank")
-    end
   end
 end