Speed improvements

lib/acts_as_taggable_on/acts_as_taggable_on/cache.rb

@@ -35,12 +35,7 @@ def caching_tag_list_on?(context)
       end
     end
 
-    module InstanceMethods
-      def tag_list_cache_set_on(context)
-        variable_name = "@#{context.to_s.singularize}_list"
-        !instance_variable_get(variable_name).nil?
-      end
-
+    module InstanceMethods      
       def save_cached_tag_list
         tag_types.map(&:to_s).each do |tag_type|
           if self.class.send("caching_#{tag_type.singularize}_list?")
@@ -50,6 +45,8 @@ def save_cached_tag_list
             end
           end
         end
+
+        true
       end
     end
   end

lib/acts_as_taggable_on/acts_as_taggable_on/core.rb

@@ -14,27 +14,28 @@ def self.included(base)
 
     module ClassMethods
       def initialize_acts_as_taggable_on_core
-        tag_types.map(&:to_s).each do |tag_type|
-          context_taggings = "#{tag_type.singularize}_taggings".to_sym
-          context_tags     = tag_type.to_sym
+        tag_types.map(&:to_s).each do |tags_type|
+          tag_type         = tags_type.to_s.singularize
+          context_taggings = "#{tag_type}_taggings".to_sym
+          context_tags     = tags_type.to_sym
 
           class_eval do
             has_many context_taggings, :as => :taggable, :dependent => :destroy, :include => :tag, :class_name => "Tagging",
-                     :conditions => ['#{Tagging.table_name}.tagger_id IS NULL AND #{Tagging.table_name}.context = ?', tag_type]
+                     :conditions => ['#{Tagging.table_name}.tagger_id IS NULL AND #{Tagging.table_name}.context = ?', tags_type]
             has_many context_tags, :through => context_taggings, :source => :tag
           end
 
           class_eval %(
-            def #{tag_type.singularize}_list
-              tag_list_on('#{tag_type}')
+            def #{tag_type}_list
+              tag_list_on('#{tags_type}')
             end
 
-            def #{tag_type.singularize}_list=(new_tags)
-              set_tag_list_on('#{tag_type}', new_tags)
+            def #{tag_type}_list=(new_tags)
+              set_tag_list_on('#{tags_type}', new_tags)
             end
 
-            def all_#{tag_type}_list
-              all_tags_list_on('#{tag_type}')
+            def all_#{tags_type}_list
+              all_tags_list_on('#{tags_type}')
             end
           )
         end        
@@ -129,6 +130,11 @@ def cached_tag_list_on(context)
         self["cached_#{context.to_s.singularize}_list"]
       end
 
+      def tag_list_cache_set_on(context)
+        variable_name = "@#{context.to_s.singularize}_list"
+        !instance_variable_get(variable_name).nil?
+      end
+
       def tag_list_cache_on(context)
         variable_name = "@#{context.to_s.singularize}_list"
         instance_variable_get(variable_name) || instance_variable_set(variable_name, TagList.new(tags_on(context).map(&:name)))
@@ -167,7 +173,7 @@ def tags_on(context)
 
       def set_tag_list_on(context, new_list)
         add_custom_context(context)
-        
+
         variable_name = "@#{context.to_s.singularize}_list"
         instance_variable_set(variable_name, TagList.from(new_list))
       end
@@ -186,30 +192,30 @@ def reload
       end
 
       def save_tags
-        transaction do          
-          tagging_contexts.each do |context|
-            tag_list = tag_list_cache_on(context).uniq
-
-            # Find existing tags or create non-existing tags:
-            tag_list = Tag.find_or_create_all_with_like_by_name(tag_list)
-
-            current_tags = tags_on(context)
-            old_tags     = current_tags - tag_list
-            new_tags     = tag_list     - current_tags
-
-            # Find taggings to remove:
-            old_taggings = taggings.where(:tagger_type => nil, :tagger_id => nil,
-                                          :context => context.to_s, :tag_id => old_tags).all
-
-            if old_taggings.present?
-              # Destroy old taggings:
-              Tagging.destroy_all :id => old_taggings.map(&:id)
-            end
+        tagging_contexts.each do |context|
+          next unless tag_list_cache_set_on(context)
 
-            # Create new taggings:
-            new_tags.each do |tag|
-              taggings.create!(:tag_id => tag.id, :context => context.to_s, :taggable => self)
-            end
+          tag_list = tag_list_cache_on(context).uniq
+
+          # Find existing tags or create non-existing tags:
+          tag_list = Tag.find_or_create_all_with_like_by_name(tag_list)
+
+          current_tags = tags_on(context)
+          old_tags     = current_tags - tag_list
+          new_tags     = tag_list     - current_tags
+
+          # Find taggings to remove:
+          old_taggings = taggings.where(:tagger_type => nil, :tagger_id => nil,
+                                        :context => context.to_s, :tag_id => old_tags).all
+
+          if old_taggings.present?
+            # Destroy old taggings:
+            Tagging.destroy_all :id => old_taggings.map(&:id)
+          end
+
+          # Create new taggings:
+          new_tags.each do |tag|
+            taggings.create!(:tag_id => tag.id, :context => context.to_s, :taggable => self)
           end
         end
 

lib/acts_as_taggable_on/acts_as_taggable_on/ownership.rb

@@ -67,34 +67,34 @@ def reload
       end
 
       def save_owned_tags
-        transaction do
-          tagging_contexts.each do |context|
-            cached_owned_tag_list_on(context).each do |owner, tag_list|
-              # Find existing tags or create non-existing tags:
-              tag_list = Tag.find_or_create_all_with_like_by_name(tag_list.uniq)            
-
-              owned_tags = owner_tags_on(owner, context)              
-              old_tags   = owned_tags - tag_list
-              new_tags   = tag_list   - owned_tags
-
-              # Find all taggings that belong to the taggable (self), are owned by the owner, 
-              # have the correct context, and are removed from the list.
-              old_taggings = Tagging.where(:taggable_id => id, :taggable_type => self.class.base_class.to_s,
-                                           :tagger_type => owner.class.to_s, :tagger_id => owner.id,
-                                           :tag_id => old_tags, :context => context).all
-
-              if old_taggings.present?
-                # Destroy old taggings:
-                Tagging.destroy_all(:id => old_taggings.map(&:id))
-              end
+        tagging_contexts.each do |context|
+          cached_owned_tag_list_on(context).each do |owner, tag_list|
+            # Find existing tags or create non-existing tags:
+            tag_list = Tag.find_or_create_all_with_like_by_name(tag_list.uniq)            
 
-              # Create new taggings:
-              new_tags.each do |tag|
-                taggings.create!(:tag_id => tag.id, :context => context.to_s, :tagger => owner, :taggable => self)
-              end
+            owned_tags = owner_tags_on(owner, context)              
+            old_tags   = owned_tags - tag_list
+            new_tags   = tag_list   - owned_tags
+
+            # Find all taggings that belong to the taggable (self), are owned by the owner, 
+            # have the correct context, and are removed from the list.
+            old_taggings = Tagging.where(:taggable_id => id, :taggable_type => self.class.base_class.to_s,
+                                         :tagger_type => owner.class.to_s, :tagger_id => owner.id,
+                                         :tag_id => old_tags, :context => context).all
+
+            if old_taggings.present?
+              # Destroy old taggings:
+              Tagging.destroy_all(:id => old_taggings.map(&:id))
+            end
+
+            # Create new taggings:
+            new_tags.each do |tag|
+              taggings.create!(:tag_id => tag.id, :context => context.to_s, :tagger => owner, :taggable => self)
             end
           end
         end
+
+        true
       end
     end
   end

spec/acts_as_taggable_on/taggable_spec.rb

@@ -31,6 +31,9 @@
     lambda {
       @taggable.save
     }.should change(Tag, :count).by(3)
+
+    @taggable.reload
+    @taggable.skill_list.sort.should == %w(ruby rails css).sort
   end
 
   it "should be able to create tags through the tag list directly" do