Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

sorting nodes and performance #29

Closed
wants to merge 4 commits into from

2 participants

@kueda

I added a method to just sort a simple array of nodes by ancestry. Simply sorting by the ancestry column performs a level-order sort, and I wanted something more like a pre-order sort (even though that's not entirely possible given the unordered ranks in the materialized path model. If there's a simpler way to do this, I'd love to know about it.

I also replaced some all.each calls with find_each for improved performance when dealing with very large trees.

@kueda

Uh, I may have screwed up a merge too, since there seems to be a bunch of other commits lumped into this... I thought I just merged in commits from stefankroes/ancestry in the fork queue. Was that not the right way to go about it? github still mystifies me a bit.

@stefankroes
Owner

I want to include your changes but I think the pull request is kind of screwed up, I will have to check later.

@kueda

Should I do it over again? Or maybe drop my repo, re-fork, and re-apply my changes?

@stefankroes
Owner

Could you?

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  README.rdoc
@@ -300,6 +300,6 @@ I will try to keep Ancestry up to date with changing versions of Rails and Ruby
Bug report? Faulty/incomplete documentation? Feature request? Please post an issue on 'http://github.com/stefankroes/ancestry/issues'. Please also contact me at s.a.kroes[at]gmail.com if it's urgent.
-Question? Contact me at s.a.kroes[at]gmail.com, make sure you read the documentation. You can also join the #ancestry channel on IRC (irc.freenode.net).
+Question? Contact me at s.a.kroes[at]gmail.com, make sure you read the documentation.
Copyright (c) 2009 Stefan Kroes, released under the MIT license
View
40 lib/ancestry/class_methods.rb
@@ -36,14 +36,36 @@ def arrange options = {}
self.base_class.ordered_by_ancestry_and options.delete(:order)
end
# Get all nodes ordered by ancestry and start sorting them into an empty hash
- scope.all(options).inject(ActiveSupport::OrderedHash.new) do |arranged_nodes, node|
+ arrange_nodes scope.all(options)
+ end
+
+ # Arrange array of nodes into a nested hash of the form
+ # {node => children}, where children = {} if the node has no children
+ def arrange_nodes(nodes)
+ # Get all nodes ordered by ancestry and start sorting them into an empty hash
+ nodes.inject(ActiveSupport::OrderedHash.new) do |arranged_nodes, node|
# Find the insertion point for that node by going through its ancestors
node.ancestor_ids.inject(arranged_nodes) do |insertion_point, ancestor_id|
insertion_point.each do |parent, children|
# Change the insertion point to children if node is a descendant of this parent
insertion_point = children if ancestor_id == parent.id
- end; insertion_point
- end[node] = ActiveSupport::OrderedHash.new; arranged_nodes
+ end
+ insertion_point
+ end[node] = ActiveSupport::OrderedHash.new
+ arranged_nodes
+ end
+ end
+
+ # Pseudo-preordered array of nodes. Children will always follow parents,
+ # but the ordering of nodes within a rank depends on their order in the
+ # array that gets passed in
+ def sort_by_ancestry(nodes)
+ arranged = nodes.is_a?(Hash) ? nodes : arrange_nodes(nodes.sort_by{|n| n.ancestry || '0'})
+ arranged.inject([]) do |sorted_nodes, pair|
+ node, children = pair
+ sorted_nodes << node
+ sorted_nodes += sort_by_ancestry(children) unless children.blank?
+ sorted_nodes
end
end
@@ -52,7 +74,7 @@ def check_ancestry_integrity! options = {}
parents = {}
exceptions = [] if options[:report] == :list
# For each node ...
- self.base_class.all.each do |node|
+ self.base_class.find_each do |node|
begin
# ... check validity of ancestry column
if !node.valid? and !node.errors[node.class.ancestry_column].blank?
@@ -86,7 +108,7 @@ def check_ancestry_integrity! options = {}
def restore_ancestry_integrity!
parents = {}
# For each node ...
- self.base_class.all.each do |node|
+ self.base_class.find_each do |node|
# ... set its ancestry to nil if invalid
if node.errors[node.class.ancestry_column].blank?
node.without_ancestry_callbacks do
@@ -104,7 +126,7 @@ def restore_ancestry_integrity!
parents[node.id] = nil if parent == node.id
end
# For each node ...
- self.base_class.all.each do |node|
+ self.base_class.find_each do |node|
# ... rebuild ancestry from parents array
ancestry, parent = nil, parents[node.id]
until parent.nil?
@@ -118,7 +140,7 @@ def restore_ancestry_integrity!
# Build ancestry from parent id's for migration purposes
def build_ancestry_from_parent_ids! parent_id = nil, ancestry = nil
- self.base_class.all(:conditions => {:parent_id => parent_id}).each do |node|
+ self.base_class.find_each(:conditions => {:parent_id => parent_id}) do |node|
node.without_ancestry_callbacks do
node.update_attribute ancestry_column, ancestry
end
@@ -129,9 +151,9 @@ def build_ancestry_from_parent_ids! parent_id = nil, ancestry = nil
# Rebuild depth cache if it got corrupted or if depth caching was just turned on
def rebuild_depth_cache!
raise Ancestry::AncestryException.new("Cannot rebuild depth cache for model without depth caching.") unless respond_to? :depth_cache_column
- self.base_class.all.each do |node|
+ self.base_class.find_each do |node|
node.update_attribute depth_cache_column, node.depth
end
end
end
-end
+end
View
13 test/has_ancestry_test.rb
@@ -688,4 +688,17 @@ def test_arrange_order_option
end
end
end
+
+ def test_sort_by_ancestry
+ AncestryTestDatabase.with_model do |model|
+ n1 = model.create!
+ n2 = model.create!(:parent => n1)
+ n3 = model.create!(:parent => n2)
+ n4 = model.create!(:parent => n2)
+ n5 = model.create!(:parent => n1)
+
+ arranged = model.sort_by_ancestry(model.all.sort_by(&:id).reverse)
+ assert_equal [n1, n2, n4, n3, n5].map(&:id), arranged.map(&:id)
+ end
+ end
end
Something went wrong with that request. Please try again.