diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 00000000..f43f9d9a --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,77 @@ +AllCops: + NewCops: enable + SuggestExtensions: false + TargetRubyVersion: 3.3 + +Gemspec/DevelopmentDependencies: + Enabled: false +Layout/HashAlignment: + Enabled: false +Layout/LineLength: + Enabled: false +Layout/SpaceInsideHashLiteralBraces: + EnforcedStyle: no_space +Layout/SpaceAroundOperators: + EnforcedStyleForExponentOperator: space + +Metrics/AbcSize: + Enabled: false +Metrics/BlockLength: + Enabled: false +Metrics/ClassLength: + Enabled: false +Metrics/CyclomaticComplexity: + Enabled: false +Metrics/MethodLength: + Enabled: false +Metrics/ModuleLength: + Enabled: false +Metrics/PerceivedComplexity: + Enabled: false + +Naming/PredicateName: + Enabled: false +Naming/VariableNumber: + Enabled: false + +Style/Documentation: + Enabled: false +Style/ExpandPathArguments: + Enabled: false +Style/GuardClause: + Enabled: false +Style/HashSyntax: + Enabled: false +Style/IfUnlessModifier: + Enabled: false +Style/Lambda: + Enabled: false +Style/MultilineIfModifier: + Enabled: false +Style/NegatedIf: + Enabled: false +Style/Next: + Enabled: false +Style/NumericPredicate: + Enabled: false +Style/ParallelAssignment: + Enabled: false +Style/PercentLiteralDelimiters: + Enabled: false +Style/StringLiterals: + Enabled: false +Style/StringLiteralsInInterpolation: + Enabled: false +Style/SymbolArray: + Enabled: false + +# specs + +Layout/ExtraSpacing: + Exclude: [ "test/**/*" ] +Style/TrailingUnderscoreVariable: + Exclude: [ "test/**/*" ] +Layout/SpaceBeforeFirstArg: + Exclude: [ "test/**/*" ] +Layout/SpaceAroundOperators: + Exclude: [ "test/**/*" ] diff --git a/Appraisals b/Appraisals index 2883b0b9..d6a967cc 100644 --- a/Appraisals +++ b/Appraisals @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # on a mac using: # bundle config --global build.mysql2 "--with-mysql-dir=$(brew --prefix mysql)" diff --git a/Gemfile b/Gemfile index aa081768..d68c3d03 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + source 'https://rubygems.org' gemspec diff --git a/Rakefile b/Rakefile index 72610ff3..acbde054 100644 --- a/Rakefile +++ b/Rakefile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'bundler/setup' require 'bundler/gem_tasks' require 'rake/testtask' diff --git a/ancestry.gemspec b/ancestry.gemspec index 3345d8ed..388a2e03 100644 --- a/ancestry.gemspec +++ b/ancestry.gemspec @@ -1,5 +1,7 @@ +# frozen_string_literal: true + lib = File.expand_path('../lib/', __FILE__) -$:.unshift lib unless $:.include?(lib) +$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'ancestry/version' Gem::Specification.new do |s| @@ -22,6 +24,7 @@ EOF "changelog_uri" => "https://github.com/stefankroes/ancestry/blob/master/CHANGELOG.md", "source_code_uri" => "https://github.com/stefankroes/ancestry/", "bug_tracker_uri" => "https://github.com/stefankroes/ancestry/issues", + "rubygems_mfa_required" => "true" } s.version = Ancestry::VERSION @@ -30,19 +33,19 @@ EOF s.homepage = 'https://github.com/stefankroes/ancestry' s.license = 'MIT' - s.files = Dir[ + s.files = Dir[ "{lib}/**/*", 'CHANGELOG.md', 'MIT-LICENSE', 'README.md' ] s.require_paths = ["lib"] - - s.required_ruby_version = '>= 2.5' + + s.required_ruby_version = '>= 2.5' s.add_runtime_dependency 'activerecord', '>= 5.2.6' s.add_development_dependency 'appraisal' s.add_development_dependency 'minitest' - s.add_development_dependency 'rake', '~> 13.0' + s.add_development_dependency 'rake', '~> 13.0' s.add_development_dependency 'simplecov' s.add_development_dependency 'yard' end diff --git a/gemfiles/gemfile_52.gemfile b/gemfiles/gemfile_52.gemfile index eb715511..2d658258 100644 --- a/gemfiles/gemfile_52.gemfile +++ b/gemfiles/gemfile_52.gemfile @@ -1,4 +1,5 @@ # This file was generated by Appraisal +# frozen_string_literal: true source "https://rubygems.org" diff --git a/gemfiles/gemfile_60.gemfile b/gemfiles/gemfile_60.gemfile index f0d662f0..b2e11095 100644 --- a/gemfiles/gemfile_60.gemfile +++ b/gemfiles/gemfile_60.gemfile @@ -1,4 +1,5 @@ # This file was generated by Appraisal +# frozen_string_literal: true source "https://rubygems.org" diff --git a/gemfiles/gemfile_61.gemfile b/gemfiles/gemfile_61.gemfile index 6a2913a1..539c700c 100644 --- a/gemfiles/gemfile_61.gemfile +++ b/gemfiles/gemfile_61.gemfile @@ -1,4 +1,5 @@ # This file was generated by Appraisal +# frozen_string_literal: true source "https://rubygems.org" diff --git a/gemfiles/gemfile_70.gemfile b/gemfiles/gemfile_70.gemfile index a938375d..89796880 100644 --- a/gemfiles/gemfile_70.gemfile +++ b/gemfiles/gemfile_70.gemfile @@ -1,4 +1,5 @@ # This file was generated by Appraisal +# frozen_string_literal: true source "https://rubygems.org" diff --git a/gemfiles/gemfile_71.gemfile b/gemfiles/gemfile_71.gemfile index b077766d..32caf094 100644 --- a/gemfiles/gemfile_71.gemfile +++ b/gemfiles/gemfile_71.gemfile @@ -1,4 +1,5 @@ # This file was generated by Appraisal +# frozen_string_literal: true source "https://rubygems.org" diff --git a/gemfiles/gemfile_72.gemfile b/gemfiles/gemfile_72.gemfile index 60890b37..c9d589eb 100644 --- a/gemfiles/gemfile_72.gemfile +++ b/gemfiles/gemfile_72.gemfile @@ -1,4 +1,5 @@ # This file was generated by Appraisal +# frozen_string_literal: true source "https://rubygems.org" diff --git a/lib/ancestry.rb b/lib/ancestry.rb index e5cd2c61..e9110c16 100644 --- a/lib/ancestry.rb +++ b/lib/ancestry.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative 'ancestry/version' require_relative 'ancestry/class_methods' require_relative 'ancestry/instance_methods' @@ -8,7 +10,7 @@ require_relative 'ancestry/materialized_path_pg' I18n.load_path += Dir[File.join(File.expand_path(File.dirname(__FILE__)), - 'ancestry', 'locales', '*.{rb,yml}').to_s] + 'ancestry', 'locales', '*.{rb,yml}').to_s] module Ancestry @@default_update_strategy = :ruby diff --git a/lib/ancestry/class_methods.rb b/lib/ancestry/class_methods.rb index 4042d587..c924063e 100644 --- a/lib/ancestry/class_methods.rb +++ b/lib/ancestry/class_methods.rb @@ -1,7 +1,9 @@ +# frozen_string_literal: true + module Ancestry module ClassMethods # Fetch tree node if necessary - def to_node object + def to_node(object) if object.is_a?(ancestry_base_class) object else @@ -10,13 +12,13 @@ def to_node object end # Scope on relative depth options - def scope_depth depth_options, depth + def scope_depth(depth_options, depth) depth_options.inject(ancestry_base_class) do |scope, option| scope_name, relative_depth = option if [:before_depth, :to_depth, :at_depth, :from_depth, :after_depth].include? scope_name scope.send scope_name, depth + relative_depth else - raise Ancestry::AncestryException.new(I18n.t("ancestry.unknown_depth_option", scope_name: scope_name)) + raise Ancestry::AncestryException, I18n.t("ancestry.unknown_depth_option", scope_name: scope_name) end end end @@ -27,11 +29,11 @@ def scope_depth depth_options, depth # To order your hashes pass the order to the arrange method instead of to the scope # Get all nodes and sort them into an empty hash - def arrange options = {} + def arrange(options = {}) if (order = options.delete(:order)) - arrange_nodes ancestry_base_class.order(order).where(options) + arrange_nodes(ancestry_base_class.order(order).where(options)) else - arrange_nodes ancestry_base_class.where(options) + arrange_nodes(ancestry_base_class.where(options)) end end @@ -63,10 +65,10 @@ def flatten_arranged_nodes(arranged, nodes = []) nodes end - # Arrangement to nested array for serialization - # You can also supply your own serialization logic using blocks - # also allows you to pass the order just as you can pass it to the arrange method - def arrange_serializable options={}, nodes=nil, &block + # Arrangement to nested array for serialization + # You can also supply your own serialization logic using blocks + # also allows you to pass the order just as you can pass it to the arrange method + def arrange_serializable(options = {}, nodes = nil, &block) nodes = arrange(options) if nodes.nil? nodes.map do |parent, children| if block_given? @@ -78,13 +80,13 @@ def arrange_serializable options={}, nodes=nil, &block end def tree_view(column, data = nil) - data = arrange unless data + data ||= arrange data.each do |parent, children| if parent.depth == 0 puts parent[column] else num = parent.depth - 1 - indent = " "*num + indent = " " * num puts " #{"|" if parent.depth > 1}#{indent}|_ #{parent[column]}" end tree_view(column, children) if children @@ -93,7 +95,7 @@ def tree_view(column, data = nil) # Pseudo-preordered array of nodes. Children will always follow parents, # This is deterministic unless the parents are missing *and* a sort block is specified - def sort_by_ancestry(nodes, &block) + def sort_by_ancestry(nodes) arranged = nodes if nodes.is_a?(Hash) unless arranged @@ -113,48 +115,43 @@ def sort_by_ancestry(nodes, &block) # compromised tree integrity is unlikely without explicitly setting cyclic parents or invalid ancestry and circumventing validation # just in case, raise an AncestryIntegrityException if issues are detected # specify :report => :list to return an array of exceptions or :report => :echo to echo any error messages - def check_ancestry_integrity! options = {} + def check_ancestry_integrity!(options = {}) parents = {} exceptions = [] if options[:report] == :list unscoped_where do |scope| # For each node ... scope.find_each do |node| - begin - # ... check validity of ancestry column - if !node.sane_ancestor_ids? - raise Ancestry::AncestryIntegrityException.new(I18n.t("ancestry.invalid_ancestry_column", - :node_id => node.id, - :ancestry_column => "#{node.read_attribute node.class.ancestry_column}" - )) - end - # ... check that all ancestors exist - node.ancestor_ids.each do |ancestor_id| - unless exists? ancestor_id - raise Ancestry::AncestryIntegrityException.new(I18n.t("ancestry.reference_nonexistent_node", - :node_id => node.id, - :ancestor_id => ancestor_id - )) - end - end - # ... check that all node parents are consistent with values observed earlier - node.path_ids.zip([nil] + node.path_ids).each do |node_id, parent_id| - parents[node_id] = parent_id unless parents.has_key? node_id - unless parents[node_id] == parent_id - raise Ancestry::AncestryIntegrityException.new(I18n.t("ancestry.conflicting_parent_id", - :node_id => node_id, - :parent_id => parent_id || 'nil', - :expected => parents[node_id] || 'nil' - )) - end + # ... check validity of ancestry column + if !node.sane_ancestor_ids? + raise Ancestry::AncestryIntegrityException, I18n.t("ancestry.invalid_ancestry_column", + :node_id => node.id, + :ancestry_column => node.read_attribute(node.class.ancestry_column)) + end + # ... check that all ancestors exist + node.ancestor_ids.each do |ancestor_id| + unless exists?(ancestor_id) + raise Ancestry::AncestryIntegrityException, I18n.t("ancestry.reference_nonexistent_node", + :node_id => node.id, + :ancestor_id => ancestor_id) end - rescue Ancestry::AncestryIntegrityException => integrity_exception - case options[:report] - when :list then exceptions << integrity_exception - when :echo then puts integrity_exception - else raise integrity_exception + end + # ... check that all node parents are consistent with values observed earlier + node.path_ids.zip([nil] + node.path_ids).each do |node_id, parent_id| + parents[node_id] = parent_id unless parents.key?(node_id) + unless parents[node_id] == parent_id + raise Ancestry::AncestryIntegrityException, I18n.t("ancestry.conflicting_parent_id", + :node_id => node_id, + :parent_id => parent_id || 'nil', + :expected => parents[node_id] || 'nil') end end + rescue Ancestry::AncestryIntegrityException => e + case options[:report] + when :list then exceptions << e + when :echo then puts e + else raise e + end end end exceptions if options[:report] == :list @@ -201,7 +198,7 @@ def restore_ancestry_integrity! end # Build ancestry from parent ids for migration purposes - def build_ancestry_from_parent_ids! column=:parent_id, parent_id = nil, ancestor_ids = [] + def build_ancestry_from_parent_ids!(column = :parent_id, parent_id = nil, ancestor_ids = []) unscoped_where do |scope| scope.where(column => parent_id).find_each do |node| node.without_ancestry_callbacks do @@ -214,7 +211,7 @@ def build_ancestry_from_parent_ids! column=:parent_id, parent_id = nil, ancestor # Rebuild depth cache if it got corrupted or if depth caching was just turned on def rebuild_depth_cache! - raise Ancestry::AncestryException.new(I18n.t("ancestry.cannot_rebuild_depth_cache")) unless respond_to? :depth_cache_column + raise(Ancestry::AncestryException, I18n.t("ancestry.cannot_rebuild_depth_cache")) unless respond_to?(:depth_cache_column) ancestry_base_class.transaction do unscoped_where do |scope| @@ -232,7 +229,7 @@ def rebuild_depth_cache_sql! end def rebuild_counter_cache! - if %w(mysql mysql2).include?(connection.adapter_name.downcase) + if %w(mysql mysql2).include?(connection.adapter_name.downcase) connection.execute %{ UPDATE #{table_name} AS dest LEFT JOIN ( diff --git a/lib/ancestry/exceptions.rb b/lib/ancestry/exceptions.rb index 9088d8e6..e2efd5d3 100644 --- a/lib/ancestry/exceptions.rb +++ b/lib/ancestry/exceptions.rb @@ -1,7 +1,9 @@ +# frozen_string_literal: true + module Ancestry class AncestryException < RuntimeError end class AncestryIntegrityException < AncestryException end -end \ No newline at end of file +end diff --git a/lib/ancestry/has_ancestry.rb b/lib/ancestry/has_ancestry.rb index 8806cfce..69e3cab1 100644 --- a/lib/ancestry/has_ancestry.rb +++ b/lib/ancestry/has_ancestry.rb @@ -1,32 +1,36 @@ +# frozen_string_literal: true + module Ancestry module HasAncestry - def has_ancestry options = {} + def has_ancestry(options = {}) # Check options - raise Ancestry::AncestryException.new(I18n.t("ancestry.option_must_be_hash")) unless options.is_a? Hash - options.each do |key, value| - unless [:ancestry_column, :orphan_strategy, :cache_depth, :depth_cache_column, :touch, :counter_cache, :primary_key_format, :update_strategy, :ancestry_format].include? key - raise Ancestry::AncestryException.new(I18n.t("ancestry.unknown_option", key: key.inspect, value: value.inspect)) - end + unless options.is_a? Hash + raise Ancestry::AncestryException, I18n.t("ancestry.option_must_be_hash") + end + + extra_keys = options.keys - [:ancestry_column, :orphan_strategy, :cache_depth, :depth_cache_column, :touch, :counter_cache, :primary_key_format, :update_strategy, :ancestry_format] + if (key = extra_keys.first) + raise Ancestry::AncestryException, I18n.t("ancestry.unknown_option", key: key.inspect, value: options[key].inspect) end ancestry_format = options[:ancestry_format] || Ancestry.default_ancestry_format if ![:materialized_path, :materialized_path2].include?(ancestry_format) - raise Ancestry::AncestryException.new(I18n.t("ancestry.unknown_format", value: ancestry_format)) + raise Ancestry::AncestryException, I18n.t("ancestry.unknown_format", value: ancestry_format) end orphan_strategy = options[:orphan_strategy] || :destroy # Create ancestry column accessor and set to option or default - self.class_variable_set('@@ancestry_column', options[:ancestry_column] || :ancestry) + class_variable_set('@@ancestry_column', options[:ancestry_column] || :ancestry) cattr_reader :ancestry_column, instance_reader: false primary_key_format = options[:primary_key_format].presence || Ancestry.default_primary_key_format - self.class_variable_set('@@ancestry_delimiter', '/') + class_variable_set('@@ancestry_delimiter', '/') cattr_reader :ancestry_delimiter, instance_reader: false # Save self as base class (for STI) - self.class_variable_set('@@ancestry_base_class', self) + class_variable_set('@@ancestry_base_class', self) cattr_reader :ancestry_base_class, instance_reader: false # Touch ancestors after updating @@ -41,9 +45,9 @@ def has_ancestry options = {} extend Ancestry::ClassMethods extend Ancestry::HasAncestry.ancestry_format_module(ancestry_format) - attribute self.ancestry_column, default: self.ancestry_root + attribute ancestry_column, default: ancestry_root - validates self.ancestry_column, ancestry_validation_options(primary_key_format) + validates ancestry_column, ancestry_validation_options(primary_key_format) update_strategy = options[:update_strategy] || Ancestry.default_update_strategy include Ancestry::MaterializedPathPg if update_strategy == :sql @@ -60,7 +64,7 @@ def has_ancestry options = {} alias_method :apply_orphan_strategy, orphan_strategy_helper before_destroy :apply_orphan_strategy elsif orphan_strategy.to_s != "none" - raise Ancestry::AncestryException.new(I18n.t("ancestry.invalid_orphan_strategy")) + raise Ancestry::AncestryException, I18n.t("ancestry.invalid_orphan_strategy") end # Create ancestry column accessor and set to option or default @@ -70,7 +74,7 @@ def has_ancestry options = {} depth_cache_sql = options[:depth_cache_column]&.to_s || 'ancestry_depth' elsif options[:cache_depth] # Create accessor for column name and set to option or default - self.cattr_accessor :depth_cache_column + cattr_accessor :depth_cache_column self.depth_cache_column = if options[:cache_depth] == true options[:depth_cache_column]&.to_s || 'ancestry_depth' @@ -119,6 +123,7 @@ def has_ancestry options = {} def acts_as_tree(*args) return super if defined?(super) + has_ancestry(*args) end diff --git a/lib/ancestry/instance_methods.rb b/lib/ancestry/instance_methods.rb index 8d898093..7681e9ff 100644 --- a/lib/ancestry/instance_methods.rb +++ b/lib/ancestry/instance_methods.rb @@ -1,8 +1,10 @@ +# frozen_string_literal: true + module Ancestry module InstanceMethods # Validate that the ancestors don't include itself def ancestry_exclude_self - errors.add(:base, I18n.t("ancestry.exclude_self", class_name: self.class.name.humanize)) if ancestor_ids.include? self.id + errors.add(:base, I18n.t("ancestry.exclude_self", class_name: self.class.name.humanize)) if ancestor_ids.include?(id) end # Update descendants with new ancestry (after update) @@ -49,7 +51,7 @@ def apply_orphan_strategy_adopt descendants.each do |descendant| descendant.without_ancestry_callbacks do - descendant.update_attribute :ancestor_ids, (descendant.ancestor_ids.delete_if { |x| x == self.id }) + descendant.update_attribute :ancestor_ids, (descendant.ancestor_ids.delete_if { |x| x == id }) end end end @@ -58,7 +60,7 @@ def apply_orphan_strategy_adopt def apply_orphan_strategy_restrict return if ancestry_callbacks_disabled? || new_record? - raise Ancestry::AncestryException.new(I18n.t("ancestry.cannot_delete_descendants")) unless is_childless? + raise(Ancestry::AncestryException, I18n.t("ancestry.cannot_delete_descendants")) unless is_childless? end # Touch each of this record's ancestors (after save) @@ -93,7 +95,7 @@ def decrease_parent_counter_cache def update_parent_counter_cache return unless saved_change_to_attribute?(self.class.ancestry_column) - if parent_id_was = parent_id_before_last_save + if (parent_id_was = parent_id_before_last_save) self.class.ancestry_base_class.decrement_counter counter_cache_column, parent_id_was end @@ -105,13 +107,13 @@ def update_parent_counter_cache def has_parent? ancestor_ids.present? end - alias :ancestors? :has_parent? + alias ancestors? has_parent? def ancestry_changed? column = self.class.ancestry_column.to_s - # These methods return nil if there are no changes. - # This was fixed in a refactoring in rails 6.0: https://github.com/rails/rails/pull/35933 - !!(will_save_change_to_attribute?(column) || saved_change_to_attribute?(column)) + # These methods return nil if there are no changes. + # This was fixed in a refactoring in rails 6.0: https://github.com/rails/rails/pull/35933 + !!(will_save_change_to_attribute?(column) || saved_change_to_attribute?(column)) end def sane_ancestor_ids? @@ -131,8 +133,9 @@ def sane_ancestor_ids? self.validation_context = current_context end - def ancestors depth_options = {} + def ancestors(depth_options = {}) return self.class.ancestry_base_class.none unless has_parent? + self.class.ancestry_base_class.scope_depth(depth_options, depth).ordered_by_ancestry.ancestors_of(self) end @@ -148,7 +151,7 @@ def path_ids_in_database ancestor_ids_in_database + [id] end - def path depth_options = {} + def path(depth_options = {}) self.class.ancestry_base_class.scope_depth(depth_options, depth).ordered_by_ancestry.inpath_of(self) end @@ -161,25 +164,25 @@ def cache_depth end def ancestor_of?(node) - node.ancestor_ids.include?(self.id) + node.ancestor_ids.include?(id) end # Parent # currently parent= does not work in after save callbacks # assuming that parent hasn't changed - def parent= parent + def parent=(parent) self.ancestor_ids = parent ? parent.path_ids : [] end - def parent_id= new_parent_id + def parent_id=(new_parent_id) self.parent = new_parent_id.present? ? unscoped_find(new_parent_id) : nil end def parent_id ancestor_ids.last if has_parent? end - alias :parent_id? :ancestors? + alias parent_id? ancestors? def parent if has_parent? @@ -190,7 +193,7 @@ def parent end def parent_of?(node) - self.id == node.parent_id + id == node.parent_id end # Root @@ -210,10 +213,10 @@ def root def is_root? !has_parent? end - alias :root? :is_root? + alias root? is_root? def root_of?(node) - self.id == node.root_id + id == node.root_id end # Children @@ -227,7 +230,7 @@ def child_ids end def has_children? - self.children.exists? + children.exists? end alias_method :children?, :has_children? @@ -237,7 +240,7 @@ def is_childless? alias_method :childless?, :is_childless? def child_of?(node) - self.parent_id == node.id + parent_id == node.id end # Siblings @@ -252,7 +255,7 @@ def sibling_ids end def has_siblings? - self.siblings.count > 1 + siblings.count > 1 end alias_method :siblings?, :has_siblings? @@ -262,16 +265,16 @@ def is_only_child? alias_method :only_child?, :is_only_child? def sibling_of?(node) - self.ancestor_ids == node.ancestor_ids + ancestor_ids == node.ancestor_ids end # Descendants - def descendants depth_options = {} + def descendants(depth_options = {}) self.class.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).descendants_of(self) end - def descendant_ids depth_options = {} + def descendant_ids(depth_options = {}) descendants(depth_options).pluck(self.class.primary_key) end @@ -281,11 +284,11 @@ def descendant_of?(node) # Indirects - def indirects depth_options = {} + def indirects(depth_options = {}) self.class.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).indirects_of(self) end - def indirect_ids depth_options = {} + def indirect_ids(depth_options = {}) indirects(depth_options).pluck(self.class.primary_key) end @@ -295,11 +298,11 @@ def indirect_of?(node) # Subtree - def subtree depth_options = {} + def subtree(depth_options = {}) self.class.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).subtree_of(self) end - def subtree_ids depth_options = {} + def subtree_ids(depth_options = {}) subtree(depth_options).pluck(self.class.primary_key) end @@ -324,33 +327,31 @@ def ancestry_callbacks_disabled? def unscoped_descendants unscoped_where do |scope| - scope.where self.class.ancestry_base_class.descendant_conditions(self) + scope.where(self.class.ancestry_base_class.descendant_conditions(self)) end end def unscoped_descendants_before_last_save unscoped_where do |scope| - scope.where self.class.ancestry_base_class.descendant_before_last_save_conditions(self) + scope.where(self.class.ancestry_base_class.descendant_before_last_save_conditions(self)) end end # works with after save context (hence before_last_save) def unscoped_current_and_previous_ancestors unscoped_where do |scope| - scope.where scope.primary_key => (ancestor_ids + ancestor_ids_before_last_save).uniq + scope.where(scope.primary_key => (ancestor_ids + ancestor_ids_before_last_save).uniq) end end - def unscoped_find id + def unscoped_find(id) unscoped_where do |scope| - scope.find id + scope.find(id) end end - def unscoped_where - self.class.ancestry_base_class.unscoped_where do |scope| - yield scope - end + def unscoped_where(&block) + self.class.ancestry_base_class.unscoped_where(&block) end end end diff --git a/lib/ancestry/materialized_path.rb b/lib/ancestry/materialized_path.rb index d53854af..d70a97cb 100644 --- a/lib/ancestry/materialized_path.rb +++ b/lib/ancestry/materialized_path.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Ancestry # store ancestry as grandparent_id/parent_id # root a=nil,id=1 children=id,id/% == 1, 1/% @@ -114,6 +116,7 @@ def generate_ancestry(ancestor_ids) def parse_ancestry_column(obj) return [] if obj.nil? || obj == ancestry_root + obj_ids = obj.split(ancestry_delimiter).delete_if(&:blank?) primary_key_is_an_integer? ? obj_ids.map!(&:to_i) : obj_ids end @@ -132,7 +135,7 @@ def concat(*args) def self.construct_depth_sql(table_name, ancestry_column, ancestry_delimiter) tmp = %{(LENGTH(#{table_name}.#{ancestry_column}) - LENGTH(REPLACE(#{table_name}.#{ancestry_column},'#{ancestry_delimiter}','')))} - tmp = tmp + "/#{ancestry_delimiter.size}" if ancestry_delimiter.size > 1 + tmp += "/#{ancestry_delimiter.size}" if ancestry_delimiter.size > 1 "(CASE WHEN #{table_name}.#{ancestry_column} IS NULL THEN 0 ELSE 1 + #{tmp} END)" end @@ -140,7 +143,7 @@ def self.construct_depth_sql(table_name, ancestry_column, ancestry_delimiter) def ancestry_validation_options(ancestry_primary_key_format) { - format: { with: ancestry_format_regexp(ancestry_primary_key_format) }, + format: {with: ancestry_format_regexp(ancestry_primary_key_format)}, allow_nil: ancestry_nil_allowed? } end @@ -158,7 +161,7 @@ module InstanceMethods def ancestors? read_attribute(self.class.ancestry_column) != self.class.ancestry_root end - alias :has_parent? :ancestors? + alias has_parent? ancestors? def ancestor_ids=(value) write_attribute(self.class.ancestry_column, self.class.generate_ancestry(value)) @@ -186,7 +189,7 @@ def parent_id_before_last_save # optimization - better to go directly to column and avoid parsing def sibling_of?(node) - self.read_attribute(self.class.ancestry_column) == node.read_attribute(node.class.ancestry_column) + read_attribute(self.class.ancestry_column) == node.read_attribute(node.class.ancestry_column) end # The ancestry value for this record's children @@ -195,7 +198,8 @@ def sibling_of?(node) # NOTE: This could have been called child_ancestry_in_database # the child records were created from the version in the database def child_ancestry - raise Ancestry::AncestryException.new(I18n.t("ancestry.no_child_for_new_record")) if new_record? + raise(Ancestry::AncestryException, I18n.t("ancestry.no_child_for_new_record")) if new_record? + [attribute_in_database(self.class.ancestry_column), id].compact.join(self.class.ancestry_delimiter) end @@ -204,9 +208,10 @@ def child_ancestry # to find the old children and bring them along (or to ) # This is not valid in a new record's after_save. def child_ancestry_before_last_save - if new_record? || respond_to?(:previously_new_record?) && previously_new_record? - raise Ancestry::AncestryException.new(I18n.t("ancestry.no_child_for_new_record")) + if new_record? || (respond_to?(:previously_new_record?) && previously_new_record?) + raise Ancestry::AncestryException, I18n.t("ancestry.no_child_for_new_record") end + [attribute_before_last_save(self.class.ancestry_column), id].compact.join(self.class.ancestry_delimiter) end end diff --git a/lib/ancestry/materialized_path2.rb b/lib/ancestry/materialized_path2.rb index 46887852..df4ef8a2 100644 --- a/lib/ancestry/materialized_path2.rb +++ b/lib/ancestry/materialized_path2.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Ancestry # store ancestry as /grandparent_id/parent_id/ # root: a=/,id=1 children=#{a}#{id}/% == /1/% @@ -47,7 +49,7 @@ def generate_ancestry(ancestor_ids) # module method def self.construct_depth_sql(table_name, ancestry_column, ancestry_delimiter) tmp = %{(LENGTH(#{table_name}.#{ancestry_column}) - LENGTH(REPLACE(#{table_name}.#{ancestry_column},'#{ancestry_delimiter}','')))} - tmp = tmp + "/#{ancestry_delimiter.size}" if ancestry_delimiter.size > 1 + tmp += "/#{ancestry_delimiter.size}" if ancestry_delimiter.size > 1 "(#{tmp} -1)" end @@ -64,15 +66,17 @@ def ancestry_format_regexp(primary_key_format) module InstanceMethods # Please see notes for MaterializedPath#child_ancestry def child_ancestry - raise Ancestry::AncestryException.new(I18n.t("ancestry.no_child_for_new_record")) if new_record? + raise(Ancestry::AncestryException, I18n.t("ancestry.no_child_for_new_record")) if new_record? + "#{attribute_in_database(self.class.ancestry_column)}#{id}#{self.class.ancestry_delimiter}" end # Please see notes for MaterializedPath#child_ancestry_before_last_save def child_ancestry_before_last_save - if new_record? || respond_to?(:previously_new_record?) && previously_new_record? - raise Ancestry::AncestryException.new(I18n.t("ancestry.no_child_for_new_record")) + if new_record? || (respond_to?(:previously_new_record?) && previously_new_record?) + raise(Ancestry::AncestryException, I18n.t("ancestry.no_child_for_new_record")) end + "#{attribute_before_last_save(self.class.ancestry_column)}#{id}#{self.class.ancestry_delimiter}" end end diff --git a/lib/ancestry/materialized_path_pg.rb b/lib/ancestry/materialized_path_pg.rb index 5cb35030..46de2d51 100644 --- a/lib/ancestry/materialized_path_pg.rb +++ b/lib/ancestry/materialized_path_pg.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Ancestry module MaterializedPathPg # Update descendants with new ancestry (after update) @@ -5,8 +7,8 @@ def update_descendants_with_new_ancestry # If enabled and node is existing and ancestry was updated and the new ancestry is sane ... # The only way the ancestry could be bad is via `update_attribute` with a bad value if !ancestry_callbacks_disabled? && sane_ancestor_ids? - old_ancestry = self.class.generate_ancestry( path_ids_before_last_save ) - new_ancestry = self.class.generate_ancestry( path_ids ) + old_ancestry = self.class.generate_ancestry(path_ids_before_last_save) + new_ancestry = self.class.generate_ancestry(path_ids) update_clause = { self.class.ancestry_column => Arel.sql("regexp_replace(#{self.class.ancestry_column}, '^#{Regexp.escape(old_ancestry)}', '#{new_ancestry}')") } diff --git a/lib/ancestry/version.rb b/lib/ancestry/version.rb index 08fbe39a..3e757431 100644 --- a/lib/ancestry/version.rb +++ b/lib/ancestry/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Ancestry VERSION = '5.0.0' end diff --git a/test/concerns/arrangement_test.rb b/test/concerns/arrangement_test.rb index 8257334c..bc0b125d 100644 --- a/test/concerns/arrangement_test.rb +++ b/test/concerns/arrangement_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class ArrangementTest < ActiveSupport::TestCase @@ -6,7 +8,7 @@ def root_node(model) end def middle_node(model) - root_node(model).children.sort_by(&:id).first + root_node(model).children.min_by(&:id) end def leaf_node(model) @@ -23,7 +25,7 @@ def assert_tree(arranged_nodes, size_at_depth) assert_equal size_at_depth[1], children.size assert_equal node.children.sort_by(&:id), children.keys.sort_by(&:id) - assert_tree(children, size_at_depth[1..-1]) + assert_tree(children, size_at_depth[1..]) end end @@ -39,7 +41,7 @@ def assert_tree_path(arranged_nodes, expected_ids) arranged_nodes.each do |node, children| assert_equal expected_ids[0], node.id - assert_tree_path(children, expected_ids[1..-1]) + assert_tree_path(children, expected_ids[1..]) end end @@ -131,20 +133,25 @@ def test_arrange_serializable AncestryTestDatabase.with_model :depth => 2, :width => 2 do |model, _roots| col = model.ancestry_column # materialized path 2 has a slash at the beginning and end - fmt = AncestryTestDatabase.materialized_path2? ? -> (a) { a ? "/#{a}/" : "/" } : -> (a) {a} - result = [{ - col=>fmt[nil], "id"=>4, "children"=> [{ - col=>fmt["4"], "id"=>6, "children" => [] - }, { - col=>fmt["4"], "id"=>5, "children" => [] - }] - }, { - col=>fmt[nil], "id"=>1, "children"=> [{ - col=>fmt["1"], "id"=>3, "children"=>[] + fmt = + if AncestryTestDatabase.materialized_path2? + ->(a) { a ? "/#{a}/" : "/" } + else + ->(a) { a } + end + result = [ + { + col => fmt[nil], "id" => 4, "children" => [ + {col => fmt["4"], "id" => 6, "children" => []}, + {col => fmt["4"], "id" => 5, "children" => []} + ] }, { - col=>fmt["1"], "id"=>2, "children"=>[] - }] - }] + col => fmt[nil], "id" => 1, "children" => [ + {col => fmt["1"], "id" => 3, "children" => []}, + {col => fmt["1"], "id" => 2, "children" => []} + ] + } + ] assert_equal model.arrange_serializable(order: "id desc"), result end @@ -152,19 +159,19 @@ def test_arrange_serializable def test_arrange_serializable_with_block AncestryTestDatabase.with_model :depth => 2, :width => 2 do |model, _roots| - expected_result = [{ - "id"=>4, "children"=>[{ - "id"=>6 - }, { - "id"=>5 - }] - }, { - "id"=>1, "children"=> [{ - "id"=>3 + expected_result = [ + { + "id" => 4, "children" => [ + {"id" => 6}, + {"id" => 5} + ] }, { - "id"=>2 - }] - }] + "id" => 1, "children" => [ + {"id" => 3}, + {"id" => 2} + ] + } + ] result = model.arrange_serializable(order: "id desc") do |parent, children| out = {} out["id"] = parent.id diff --git a/test/concerns/build_ancestry_test.rb b/test/concerns/build_ancestry_test.rb index f1d14e81..62879438 100644 --- a/test/concerns/build_ancestry_test.rb +++ b/test/concerns/build_ancestry_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class BuildAncestryTest < ActiveSupport::TestCase diff --git a/test/concerns/counter_cache_test.rb b/test/concerns/counter_cache_test.rb index 2093332c..a32fc34c 100644 --- a/test/concerns/counter_cache_test.rb +++ b/test/concerns/counter_cache_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class CounterCacheTest < ActiveSupport::TestCase diff --git a/test/concerns/db_test.rb b/test/concerns/db_test.rb index fe2001c8..67878bd8 100644 --- a/test/concerns/db_test.rb +++ b/test/concerns/db_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class DbTest < ActiveSupport::TestCase diff --git a/test/concerns/default_scopes_test.rb b/test/concerns/default_scopes_test.rb index 4abfa383..965ed585 100644 --- a/test/concerns/default_scopes_test.rb +++ b/test/concerns/default_scopes_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class DefaultScopesTest < ActiveSupport::TestCase diff --git a/test/concerns/depth_caching_test.rb b/test/concerns/depth_caching_test.rb index 61a9158a..2519450f 100644 --- a/test/concerns/depth_caching_test.rb +++ b/test/concerns/depth_caching_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class DepthCachingTest < ActiveSupport::TestCase diff --git a/test/concerns/depth_constraints_test.rb b/test/concerns/depth_constraints_test.rb index 86912a5c..95a2c6b8 100644 --- a/test/concerns/depth_constraints_test.rb +++ b/test/concerns/depth_constraints_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class DepthConstraintsTest < ActiveSupport::TestCase diff --git a/test/concerns/has_ancestry_test.rb b/test/concerns/has_ancestry_test.rb index 01cc7e7f..3a46d06a 100644 --- a/test/concerns/has_ancestry_test.rb +++ b/test/concerns/has_ancestry_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class HasAncestryTreeTest < ActiveSupport::TestCase @@ -96,7 +98,7 @@ def test_setup_test_nodes end def test_primary_key_is_an_integer - AncestryTestDatabase.with_model(extra_columns: { string_id: :string }) do |model| + AncestryTestDatabase.with_model(extra_columns: {string_id: :string}) do |model| model.primary_key = :string_id assert !model.primary_key_is_an_integer? diff --git a/test/concerns/hooks_test.rb b/test/concerns/hooks_test.rb index 465b6068..033f3a12 100644 --- a/test/concerns/hooks_test.rb +++ b/test/concerns/hooks_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class ArrangementTest < ActiveSupport::TestCase @@ -7,7 +9,6 @@ def test_has_ancestry_in_after_save :orphan_strategy => :adopt, :extra_columns => {:name => :string, :name_path => :string} ) do |model| - model.class_eval do before_save :before_save_hook @@ -33,9 +34,8 @@ def before_save_hook def test_update_descendants_with_changed_parent_value AncestryTestDatabase.with_model( - extra_columns: { name: :string, name_path: :string } + extra_columns: {name: :string, name_path: :string} ) do |model| - model.class_eval do before_save :update_name_path # this example will only work if the name field is unique across all levels @@ -76,9 +76,9 @@ def update_descendants_hook(descendants_clause, old_ancestry, new_ancestry) assert_equal("changed", m5.reload.name_path) assert_equal([m5.id], m2.reload.ancestor_ids) assert_equal("changed/child", m2.reload.name_path) - assert_equal([m5.id,m2.id], m3.reload.ancestor_ids) + assert_equal([m5.id, m2.id], m3.reload.ancestor_ids) assert_equal("changed/child/grandchild", m3.reload.name_path) - assert_equal([m5.id,m2.id,m3.id], m4.reload.ancestor_ids) + assert_equal([m5.id, m2.id, m3.id], m4.reload.ancestor_ids) assert_equal("changed/child/grandchild/grandchild's grand", m4.reload.name_path) end end @@ -132,15 +132,16 @@ def before_hook # see f94b22ba https://github.com/stefankroes/ancestry/pull/263 def test_node_creation_in_after_commit AncestryTestDatabase.with_model do |model| - children=[] + children = [] model.instance_eval do attr_accessor :idx - self.after_commit do - children << self.children.create!(:idx => self.idx - 1) if self.idx > 0 + + after_commit do + children << self.children.create!(:idx => idx - 1) if idx > 0 end end model.create!(:idx => 3) - assert_equal [1,2,3], children.first.ancestor_ids + assert_equal [1, 2, 3], children.first.ancestor_ids end end end diff --git a/test/concerns/integrity_checking_and_restoration_test.rb b/test/concerns/integrity_checking_and_restoration_test.rb index 2b8071f9..474a66e4 100644 --- a/test/concerns/integrity_checking_and_restoration_test.rb +++ b/test/concerns/integrity_checking_and_restoration_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class IntegrityCheckingAndRestaurationTest < ActiveSupport::TestCase @@ -49,7 +51,7 @@ def test_integrity_checking end end - def assert_integrity_restoration model + def assert_integrity_restoration(model) assert_raise Ancestry::AncestryIntegrityException do model.check_ancestry_integrity! end @@ -57,8 +59,8 @@ def assert_integrity_restoration model assert_nothing_raised do model.check_ancestry_integrity! end - assert model.all.any? {|node| node.has_parent? }, "Expected some nodes not to be roots" - assert_equal model.count, model.roots.collect {|node| node.descendants.count + 1 }.sum + assert model.all.any?(&:has_parent?), "Expected some nodes not to be roots" + assert_equal model.count, model.roots.collect { |node| node.descendants.count + 1 }.sum end def test_integrity_restoration diff --git a/test/concerns/materialized_path2_test.rb b/test/concerns/materialized_path2_test.rb index 83a1ce98..aa566267 100644 --- a/test/concerns/materialized_path2_test.rb +++ b/test/concerns/materialized_path2_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class MaterializedPath2Test < ActiveSupport::TestCase diff --git a/test/concerns/materialized_path_test.rb b/test/concerns/materialized_path_test.rb index 328f7d12..9a2c1a38 100644 --- a/test/concerns/materialized_path_test.rb +++ b/test/concerns/materialized_path_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class MaterializedPathTest < ActiveSupport::TestCase @@ -14,23 +16,23 @@ def test_ancestry_column_values # saved node.save! - assert_ancestry node, nil, child: "#{node.id}" + assert_ancestry node, nil, child: node.id.to_s # changed node.ancestor_ids = [root.id] - assert_ancestry node, "#{root.id}", db: nil, child: "#{node.id}" + assert_ancestry node, root.id.to_s, db: nil, child: node.id.to_s # changed saved node.save! - assert_ancestry node, "#{root.id}", child: "#{root.id}/#{node.id}" + assert_ancestry node, root.id.to_s, child: "#{root.id}/#{node.id}" # reloaded node.reload - assert_ancestry node, "#{root.id}", child: "#{root.id}/#{node.id}" + assert_ancestry node, root.id.to_s, child: "#{root.id}/#{node.id}" # fresh node node = model.find(node.id) - assert_ancestry node, "#{root.id}", child: "#{root.id}/#{node.id}" + assert_ancestry node, root.id.to_s, child: "#{root.id}/#{node.id}" end end diff --git a/test/concerns/orphan_strategies_test.rb b/test/concerns/orphan_strategies_test.rb index 19147681..18d6a260 100644 --- a/test/concerns/orphan_strategies_test.rb +++ b/test/concerns/orphan_strategies_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class OphanStrategiesTest < ActiveSupport::TestCase @@ -10,7 +12,7 @@ def test_setting_invalid_orphan_strategy end def test_orphan_rootify_strategy - AncestryTestDatabase.with_model orphan_strategy: :rootify, :depth => 3, :width => 3 do |model, roots| + AncestryTestDatabase.with_model orphan_strategy: :rootify, :depth => 3, :width => 3 do |_model, roots| root = roots.first.first children = root.children.to_a root.destroy @@ -48,7 +50,7 @@ def verify_parent_exists end def test_orphan_restrict_strategy - AncestryTestDatabase.with_model orphan_strategy: :restrict, :depth => 3, :width => 3 do |model, roots| + AncestryTestDatabase.with_model orphan_strategy: :restrict, :depth => 3, :width => 3 do |_model, roots| root = roots.first.first assert_raise Ancestry::AncestryException do root.destroy @@ -61,11 +63,11 @@ def test_orphan_restrict_strategy def test_orphan_adopt_strategy AncestryTestDatabase.with_model orphan_strategy: :adopt do |model| - n1 = model.create! #create a root node - n2 = model.create!(:parent => n1) #create child with parent=root - n3 = model.create!(:parent => n2) #create child with parent=n2, depth = 2 - n4 = model.create!(:parent => n2) #create child with parent=n2, depth = 2 - n5 = model.create!(:parent => n4) #create child with parent=n4, depth = 3 + n1 = model.create! # create a root node + n2 = model.create!(:parent => n1) # create child with parent=root + n3 = model.create!(:parent => n2) # create child with parent=n2, depth = 2 + n4 = model.create!(:parent => n2) # create child with parent=n2, depth = 2 + n5 = model.create!(:parent => n4) # create child with parent=n4, depth = 3 n2.destroy # delete a node with desecendants n3.reload n5.reload @@ -82,7 +84,7 @@ def test_orphan_adopt_strategy # DEPRECATED - please see test_apply_orphan_strategy_none for pattern instead def test_override_apply_orphan_strategy - AncestryTestDatabase.with_model orphan_strategy: :destroy do |model, roots| + AncestryTestDatabase.with_model orphan_strategy: :destroy do |model, _roots| root = model.create! child = model.create!(:parent => root) model.class_eval do @@ -99,7 +101,7 @@ def apply_orphan_strategy end def test_apply_orphan_strategy_none - AncestryTestDatabase.with_model orphan_strategy: :none do |model, roots| + AncestryTestDatabase.with_model orphan_strategy: :none do |model, _roots| root = model.create! child = model.create!(:parent => root) model.class_eval do @@ -125,7 +127,7 @@ def apply_orphan_strategy_abc end end - root = model.create! + root = model.create! 3.times { root.children.create! } model.create! # a node that is not affected assert_difference 'model.count', -4 do @@ -145,7 +147,7 @@ def apply_orphan_strategy_abc has_ancestry orphan_strategy: :abc, ancestry_column: AncestryTestDatabase.ancestry_column end - root = model.create! + root = model.create! 3.times { root.children.create! } model.create! # a node that is not affected assert_difference 'model.count', -4 do @@ -156,17 +158,17 @@ def apply_orphan_strategy_abc def test_basic_delete AncestryTestDatabase.with_model do |model| - n1 = model.create! #create a root node - n2 = model.create!(:parent => n1) #create child with parent=root + n1 = model.create! # create a root node + n2 = model.create!(:parent => n1) # create child with parent=root n2.destroy! - model.find(n1.id) # parent should exist + model.find(n1.id) # parent should exist - n1 = model.create! #create a root node - n2 = model.create!(:parent => n1) #create child with parent=root + n1 = model.create! # create a root node + n2 = model.create!(:parent => n1) # create child with parent=root n1.destroy! assert_nil(model.find_by(:id => n2.id)) # child should not exist - n1 = model.create! #create a root node + n1 = model.create! # create a root node n1.destroy! end end diff --git a/test/concerns/relations_test.rb b/test/concerns/relations_test.rb index 0aa3a8aa..30f3f891 100644 --- a/test/concerns/relations_test.rb +++ b/test/concerns/relations_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class RelationsTest < ActiveSupport::TestCase diff --git a/test/concerns/scopes_test.rb b/test/concerns/scopes_test.rb index 8efb7875..b31b168e 100644 --- a/test/concerns/scopes_test.rb +++ b/test/concerns/scopes_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' # all class nodes used to look up objects belong here @@ -64,7 +66,7 @@ def test_order_by_reverse AncestryTestDatabase.with_model(:width => 1, :depth => 3) do |model, _roots| child = model.last assert child - assert_nothing_raised do #IrreversibleOrderError + assert_nothing_raised do # IrreversibleOrderError assert child.ancestors.last end end @@ -104,9 +106,9 @@ def test_scoping_in_callbacks model.class_eval do define_method :after_create_callback do # We don't want to be in the #children scope here when creating the child - self.parent + parent self.parent_id = record.id if record - self.root + root end end diff --git a/test/concerns/setter_test.rb b/test/concerns/setter_test.rb index cc1e8479..adfccbb6 100644 --- a/test/concerns/setter_test.rb +++ b/test/concerns/setter_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class IntegrityCheckingAndRestaurationTest < ActiveSupport::TestCase diff --git a/test/concerns/sort_by_ancestry_test.rb b/test/concerns/sort_by_ancestry_test.rb index 768d2809..03a34448 100644 --- a/test/concerns/sort_by_ancestry_test.rb +++ b/test/concerns/sort_by_ancestry_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class SortByAncestryTest < ActiveSupport::TestCase @@ -5,7 +7,7 @@ class SortByAncestryTest < ActiveSupport::TestCase # This highlights where/why a non-correct sorting order is returned CORRECT = (ENV["CORRECT"] == "true") - RANK_SORT = -> (a, b) { a.rank <=> b.rank } + RANK_SORT = ->(a, b) { a.rank <=> b.rank } # tree is of the form: # - n1 @@ -17,7 +19,7 @@ class SortByAncestryTest < ActiveSupport::TestCase # @returns [Array] list of nodes def build_tree(model) # inflate the node id to test id wrap around edge cases - ENV["NODES"].to_i.times { model.create!.destroy } if ENV["NODES"] + ENV["NODES"]&.to_i&.times { model.create!.destroy } n1 = model.create! n2 = model.create!(:parent => n1) @@ -57,7 +59,7 @@ def test_sort_by_ancestry_no_parents_siblings end # TODO: thinking about dropping this one - # only keep if we can find a + # only keep if we can find a way to sort in the db def test_sort_by_ancestry_no_parents_same_level AncestryTestDatabase.with_model do |model| _, _, n3, n4, n5, _ = build_tree(model) @@ -116,7 +118,7 @@ def test_sort_by_ancestry_missing_parent_middle_of_tree def build_ranked_tree(model) # inflate the node id to test id wrap around edge cases # NODES=4..9 seem like edge cases - ENV["NODES"].to_i.times { model.create!.destroy } if ENV["NODES"] + ENV["NODES"]&.to_i&.times { model.create!.destroy } n1 = model.create!(:rank => 0) n2 = model.create!(:rank => 1) diff --git a/test/concerns/sti_support_test.rb b/test/concerns/sti_support_test.rb index 2911e81c..123f481f 100644 --- a/test/concerns/sti_support_test.rb +++ b/test/concerns/sti_support_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class StiSupportTest < ActiveSupport::TestCase @@ -65,7 +67,7 @@ def test_sti_support_with_from_subclass def test_sti_support_for_counter_cache AncestryTestDatabase.with_model :counter_cache => true, :extra_columns => {:type => :string} do |model| - # NOTE had to use subclasses other than Subclass1/Subclass2 from above + # NOTE: had to use subclasses other than Subclass1/Subclass2 from above # due to (I think) Rails caching those STI classes and that not getting # reset between specs diff --git a/test/concerns/touching_test.rb b/test/concerns/touching_test.rb index be18e746..2b912416 100644 --- a/test/concerns/touching_test.rb +++ b/test/concerns/touching_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class TouchingTest < ActiveSupport::TestCase @@ -6,7 +8,6 @@ def test_touch_option_disabled :extra_columns => {:name => :string, :updated_at => :datetime}, :touch => false ) do |model| - wayback = Time.new(1984) recently = Time.now - 1.minute @@ -25,7 +26,6 @@ def test_touch_option_enabled_propagates_with_modification :extra_columns => {:updated_at => :datetime}, :touch => true ) do |model| - way_back = Time.new(1984) recently = Time.now - 1.minute @@ -52,7 +52,6 @@ def test_touch_option_enabled_propagates_with_modification def test_touch_propogates_multiple_levels AncestryTestDatabase.with_model(:extra_columns => {:name => :string, :updated_at => :datetime}, :touch => true) do |model| - way_back = Time.new(1984) recently = Time.now - 1.minute @@ -82,7 +81,6 @@ def test_touch_option_enabled_doesnt_propagate_without_modification :extra_columns => {:updated_at => :datetime}, :touch => true ) do |model| - way_back = Time.new(1984) recently = Time.now - 1.minute @@ -105,7 +103,6 @@ def test_touch_option_with_scope :extra_columns => {:updated_at => :datetime}, :touch => true ) do |model| - way_back = Time.new(1984) recently = Time.now - 1.minute diff --git a/test/concerns/tree_navigation_test.rb b/test/concerns/tree_navigation_test.rb index ad8b2afc..be61967d 100644 --- a/test/concerns/tree_navigation_test.rb +++ b/test/concerns/tree_navigation_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' # this is testing attribute getters @@ -7,15 +9,15 @@ class TreeNavigationTest < ActiveSupport::TestCase # across: |siblings|subtree |path | ATTRIBUTE_MATRIX = { root: {attribute_id: :root_id}, - parent: {attribute_id: :parent_id, exists: :has_parent?, db: true}, - ancestors: {attribute_ids: :ancestor_ids, exists: :ancestors?, db: true}, - children: {attribute_ids: :child_ids, exists: :children?}, + parent: {attribute_id: :parent_id, exists: :has_parent?, db: true}, + ancestors: {attribute_ids: :ancestor_ids, exists: :ancestors?, db: true}, + children: {attribute_ids: :child_ids, exists: :children?}, descendants: {attribute_ids: :descendant_ids}, indirects: {attribute_ids: :indirect_ids}, - siblings: {attribute_ids: :sibling_ids, exists: :siblings?}, + siblings: {attribute_ids: :sibling_ids, exists: :siblings?}, subtree: {attribute_ids: :subtree_ids}, - path: {attribute_ids: :path_ids, db: true}, - } + path: {attribute_ids: :path_ids, db: true} + }.freeze # NOTE: has_ancestors? is an alias for parent? / ancestors? but not tested # class level getters are in test/concerns/scopes_test.rb @@ -194,7 +196,8 @@ def test_node_in_database_children # in database (again - but in a different hierarchy) node11.save! - node1.reload ; node2.reload + node1.reload + node2.reload # are these necessary? # do we want this to work without? @@ -242,19 +245,26 @@ def update_descendants_with_new_ancestry raise "callback disabled for #{id}" if id == 2 else raise "callback eabled for #{id}" if id != 2 + # want to make sure we're pointing at the correct nodes actual = unscoped_descendants_before_last_save.order(:id).map(&:id) raise "unscoped_descendants_before_last_save was #{actual}" unless actual == [3, 4, 5] + actual = path_ids_before_last_save raise "bad path_ids(before) is #{actual}" unless actual == [1, 2] + actual = path_ids raise "bad path_ids is #{actual}" unless actual == [6, 2] + actual = parent_id_before_last_save raise "bad parent_id(before) id #{actual}" unless actual == 1 + actual = parent_id raise "bad parent_id(before) id #{actual}" unless actual == 6 + actual = ancestor_ids_before_last_save raise "bad ancestor_ids(before) id #{actual}" unless actual == [1] + actual = ancestor_ids raise "bad ancestor_ids(before) id #{actual}" unless actual == [6] end @@ -308,7 +318,7 @@ def assert_attribute(node, attribute_name, value, db: :value, exists: :value) # @param value [Array] expected output # @param attribute_name [Symbol] attribute to test # @param exists [true|false] test the exists "attribute? (default values.present?) - # @param db [Array[AR]] value that should be reflected _in_database (default: use values) + # @param db [Array[AR]] value that should be reflected _in_database (default: use values) # skips if not supported in matrix def assert_attributes(node, attribute_name, values, db: :values, exists: :values) attribute_ids = ATTRIBUTE_MATRIX[attribute_name][:attribute_ids] diff --git a/test/concerns/tree_predicate_test.rb b/test/concerns/tree_predicate_test.rb index c57d6304..38be4841 100644 --- a/test/concerns/tree_predicate_test.rb +++ b/test/concerns/tree_predicate_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative '../environment' class TreePredicateTest < ActiveSupport::TestCase @@ -19,7 +21,7 @@ def test_tree_predicates # Children assertions assert root.has_children? assert !root.is_childless? - assert children.map { |n| n.is_childless? }.all? + assert children.map(&:is_childless?).all? assert children.map { |n| !root.child_of?(n) }.all? assert children.map { |n| n.child_of?(root) }.all? # Siblings assertions diff --git a/test/environment.rb b/test/environment.rb index dc3e848e..f5ed1b0e 100644 --- a/test/environment.rb +++ b/test/environment.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rubygems' require 'bundler/setup' @@ -24,7 +26,7 @@ class AncestryTestDatabase def self.setup # Silence I18n and Activerecord logging I18n.enforce_available_locales = false if I18n.respond_to? :enforce_available_locales= - ActiveRecord::Base.logger = Logger.new(STDERR) + ActiveRecord::Base.logger = Logger.new($stderr) ActiveRecord::Base.logger.level = Logger::Severity::UNKNOWN # Assume Travis CI database config if no custom one exists @@ -35,14 +37,15 @@ def self.setup end # Setup database connection - all_config = if YAML.respond_to?(:safe_load_file) - YAML.safe_load_file(filename, aliases: true) - else - YAML.load_file(filename) - end + all_config = + if YAML.respond_to?(:safe_load_file) + YAML.safe_load_file(filename, aliases: true) + else + YAML.load_file(filename) + end config = all_config[db_type] if config.blank? - $stderr.puts "","","ERROR: Could not find '#{db_type}' in #{filename}" + $stderr.puts "", "", "ERROR: Could not find '#{db_type}' in #{filename}" $stderr.puts "Pick from: #{all_config.keys.join(", ")}", "", "" exit(1) end @@ -59,13 +62,12 @@ def self.setup puts "testing #{db_type} #{Ancestry.default_update_strategy == :sql ? "(sql) " : ""}(with #{column_type} #{ancestry_column})" puts "column format: #{Ancestry.default_ancestry_format} options: #{column_options.inspect}" - - rescue => err + rescue StandardError => e if ENV["CI"] raise else puts "\nSkipping tests for '#{db_type}'" - puts " #{err}\n\n" + puts " #{e}\n\n" exit 0 end end @@ -102,18 +104,18 @@ def self.column_options(force_allow_nil: false) if column_type == "string" { :collation => ancestry_collation == "default" ? nil : ancestry_collation, - :null => materialized_path2? ? false : true + :null => !materialized_path2? } else { :limit => 3000, - :null => materialized_path2? ? false : true + :null => !materialized_path2? } end force_allow_nil ? @column_options.merge(:null => true) : @column_options end - def self.with_model options = {} + def self.with_model(options = {}) depth = options.delete(:depth) || 0 width = options.delete(:width) || 0 skip_ancestry = options.delete(:skip_ancestry) @@ -121,7 +123,7 @@ def self.with_model options = {} default_scope_params = options.delete(:default_scope_params) options[:ancestry_column] ||= ancestry_column - table_options={} + table_options = {} table_options[:id] = options.delete(:id) if options.key?(:id) ActiveRecord::Base.connection.create_table 'test_nodes', **table_options do |table| @@ -145,13 +147,13 @@ def self.with_model options = {} table.integer counter_cache_column, default: 0, null: false end - extra_columns.each do |name, type| + extra_columns&.each do |name, type| table.send type, name - end unless extra_columns.nil? + end end testmethod = caller[0][/`.*'/][1..-2] - model_name = testmethod.camelize + "TestNode" + model_name = "#{testmethod.camelize}TestNode" begin model = Class.new(ActiveRecord::Base) @@ -177,13 +179,15 @@ def self.with_model options = {} end end - def self.create_test_nodes model, depth, width, parent = nil - unless depth == 0 + def self.create_test_nodes(model, depth, width, parent = nil) + if depth == 0 + [] + else Array.new width do node = model.create!(:parent => parent) [node, create_test_nodes(model, depth - 1, width, node)] end - else; []; end + end end def self.postgres? @@ -192,9 +196,9 @@ def self.postgres? def self.materialized_path2? return @materialized_path2 if defined?(@materialized_path2) + @materialized_path2 = (ENV["FORMAT"] == "materialized_path2") end - private def self.db_type ENV["DB"].presence || "sqlite3" diff --git a/test/test_helpers.rb b/test/test_helpers.rb index 61caee1c..74b9a405 100644 --- a/test/test_helpers.rb +++ b/test/test_helpers.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module TestHelpers def assert_ancestry(node, value, child: :skip, db: :value) column_name = node.class.ancestry_column