Merge branch 'ai2809' into 'master'

Use materialized path for parents lookup http://noosfero.org/Development/ActionItem2809 See merge request !7

Merge branch 'ai2809' into 'master'
Use materialized path for parents lookup http://noosfero.org/Development/ActionItem2809 See merge request !7
Braulio Bhavamitra
2 parents 980d4cac 9f0569e6
Showing 5 changed files with 230 additions and 95 deletions Show diff stats
db/migrate/20120820120000_index_parent_id_from_all_tables.rb
db/migrate/20120820142056_add_ancestry_to_categories.rb
db/schema.rb
lib/acts_as_filesystem.rb
test/unit/acts_as_filesystem_test.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
@@ -0,0 +1,11 @@
+class AddAncestryToCategories < ActiveRecord::Migration
+  def self.up
+    add_column :categories, :ancestry, :text
+
+    Category.build_ancestry
+  end
+
+  def self.down
+    remove_column :categories, :ancestry
+  end
+end
@@ -104,6 +104,7 @@ ActiveRecord::Schema.define(:version =&gt; 20150122165042) do
   add_index "article_versions", ["path", "profile_id"], :name => "index_article_versions_on_path_and_profile_id"
   add_index "article_versions", ["path"], :name => "index_article_versions_on_path"
   add_index "article_versions", ["published_at", "id"], :name => "index_article_versions_on_published_at_and_id"
+  add_index "article_versions", ["parent_id"], :name => "index_article_versions_on_parent_id"
   create_table "articles", :force => true do |t|
     t.string   "name"
@@ -217,8 +218,11 @@ ActiveRecord::Schema.define(:version =&gt; 20150122165042) do
     t.string  "acronym"
     t.string  "abbreviation"
     t.string  "display_color",   :limit => 6
+    t.text    "ancestry"
   end
+  add_index "categories", ["parent_id"], :name => "index_categories_on_parent_id"
+
   create_table "categories_profiles", :id => false, :force => true do |t|
     t.integer "profile_id"
     t.integer "category_id"
@@ -251,6 +255,7 @@ ActiveRecord::Schema.define(:version =&gt; 20150122165042) do
     t.string   "source_type"
     t.string   "user_agent"
     t.string   "referrer"
+    t.integer  "group_id"
   end
   add_index "comments", ["source_id", "spam"], :name => "index_comments_on_source_id_and_spam"
@@ -357,6 +362,8 @@ ActiveRecord::Schema.define(:version =&gt; 20150122165042) do
     t.boolean "thumbnails_processed", :default => false
   end
+  add_index "images", ["parent_id"], :name => "index_images_on_parent_id"
+
   create_table "inputs", :force => true do |t|
     t.integer  "product_id",                                    :null => false
     t.integer  "product_category_id",                           :null => false
@@ -649,7 +656,11 @@ ActiveRecord::Schema.define(:version =&gt; 20150122165042) do
     t.boolean "pending",   :default => false
   end
+<<<<<<< HEAD
   add_index "tags", ["name"], :name => "index_tags_on_name", :unique => true
+=======
+  add_index "tags", ["parent_id"], :name => "index_tags_on_parent_id"
+>>>>>>> Update schema.rb
   create_table "tasks", :force => true do |t|
     t.text     "data"
@@ -689,6 +700,8 @@ ActiveRecord::Schema.define(:version =&gt; 20150122165042) do
     t.string  "thumbnail"
   end
+  add_index "thumbnails", ["parent_id"], :name => "index_thumbnails_on_parent_id"
+
   create_table "units", :force => true do |t|
     t.string  "singular",       :null => false
     t.string  "plural",         :null => false
 module ActsAsFileSystem
-  module ClassMethods
+  module ActsMethods
     # Declares the ActiveRecord model to acts like a filesystem: objects are
     # arranged in a tree (liks acts_as_tree), and . The underlying table must
@@ -14,66 +14,81 @@ 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
+      acts_as_tree :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
+      extend ClassMethods
+      include InstanceMethods
+      if self.has_path?
+        after_update :update_children_path
+        before_create :set_path
+        include InstanceMethods::PathMethods
       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
+      before_save :set_ancestry
+    end
+
+  end
+
+  module ClassMethods
+
+    def build_ancestry(parent_id = nil, ancestry = '')
+      ActiveRecord::Base.transaction do
+        self.base_class.all(:conditions => {:parent_id => parent_id}).each do |node|
+          node.ancestry = ancestry
+          node.send :create_or_update_without_callbacks
+
+          build_ancestry node.id, (ancestry.empty? ? "#{node.formatted_ancestry_id}" :
+                                   "#{ancestry}#{node.ancestry_sep}#{node.formatted_ancestry_id}")
         end
-        record.recalculate_path = false
-        true
       end
+      #raise "Couldn't reach and set ancestry on every record" if self.base_class.count(:conditions => ['ancestry is null']) != 0
+    end
+
+    def has_path?
+      (['name', 'slug', 'path'] - self.column_names).blank?
     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)
+    def ancestry_column
+      'ancestry'
+    end
+    def ancestry_sep
+      '.'
+    end
+    def has_ancestry?
+      self.class.column_names.include? self.ancestry_column
+    end
+
+    def formatted_ancestry_id
+      "%010d" % self.id if self.id
     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 ancestry
+      self[ancestry_column]
+    end
+    def ancestor_ids
+      return nil if !has_ancestry? or ancestry.nil?
+      @ancestor_ids ||= ancestry.split(ancestry_sep).map{ |id| id.to_i }
+    end
+
+    def ancestry=(value)
+      self[ancestry_column] = value
+    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(true)[0...-1].map{ |p| p.formatted_ancestry_id }.join(ancestry_sep)
+      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.
+    # 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,61 +97,36 @@ 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
+    def top_ancestor
+      if has_ancestry? and !ancestry.nil?
+        self.class.base_class.find_by_id self.top_ancestor_id
+      else
+        self.hierarchy.first
       end
     end
-
-    # calculates the full path to this category using parent's path.
-    def calculate_path
-      if self.top_level?
-        self.slug
+    def top_ancestor_id
+      if has_ancestry? and !ancestry.nil?
+        self.ancestor_ids.first
       else
-        self.parent.calculate_path + "/" + self.slug
+        self.hierarchy.first.id
       end
     end
-    def top_ancestor
-      self.top_level? ? self : self.parent.top_ancestor
-    end
-
-    def explode_path
-      path.split(/\//)
-    end
-
     # returns the full hierarchy from the top-level item to this one. For
     # example, if item1 has a children item2 and item2 has a children item3,
     # then item3's hierarchy would be [item1, item2, item3].
@@ -145,16 +135,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 !reload and !recalculate_path and ancestor_ids
+          objects = self.class.base_class.all(:conditions => {:id => ancestor_ids})
+          ancestor_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,8 +176,86 @@ module ActsAsFileSystem
       res
     end
+    #####
+    # Path methods
+    # These methods are used when _path_, _name_ and _slug_ attributes exist
+    # and should be calculated based on the tree
+    #####
+    module PathMethods
+      # used to know when to trigger batch renaming
+      attr_accessor :recalculate_path
+
+      # 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
+
+      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 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
 end
-ActiveRecord::Base.extend ActsAsFileSystem::ClassMethods
+ActiveRecord::Base.extend ActsAsFileSystem::ActsMethods
@@ -7,13 +7,34 @@ class ActsAsFilesystemTest &lt; 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
@@ -0,0 +1,17 @@		@@ -0,0 +1,17 @@
	1	+class IndexParentIdFromAllTables < ActiveRecord::Migration
	2	+ def self.up
	3	+ add_index :article_versions, :parent_id
	4	+ add_index :categories, :parent_id
	5	+ add_index :images, :parent_id
	6	+ add_index :tags, :parent_id
	7	+ add_index :thumbnails, :parent_id
	8	+ end
	9	+
	10	+ def self.down
	11	+ remove_index :article_versions, :parent_id
	12	+ remove_index :categories, :parent_id
	13	+ remove_index :images, :parent_id
	14	+ remove_index :tags, :parent_id
	15	+ remove_index :thumbnails, :parent_id
	16	+ end
	17	+end
@@ -0,0 +1,11 @@		@@ -0,0 +1,11 @@
	1	+class AddAncestryToCategories < ActiveRecord::Migration
	2	+ def self.up
	3	+ add_column :categories, :ancestry, :text
	4	+
	5	+ Category.build_ancestry
	6	+ end
	7	+
	8	+ def self.down
	9	+ remove_column :categories, :ancestry
	10	+ end
	11	+end
	@@ -7,13 +7,34 @@ class ActsAsFilesystemTest < ActiveSupport::TestCase		@@ -7,13 +7,34 @@ class ActsAsFilesystemTest < ActiveSupport::TestCase
7	should 'provide a hierarchy list' do	7	should 'provide a hierarchy list' do
8	profile = create_user('testinguser').person	8	profile = create_user('testinguser').person
9		9
10	- a1 = profile.articles.build(:name => 'a1'); a1.save!
11	- a2 = profile.articles.build(:name => 'a2'); a2.parent = a1; a2.save!
12	- a3 = profile.articles.build(:name => 'a3'); a3.parent = a2; a3.save!	10	+ a1 = profile.articles.create(:name => 'a1')
		11	+ a2 = profile.articles.create(:name => 'a2', :parent => a1)
		12	+ a3 = profile.articles.create(:name => 'a3', :parent => a2)
13		13
14	assert_equal [a1, a2, a3], a3.hierarchy	14	assert_equal [a1, a2, a3], a3.hierarchy
15	end	15	end
16		16
		17	+ should 'set ancestry' do
		18	+ c1 = create(Category, :name => 'c1')
		19	+ c2 = create(Category, :name => 'c2', :parent => c1)
		20	+ c3 = create(Category, :name => 'c3', :parent => c2)
		21	+
		22	+ assert_not_nil c1.ancestry
		23	+ assert_not_nil c2.ancestry
		24	+ assert_equal "%010d.%010d" % [c1.id, c2.id], c3.ancestry
		25	+ assert_equal [c1, c2, c3], c3.hierarchy
		26	+ end
		27	+
		28	+ should 'provide the level' do
		29	+ c1 = create(Category, :name => 'c1')
		30	+ c2 = create(Category, :name => 'c2', :parent => c1)
		31	+ c3 = create(Category, :name => 'c3', :parent => c2)
		32	+
		33	+ assert_equal 0, c1.level
		34	+ assert_equal 1, c2.level
		35	+ assert_equal 2, c3.level
		36	+ end
		37	+
17	should 'be able to optionally reload the hierarchy' do	38	should 'be able to optionally reload the hierarchy' do
18	a = Article.new	39	a = Article.new
19	list = a.hierarchy	40	list = a.hierarchy