From 918e443d945e2c0291209092b5d9d29a74c1970e Mon Sep 17 00:00:00 2001 From: Kirill Shnurov Date: Sun, 29 Jan 2023 18:55:53 +0800 Subject: [PATCH 1/2] remove ancestry_* writers --- lib/ancestry/class_methods.rb | 2 +- lib/ancestry/has_ancestry.rb | 12 ++++++------ lib/ancestry/instance_methods.rb | 22 ++++++++++------------ lib/ancestry/materialized_path.rb | 24 ++++++++++++------------ lib/ancestry/materialized_path2.rb | 8 ++++---- lib/ancestry/materialized_path_pg.rb | 5 ++--- test/concerns/has_ancestry_test.rb | 9 --------- 7 files changed, 35 insertions(+), 47 deletions(-) diff --git a/lib/ancestry/class_methods.rb b/lib/ancestry/class_methods.rb index 3f6745b9..f76deb67 100644 --- a/lib/ancestry/class_methods.rb +++ b/lib/ancestry/class_methods.rb @@ -125,7 +125,7 @@ def check_ancestry_integrity! options = {} 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.ancestry_column}" + :ancestry_column => "#{node.read_attribute node.class.ancestry_column}" )) end # ... check that all ancestors exist diff --git a/lib/ancestry/has_ancestry.rb b/lib/ancestry/has_ancestry.rb index 4d9bcdf1..5e6409e1 100644 --- a/lib/ancestry/has_ancestry.rb +++ b/lib/ancestry/has_ancestry.rb @@ -21,18 +21,18 @@ def has_ancestry options = {} orphan_strategy = options[:orphan_strategy] || :destroy # Create ancestry column accessor and set to option or default - cattr_accessor :ancestry_column - self.ancestry_column = options[:ancestry_column] || :ancestry + self.class_variable_set('@@ancestry_column', options[:ancestry_column] || :ancestry) + cattr_reader :ancestry_column, instance_reader: false cattr_accessor :ancestry_primary_key_format self.ancestry_primary_key_format = options[:primary_key_format].presence || Ancestry.default_primary_key_format - cattr_accessor :ancestry_delimiter - self.ancestry_delimiter = '/' + self.class_variable_set('@@ancestry_delimiter', '/') + cattr_reader :ancestry_delimiter, instance_reader: false # Save self as base class (for STI) - cattr_accessor :ancestry_base_class - self.ancestry_base_class = self + self.class_variable_set('@@ancestry_base_class', self) + cattr_reader :ancestry_base_class # Touch ancestors after updating cattr_accessor :touch_ancestors diff --git a/lib/ancestry/instance_methods.rb b/lib/ancestry/instance_methods.rb index 91f7824c..7a089d67 100644 --- a/lib/ancestry/instance_methods.rb +++ b/lib/ancestry/instance_methods.rb @@ -62,7 +62,7 @@ def apply_orphan_strategy_restrict # Touch each of this record's ancestors (after save) def touch_ancestors_callback - if !ancestry_callbacks_disabled? && self.ancestry_base_class.touch_ancestors + if !ancestry_callbacks_disabled? && self.class.ancestry_base_class.touch_ancestors # Touch each of the old *and* new ancestors unscoped_current_and_previous_ancestors.each do |ancestor| ancestor.without_ancestry_callbacks do @@ -90,9 +90,7 @@ def decrease_parent_counter_cache end def update_parent_counter_cache - changed = saved_change_to_attribute?(self.ancestry_base_class.ancestry_column) - - return unless changed + return unless saved_change_to_attribute?(self.class.ancestry_column) if parent_id_was = parent_id_before_last_save self.ancestry_base_class.decrement_counter counter_cache_column, parent_id_was @@ -109,7 +107,7 @@ def has_parent? alias :ancestors? :has_parent? def ancestry_changed? - column = self.ancestry_base_class.ancestry_column.to_s + 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)) @@ -119,7 +117,7 @@ def sane_ancestor_ids? current_context, self.validation_context = validation_context, nil errors.clear - attribute = ancestry_base_class.ancestry_column + attribute = self.class.ancestry_column ancestry_value = send(attribute) return true unless ancestry_value @@ -154,7 +152,7 @@ def depth end def cache_depth - write_attribute self.ancestry_base_class.depth_cache_column, depth + write_attribute self.class.ancestry_base_class.depth_cache_column, depth end def ancestor_of?(node) @@ -220,7 +218,7 @@ def children end def child_ids - children.pluck(self.ancestry_base_class.primary_key) + children.pluck(self.class.primary_key) end def has_children? @@ -245,7 +243,7 @@ def siblings # NOTE: includes self def sibling_ids - siblings.pluck(self.ancestry_base_class.primary_key) + siblings.pluck(self.class.primary_key) end def has_siblings? @@ -269,7 +267,7 @@ def descendants depth_options = {} end def descendant_ids depth_options = {} - descendants(depth_options).pluck(self.ancestry_base_class.primary_key) + descendants(depth_options).pluck(self.class.primary_key) end def descendant_of?(node) @@ -283,7 +281,7 @@ def indirects depth_options = {} end def indirect_ids depth_options = {} - indirects(depth_options).pluck(self.ancestry_base_class.primary_key) + indirects(depth_options).pluck(self.class.primary_key) end def indirect_of?(node) @@ -297,7 +295,7 @@ def subtree depth_options = {} end def subtree_ids depth_options = {} - subtree(depth_options).pluck(self.ancestry_base_class.primary_key) + subtree(depth_options).pluck(self.class.primary_key) end # Callback disabling diff --git a/lib/ancestry/materialized_path.rb b/lib/ancestry/materialized_path.rb index e7a4bcc1..710e12b3 100644 --- a/lib/ancestry/materialized_path.rb +++ b/lib/ancestry/materialized_path.rb @@ -112,29 +112,29 @@ def ancestry_format_regexp module InstanceMethods # optimization - better to go directly to column and avoid parsing def ancestors? - read_attribute(self.ancestry_base_class.ancestry_column) != self.ancestry_base_class.ancestry_root + read_attribute(self.class.ancestry_column) != self.class.ancestry_root end alias :has_parent? :ancestors? def ancestor_ids=(value) - write_attribute(self.ancestry_base_class.ancestry_column, generate_ancestry(value)) + write_attribute(self.class.ancestry_column, generate_ancestry(value)) end def ancestor_ids - parse_ancestry_column(read_attribute(self.ancestry_base_class.ancestry_column)) + parse_ancestry_column(read_attribute(self.class.ancestry_column)) end def ancestor_ids_before_last_save - parse_ancestry_column(attribute_before_last_save(self.ancestry_base_class.ancestry_column)) + parse_ancestry_column(attribute_before_last_save(self.class.ancestry_column)) end def parent_id_before_last_save - parse_ancestry_column(attribute_before_last_save(self.ancestry_base_class.ancestry_column)).last + parse_ancestry_column(attribute_before_last_save(self.class.ancestry_column)).last end # optimization - better to go directly to column and avoid parsing def sibling_of?(node) - self.read_attribute(self.ancestry_base_class.ancestry_column) == node.read_attribute(self.ancestry_base_class.ancestry_column) + self.read_attribute(self.class.ancestry_column) == node.read_attribute(node.class.ancestry_column) end # private (public so class methods can find it) @@ -143,26 +143,26 @@ def sibling_of?(node) def child_ancestry # New records cannot have children raise Ancestry::AncestryException.new(I18n.t("ancestry.no_child_for_new_record")) if new_record? - [attribute_in_database(self.ancestry_base_class.ancestry_column), id].compact.join(self.ancestry_base_class.ancestry_delimiter) + [attribute_in_database(self.class.ancestry_column), id].compact.join(self.class.ancestry_delimiter) end def child_ancestry_before_save # New records cannot have children raise Ancestry::AncestryException.new(I18n.t("ancestry.no_child_for_new_record")) if new_record? - [attribute_before_last_save(self.ancestry_base_class.ancestry_column), id].compact.join(self.ancestry_base_class.ancestry_delimiter) + [attribute_before_last_save(self.class.ancestry_column), id].compact.join(self.class.ancestry_delimiter) end def parse_ancestry_column(obj) - return [] if obj.nil? || obj == self.ancestry_base_class.ancestry_root - obj_ids = obj.split(self.ancestry_base_class.ancestry_delimiter).delete_if(&:blank?) + return [] if obj.nil? || obj == self.class.ancestry_root + obj_ids = obj.split(self.class.ancestry_delimiter).delete_if(&:blank?) self.class.primary_key_is_an_integer? ? obj_ids.map!(&:to_i) : obj_ids end def generate_ancestry(ancestor_ids) if ancestor_ids.present? && ancestor_ids.any? - ancestor_ids.join(self.ancestry_base_class.ancestry_delimiter) + ancestor_ids.join(self.class.ancestry_delimiter) else - self.ancestry_base_class.ancestry_root + self.class.ancestry_root end end end diff --git a/lib/ancestry/materialized_path2.rb b/lib/ancestry/materialized_path2.rb index caa2fc23..516e04b8 100644 --- a/lib/ancestry/materialized_path2.rb +++ b/lib/ancestry/materialized_path2.rb @@ -42,20 +42,20 @@ module InstanceMethods def child_ancestry # New records cannot have children raise Ancestry::AncestryException.new(I18n.t("ancestry.no_child_for_new_record")) if new_record? - "#{attribute_in_database(self.ancestry_base_class.ancestry_column)}#{id}#{self.ancestry_base_class.ancestry_delimiter}" + "#{attribute_in_database(self.class.ancestry_column)}#{id}#{self.class.ancestry_delimiter}" end def child_ancestry_before_save # New records cannot have children raise Ancestry::AncestryException.new(I18n.t("ancestry.no_child_for_new_record")) if new_record? - "#{attribute_before_last_save(self.ancestry_base_class.ancestry_column)}#{id}#{self.ancestry_base_class.ancestry_delimiter}" + "#{attribute_before_last_save(self.class.ancestry_column)}#{id}#{self.class.ancestry_delimiter}" end def generate_ancestry(ancestor_ids) if ancestor_ids.present? && ancestor_ids.any? - "#{self.ancestry_base_class.ancestry_delimiter}#{ancestor_ids.join(self.ancestry_base_class.ancestry_delimiter)}#{self.ancestry_base_class.ancestry_delimiter}" + "#{self.class.ancestry_delimiter}#{ancestor_ids.join(self.class.ancestry_delimiter)}#{self.class.ancestry_delimiter}" else - self.ancestry_base_class.ancestry_root + self.class.ancestry_root end end end diff --git a/lib/ancestry/materialized_path_pg.rb b/lib/ancestry/materialized_path_pg.rb index 73f7b957..24e7bf3b 100644 --- a/lib/ancestry/materialized_path_pg.rb +++ b/lib/ancestry/materialized_path_pg.rb @@ -4,14 +4,13 @@ module MaterializedPathPg def update_descendants_with_new_ancestry # If enabled and node is existing and ancestry was updated and the new ancestry is sane ... if !ancestry_callbacks_disabled? && !new_record? && ancestry_changed? && sane_ancestor_ids? - ancestry_column = ancestry_base_class.ancestry_column old_ancestry = generate_ancestry( path_ids_before_last_save ) new_ancestry = generate_ancestry( path_ids ) update_clause = [ - "#{ancestry_column} = regexp_replace(#{ancestry_column}, '^#{Regexp.escape(old_ancestry)}', '#{new_ancestry}')" + "#{self.class.ancestry_column} = regexp_replace(#{self.class.ancestry_column}, '^#{Regexp.escape(old_ancestry)}', '#{new_ancestry}')" ] - if ancestry_base_class.respond_to?(:depth_cache_column) && respond_to?(ancestry_base_class.depth_cache_column) + if self.class.ancestry_base_class.respond_to?(:depth_cache_column) && respond_to?(self.class.ancestry_base_class.depth_cache_column) depth_cache_column = ancestry_base_class.depth_cache_column.to_s update_clause << "#{depth_cache_column} = length(regexp_replace(regexp_replace(ancestry, '^#{Regexp.escape(old_ancestry)}', '#{new_ancestry}'), '[^#{ancestry_base_class.ancestry_delimiter}]', '', 'g')) #{ancestry_base_class.ancestry_format == :materialized_path2 ? '-' : '+'} 1" end diff --git a/test/concerns/has_ancestry_test.rb b/test/concerns/has_ancestry_test.rb index 062b3e8e..460e93ed 100644 --- a/test/concerns/has_ancestry_test.rb +++ b/test/concerns/has_ancestry_test.rb @@ -13,15 +13,6 @@ def test_non_default_ancestry_column end end - def test_setting_ancestry_column - AncestryTestDatabase.with_model do |model| - model.ancestry_column = :ancestors - assert_equal :ancestors, model.ancestry_column - model.ancestry_column = :ancestry - assert_equal :ancestry, model.ancestry_column - end - end - def test_invalid_has_ancestry_options assert_raise Ancestry::AncestryException do Class.new(ActiveRecord::Base).has_ancestry :this_option_doesnt_exist => 42 From 0c0240aefe990763ae05a56cec827818bfcffaa0 Mon Sep 17 00:00:00 2001 From: Kirill Shnurov Date: Sun, 29 Jan 2023 22:56:35 +0800 Subject: [PATCH 2/2] remove ancestry_base_class instance reader --- lib/ancestry/class_methods.rb | 14 +++++++------- lib/ancestry/has_ancestry.rb | 2 +- lib/ancestry/instance_methods.rb | 28 ++++++++++++++-------------- lib/ancestry/materialized_path_pg.rb | 6 +++--- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/ancestry/class_methods.rb b/lib/ancestry/class_methods.rb index f76deb67..19908452 100644 --- a/lib/ancestry/class_methods.rb +++ b/lib/ancestry/class_methods.rb @@ -2,7 +2,7 @@ module Ancestry module ClassMethods # Fetch tree node if necessary def to_node object - if object.is_a?(self.ancestry_base_class) + if object.is_a?(ancestry_base_class) object else unscoped_where { |scope| scope.find(object.try(primary_key) || object) } @@ -11,7 +11,7 @@ def to_node object # Scope on relative depth options def scope_depth depth_options, depth - depth_options.inject(self.ancestry_base_class) do |scope, option| + 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 @@ -29,9 +29,9 @@ def scope_depth depth_options, depth # Get all nodes and sort them into an empty hash def arrange options = {} if (order = options.delete(:order)) - arrange_nodes self.ancestry_base_class.order(order).where(options) + arrange_nodes ancestry_base_class.order(order).where(options) else - arrange_nodes self.ancestry_base_class.where(options) + arrange_nodes ancestry_base_class.where(options) end end @@ -164,7 +164,7 @@ def check_ancestry_integrity! options = {} def restore_ancestry_integrity! parent_ids = {} # Wrap the whole thing in a transaction ... - self.ancestry_base_class.transaction do + ancestry_base_class.transaction do unscoped_where do |scope| # For each node ... scope.find_each do |node| @@ -216,7 +216,7 @@ def build_ancestry_from_parent_ids! column=:parent_id, parent_id = nil, ancestor def rebuild_depth_cache! raise Ancestry::AncestryException.new(I18n.t("ancestry.cannot_rebuild_depth_cache")) unless respond_to? :depth_cache_column - self.ancestry_base_class.transaction do + ancestry_base_class.transaction do unscoped_where do |scope| scope.find_each do |node| node.update_attribute depth_cache_column, node.depth @@ -226,7 +226,7 @@ def rebuild_depth_cache! end def unscoped_where - yield self.ancestry_base_class.default_scoped.unscope(:where) + yield ancestry_base_class.default_scoped.unscope(:where) end ANCESTRY_UNCAST_TYPES = [:string, :uuid, :text].freeze diff --git a/lib/ancestry/has_ancestry.rb b/lib/ancestry/has_ancestry.rb index 5e6409e1..659eef02 100644 --- a/lib/ancestry/has_ancestry.rb +++ b/lib/ancestry/has_ancestry.rb @@ -32,7 +32,7 @@ def has_ancestry options = {} # Save self as base class (for STI) self.class_variable_set('@@ancestry_base_class', self) - cattr_reader :ancestry_base_class + cattr_reader :ancestry_base_class, instance_reader: false # Touch ancestors after updating cattr_accessor :touch_ancestors diff --git a/lib/ancestry/instance_methods.rb b/lib/ancestry/instance_methods.rb index 7a089d67..41645031 100644 --- a/lib/ancestry/instance_methods.rb +++ b/lib/ancestry/instance_methods.rb @@ -74,7 +74,7 @@ def touch_ancestors_callback # Counter Cache def increase_parent_counter_cache - self.ancestry_base_class.increment_counter counter_cache_column, parent_id + self.class.ancestry_base_class.increment_counter counter_cache_column, parent_id end def decrease_parent_counter_cache @@ -86,14 +86,14 @@ def decrease_parent_counter_cache return if defined?(@_trigger_destroy_callback) && !@_trigger_destroy_callback return if ancestry_callbacks_disabled? - self.ancestry_base_class.decrement_counter counter_cache_column, parent_id + self.class.ancestry_base_class.decrement_counter counter_cache_column, parent_id end def update_parent_counter_cache return unless saved_change_to_attribute?(self.class.ancestry_column) if parent_id_was = parent_id_before_last_save - self.ancestry_base_class.decrement_counter counter_cache_column, parent_id_was + self.class.ancestry_base_class.decrement_counter counter_cache_column, parent_id_was end parent_id && increase_parent_counter_cache @@ -131,8 +131,8 @@ def sane_ancestor_ids? end def ancestors depth_options = {} - return self.ancestry_base_class.none unless has_parent? - self.ancestry_base_class.scope_depth(depth_options, depth).ordered_by_ancestry.ancestors_of(self) + 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 def path_ids @@ -144,7 +144,7 @@ def path_ids_before_last_save end def path depth_options = {} - self.ancestry_base_class.scope_depth(depth_options, depth).ordered_by_ancestry.inpath_of(self) + self.class.ancestry_base_class.scope_depth(depth_options, depth).ordered_by_ancestry.inpath_of(self) end def depth @@ -214,7 +214,7 @@ def root_of?(node) # Children def children - self.ancestry_base_class.children_of(self) + self.class.ancestry_base_class.children_of(self) end def child_ids @@ -238,7 +238,7 @@ def child_of?(node) # Siblings def siblings - self.ancestry_base_class.siblings_of(self) + self.class.ancestry_base_class.siblings_of(self) end # NOTE: includes self @@ -263,7 +263,7 @@ def sibling_of?(node) # Descendants def descendants depth_options = {} - self.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).descendants_of(self) + self.class.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).descendants_of(self) end def descendant_ids depth_options = {} @@ -277,7 +277,7 @@ def descendant_of?(node) # Indirects def indirects depth_options = {} - self.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).indirects_of(self) + self.class.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).indirects_of(self) end def indirect_ids depth_options = {} @@ -291,7 +291,7 @@ def indirect_of?(node) # Subtree def subtree depth_options = {} - self.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).subtree_of(self) + self.class.ancestry_base_class.ordered_by_ancestry.scope_depth(depth_options, depth).subtree_of(self) end def subtree_ids depth_options = {} @@ -314,13 +314,13 @@ def ancestry_callbacks_disabled? private def unscoped_descendants unscoped_where do |scope| - scope.where self.ancestry_base_class.descendant_conditions(self) + scope.where self.class.ancestry_base_class.descendant_conditions(self) end end def unscoped_descendants_before_save unscoped_where do |scope| - scope.where self.ancestry_base_class.descendant_before_save_conditions(self) + scope.where self.class.ancestry_base_class.descendant_before_save_conditions(self) end end @@ -338,7 +338,7 @@ def unscoped_find id end def unscoped_where - self.ancestry_base_class.unscoped_where do |scope| + self.class.ancestry_base_class.unscoped_where do |scope| yield scope end end diff --git a/lib/ancestry/materialized_path_pg.rb b/lib/ancestry/materialized_path_pg.rb index 24e7bf3b..4b7a129a 100644 --- a/lib/ancestry/materialized_path_pg.rb +++ b/lib/ancestry/materialized_path_pg.rb @@ -10,9 +10,9 @@ def update_descendants_with_new_ancestry "#{self.class.ancestry_column} = regexp_replace(#{self.class.ancestry_column}, '^#{Regexp.escape(old_ancestry)}', '#{new_ancestry}')" ] - if self.class.ancestry_base_class.respond_to?(:depth_cache_column) && respond_to?(self.class.ancestry_base_class.depth_cache_column) - depth_cache_column = ancestry_base_class.depth_cache_column.to_s - update_clause << "#{depth_cache_column} = length(regexp_replace(regexp_replace(ancestry, '^#{Regexp.escape(old_ancestry)}', '#{new_ancestry}'), '[^#{ancestry_base_class.ancestry_delimiter}]', '', 'g')) #{ancestry_base_class.ancestry_format == :materialized_path2 ? '-' : '+'} 1" + if self.class.respond_to?(:depth_cache_column) && respond_to?(self.class.depth_cache_column) + depth_cache_column = self.class.depth_cache_column.to_s + update_clause << "#{depth_cache_column} = length(regexp_replace(regexp_replace(ancestry, '^#{Regexp.escape(old_ancestry)}', '#{new_ancestry}'), '[^#{self.class.ancestry_delimiter}]', '', 'g')) #{self.class.ancestry_format == :materialized_path2 ? '-' : '+'} 1" end unscoped_descendants_before_save.update_all update_clause.join(', ')