diff --git a/API-CHANGES.md b/API-CHANGES.md index 211d0c3..9faa90a 100644 --- a/API-CHANGES.md +++ b/API-CHANGES.md @@ -7,6 +7,28 @@ _Note_: API changes are expected to reduce significantly after the `1.x` release. In most cases, an alternative will be provided to ensure relatively smooth transition to the new APIs. +## Release 2.2.0 Changes + +* [Tree::TreeNode#add][add] now raises `ArgumentError` when attempting to add + an ancestor node as a child, preventing cycles. + +* [Tree::TreeNode#remove_all!][remove_all] now detaches children by clearing + their parent links. + +* [Tree::TreeNode#rename_child][rename_child] now raises `ArgumentError` if the + new name collides with an existing sibling. + +* [Tree::BinaryTreeNode#set_child_at][set_child_at] now raises `ArgumentError` + for invalid indices and cleans up parent/hash references when replacing or + clearing a child. + +* [Tree::TreeNode#postordered_each][postordered_each] and + [Tree::TreeNode#breadth_each][breadth_each] now skip `nil` children to + support binary trees with missing children. + +* [Tree::TreeNode#each_level][each_level] now returns a level-wise enumerator + when called without a block. + ## Release 2.1.0 Changes * Minimum Ruby version has been bumped to 2.7 and above @@ -143,6 +165,7 @@ smooth transition to the new APIs. [detached_subtree_copy]: rdoc-ref:Tree::TreeNode#detached_subtree_copy [dup]: rdoc-ref:Tree::TreeNode#dup [each]: rdoc-ref:Tree::TreeNode#each +[each_level]: rdoc-ref:Tree::TreeNode#each_level [in_degree]: rdoc-ref:Tree::Utils::TreeMetricsHandler#in_degree [initialize]: rdoc-ref:Tree::TreeNode#initialize [inordered_each]: rdoc-ref:Tree::BinaryTreeNode#inordered_each @@ -155,5 +178,8 @@ smooth transition to the new APIs. [postordered_each]: rdoc-ref:Tree::TreeNode#postordered_each [preordered_each]: rdoc-ref:Tree::TreeNode#preordered_each [previous_sibling]: rdoc-ref:Tree::TreeNode#previous_sibling +[remove_all]: rdoc-ref:Tree::TreeNode#remove_all! +[rename_child]: rdoc-ref:Tree::TreeNode#rename_child +[set_child_at]: rdoc-ref:Tree::BinaryTreeNode#set_child_at [siblings]: rdoc-ref:Tree::TreeNode#siblings [to_json]: rdoc-ref:Tree::Utils::JSONConverter#to_json diff --git a/Gemfile.lock b/Gemfile.lock index 6974d45..3e3155f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - rubytree (2.1.1) + rubytree (2.2.0) json (~> 2.0, > 2.9) GEM diff --git a/History.md b/History.md index 37a146c..1d571ac 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,23 @@ # History of Changes +### 2.2.0 / 2026-02-06 + +* Prevent cycles by rejecting attempts to add an ancestor as a child. + +* Ensure `remove_all!` detaches children by clearing their parent links. + +* Raise on sibling name collisions in `rename_child`. + +* Harden binary tree child assignment (`set_child_at`) with proper index errors + and cleanup of parent/hash references. + +* Make traversals resilient to missing children by skipping `nil` nodes in + `postordered_each` and `breadth_each`. + +* Return a level-wise enumerator from `each_level` when no block is given. + +* Improve `to_s` formatting to show `` for nil content. + ### 2.1.1 / 2024-12-19 * 2.1.1 is a minor update that updates all dependencies and updates the guard diff --git a/Rakefile b/Rakefile index c56e6ab..717c92e 100644 --- a/Rakefile +++ b/Rakefile @@ -2,7 +2,7 @@ # # Rakefile - This file is part of the RubyTree package. # -# Copyright (c) 2006-2022 Anupam Sengupta +# Copyright (c) 2006-2024 Anupam Sengupta # # All rights reserved. # @@ -160,9 +160,16 @@ namespace :gem do pkg.need_tar = true end - desc 'Push the gem into the Rubygems repository' + desc 'Push the gem into the Rubygems and Github repositories' task push: :gem do + github_repo = 'https://rubygems.pkg.github.com/evolve75' + + # This pushes to the standard RubyGems registry sh "gem push pkg/#{GEM_NAME}" + + # For github, the credentials key is assumed to be github + # See: https://docs.github.com/en/packages/working-with-a-github-packages-registry/ + sh "gem push --key github --host #{github_repo} pkg/#{GEM_NAME}" end end diff --git a/lib/tree.rb b/lib/tree.rb index ef23fe1..b169583 100644 --- a/lib/tree.rb +++ b/lib/tree.rb @@ -318,7 +318,8 @@ def marshal_load(dumped_tree_array) # # @return [String] A string representation of the node. def to_s - "Node Name: #{@name} Content: #{@content.to_s || ''} " \ + content_str = @content.nil? ? '' : @content.to_s + "Node Name: #{@name} Content: #{content_str} " \ "Parent: #{root? ? '' : @parent.name.to_s} " \ "Children: #{@children.length} Total Nodes: #{size}" end @@ -394,6 +395,10 @@ def add(child, at_index = -1) raise ArgumentError, 'Attempting add root as a child' if child.equal?(root) + if (ancestors = parentage) && ancestors.include?(child) + raise ArgumentError, 'Attempting add ancestor as a child' + end + # Lazy man's unique test, won't test if children of child are unique in # this tree too. raise "Child #{child.name} already added!"\ @@ -453,6 +458,9 @@ def rename_child(old_name, new_name) raise ArgumentError, "Invalid child name specified: #{old_name}"\ unless @children_hash.key?(old_name) + raise ArgumentError, "Child name already exists: #{new_name}"\ + if @children_hash.key?(new_name) + @children_hash[new_name] = @children_hash.delete(old_name) @children_hash[new_name].name = new_name end @@ -539,7 +547,10 @@ def remove_from_parent! # @see #remove! # @see #remove_from_parent! def remove_all! - @children.each(&:remove_all!) + @children.each do |child| + child.remove_all! + child.set_as_root! + end @children_hash.clear @children.clear @@ -669,7 +680,7 @@ def postordered_each peek_node.visited = true # Add the children to the stack. Use the marking structure. marked_children = - peek_node.node.children.map { |node| marked_node.new(node, false) } + peek_node.node.children.compact.map { |node| marked_node.new(node, false) } node_stack = marked_children.concat(node_stack) next else @@ -700,9 +711,11 @@ def breadth_each # Use a queue to do breadth traversal until node_queue.empty? node_to_traverse = node_queue.shift + next unless node_to_traverse + yield node_to_traverse # Enqueue the children from left to right. - node_to_traverse.children { |child| node_queue.push child } + node_to_traverse.children { |child| node_queue.push child if child } end self if block_given? @@ -770,7 +783,7 @@ def each_level end self else - each + to_enum(:each_level) end end diff --git a/lib/tree/binarytree.rb b/lib/tree/binarytree.rb index 15f3589..8f773eb 100644 --- a/lib/tree/binarytree.rb +++ b/lib/tree/binarytree.rb @@ -203,12 +203,29 @@ def inordered_each # # @raise [ArgumentError] If the index is out of limits. def set_child_at(child, at_index) - raise ArgumentError 'A binary tree cannot have more than two children.'\ - unless (0..1).include? at_index + raise ArgumentError, 'A binary tree cannot have more than two children.'\ + unless (0..1).include? at_index - @children[at_index] = child - @children_hash[child.name] = child if child # Assign the name mapping - child.parent = self if child + old_child = @children[at_index] + if old_child && old_child != child + still_present = @children.each_with_index.any? do |existing, idx| + idx != at_index && existing.equal?(old_child) + end + + unless still_present + @children_hash.delete(old_child.name) + old_child.set_as_root! + end + end + + if child + child.parent&.remove!(child) unless child.parent == self + @children[at_index] = child + @children_hash[child.name] = child # Assign the name mapping + child.parent = self + else + @children[at_index] = nil + end child end diff --git a/lib/tree/version.rb b/lib/tree/version.rb index 167e679..dd7e64e 100644 --- a/lib/tree/version.rb +++ b/lib/tree/version.rb @@ -35,5 +35,5 @@ module Tree # Rubytree Package Version - VERSION = '2.1.1' + VERSION = '2.2.0' end diff --git a/rubytree.gemspec b/rubytree.gemspec index af7e3c4..97958b3 100644 --- a/rubytree.gemspec +++ b/rubytree.gemspec @@ -44,7 +44,8 @@ Gem::Specification.new do |s| END_DESC s.metadata = { - 'rubygems_mfa_required' => 'true' + 'rubygems_mfa_required' => 'true', + 'github_repo' => 'ssh://github.com/evolve75/rubytree' } s.files = Dir['lib/**/*.rb'] # The actual code diff --git a/test/test_binarytree.rb b/test/test_binarytree.rb index f89755d..7d4b820 100755 --- a/test/test_binarytree.rb +++ b/test/test_binarytree.rb @@ -229,6 +229,9 @@ def test_left_child_equals assert_nil(@root.left_child, 'The left child should now be nil') assert_nil(@root.first_child, 'The first child is now nil') assert_equal('B Child at Right', @root.last_child.name, 'The last child should now be the right child') + assert(@left_child1.root?, 'The old left child should now be a root') + assert_nil(@left_child1.parent, 'The old left child should not have a parent') + assert_nil(@root['A Child at Left'], 'Lookup by old left name should be nil') end # Test right_child= method. @@ -249,6 +252,15 @@ def test_right_child_equals assert_nil(@root.right_child, 'The right child should now be nil') assert_equal('A Child at Left', @root.first_child.name, 'The first child should now be the left child') assert_nil(@root.last_child, 'The first child is now nil') + assert(@right_child1.root?, 'The old right child should now be a root') + assert_nil(@right_child1.parent, 'The old right child should not have a parent') + assert_nil(@root['B Child at Right'], 'Lookup by old right name should be nil') + end + + # Test invalid index error for set_child_at. + def test_set_child_at_invalid_index + error = assert_raise(ArgumentError) { @root.send(:set_child_at, @left_child1, 2) } + assert_match(/cannot have more than two children/i, error.message) end # Test isLeft_child? method. @@ -297,5 +309,27 @@ def test_swap_children assert_equal(@right_child1, @root[0], 'right_child1 should now be the first child') assert_equal(@left_child1, @root[1], 'left_child1 should now be the last child') end + + # Test traversals when nil children exist. + def test_traversal_with_nil_children + @root << @left_child1 + @root << @right_child1 + + @root.right_child = nil + + breadth_nodes = [] + post_nodes = [] + + assert_nothing_raised do + @root.breadth_each { |node| breadth_nodes << node } + end + + assert_nothing_raised do + @root.postordered_each { |node| post_nodes << node } + end + + assert_equal([@root, @left_child1], breadth_nodes) + assert_equal([@left_child1, @root], post_nodes) + end end end diff --git a/test/test_tree.rb b/test/test_tree.rb index c56da69..d2fed98 100755 --- a/test/test_tree.rb +++ b/test/test_tree.rb @@ -94,6 +94,7 @@ def test_root_setup assert_not_nil(@root.name, 'Name should not be nil') assert_equal('ROOT', @root.name, "Name should be 'ROOT'") assert_equal('Root Node', @root.content, "Content should be 'Root Node'") + assert(@root.to_s.include?('Content: Root Node'), 'to_s should include content value') assert(@root.root?, 'Should identify as root') assert(!@root.children?, 'Cannot have any children') assert(@root.content?, 'This root should have content') @@ -117,6 +118,11 @@ def test_root assert_equal(2, @root.node_height, "Root's height after adding the children should be 2") end + def test_to_s_empty_content + empty = Tree::TreeNode.new('EMPTY') + assert_match(/Content: /, empty.to_s) + end + def test_from_hash # A # / | \ @@ -491,6 +497,10 @@ def test_add # Test the addition of a nil node. assert_raise(ArgumentError) { @root.add(nil) } + + # Test adding an ancestor as a child (cycle prevention). + error = assert_raise(ArgumentError) { @child4.add(@child3) } + assert_match(/Attempting add ancestor as a child/, error.message) end # Test the addition of a duplicate node (duplicate being defined as a node with the same name). @@ -689,6 +699,16 @@ def test_remove_all_bang assert(!@root.children?, 'Should have no children') assert_equal(1, @root.size, 'Should have one node') + + # Removed children should be detached (root? == true). + assert(@child1.root?, 'Child1 should be a root after remove_all!') + assert(@child2.root?, 'Child2 should be a root after remove_all!') + assert(@child3.root?, 'Child3 should be a root after remove_all!') + assert(@child4.root?, 'Child4 should be a root after remove_all!') + assert_nil(@child1.parent, 'Child1 parent should be nil after remove_all!') + assert_nil(@child2.parent, 'Child2 parent should be nil after remove_all!') + assert_nil(@child3.parent, 'Child3 parent should be nil after remove_all!') + assert_nil(@child4.parent, 'Child4 parent should be nil after remove_all!') end # Test the remove_from_parent! method. @@ -832,6 +852,18 @@ def test_each_leaf assert(result_array.include?(@child4), 'Should have child 4') end + # Test the each_level method without a block (Enumerator). + def test_each_level + setup_test_tree + + levels = @root.each_level.to_a + + assert_equal(3, levels.length, 'Should have three levels') + assert_equal([@root], levels[0]) + assert_equal([@child1, @child2, @child3], levels[1]) + assert_equal([@child4], levels[2]) + end + # Test the parent method. def test_parent setup_test_tree @@ -1531,6 +1563,9 @@ def test_rename_child assert_raise(ArgumentError) { @root.rename_child('Not_Present_Child1', 'ALT_Child1') } + error = assert_raise(ArgumentError) { @root.rename_child('Child1', 'Child2') } + assert_match(/Child name already exists: Child2/, error.message) + @root.rename_child('Child1', 'ALT_Child1') assert_equal('ALT_Child1', @child1.name, "Name should be 'ALT_Child1'") assert_equal(@child1, @root['ALT_Child1'], 'Should be able to access from parent using new name')