Revert "AO3-4858 Cache tag counts to speed up work posting" (#2805)

* Revert "Revert "AO3-4858 We were using taggings_count in SQL queries which will not work" (#2804)"

This reverts commit 199ad38.

* Revert "AO3-4918 Amend deploy script to reload unicorns within maintenance window  (#2801)"

This reverts commit b3908ac.

* Revert "AO3-4901 Divide tag set cukes into smaller files and own test group (#2789)"

This reverts commit 3da0998.

* Revert "AO3-4887 Challenge Assignment controller test for no_challenge (#2775)"

This reverts commit 57d5d72.

* Revert "AO3-4858 Queries that hit mysql have to refer to taggings_count_cache rather than taggings_count (#2799)"

This reverts commit a23103b.

* Revert "AO3-4869 Extend tests for external_authors controller (#2755)"

This reverts commit c8953f6.

* Revert "AO3-4858 Cache tag counts to speed up work posting (#2796)"

This reverts commit 7f3b54a.

app/controllers/tags_controller.rb

@@ -97,7 +97,7 @@ def show
     @tag_children = Rails.cache.fetch "views/tags/#{@tag.cache_key}/children" do
       children = {}
       (@tag.child_types - %w(SubTag)).each do |child_type|
-        tags = @tag.send(child_type.underscore.pluralize).order('taggings_count_cache DESC').limit(ArchiveConfig.TAG_LIST_LIMIT + 1)
+        tags = @tag.send(child_type.underscore.pluralize).order('taggings_count DESC').limit(ArchiveConfig.TAG_LIST_LIMIT + 1)
         children[child_type] = tags.to_a.uniq unless tags.blank?
       end
       children

app/models/tagging.rb

@@ -1,5 +1,5 @@
 class Tagging < ActiveRecord::Base
-  belongs_to :tagger, polymorphic: true
+  belongs_to :tagger, polymorphic: true, counter_cache: true
   belongs_to :taggable, polymorphic: true, touch: true
 
   validates_presence_of :tagger, :taggable
@@ -27,4 +27,4 @@ def remove_filter_tagging
   def self.find_by_tag(taggable, tag)
     Tagging.find_by_tagger_id_and_taggable_id_and_tagger_type_and_taggable_type(tag.id, taggable.id, 'Tag', taggable.class.name)
   end
-end
+end

app/models/work.rb

@@ -202,9 +202,6 @@ def expire_caches
     self.filters.each do |tag|
       tag.update_works_index_timestamp!
     end
-    self.tags.each do |tag|
-      tag.update_tag_cache
-    end
     Work.expire_work_tag_groups_id(self.id)
   end
 

config/resque_schedule.yml

@@ -12,16 +12,9 @@ run_background_reindex_queue:
   args: background
   description: "Kick off a reindex of all background reindexes"
 
-run_add_counts_to_queue:
-  every: 30m
-  class: "ScheduledTagJob"
-  queue: utilities
-  args: add_counts_to_queue
-  description: "update the cache of counts of usage for tags"
-
 run_stats_reindex_queue:
   every: 30m
   class: "ScheduledReindexJob"
   queue: utilities
   args: stats 
-  description: "Kick off a reindex of works with stats updates"
+  description: "Kick off a reindex of works with stats updates"

features/fixtures/tags.yml

@@ -38,6 +38,7 @@ char1a:
   id: 9
   name: Persone A
   type: Character
+  taggings_count: 1
 ghostsoup:
   id: 10
   name: Ghost Soup
@@ -58,4 +59,4 @@ rating3:
 rating4:
   id: 14
   name: Explicit
-  type: Rating
+  type: Rating

features/tags_and_wrangling/tag_wrangling_more.feature

@@ -24,7 +24,7 @@ Feature: Tag wrangling: assigning wranglers, using the filters on the Wranglers
     Then I should see "Edit first fandom Tag"
 
     # assigning media to a fandom
-    When I fill in "tag[media_string]" with "TV Shows"
+    When I fill in "Media" with "TV Shows"
       And I press "Save changes"
     Then I should see "Tag was updated"
     When I follow "Tag Wrangling"

lib/tasks/tag_tasks.rake

@@ -1,6 +1,6 @@
 namespace :Tag do
   desc "Reset common taggings - slow"
-  task(reset_common: :environment) do
+  task(:reset_common => :environment) do
     Work.find(:all).each do |w|
       print "." if w.id.modulo(100) == 0; STDOUT.flush
       #w.update_common_tags
@@ -9,35 +9,36 @@ namespace :Tag do
   end
 
   desc "Reset tag count"
-  task(reset_count: :environment) do
+  task(:reset_count => :environment) do
     Tag.find(:all).each do |t|
-      t.taggings_count
+      Tag.update_counters t.id, :taggings_count => -t.taggings_count
+      Tag.update_counters t.id, :taggings_count => t.taggings.length
     end
     puts "Tag count reset."
   end
 
   desc "Reset taggings count for obviously wrong taggings_count"
-  task(fix_taggings_count: :environment) do
+  task(:fix_taggings_count => :environment) do
     tag_scope = Tag.where("taggings_count < 0")
     tag_count = tag_scope.count
     tag_scope.each_with_index do |tag, index|
       puts "#{index} / #{tag_count}"
-      tag.taggings_count
+      Tag.reset_counters(tag.id, :taggings)
     end
     puts "Taggings count for less-than-zero counts has been reset."
   end
 
   desc "Update relationship has_characters"
-  task(update_has_characters: :environment) do
+  task(:update_has_characters => :environment) do
     Relationship.all.each do |relationship|
       relationship.update_attribute(:has_characters, true) unless relationship.characters.blank?
     end
   end
 
   desc "Delete unused tags"
-  task(delete_unused: :environment) do
+  task(:delete_unused => :environment) do
     deleted_names = []
-    Tag.find(:all, conditions: {canonical: false, merger_id: nil, taggings_count_cache: 0}).each do |t|
+    Tag.find(:all, :conditions => {:canonical => false, :merger_id => nil, :taggings_count => 0}).each do |t|
       if t.taggings.count == 0 && t.child_taggings.count == 0 && t.set_taggings.count == 0
         deleted_names << t.name
         t.destroy
@@ -50,26 +51,26 @@ namespace :Tag do
   end
 
   desc "Delete unused admin post tags"
-  task(delete_unused_admin_post_tags: :environment) do
+  task(:delete_unused_admin_post_tags => :environment) do
     AdminPostTag.joins("LEFT JOIN `admin_post_taggings` ON admin_post_taggings.admin_post_tag_id = admin_post_tags.id").where("admin_post_taggings.id IS NULL").destroy_all
   end
 
   desc "Clean up orphaned taggings"
-  task(clean_up_taggings: :environment) do
+  task(:clean_up_taggings => :environment) do
     Tagging.find_each {|t| t.destroy if t.taggable.nil?}
     CommonTagging.find_each {|t| t.destroy if t.common_tag.nil?}
   end
   desc "Reset filter taggings"
-  task(reset_filters: :environment) do
+  task(:reset_filters => :environment) do
     FilterTagging.delete_all
     FilterTagging.build_from_taggings
   end
   desc "Reset filter counts"
-  task(reset_filter_counts: :environment) do
+  task(:reset_filter_counts => :environment) do
     FilterCount.set_all
   end
   desc "Reset filter counts from date"
-  task(unsuspend_filter_counts: :environment) do
+  task(:unsuspend_filter_counts => :environment) do
     admin_settings = Rails.cache.fetch("admin_settings") { AdminSetting.first }
     if admin_settings && admin_settings.suspend_filter_counts_at
       FilterTagging.update_filter_counts_since(admin_settings.suspend_filter_counts_at)

spec/models/tag_spec.rb

@@ -11,54 +11,6 @@
     User.current_user = nil
   end
 
-  context 'checking count caching' do
-    before(:each) do
-      # Set the minimal amount of time a tag can be cached for.
-      ArchiveConfig.TAGGINGS_COUNT_MIN_TIME = 1
-      # Set so that we need few uses of a tag to start caching it.
-      ArchiveConfig.TAGGINGS_COUNT_CACHE_DIVISOR = 2
-      # Set the minimum number of uses needed for before caching is started.
-      ArchiveConfig.TAGGINGS_COUNT_MIN_CACHE_COUNT = 3
-      @fandom_tag = FactoryGirl.create(:fandom)
-    end
-
-    it 'should not cache tags which are not used much' do
-      work = FactoryGirl.create(:work, fandom_string: @fandom_tag.name)
-      @fandom_tag.reload
-      expect(@fandom_tag.taggings_count_cache).to eq 1
-      expect(@fandom_tag.taggings_count).to eq 1
-      expect(@fandom_tag.large_tag).not_to be_truthy
-    end
-
-    it 'will start caching a when tag when that tag is used significantly' do
-      (1..ArchiveConfig.TAGGINGS_COUNT_MIN_CACHE_COUNT + 1).each do |try|
-        work = FactoryGirl.create(:work, fandom_string: @fandom_tag.name)
-        work.save
-        @fandom_tag.reload
-        expect(@fandom_tag.taggings_count_cache).to eq try
-        expect(@fandom_tag.taggings_count).to eq try
-      end
-      work = FactoryGirl.create(:work, fandom_string: @fandom_tag.name)
-      work.save
-      @fandom_tag.reload
-      # This value should be cached and wrong
-      expect(@fandom_tag.taggings_count_cache).to eq ArchiveConfig.TAGGINGS_COUNT_MIN_CACHE_COUNT + 1
-      expect(@fandom_tag.taggings_count).to eq ArchiveConfig.TAGGINGS_COUNT_MIN_CACHE_COUNT + 1
-      expect(@fandom_tag.large_tag).not_to be_truthy
-    end
-
-    it "flags a tag as large based on its number of uses" do
-      (1..40 * ArchiveConfig.TAGGINGS_COUNT_CACHE_DIVISOR - 1).each do |try|
-        @fandom_tag.taggings_count = try
-        @fandom_tag.reload
-        expect(@fandom_tag.large_tag).not_to be_truthy
-      end
-      @fandom_tag.taggings_count = 40 * ArchiveConfig.TAGGINGS_COUNT_CACHE_DIVISOR
-      @fandom_tag.reload
-      expect(@fandom_tag.large_tag).to be_truthy
-    end
-  end
-
   it "should not be valid without a name" do
     expect(@tag.save).not_to be_truthy
 

spec/models/work_search_spec.rb

@@ -3,10 +3,6 @@
 describe WorkSearch do
 
   before(:each) do
-    work.save
-    # This doesn't work properly in the factory.
-    second_work.collection_ids = [collection.id]
-    second_work.save
     Tire.index(Work.index_name).delete
     Work.create_elasticsearch_index
     Work.import
@@ -43,6 +39,7 @@
       fandom_string: "Harry Potter",
       character_string: "Harry Potter, Ron Weasley, Hermione Granger",
       posted: true,
+      collection_ids: [1],
       language_id: 2
     )
   end
@@ -150,13 +147,11 @@
       it "should only return works in that fandom" do
         work_search = WorkSearch.new(fandom_names: "Harry Potter")
         expect(work_search.search_results).not_to include work
-        expect(work_search.search_results).to include second_work
       end
 
       it "should not choke on exclamation points" do
         work_search = WorkSearch.new(fandom_names: "Potter!")
         expect(work_search.search_results).to include second_work
-        expect(work_search.search_results).not_to include work
       end
     end