From 95ebdaf6cdf408c42692148482de6d8257c3b419 Mon Sep 17 00:00:00 2001
From: Braulio Bhavamitra <brauliobo@gmail.com>
Date: Tue, 21 Aug 2012 13:29:00 +0000
Subject: [PATCH] Optimize category tree lookup with a materialized path

---
 db/migrate/20120820120000_index_parent_id_from_all_tables.rb |  17 +++++++++++++++++
 db/migrate/20120820142056_add_ancestry_to_categories.rb      |  14 ++++++++++++++
 db/schema.rb                                                 |   1 +
 lib/acts_as_filesystem.rb                                    | 209 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------
 test/unit/acts_as_filesystem_test.rb                         |  27 ++++++++++++++++++++++++---
 5 files changed, 172 insertions(+), 96 deletions(-)
 create mode 100644 db/migrate/20120820120000_index_parent_id_from_all_tables.rb
 create mode 100644 db/migrate/20120820142056_add_ancestry_to_categories.rb

diff --git a/db/migrate/20120820120000_index_parent_id_from_all_tables.rb b/db/migrate/20120820120000_index_parent_id_from_all_tables.rb
new file mode 100644
index 0000000..4010a48
--- /dev/null
+++ b/db/migrate/20120820120000_index_parent_id_from_all_tables.rb
@@ -0,0 +1,17 @@
+class IndexParentIdFromAllTables < ActiveRecord::Migration
+  def self.up
+    add_index :article_versions, :parent_id
+    add_index :categories, :parent_id
+    add_index :images, :parent_id
+    add_index :tags, :parent_id
+    add_index :thumbnails, :parent_id
+  end
+
+  def self.down
+    remove_index :article_versions, :parent_id
+    remove_index :categories, :parent_id
+    remove_index :images, :parent_id
+    remove_index :tags, :parent_id
+    remove_index :thumbnails, :parent_id
+  end
+end
diff --git a/db/migrate/20120820142056_add_ancestry_to_categories.rb b/db/migrate/20120820142056_add_ancestry_to_categories.rb
new file mode 100644
index 0000000..987a1cb
--- /dev/null
+++ b/db/migrate/20120820142056_add_ancestry_to_categories.rb
@@ -0,0 +1,14 @@
+class AddAncestryToCategories < ActiveRecord::Migration
+  def self.up
+    add_column :categories, :ancestry, :text
+
+    Category.all.each do |category|
+      category.set_ancestry
+      category.save!
+    end
+  end
+
+  def self.down
+    remove_column :categories, :ancestry
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 3a51dd8..0e33b34 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -217,6 +217,7 @@ ActiveRecord::Schema.define(:version => 20150122165042) do
     t.string  "acronym"
     t.string  "abbreviation"
     t.string  "display_color",   :limit => 6
+    t.text    "ancestry"
   end
 
   create_table "categories_profiles", :id => false, :force => true do |t|
diff --git a/lib/acts_as_filesystem.rb b/lib/acts_as_filesystem.rb
index b9aea69..a810b52 100644
--- a/lib/acts_as_filesystem.rb
+++ b/lib/acts_as_filesystem.rb
@@ -14,66 +14,72 @@ module ActsAsFileSystem
     #   the parent, a "/" and the slug of the object)
     # * children_count - a cache of the number of children elements.
     def acts_as_filesystem
-
       include ActsAsFileSystem::InstanceMethods
 
       # a filesystem is a tree
-      acts_as_tree :order => 'name', :counter_cache => :children_count
-
-      # calculate the right path
-      before_create do |record|
-        if record.path == record.slug && (! record.top_level?)
-          record.path = record.calculate_path
-        end
-        true
-      end
-
-      # when renaming a category, all children categories must have their paths
-      # recalculated
-      after_update do |record|
-        if record.recalculate_path
-          record.children.each do |item|
-            item.path = item.calculate_path
-            item.recalculate_path = true
-            item.save!
-          end
-        end
-        record.recalculate_path = false
-        true
-      end
+      acts_as_tree :counter_cache => :children_count
 
+      before_create :set_path
+      before_save :set_ancestry
+      after_update :update_children_path
     end
+
   end
 
   module InstanceMethods
+
     # used to know when to trigger batch renaming
     attr_accessor :recalculate_path
 
-    # calculates the full name of a category by accessing the name of all its
-    # ancestors.
-    #
-    # If you have this category hierarchy:
-    #   Category "A"
-    #     Category "B"
-    #       Category "C"
-    #
-    # Then Category "C" will have "A/B/C" as its full name.
-    def full_name(sep = '/')
-      self.hierarchy.map {|item| item.name || '?' }.join(sep)
+    # calculates the full path to this record using parent's path.
+    def calculate_path
+      self.hierarchy.map{ |obj| obj.slug }.join('/')
+    end
+    def set_path
+      if self.path == self.slug && !self.top_level?
+        self.path = self.calculate_path
+      end
+    end
+    def explode_path
+      path.split(/\//)
     end
 
-    # gets the name without leading parents. Usefull when dividing categories
-    # in top-level groups and full names must not include the top-level
-    # category which is already a emphasized label
-    def full_name_without_leading(count, sep = '/')
-      parts = self.full_name(sep).split(sep)
-      count.times { parts.shift }
-      parts.join(sep)
+    def has_ancestry?
+      self.class.column_names.include? 'ancestry'
+    end
+    def ancestry
+      self['ancestry']
+    end
+    def ancestry=(value)
+      self['ancestry'] = value
+    end
+    # get the serialized tree from database column 'ancetry'
+    # and convert it to an array
+    def ancestry_ids
+      return nil if !has_ancestry? or ancestry.nil?
+      @ancestry_ids ||= ancestry.split('.').map{ |id| id.to_i }
+    end
+    def set_ancestry
+      return unless self.has_ancestry?
+      if self.ancestry.nil? or (new_record? or parent_id_changed?) or recalculate_path
+        self.ancestry = self.hierarchy[0...-1].map{ |p| "%010d" % p.id }.join('.')
+      end
     end
 
-    # calculates the level of the category in the category hierarchy. Top-level
-    # categories have level 0; the children of the top-level categories have
-    # level 1; the children of categories with level 1 have level 2, and so on.
+    def update_children_path
+      if self.recalculate_path
+        self.children.each do |child|
+          child.path = child.calculate_path
+          child.recalculate_path = true
+          child.save!
+        end
+      end
+      self.recalculate_path = false
+    end
+
+    # calculates the level of the record in the records hierarchy. Top-level
+    # records have level 0; the children of the top-level records have
+    # level 1; the children of records with level 1 have level 2, and so on.
     #
     #      A    level 0
     #     / \
@@ -82,59 +88,26 @@ module ActsAsFileSystem
     #   E F G H level 2
     #     ...
     def level
-      self.parent ? (self.parent.level + 1) : 0
+      self.hierarchy.size - 1
     end
 
-    # Is this category a top-level category?
+    # Is this record a top-level record?
     def top_level?
       self.parent.nil?
     end
 
-    # Is this category a leaf in the hierarchy tree of categories?
+    # Is this record a leaf in the hierarchy tree of records?
     #
-    # Being a leaf means that this category has no subcategories.
+    # Being a leaf means that this record has no subrecord.
     def leaf?
       self.children.empty?
     end
 
-    def set_name(value)
-      if self.name != value
-        self.recalculate_path = true
-      end
-      self[:name] = value
-    end
-
-    # sets the name of the category. Also sets #slug accordingly.
-    def name=(value)
-      self.set_name(value)
-      unless self.name.blank?
-        self.slug = self.name.to_slug
-      end
-    end
-
-    # sets the slug of the category. Also sets the path with the new slug value.
-    def slug=(value)
-      self[:slug] = value
-      unless self.slug.blank?
-        self.path = self.calculate_path
-      end
-    end
-
-    # calculates the full path to this category using parent's path.
-    def calculate_path
-      if self.top_level?
-        self.slug
-      else
-        self.parent.calculate_path + "/" + self.slug
-      end
-    end
-
     def top_ancestor
-      self.top_level? ? self : self.parent.top_ancestor
+      self.hierarchy.first
     end
-
-    def explode_path
-      path.split(/\//)
+    def top_ancestor_id
+      self.ancestry_ids.first
     end
 
     # returns the full hierarchy from the top-level item to this one. For
@@ -145,16 +118,21 @@ module ActsAsFileSystem
     # when the ActiveRecord object was modified in some way, or just after
     # changing parent)
     def hierarchy(reload = false)
-      if reload
-        @hierarchy = nil
-      end
+      @hierarchy = nil if reload or recalculate_path
 
-      unless @hierarchy
+      if @hierarchy.nil?
         @hierarchy = []
-        item = self
-        while item
-          @hierarchy.unshift(item)
-          item = item.parent
+
+        if ancestry_ids
+          objects = self.class.base_class.all(:conditions => {:id => ancestry_ids})
+          ancestry_ids.each{ |id| @hierarchy << objects.find{ |t| t.id == id } }
+          @hierarchy << self
+        else
+          item = self
+          while item
+            @hierarchy.unshift(item)
+            item = item.parent
+          end
         end
       end
 
@@ -181,6 +159,51 @@ module ActsAsFileSystem
       res
     end
 
+    # calculates the full name of a record by accessing the name of all its
+    # ancestors.
+    #
+    # If you have this record hierarchy:
+    #   Record "A"
+    #     Record "B"
+    #       Record "C"
+    #
+    # Then Record "C" will have "A/B/C" as its full name.
+    def full_name(sep = '/')
+      self.hierarchy.map {|item| item.name || '?' }.join(sep)
+    end
+
+    # gets the name without leading parents. Useful when dividing records
+    # in top-level groups and full names must not include the top-level
+    # record which is already a emphasized label
+    def full_name_without_leading(count, sep = '/')
+      parts = self.full_name(sep).split(sep)
+      count.times { parts.shift }
+      parts.join(sep)
+    end
+
+    def set_name(value)
+      if self.name != value
+        self.recalculate_path = true
+      end
+      self[:name] = value
+    end
+
+    # sets the name of the record. Also sets #slug accordingly.
+    def name=(value)
+      self.set_name(value)
+      unless self.name.blank?
+        self.slug = self.name.to_slug
+      end
+    end
+
+    # sets the slug of the record. Also sets the path with the new slug value.
+    def slug=(value)
+      self[:slug] = value
+      unless self.slug.blank?
+        self.path = self.calculate_path
+      end
+    end
+
   end
 end
 
diff --git a/test/unit/acts_as_filesystem_test.rb b/test/unit/acts_as_filesystem_test.rb
index 874d2c5..d0dd490 100644
--- a/test/unit/acts_as_filesystem_test.rb
+++ b/test/unit/acts_as_filesystem_test.rb
@@ -7,13 +7,34 @@ class ActsAsFilesystemTest < ActiveSupport::TestCase
   should 'provide a hierarchy list' do
     profile = create_user('testinguser').person
 
-    a1 = profile.articles.build(:name => 'a1'); a1.save!
-    a2 = profile.articles.build(:name => 'a2'); a2.parent = a1; a2.save!
-    a3 = profile.articles.build(:name => 'a3'); a3.parent = a2; a3.save!
+    a1 = profile.articles.create(:name => 'a1')
+    a2 = profile.articles.create(:name => 'a2', :parent => a1)
+    a3 = profile.articles.create(:name => 'a3', :parent => a2)
 
     assert_equal [a1, a2, a3], a3.hierarchy
   end
 
+  should 'set ancestry' do
+    c1 = create(Category, :name => 'c1')
+    c2 = create(Category, :name => 'c2', :parent => c1)
+    c3 = create(Category, :name => 'c3', :parent => c2)
+
+    assert_not_nil c1.ancestry
+    assert_not_nil c2.ancestry
+    assert_equal "%010d.%010d" % [c1.id, c2.id], c3.ancestry
+    assert_equal [c1, c2, c3], c3.hierarchy
+  end
+
+  should 'provide the level' do
+    c1 = create(Category, :name => 'c1')
+    c2 = create(Category, :name => 'c2', :parent => c1)
+    c3 = create(Category, :name => 'c3', :parent => c2)
+
+    assert_equal 0, c1.level
+    assert_equal 1, c2.level
+    assert_equal 2, c3.level
+  end
+
   should 'be able to optionally reload the hierarchy' do
     a = Article.new
     list = a.hierarchy
--
libgit2 0.21.2