From 0d7667ac315a7382146f39395914406fbb923f90 Mon Sep 17 00:00:00 2001 From: Brian Durand Date: Sat, 4 Jul 2026 22:47:16 -0700 Subject: [PATCH] Bug fixes --- CHANGELOG.md | 21 ++ README.md | 4 + VERSION | 2 +- lib/support_table_data.rb | 208 +++++++++++++----- .../documentation/source_file.rb | 2 +- lib/support_table_data/railtie.rb | 2 +- lib/support_table_data/tasks/utils.rb | 10 +- spec/data/sizes.yml | 19 ++ spec/data/sizes_override.yml | 2 + spec/models.rb | 8 + spec/models/size.rb | 10 + .../documentation/source_file_spec.rb | 24 ++ spec/support_table_data_spec.rb | 96 +++++++- 13 files changed, 341 insertions(+), 67 deletions(-) create mode 100644 spec/data/sizes.yml create mode 100644 spec/data/sizes_override.yml create mode 100644 spec/models/size.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index abeff51..39a8cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,27 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## 1.6.2 + +### Fixed + +- `sync_table_data!` with `delete_missing: true` now raises an `ArgumentError` instead of deleting every row in the table when the data files contain no rows (for example, when a data file was accidentally emptied or truncated). +- Generated predicate methods (e.g. `record.active?`) now cast the data file value to the attribute type before comparing. Previously the raw file value was compared to the cast database attribute, so predicates silently returned `false` whenever the types differed (guaranteed for CSV data files, where all values are strings). +- Single table inheritance subclasses no longer raise `NoMethodError` from `instance_names`, `instance_keys`, `protected_instance?`, and other class methods. Subclasses now share the support table state defined on their base class. +- Named instance helper methods are now redefined when a later data file overrides an attribute or key value, so the helpers always return the merged values that are synced to the database. Previously they permanently returned the values from the first file that defined the named instance. +- `named_instance_attribute_helpers` can now be called again with an attribute that was already registered without raising an `ArgumentError`. +- YAML data files can now use anchors/aliases and date/time values. Previously these raised `Psych::AliasesNotEnabled` or `Psych::DisallowedClass` errors. +- `protected_instance?` no longer returns stale results when data files are added after the protected keys were first computed. +- Fixed broken cycle detection in the autosave association check during syncs that could cause infinite recursion on cyclic autosave associations. +- `sync_table_data!` now retries once on `ActiveRecord::RecordNotUnique` errors caused by concurrent syncs inserting the same rows from another process. +- `sync_table_data!` now returns an empty array instead of `nil` when the table does not exist. +- `named_instance` now raises a clear `ActiveRecord::RecordNotFound` error for undefined named instances instead of querying the database for a `nil` key (which could silently return a row with a `NULL` key value). +- Memoized class-level state is now consistently synchronized with the class mutex to avoid races on non-MRI Ruby implementations. +- Setting `config.support_table.auto_sync = false` before the gem is loaded is no longer overwritten back to `true` by the Railtie. +- The documentation tasks no longer corrupt model source files that contain duplicated generated YARD doc blocks (e.g. from a bad merge); the regex that finds the generated block is no longer greedy. +- Data file names containing extra dots no longer break the class name detection used by `SupportTableData.sync_all!` to eager load models. +- Error messages for invalid named instance definitions now include the model class name instead of repeating the instance name. + ## 1.6.1 ### Fixed diff --git a/README.md b/README.md index 535693f..2a74f9c 100644 --- a/README.md +++ b/README.md @@ -279,6 +279,10 @@ SupportTableData.sync_all!(delete_missing: true) > [!CAUTION] > Use `delete_missing` with care. It will delete any records in the table that are not defined in the data files, which may include user-created data or fail due to foreign key constraints. +As a safeguard, `sync_table_data!` will raise an `ArgumentError` rather than delete anything when `delete_missing` is enabled but the data files contain no rows (for instance, when a data file was accidentally emptied or truncated). + +It is recommended to add a unique database index on the key attribute column. Concurrent syncs from multiple processes (for example, parallel deployment jobs) could otherwise insert duplicate rows. If a sync hits a uniqueness violation from a concurrent insert, it will automatically retry once to pick up the other process' changes. + The number of records contained in data files should be fairly small (ideally fewer than 100). It is possible to load just a subset of rows in a large table because only the rows listed in the data files will be synced. You can use this feature if your table allows user-entered data, but has a few rows that must exist for the code to work. Loading data is done inside a database transaction. No changes will be persisted to the database unless all rows for a model can be synced. diff --git a/VERSION b/VERSION index 9c6d629..fdd3be6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.6.1 +1.6.2 diff --git a/lib/support_table_data.rb b/lib/support_table_data.rb index 8e4dfd4..83f0300 100644 --- a/lib/support_table_data.rb +++ b/lib/support_table_data.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/module/redefine_method" + # This concern can be mixed into models that represent static support tables. These are small tables # that have a limited number of rows, and have values that are often tied to the logic in the code. # @@ -97,15 +99,22 @@ def support_table_yard_docs=(value) # files will be deleted. Use with caution. # @return [Array] List of saved changes for each record that was created or modified. def sync_table_data!(delete_missing: false) - return unless table_exists? + return [] unless table_exists? - canonical_data = support_table_data.each_with_object({}) do |attributes, hash| - hash[attributes[support_table_key_attribute].to_s] = attributes - end - records = where(support_table_key_attribute => canonical_data.keys) - changes = [] + retried = false begin + canonical_data = support_table_data.each_with_object({}) do |attributes, hash| + hash[attributes[support_table_key_attribute].to_s] = attributes + end + + if delete_missing && canonical_data.empty? && exists? + raise ArgumentError.new("Refusing to sync #{name} with delete_missing enabled because the data files contain no rows; this would delete every row in the table") + end + + records = where(support_table_key_attribute => canonical_data.keys) + changes = [] + ActiveSupport::Notifications.instrument("support_table_data.sync", class: self) do synced_ids = [] @@ -141,11 +150,17 @@ def sync_table_data!(delete_missing: false) end end end + changes rescue ActiveRecord::RecordInvalid => e raise SupportTableData::ValidationError.new(e.record) - end + rescue ActiveRecord::RecordNotUnique + # A concurrent sync from another process may have inserted the same rows. + # The transaction was rolled back, so retry once to pick up those rows. + raise if retried - changes + retried = true + retry + end end # Add a data file that contains the support table data. This method can be called multiple times to @@ -157,9 +172,10 @@ def sync_table_data!(delete_missing: false) # @return [void] def add_support_table_data(data_file_path) root_dir = (support_table_data_directory || SupportTableData.data_directory || Dir.pwd) - @mutex.synchronize do - @support_table_data_files += [File.expand_path(data_file_path, root_dir)] + support_table_mutex.synchronize do + @support_table_data_files = support_table_data_files + [File.expand_path(data_file_path, root_dir)] @support_table_instance_keys = nil + @protected_keys = nil end define_support_table_named_instances end @@ -172,9 +188,11 @@ def add_support_table_data(data_file_path) # @param attributes [String, Symbol] The names of the attributes to add helper methods for. # @return [void] def named_instance_attribute_helpers(*attributes) - @mutex.synchronize do + support_table_mutex.synchronize do attributes.flatten.collect(&:to_s).each do |attribute| - @support_table_attribute_helpers = @support_table_attribute_helpers.merge(attribute => []) + next if support_table_attribute_helpers_map.include?(attribute) + + @support_table_attribute_helpers = support_table_attribute_helpers_map.merge(attribute => []) end end define_support_table_named_instances @@ -185,7 +203,7 @@ def named_instance_attribute_helpers(*attributes) # # @return [Array] List of attribute names. def support_table_attribute_helpers - @support_table_attribute_helpers.keys + support_table_attribute_helpers_map.keys end # Get the data for the support table from the data files. @@ -193,7 +211,7 @@ def support_table_attribute_helpers # @return [Array] List of attributes for all records in the data files. def support_table_data data = {} - @support_table_data_files.each do |data_file_path| + support_table_data_files.each do |data_file_path| file_data = support_table_parse_data_file(data_file_path) file_data = file_data.values if file_data.is_a?(Hash) file_data = Array(file_data).flatten @@ -218,7 +236,7 @@ def named_instance_data(name) data = {} name = name.to_s - @support_table_data_files.each do |data_file_path| + support_table_data_files.each do |data_file_path| file_data = support_table_parse_data_file(data_file_path) next unless file_data.is_a?(Hash) @@ -237,7 +255,7 @@ def named_instance_data(name) # # @return [Array] List of all instance names. def instance_names - @support_table_instance_names.keys + support_table_instance_names_map.keys end # Load a named instance from the database. @@ -247,36 +265,54 @@ def instance_names # @raise [ActiveRecord::RecordNotFound] If the instance does not exist. def named_instance(instance_name) instance_name = instance_name.to_s - find_by!(support_table_key_attribute => @support_table_instance_names[instance_name]) + instances = support_table_instance_names_map + unless instances.include?(instance_name) + raise ActiveRecord::RecordNotFound.new("Couldn't find #{name} named instance #{instance_name.inspect}") + end + + find_by!(support_table_key_attribute => instances[instance_name]) end # Get the key values for all instances loaded from the data files. # # @return [Array] List of all the key attribute values. def instance_keys - if @support_table_instance_keys.nil? - values = [] - support_table_data.each do |attributes| - key_value = attributes[support_table_key_attribute] - instance = new - instance.send(:"#{support_table_key_attribute}=", key_value) - values << instance.send(support_table_key_attribute) + keys = @support_table_instance_keys + if keys.nil? + support_table_mutex.synchronize do + keys = @support_table_instance_keys + if keys.nil? + values = [] + support_table_data.each do |attributes| + key_value = attributes[support_table_key_attribute] + instance = new + instance.send(:"#{support_table_key_attribute}=", key_value) + values << instance.send(support_table_key_attribute) + end + keys = values.uniq + @support_table_instance_keys = keys + end end - @support_table_instance_keys = values.uniq end - @support_table_instance_keys + keys end # Return true if the instance has data being managed from a data file. # # @return [Boolean] def protected_instance?(instance) - unless defined?(@protected_keys) - keys = support_table_data.collect { |attributes| attributes[support_table_key_attribute].to_s } - @protected_keys = keys + keys = @protected_keys + if keys.nil? + support_table_mutex.synchronize do + keys = @protected_keys + if keys.nil? + keys = support_table_data.collect { |attributes| attributes[support_table_key_attribute].to_s } + @protected_keys = keys + end + end end - @protected_keys.include?(instance[support_table_key_attribute].to_s) + keys.include?(instance[support_table_key_attribute].to_s) end # Explicitly define other support tables that this model depends on. A support table depends @@ -290,22 +326,36 @@ def protected_instance?(instance) # @param class_names [String] List of class names that this support table depends on. # @return [void] def support_table_dependency(*class_names) - @support_table_dependencies += class_names.flatten.collect(&:to_s) + support_table_mutex.synchronize do + @support_table_dependencies = support_table_dependency_names + class_names.flatten.collect(&:to_s) + end end private def define_support_table_named_instances - @support_table_data_files.each do |file_path| + merged_data = {} + + support_table_data_files.each do |file_path| data = support_table_parse_data_file(file_path) next unless data.is_a?(Hash) data.each do |name, attributes| - @mutex.synchronize do - define_support_table_named_instance_methods(name, attributes) + name = name.to_s + existing = merged_data[name] + merged_data[name] = if existing.is_a?(Hash) && attributes.is_a?(Hash) + existing.merge(attributes) + else + attributes end end end + + merged_data.each do |name, attributes| + support_table_mutex.synchronize do + define_support_table_named_instance_methods(name, attributes) + end + end end def define_support_table_named_instance_methods(name, attributes) @@ -313,32 +363,43 @@ def define_support_table_named_instance_methods(name, attributes) return if method_name.start_with?("_") unless attributes.is_a?(Hash) - raise ArgumentError.new("Cannot define named instance #{method_name} on #{name}; value must be a Hash") + raise ArgumentError.new("Cannot define named instance #{method_name} on #{self.name}; value must be a Hash") end unless method_name.match?(/\A[a-z][a-z0-9_]+\z/) - raise ArgumentError.new("Cannot define named instance #{method_name} on #{name}; name contains illegal characters") + raise ArgumentError.new("Cannot define named instance #{method_name} on #{self.name}; name contains illegal characters") end key_value = attributes[support_table_key_attribute] + instance_names_map = support_table_instance_names_map - unless @support_table_instance_names.include?(method_name) + if instance_names_map.include?(method_name) + if instance_names_map[method_name] != key_value + define_support_table_instance_helper(method_name, support_table_key_attribute, key_value, redefine: true) + define_support_table_predicates_helper("#{method_name}?", support_table_key_attribute, key_value, redefine: true) + @support_table_instance_names = instance_names_map.merge(method_name => key_value) + end + else define_support_table_instance_helper(method_name, support_table_key_attribute, key_value) define_support_table_predicates_helper("#{method_name}?", support_table_key_attribute, key_value) - @support_table_instance_names = @support_table_instance_names.merge(method_name => key_value) + @support_table_instance_names = instance_names_map.merge(method_name => key_value) end - @support_table_attribute_helpers.each do |attribute_name, defined_methods| + support_table_attribute_helpers_map.each do |attribute_name, defined_methods| attribute_method_name = "#{method_name}_#{attribute_name}" - next if defined_methods.include?(attribute_method_name) - - define_support_table_instance_attribute_helper(attribute_method_name, attributes[attribute_name]) - defined_methods << attribute_method_name + if defined_methods.include?(attribute_method_name) + define_support_table_instance_attribute_helper(attribute_method_name, attributes[attribute_name], redefine: true) + else + define_support_table_instance_attribute_helper(attribute_method_name, attributes[attribute_name]) + defined_methods << attribute_method_name + end end end - def define_support_table_instance_helper(method_name, attribute_name, attribute_value) - if respond_to?(method_name, true) + def define_support_table_instance_helper(method_name, attribute_name, attribute_value, redefine: false) + if redefine + singleton_class.silence_redefinition_of_method(method_name) + elsif respond_to?(method_name, true) raise ArgumentError.new("Could not define support table helper method #{name}.#{method_name} because it is already a defined method") end @@ -349,8 +410,10 @@ def self.#{method_name} RUBY end - def define_support_table_instance_attribute_helper(method_name, attribute_value) - if respond_to?(method_name, true) + def define_support_table_instance_attribute_helper(method_name, attribute_value, redefine: false) + if redefine + singleton_class.silence_redefinition_of_method(method_name) + elsif respond_to?(method_name, true) raise ArgumentError.new("Could not define support table helper method #{name}.#{method_name} because it is already a defined method") end @@ -361,14 +424,16 @@ def self.#{method_name} RUBY end - def define_support_table_predicates_helper(method_name, attribute_name, attribute_value) - if method_defined?(method_name) || private_method_defined?(method_name) + def define_support_table_predicates_helper(method_name, attribute_name, attribute_value, redefine: false) + if redefine + silence_redefinition_of_method(method_name) + elsif method_defined?(method_name) || private_method_defined?(method_name) raise ArgumentError.new("Could not define support table helper method #{name}##{method_name} because it is already a defined method") end class_eval <<~RUBY, __FILE__, __LINE__ + 1 def #{method_name} - #{attribute_name} == #{attribute_value.inspect} + #{attribute_name} == self.class.type_for_attribute(#{attribute_name.inspect}).cast(#{attribute_value.inspect}) end RUBY end @@ -390,7 +455,13 @@ def support_table_parse_data_file(file_path) end else require "yaml" unless defined?(YAML) - data = YAML.safe_load(file_data) + require "date" unless defined?(Date) + data = if Psych::VERSION.to_f >= 3.1 + YAML.safe_load(file_data, permitted_classes: [Date, Time], aliases: true) + else + # Positional arguments for Psych < 3.1 (Ruby 2.5). + YAML.safe_load(file_data, [Date, Time], [], true) + end end data @@ -399,7 +470,7 @@ def support_table_parse_data_file(file_path) def support_table_record_changed?(record, seen = Set.new) return true if record.changed? - seen << self + seen << record record.class.reflect_on_all_associations.detect do |reflection| next false if reflection.belongs_to? next false unless reflection.options[:autosave] @@ -409,6 +480,31 @@ def support_table_record_changed?(record, seen = Set.new) end end end + + # The class level state used by the concern is stored in instance variables on the + # class where the concern was included. These readers fall back to the superclass + # so that single table inheritance subclasses share the state defined on their + # base class rather than crashing on uninitialized instance variables. + + def support_table_mutex + @mutex || (superclass.include?(SupportTableData) ? superclass.send(:support_table_mutex) : nil) + end + + def support_table_data_files + @support_table_data_files || (superclass.include?(SupportTableData) ? superclass.send(:support_table_data_files) : []) + end + + def support_table_instance_names_map + @support_table_instance_names || (superclass.include?(SupportTableData) ? superclass.send(:support_table_instance_names_map) : {}) + end + + def support_table_attribute_helpers_map + @support_table_attribute_helpers || (superclass.include?(SupportTableData) ? superclass.send(:support_table_attribute_helpers_map) : {}) + end + + def support_table_dependency_names + @support_table_dependencies || (superclass.include?(SupportTableData) ? superclass.send(:support_table_dependency_names) : []) + end end class << self @@ -481,7 +577,7 @@ def support_table_classes(*extra_classes) if SupportTableData.data_directory && File.exist?(SupportTableData.data_directory) && File.directory?(SupportTableData.data_directory) Dir.glob(File.join(SupportTableData.data_directory, "**", "*")).sort.each do |file_name| file_name = file_name.delete_prefix("#{SupportTableData.data_directory}#{File::SEPARATOR}") - class_name = file_name.sub(/\.[^.]*/, "").singularize.camelize + class_name = file_name.sub(/\.[^.]*\z/, "").singularize.camelize class_name.safe_constantize end end @@ -514,7 +610,7 @@ def support_table_classes(*extra_classes) # # @return [Array] def support_table_dependencies(klass) - dependencies = klass.instance_variable_get(:@support_table_dependencies).collect(&:constantize) + dependencies = klass.send(:support_table_dependency_names).collect(&:constantize) klass.reflections.values.each do |reflection| next if reflection.polymorphic? @@ -523,8 +619,8 @@ def support_table_dependencies(klass) next unless reflection.belongs_to? || reflection.through_reflection? next if dependencies.include?(reflection.klass) - explicit_dependencies = reflection.klass.instance_variable_get(:@support_table_dependencies) - next if explicit_dependencies&.include?(klass.name) + explicit_dependencies = reflection.klass.send(:support_table_dependency_names) + next if explicit_dependencies.include?(klass.name) dependencies << reflection.klass rescue => e diff --git a/lib/support_table_data/documentation/source_file.rb b/lib/support_table_data/documentation/source_file.rb index 40d2ce2..67591f0 100644 --- a/lib/support_table_data/documentation/source_file.rb +++ b/lib/support_table_data/documentation/source_file.rb @@ -7,7 +7,7 @@ class SourceFile BEGIN_YARD_COMMENT = "# Begin YARD docs for support_table_data" END_YARD_COMMENT = "# End YARD docs for support_table_data" - YARD_COMMENT_REGEX = /^(?[ \t]*)#{BEGIN_YARD_COMMENT}.*^[ \t]*#{END_YARD_COMMENT}$/m + YARD_COMMENT_REGEX = /^(?[ \t]*)#{BEGIN_YARD_COMMENT}.*?^[ \t]*#{END_YARD_COMMENT}$/m CLASS_DEF_REGEX = /^[ \t]*class [a-zA-Z_0-9:]+.*?$/ UPDATE_COMMAND_COMMENT = "# To update these docs, run `bundle exec rake support_table_data:yard_docs`" diff --git a/lib/support_table_data/railtie.rb b/lib/support_table_data/railtie.rb index 4e0dd33..6c32d0d 100644 --- a/lib/support_table_data/railtie.rb +++ b/lib/support_table_data/railtie.rb @@ -7,7 +7,7 @@ class Railtie < Rails::Railtie end config.support_table.data_directory ||= "db/support_tables" - config.support_table.auto_sync ||= true + config.support_table.auto_sync = true if config.support_table.auto_sync.nil? initializer "support_table_data" do |app| SupportTableData.data_directory ||= app.root.join(app.config.support_table&.data_directory).to_s diff --git a/lib/support_table_data/tasks/utils.rb b/lib/support_table_data/tasks/utils.rb index c552bd6..ce5328d 100644 --- a/lib/support_table_data/tasks/utils.rb +++ b/lib/support_table_data/tasks/utils.rb @@ -30,13 +30,9 @@ def support_table_sources(file_path = nil) ActiveRecord::Base.descendants.each do |klass| next unless klass.include?(SupportTableData) - - begin - next if klass.instance_names.empty? - rescue NoMethodError - # Skip models where instance_names is not properly initialized - next - end + # Skip STI subclasses; only the class that owns the data files gets documented. + next unless klass.instance_variable_defined?(:@support_table_data_files) + next if klass.instance_names.empty? model_file_path = SupportTableData::Tasks::Utils.model_file_path(klass) next unless model_file_path&.file? && model_file_path.readable? diff --git a/spec/data/sizes.yml b/spec/data/sizes.yml new file mode 100644 index 0000000..55797a6 --- /dev/null +++ b/spec/data/sizes.yml @@ -0,0 +1,19 @@ +small: &small + id: "1" + name: small + label: Small + active: true + introduced_on: 2020-01-15 + +medium: + <<: *small + id: 2 + name: medium + label: Medium + +large: + id: 3 + name: large + label: Big + active: false + introduced_on: 2021-06-30 diff --git a/spec/data/sizes_override.yml b/spec/data/sizes_override.yml new file mode 100644 index 0000000..dd6ce4a --- /dev/null +++ b/spec/data/sizes_override.yml @@ -0,0 +1,2 @@ +large: + label: Large diff --git a/spec/models.rb b/spec/models.rb index 706eb2b..a398752 100644 --- a/spec/models.rb +++ b/spec/models.rb @@ -46,6 +46,13 @@ t.string :type t.integer :side_count end + + connection.create_table(:sizes) do |t| + t.string :name + t.string :label + t.boolean :active + t.date :introduced_on + end end # Lazy load model classes @@ -57,6 +64,7 @@ autoload :Polygon, File.expand_path("models/polygon.rb", __dir__) autoload :Rectangle, File.expand_path("models/rectangle.rb", __dir__) autoload :Shade, File.expand_path("models/shade.rb", __dir__) +autoload :Size, File.expand_path("models/size.rb", __dir__) autoload :ShadeHue, File.expand_path("models/shade_hue.rb", __dir__) autoload :Thing, File.expand_path("models/thing.rb", __dir__) autoload :Triangle, File.expand_path("models/triangle.rb", __dir__) diff --git a/spec/models/size.rb b/spec/models/size.rb new file mode 100644 index 0000000..9be621d --- /dev/null +++ b/spec/models/size.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class Size < ActiveRecord::Base + include SupportTableData + + named_instance_attribute_helpers :label + + add_support_table_data "sizes.yml" + add_support_table_data "sizes_override.yml" +end diff --git a/spec/support_table_data/documentation/source_file_spec.rb b/spec/support_table_data/documentation/source_file_spec.rb index 69f12c5..01c9b01 100644 --- a/spec/support_table_data/documentation/source_file_spec.rb +++ b/spec/support_table_data/documentation/source_file_spec.rb @@ -45,6 +45,30 @@ class Color expect(result).not_to include("End YARD docs") end + it "does not swallow user code between duplicated documentation blocks" do + source_with_duplicate_blocks = <<~RUBY + class Color < ActiveRecord::Base + # Begin YARD docs for support_table_data + # Some YARD docs + # End YARD docs for support_table_data + + def user_method + end + + # Begin YARD docs for support_table_data + # Some other YARD docs + # End YARD docs for support_table_data + end + RUBY + + source_file = SupportTableData::Documentation::SourceFile.new(Color, color_path) + allow(source_file).to receive(:source).and_return(source_with_duplicate_blocks) + + result = source_file.source_without_yard_docs + + expect(result).to include("def user_method") + end + it "preserves trailing newline if present in original" do source_with_newline = "class Color\nend\n" source_file = SupportTableData::Documentation::SourceFile.new(Color, color_path) diff --git a/spec/support_table_data_spec.rb b/spec/support_table_data_spec.rb index 35d6f6d..88d6e5b 100644 --- a/spec/support_table_data_spec.rb +++ b/spec/support_table_data_spec.rb @@ -88,6 +88,45 @@ expect(Group.count).to eq 3 expect(Group.pluck(:name)).to match_array ["primary", "secondary", "gray"] end + + it "refuses to delete all rows when delete_missing is true and the data files contain no rows" do + Group.sync_table_data! + allow(Group).to receive(:support_table_data).and_return([]) + expect { Group.sync_table_data!(delete_missing: true) }.to raise_error(ArgumentError, /Refusing to sync Group/) + expect(Group.count).to eq 3 + end + + it "does not raise on empty data files with delete_missing when the table is already empty" do + Invalid.delete_all + allow(Invalid).to receive(:support_table_data).and_return([]) + expect(Invalid.sync_table_data!(delete_missing: true)).to eq [] + end + + it "returns an empty array if the table does not exist" do + allow(Group).to receive(:table_exists?).and_return(false) + expect(Group.sync_table_data!).to eq [] + end + + it "retries once if a concurrent sync inserted the same rows" do + calls = 0 + allow(Group).to receive(:transaction).and_wrap_original do |original, *args, &block| + calls += 1 + raise ActiveRecord::RecordNotUnique.new("duplicate key") if calls == 1 + original.call(*args, &block) + end + expect(Group.sync_table_data!.size).to eq 3 + expect(calls).to eq 2 + end + + it "reraises the error if the retried sync also hits a uniqueness violation" do + calls = 0 + allow(Group).to receive(:transaction) do + calls += 1 + raise ActiveRecord::RecordNotUnique.new("duplicate key") + end + expect { Group.sync_table_data! }.to raise_error(ActiveRecord::RecordNotUnique) + expect(calls).to eq 2 + end end describe "sync_all!" do @@ -202,6 +241,39 @@ it "raises an error if the method is already defined" do expect { Invalid.add_support_table_data("invalid.yml") }.to raise_error(ArgumentError) end + + it "casts the data file value when comparing in predicate methods" do + Size.sync_table_data! + small = Size.find_by!(name: "small") + expect(small.id).to eq 1 + expect(small.small?).to eq true + expect(small.medium?).to eq false + end + + it "supports YAML aliases and date values in data files" do + medium = Size.named_instance_data("medium") + expect(medium["active"]).to eq true + expect(medium["introduced_on"]).to eq Date.new(2020, 1, 15) + expect(medium["label"]).to eq "Medium" + end + + it "redefines attribute helpers when a later data file overrides an attribute" do + expect(Size.small_label).to eq "Small" + expect(Size.large_label).to eq "Large" + end + end + + describe "single table inheritance" do + it "shares support table state with subclasses" do + expect(Rectangle.instance_names).to eq Polygon.instance_names + expect(Rectangle.instance_keys).to eq Polygon.instance_keys + end + + it "determines protected instances for subclass records" do + Polygon.sync_table_data! + expect(Polygon.rectangle.protected_instance?).to eq true + expect(Triangle.new(name: "Scalene").protected_instance?).to eq false + end end describe "instance_names" do @@ -226,6 +298,12 @@ expect(Group.gray_name).to eq "gray" end + it "is idempotent when called again with an attribute that is already registered" do + expect { Group.named_instance_attribute_helpers(:name) }.to_not raise_error + expect(Group.primary_name).to eq "primary" + expect(Group.support_table_attribute_helpers).to match_array ["group_id", "name"] + end + it "can get a list of the defined attribute helpers" do expect(Group.support_table_attribute_helpers).to match_array ["group_id", "name"] expect(Color.support_table_attribute_helpers).to match_array [] @@ -249,11 +327,27 @@ expect(orange.protected_instance?).to eq true expect(brown.protected_instance?).to eq false end + + it "picks up instances from data files added after the protected keys were memoized" do + klass = Class.new(ActiveRecord::Base) do + include SupportTableData + + self.table_name = "colors" + end + klass.add_support_table_data("colors/named_colors.yml") + + light_gray = klass.new + light_gray.id = 8 + expect(light_gray.protected_instance?).to eq false + + klass.add_support_table_data("colors/colors.json") + expect(light_gray.protected_instance?).to eq true + end end describe "support_table_classes" do it "gets a list of all loaded support table classes with dependencies listed first" do - expect(SupportTableData.support_table_classes).to eq [Shade, Group, Hue, Color, Invalid, Polygon] + expect(SupportTableData.support_table_classes).to eq [Shade, Group, Hue, Color, Invalid, Polygon, Size] end end