From: Jean-Philippe Lang Date: Wed, 7 Jan 2015 20:19:49 +0000 (+0000) Subject: Replaces awesome_nested_set gem with a simple and more robust implementation of neste... X-Git-Tag: 3.0.0~171 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=1a851318fdce55a7ffb2290de692282e294987f8;p=redmine.git Replaces awesome_nested_set gem with a simple and more robust implementation of nested sets. The concurrency tests added in this commit trigger dead locks and/or nested set inconsistency with awesome_nested_set. git-svn-id: http://svn.redmine.org/redmine/trunk@13841 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/Gemfile b/Gemfile index a18f95546..de1a1fb99 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,6 @@ gem "coderay", "~> 1.1.0" gem "builder", ">= 3.0.4" gem "request_store", "1.0.5" gem "mime-types" -gem "awesome_nested_set", "3.0.0" gem "protected_attributes" gem "actionpack-action_caching" gem "actionpack-xml_parser" diff --git a/app/models/issue.rb b/app/models/issue.rb index 582973343..75e59ea32 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -19,6 +19,8 @@ class Issue < ActiveRecord::Base include Redmine::SafeAttributes include Redmine::Utils::DateCalculation include Redmine::I18n + before_save :set_parent_id + include Redmine::NestedSet::IssueNestedSet belongs_to :project belongs_to :tracker @@ -41,7 +43,6 @@ class Issue < ActiveRecord::Base has_many :relations_from, :class_name => 'IssueRelation', :foreign_key => 'issue_from_id', :dependent => :delete_all has_many :relations_to, :class_name => 'IssueRelation', :foreign_key => 'issue_to_id', :dependent => :delete_all - acts_as_nested_set :scope => 'root_id', :dependent => :destroy acts_as_attachable :after_add => :attachment_added, :after_remove => :attachment_removed acts_as_customizable acts_as_watchable @@ -185,7 +186,7 @@ class Issue < ActiveRecord::Base # the lock_version condition should not be an issue but we handle it. def destroy super - rescue ActiveRecord::RecordNotFound + rescue ActiveRecord::StaleObjectError, ActiveRecord::RecordNotFound # Stale or already deleted begin reload @@ -615,10 +616,8 @@ class Issue < ActiveRecord::Base errors.add :parent_issue_id, :invalid elsif !new_record? # moving an existing issue - if @parent_issue.root_id != root_id - # we can always move to another tree - elsif move_possible?(@parent_issue) - # move accepted inside tree + if move_possible?(@parent_issue) + # move accepted else errors.add :parent_issue_id, :invalid end @@ -1184,6 +1183,10 @@ class Issue < ActiveRecord::Base end end + def set_parent_id + self.parent_id = parent_issue_id + end + # Returns true if issue's project is a valid # parent issue project def valid_parent_project?(issue=parent) @@ -1366,14 +1369,7 @@ class Issue < ActiveRecord::Base end def update_nested_set_attributes - if root_id.nil? - # issue was just created - self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id) - Issue.where(["id = ?", id]).update_all(["root_id = ?", root_id]) - if @parent_issue - move_to_child_of(@parent_issue) - end - elsif parent_issue_id != parent_id + if parent_id_changed? update_nested_set_attributes_on_parent_change end remove_instance_variable(:@parent_issue) if instance_variable_defined?(:@parent_issue) @@ -1381,31 +1377,7 @@ class Issue < ActiveRecord::Base # Updates the nested set for when an existing issue is moved def update_nested_set_attributes_on_parent_change - former_parent_id = parent_id - # moving an existing issue - if @parent_issue && @parent_issue.root_id == root_id - # inside the same tree - move_to_child_of(@parent_issue) - else - # to another tree - unless root? - move_to_right_of(root) - end - old_root_id = root_id - in_tenacious_transaction do - @parent_issue.reload_nested_set if @parent_issue - self.reload_nested_set - self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id) - cond = ["root_id = ? AND lft >= ? AND rgt <= ? ", old_root_id, lft, rgt] - self.class.base_class.select('id').lock(true).where(cond) - offset = rdm_right_most_bound + 1 - lft - Issue.where(cond). - update_all(["root_id = ?, lft = lft + ?, rgt = rgt + ?", root_id, offset, offset]) - end - if @parent_issue - move_to_child_of(@parent_issue) - end - end + former_parent_id = parent_id_was # delete invalid relations of all descendants self_and_descendants.each do |issue| issue.relations.each do |relation| @@ -1416,16 +1388,11 @@ class Issue < ActiveRecord::Base recalculate_attributes_for(former_parent_id) if former_parent_id end - def rdm_right_most_bound - right_most_node = - self.class.base_class.unscoped. - order("#{quoted_right_column_full_name} desc").limit(1).lock(true).first - right_most_node ? (right_most_node[right_column_name] || 0 ) : 0 - end - private :rdm_right_most_bound - def update_parent_attributes - recalculate_attributes_for(parent_id) if parent_id + if parent_id + recalculate_attributes_for(parent_id) + association(:parent).reset + end end def recalculate_attributes_for(issue_id) diff --git a/app/models/project.rb b/app/models/project.rb index 371d3dd11..77f52737f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -17,6 +17,7 @@ class Project < ActiveRecord::Base include Redmine::SafeAttributes + include Redmine::NestedSet::ProjectNestedSet # Project statuses STATUS_ACTIVE = 1 @@ -58,7 +59,6 @@ class Project < ActiveRecord::Base :join_table => "#{table_name_prefix}custom_fields_projects#{table_name_suffix}", :association_foreign_key => 'custom_field_id' - acts_as_nested_set :dependent => :destroy acts_as_attachable :view_permission => :view_files, :edit_permission => :manage_files, :delete_permission => :manage_files @@ -81,8 +81,8 @@ class Project < ActiveRecord::Base # reserved words validates_exclusion_of :identifier, :in => %w( new ) - after_save :update_position_under_parent, :if => Proc.new {|project| project.name_changed?} after_save :update_inherited_members, :if => Proc.new {|project| project.inherit_members_changed?} + after_save :remove_inherited_member_roles, :add_inherited_member_roles, :if => Proc.new {|project| project.parent_id_changed?} before_destroy :delete_all_members scope :has_module, lambda {|mod| @@ -414,7 +414,9 @@ class Project < ActiveRecord::Base # Nothing to do true elsif p.nil? || (p.active? && move_possible?(p)) - set_or_update_position_under(p) + self.parent = p + save + p.reload if p Issue.update_versions_from_hierarchy_change(self) true else @@ -423,17 +425,6 @@ class Project < ActiveRecord::Base end end - # Recalculates all lft and rgt values based on project names - # Unlike Project.rebuild!, these values are recalculated even if the tree "looks" valid - # Used in BuildProjectsTree migration - def self.rebuild_tree! - transaction do - update_all "lft = NULL, rgt = NULL" - rebuild!(false) - all.each { |p| p.set_or_update_position_under(p.parent) } - end - end - # Returns an array of the trackers used by the project and its active sub projects def rolled_up_trackers @rolled_up_trackers ||= @@ -781,11 +772,6 @@ class Project < ActiveRecord::Base private - def after_parent_changed(parent_was) - remove_inherited_member_roles - add_inherited_member_roles - end - def update_inherited_members if parent if inherit_members? && !inherit_members_was @@ -816,6 +802,7 @@ class Project < ActiveRecord::Base end member.save! end + memberships.reset end end @@ -1043,34 +1030,4 @@ class Project < ActiveRecord::Base end update_attribute :status, STATUS_ARCHIVED end - - def update_position_under_parent - set_or_update_position_under(parent) - end - - public - - # Inserts/moves the project so that target's children or root projects stay alphabetically sorted - def set_or_update_position_under(target_parent) - parent_was = parent - sibs = (target_parent.nil? ? self.class.roots : target_parent.children) - to_be_inserted_before = sibs.sort_by {|c| c.name.to_s.downcase}.detect {|c| c.name.to_s.downcase > name.to_s.downcase } - - if to_be_inserted_before - move_to_left_of(to_be_inserted_before) - elsif target_parent.nil? - if sibs.empty? - # move_to_root adds the project in first (ie. left) position - move_to_root - else - move_to_right_of(sibs.last) unless self == sibs.last - end - else - # move_to_child_of adds the project in last (ie.right) position - move_to_child_of(target_parent) - end - if parent_was != target_parent - after_parent_changed(parent_was) - end - end end diff --git a/config/initializers/10-patches.rb b/config/initializers/10-patches.rb index ff74c308c..63532b491 100644 --- a/config/initializers/10-patches.rb +++ b/config/initializers/10-patches.rb @@ -193,16 +193,3 @@ if Rails::VERSION::MAJOR < 4 && RUBY_VERSION >= "2.1" end end end - -module CollectiveIdea - module Acts - module NestedSet - module Model - def leaf_with_new_record? - new_record? || leaf_without_new_record? - end - alias_method_chain :leaf?, :new_record - end - end - end -end diff --git a/lib/redmine/nested_set/issue_nested_set.rb b/lib/redmine/nested_set/issue_nested_set.rb new file mode 100644 index 000000000..dac340ba3 --- /dev/null +++ b/lib/redmine/nested_set/issue_nested_set.rb @@ -0,0 +1,195 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +module Redmine + module NestedSet + module IssueNestedSet + def self.included(base) + base.class_eval do + belongs_to :parent, :class_name => self.name + + before_create :add_to_nested_set, :if => lambda {|issue| issue.parent.present?} + after_create :add_as_root, :if => lambda {|issue| issue.parent.blank?} + before_update :handle_parent_change, :if => lambda {|issue| issue.parent_id_changed?} + before_destroy :destroy_children + end + base.extend ClassMethods + base.send :include, Redmine::NestedSet::Traversing + end + + private + + def target_lft + scope_for_max_rgt = self.class.where(:root_id => root_id).where(:parent_id => parent_id) + if id + #scope_for_max_rgt = scope_for_max_rgt.where("id < ?", id) + end + max_rgt = scope_for_max_rgt.maximum(:rgt) + if max_rgt + max_rgt + 1 + elsif parent + parent.lft + 1 + else + 1 + end + end + + def add_to_nested_set(lock=true) + lock_nested_set if lock + parent.send :reload_nested_set_values + self.root_id = parent.root_id + self.lft = target_lft + self.rgt = lft + 1 + self.class.where(:root_id => root_id).where("lft >= ? OR rgt >= ?", lft, lft).update_all([ + "lft = CASE WHEN lft >= :lft THEN lft + 2 ELSE lft END, " + + "rgt = CASE WHEN rgt >= :lft THEN rgt + 2 ELSE rgt END", + {:lft => lft} + ]) + end + + def add_as_root + self.root_id = id + self.lft = 1 + self.rgt = 2 + self.class.where(:id => id).update_all(:root_id => root_id, :lft => lft, :rgt => rgt) + end + + def handle_parent_change + lock_nested_set + reload_nested_set_values + if parent_id_was + remove_from_nested_set + end + if parent + move_to_nested_set + end + reload_nested_set_values + end + + def move_to_nested_set + if parent + previous_root_id = root_id + self.root_id = parent.root_id + + lft_after_move = target_lft + self.class.where(:root_id => parent.root_id).update_all([ + "lft = CASE WHEN lft >= :lft THEN lft + :shift ELSE lft END, " + + "rgt = CASE WHEN rgt >= :lft THEN rgt + :shift ELSE rgt END", + {:lft => lft_after_move, :shift => (rgt - lft + 1)} + ]) + + self.class.where(:root_id => previous_root_id).update_all([ + "root_id = :root_id, lft = lft + :shift, rgt = rgt + :shift", + {:root_id => parent.root_id, :shift => lft_after_move - lft} + ]) + + self.lft, self.rgt = lft_after_move, (rgt - lft + lft_after_move) + parent.send :reload_nested_set_values + end + end + + def remove_from_nested_set + self.class.where(:root_id => root_id).where("lft >= ? AND rgt <= ?", lft, rgt). + update_all(["root_id = :id, lft = lft - :shift, rgt = rgt - :shift", {:id => id, :shift => lft - 1}]) + + self.class.where(:root_id => root_id).update_all([ + "lft = CASE WHEN lft >= :lft THEN lft - :shift ELSE lft END, " + + "rgt = CASE WHEN rgt >= :lft THEN rgt - :shift ELSE rgt END", + {:lft => lft, :shift => rgt - lft + 1} + ]) + self.root_id = id + self.lft, self.rgt = 1, (rgt - lft + 1) + end + + def destroy_children + unless @without_nested_set_update + lock_nested_set + reload_nested_set_values + end + children.each {|c| c.send :destroy_without_nested_set_update} + reload + unless @without_nested_set_update + self.class.where(:root_id => root_id).where("lft > ? OR rgt > ?", lft, lft).update_all([ + "lft = CASE WHEN lft > :lft THEN lft - :shift ELSE lft END, " + + "rgt = CASE WHEN rgt > :lft THEN rgt - :shift ELSE rgt END", + {:lft => lft, :shift => rgt - lft + 1} + ]) + end + end + + def destroy_without_nested_set_update + @without_nested_set_update = true + destroy + end + + def reload_nested_set_values + self.root_id, self.lft, self.rgt = self.class.where(:id => id).pluck(:root_id, :lft, :rgt).first + end + + def save_nested_set_values + self.class.where(:id => id).update_all(:root_id => root_id, :lft => lft, :rgt => rgt) + end + + def move_possible?(issue) + !is_or_is_ancestor_of?(issue) + end + + def lock_nested_set + lock = true + if self.class.connection.adapter_name =~ /sqlserver/i + lock = "WITH (ROWLOCK HOLDLOCK UPDLOCK)" + end + sets_to_lock = [id, parent_id].compact + self.class.reorder(:id).where("root_id IN (SELECT root_id FROM #{self.class.table_name} WHERE id IN (?))", sets_to_lock).lock(lock).ids + end + + def nested_set_scope + self.class.order(:lft).where(:root_id => root_id) + end + + def same_nested_set_scope?(issue) + root_id == issue.root_id + end + + module ClassMethods + def rebuild_tree! + transaction do + reorder(:id).lock.ids + update_all(:root_id => nil, :lft => nil, :rgt => nil) + where(:parent_id => nil).update_all(["root_id = id, lft = ?, rgt = ?", 1, 2]) + roots_with_children = joins("JOIN #{table_name} parent ON parent.id = #{table_name}.parent_id AND parent.id = parent.root_id").uniq.pluck("parent.id") + roots_with_children.each do |root_id| + rebuild_nodes(root_id) + end + end + end + + private + + def rebuild_nodes(parent_id = nil) + nodes = where(:parent_id => parent_id, :rgt => nil, :lft => nil).order(:id).to_a + + nodes.each do |node| + node.send :add_to_nested_set, false + node.send :save_nested_set_values + rebuild_nodes node.id + end + end + end + end + end +end diff --git a/lib/redmine/nested_set/project_nested_set.rb b/lib/redmine/nested_set/project_nested_set.rb new file mode 100644 index 000000000..699927508 --- /dev/null +++ b/lib/redmine/nested_set/project_nested_set.rb @@ -0,0 +1,159 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +module Redmine + module NestedSet + module ProjectNestedSet + def self.included(base) + base.class_eval do + belongs_to :parent, :class_name => self.name + + before_create :add_to_nested_set + before_update :move_in_nested_set, :if => lambda {|project| project.parent_id_changed? || project.name_changed?} + before_destroy :destroy_children + end + base.extend ClassMethods + base.send :include, Redmine::NestedSet::Traversing + end + + private + + def target_lft + siblings_rgt = self.class.where(:parent_id => parent_id).where("name < ?", name).maximum(:rgt) + if siblings_rgt + siblings_rgt + 1 + elsif parent_id + parent_lft = self.class.where(:id => parent_id).pluck(:lft).first + raise "Project id=#{id} with parent_id=#{parent_id}: parent missing or without 'lft' value" unless parent_lft + parent_lft + 1 + else + 1 + end + end + + def add_to_nested_set(lock=true) + lock_nested_set if lock + self.lft = target_lft + self.rgt = lft + 1 + self.class.where("lft >= ? OR rgt >= ?", lft, lft).update_all([ + "lft = CASE WHEN lft >= :lft THEN lft + 2 ELSE lft END, " + + "rgt = CASE WHEN rgt >= :lft THEN rgt + 2 ELSE rgt END", + {:lft => lft} + ]) + end + + def move_in_nested_set + lock_nested_set + reload_nested_set_values + a = lft + b = rgt + c = target_lft + unless c == a + if c > a + # Moving to the right + d = c - (b - a + 1) + scope = self.class.where(["lft BETWEEN :a AND :c - 1 OR rgt BETWEEN :a AND :c - 1", {:a => a, :c => c}]) + scope.update_all([ + "lft = CASE WHEN lft BETWEEN :a AND :b THEN lft + (:d - :a) WHEN lft BETWEEN :b + 1 AND :c - 1 THEN lft - (:b - :a + 1) ELSE lft END, " + + "rgt = CASE WHEN rgt BETWEEN :a AND :b THEN rgt + (:d - :a) WHEN rgt BETWEEN :b + 1 AND :c - 1 THEN rgt - (:b - :a + 1) ELSE rgt END", + {:a => a, :b => b, :c => c, :d => d} + ]) + elsif c < a + # Moving to the left + scope = self.class.where("lft BETWEEN :c AND :b OR rgt BETWEEN :c AND :b", {:a => a, :b => b, :c => c}) + scope.update_all([ + "lft = CASE WHEN lft BETWEEN :a AND :b THEN lft - (:a - :c) WHEN lft BETWEEN :c AND :a - 1 THEN lft + (:b - :a + 1) ELSE lft END, " + + "rgt = CASE WHEN rgt BETWEEN :a AND :b THEN rgt - (:a - :c) WHEN rgt BETWEEN :c AND :a - 1 THEN rgt + (:b - :a + 1) ELSE rgt END", + {:a => a, :b => b, :c => c, :d => d} + ]) + end + reload_nested_set_values + end + end + + def destroy_children + unless @without_nested_set_update + lock_nested_set + reload_nested_set_values + end + children.each {|c| c.send :destroy_without_nested_set_update} + unless @without_nested_set_update + self.class.where("lft > ? OR rgt > ?", lft, lft).update_all([ + "lft = CASE WHEN lft > :lft THEN lft - :shift ELSE lft END, " + + "rgt = CASE WHEN rgt > :lft THEN rgt - :shift ELSE rgt END", + {:lft => lft, :shift => rgt - lft + 1} + ]) + end + end + + def destroy_without_nested_set_update + @without_nested_set_update = true + destroy + end + + def reload_nested_set_values + self.lft, self.rgt = Project.where(:id => id).pluck(:lft, :rgt).first + end + + def save_nested_set_values + self.class.where(:id => id).update_all(:lft => lft, :rgt => rgt) + end + + def move_possible?(project) + !is_or_is_ancestor_of?(project) + end + + def lock_nested_set + lock = true + if self.class.connection.adapter_name =~ /sqlserver/i + lock = "WITH (ROWLOCK HOLDLOCK UPDLOCK)" + end + self.class.order(:id).lock(lock).ids + end + + def nested_set_scope + self.class.order(:lft) + end + + def same_nested_set_scope?(project) + true + end + + module ClassMethods + def rebuild_tree! + transaction do + reorder(:id).lock.ids + update_all(:lft => nil, :rgt => nil) + rebuild_nodes + end + end + + private + + def rebuild_nodes(parent_id = nil) + nodes = Project.where(:parent_id => parent_id).where(:rgt => nil, :lft => nil).reorder(:name) + + nodes.each do |node| + node.send :add_to_nested_set, false + node.send :save_nested_set_values + rebuild_nodes node.id + end + end + end + end + end +end diff --git a/lib/redmine/nested_set/traversing.rb b/lib/redmine/nested_set/traversing.rb new file mode 100644 index 000000000..f1684c00b --- /dev/null +++ b/lib/redmine/nested_set/traversing.rb @@ -0,0 +1,116 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +module Redmine + module NestedSet + module Traversing + def self.included(base) + base.class_eval do + scope :roots, lambda {where :parent_id => nil} + scope :leaves, lambda {where "#{table_name}.rgt - #{table_name}.lft = ?", 1} + end + end + + # Returns true if the element has no parent + def root? + parent_id.nil? + end + + # Returns true if the element has a parent + def child? + !root? + end + + # Returns true if the element has no children + def leaf? + new_record? || (rgt - lft == 1) + end + + # Returns the root element (ancestor with no parent) + def root + self_and_ancestors.first + end + + # Returns the children + def children + if id.nil? + nested_set_scope.none + else + self.class.order(:lft).where(:parent_id => id) + end + end + + # Returns the descendants that have no children + def leaves + descendants.where("#{self.class.table_name}.rgt - #{self.class.table_name}.lft = ?", 1) + end + + # Returns the siblings + def siblings + nested_set_scope.where(:parent_id => parent_id).where("id <> ?", id) + end + + # Returns the ancestors + def ancestors + if root? + nested_set_scope.none + else + nested_set_scope.where("#{self.class.table_name}.lft < ? AND #{self.class.table_name}.rgt > ?", lft, rgt) + end + end + + # Returns the element and its ancestors + def self_and_ancestors + nested_set_scope.where("#{self.class.table_name}.lft <= ? AND #{self.class.table_name}.rgt >= ?", lft, rgt) + end + + # Returns true if the element is an ancestor of other + def is_ancestor_of?(other) + same_nested_set_scope?(other) && other.lft > lft && other.rgt < rgt + end + + # Returns true if the element equals other or is an ancestor of other + def is_or_is_ancestor_of?(other) + other == self || is_ancestor_of?(other) + end + + # Returns the descendants + def descendants + if leaf? + nested_set_scope.none + else + nested_set_scope.where("#{self.class.table_name}.lft > ? AND #{self.class.table_name}.rgt < ?", lft, rgt) + end + end + + # Returns the element and its descendants + def self_and_descendants + nested_set_scope.where("#{self.class.table_name}.lft >= ? AND #{self.class.table_name}.rgt <= ?", lft, rgt) + end + + # Returns true if the element is a descendant of other + def is_descendant_of?(other) + same_nested_set_scope?(other) && other.lft < lft && other.rgt > rgt + end + + # Returns true if the element equals other or is a descendant of other + def is_or_is_descendant_of?(other) + other == self || is_descendant_of?(other) + end + end + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index bc2d178c1..9f4274830 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -30,7 +30,6 @@ require Rails.root.join('test', 'mocks', 'open_id_authentication_mock.rb').to_s require File.expand_path(File.dirname(__FILE__) + '/object_helpers') include ObjectHelpers -require 'awesome_nested_set/version' require 'net/ldap' class ActionView::TestCase @@ -214,15 +213,9 @@ class ActiveSupport::TestCase mail.parts.first.body.encoded end - # awesome_nested_set new node lft and rgt value changed this refactor revision. - # https://github.com/collectiveidea/awesome_nested_set/commit/199fca9bb938e40200cd90714dc69247ef017c61 - # The reason of behavior change is that "self.class.base_class.unscoped" was added to this line. - # https://github.com/collectiveidea/awesome_nested_set/commit/199fca9bb9#diff-f61b59a5e6319024e211b0ffdd0e4ef1R273 - # It seems correct behavior because of this line comment. - # https://github.com/collectiveidea/awesome_nested_set/blame/199fca9bb9/lib/awesome_nested_set/model.rb#L278 + # Returns the lft value for a new root issue def new_issue_lft - # ::AwesomeNestedSet::VERSION > "2.1.6" ? Issue.maximum(:rgt) + 1 : 1 - Issue.maximum(:rgt) + 1 + 1 end end diff --git a/test/unit/issue_nested_set_concurrency_test.rb b/test/unit/issue_nested_set_concurrency_test.rb new file mode 100644 index 000000000..cf45b81ae --- /dev/null +++ b/test/unit/issue_nested_set_concurrency_test.rb @@ -0,0 +1,73 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.expand_path('../../test_helper', __FILE__) + +class IssueNestedSetConcurrencyTest < ActiveSupport::TestCase + fixtures :projects, :users, + :trackers, :projects_trackers, + :enabled_modules, + :issue_statuses, + :enumerations + + self.use_transactional_fixtures = false + + def setup + CustomField.delete_all + end + + def teardown + Issue.delete_all + end + + def test_concurrency + skip if sqlite? + with_settings :notified_events => [] do + # Generates an issue and destroys it in order + # to load all needed classes before starting threads + i = Issue.generate! + i.destroy + + root = Issue.generate! + assert_difference 'Issue.count', 60 do + threads = [] + 3.times do |i| + threads << Thread.new(i) do + ActiveRecord::Base.connection_pool.with_connection do + begin + 10.times do + i = Issue.generate! :parent_issue_id => root.id + c1 = Issue.generate! :parent_issue_id => i.id + c2 = Issue.generate! :parent_issue_id => i.id + c3 = Issue.generate! :parent_issue_id => i.id + c2.reload.destroy + c1.reload.destroy + end + rescue Exception => e + Thread.current[:exception] = e.message + end + end + end + end + threads.each do |thread| + thread.join + assert_nil thread[:exception] + end + end + end + end +end diff --git a/test/unit/project_nested_set_concurrency_test.rb b/test/unit/project_nested_set_concurrency_test.rb new file mode 100644 index 000000000..873d32a3a --- /dev/null +++ b/test/unit/project_nested_set_concurrency_test.rb @@ -0,0 +1,76 @@ +# Redmine - project management software +# Copyright (C) 2006-2014 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.expand_path('../../test_helper', __FILE__) + +class ProjectNestedSetConcurrencyTest < ActiveSupport::TestCase + self.use_transactional_fixtures = false + + def setup + CustomField.delete_all + end + + def teardown + Project.delete_all + end + + def test_concurrency + skip if sqlite? + # Generates a project and destroys it in order + # to load all needed classes before starting threads + p = generate_project! + p.destroy + + assert_difference 'Project.count', 60 do + threads = [] + 3.times do |i| + threads << Thread.new(i) do + ActiveRecord::Base.connection_pool.with_connection do + begin + 10.times do + p = generate_project! + c1 = generate_project! :parent_id => p.id + c2 = generate_project! :parent_id => p.id + c3 = generate_project! :parent_id => p.id + c2.reload.destroy + c1.reload.destroy + end + rescue Exception => e + Thread.current[:exception] = e.message + end + end + end + end + threads.each do |thread| + thread.join + assert_nil thread[:exception] + end + end + end + + # Generates a bare project with random name + # and identifier + def generate_project!(attributes={}) + identifier = "a"+Redmine::Utils.random_hex(6) + Project.generate!({ + :identifier => identifier, + :name => identifier, + :tracker_ids => [], + :enabled_module_names => [] + }.merge(attributes)) + end +end