Remove SearchObserver, aim is to remove all observers

rails-observers gem is mostly unmaintained and is a pain to carry forward
new implementation contains significantly less magic as a bonus

app/models/category.rb

@@ -41,20 +41,26 @@ class Category < ActiveRecord::Base
   validate :email_in_validator
 
   validate :ensure_slug
+
+  after_create :create_category_definition
+
   before_save :apply_permissions
   before_save :downcase_email
   before_save :downcase_name
-  after_create :create_category_definition
-
-  after_save :publish_category
-  after_destroy :publish_category_deletion
-
-  after_update :rename_category_definition, if: :name_changed?
-
-  after_create :delete_category_permalink
-  after_update :create_category_permalink, if: :slug_changed?
 
   after_save :publish_discourse_stylesheet
+  after_save :publish_category
+  after_save :reset_topic_ids_cache
+  after_save :clear_url_cache
+  after_save :index_search
+
+  after_destroy :reset_topic_ids_cache
+  after_destroy :publish_category_deletion
+
+  after_create :delete_category_permalink
+
+  after_update :rename_category_definition, if: :name_changed?
+  after_update :create_category_permalink, if: :slug_changed?
 
   has_one :category_search_data
   belongs_to :parent_category, class_name: 'Category'
@@ -65,8 +71,6 @@ class Category < ActiveRecord::Base
   has_many :category_tag_groups, dependent: :destroy
   has_many :tag_groups, through: :category_tag_groups
 
-  after_save :reset_topic_ids_cache
-  after_destroy :reset_topic_ids_cache
 
   scope :latest, -> { order('topic_count DESC') }
 
@@ -425,9 +429,7 @@ SQL
 
   @@url_cache = DistributedCache.new('category_url')
 
-  after_save do
+  def clear_url_cache
-    # parent takes part in url calculation
-    # any change could invalidate multiples
     @@url_cache.clear
   end
 
@@ -491,6 +493,10 @@ SQL
     DiscourseStylesheets.cache.clear
   end
 
+  def index_search
+    SearchIndexer.index(self)
+  end
+
   def self.find_by_slug(category_slug, parent_category_slug=nil)
     if parent_category_slug
       parent_category_id = self.where(slug: parent_category_slug, parent_category_id: nil).pluck(:id).first

app/models/post.rb

@@ -51,6 +51,8 @@ class Post < ActiveRecord::Base
 
   validates_with ::Validators::PostValidator
 
+  after_save :index_search
+
   # We can pass several creating options to a post via attributes
   attr_accessor :image_sizes, :quoted_post_numbers, :no_bump, :invalidate_oneboxes, :cooking_options, :skip_unique_check
 
@@ -636,6 +638,10 @@ class Post < ActiveRecord::Base
     PostTiming.where(topic_id: topic_id, post_number: post_number, user_id: user.id).exists?
   end
 
+  def index_search
+    SearchIndexer.index(self)
+  end
+
   private
 
   def parse_quote_into_arguments(quote)

app/models/topic.rb

@@ -198,6 +198,8 @@ class Topic < ActiveRecord::Base
       TagUser.auto_track(topic_id: id)
       self.tags_changed = false
     end
+
+    SearchIndexer.index(self)
   end
 
   def initialize_default_values

app/models/user.rb

@@ -99,6 +99,7 @@ class User < ActiveRecord::Base
   after_save :refresh_avatar
   after_save :badge_grant
   after_save :expire_old_email_tokens
+  after_save :index_search
 
   before_destroy do
     # These tables don't have primary keys, so destroying them with activerecord is tricky:
@@ -913,6 +914,10 @@ class User < ActiveRecord::Base
     end
   end
 
+  def index_search
+    SearchIndexer.index(self)
+  end
+
   def clear_global_notice_if_needed
     if admin && SiteSetting.has_login_hint
       SiteSetting.has_login_hint = false

app/services/search_indexer.rb

@@ -1,7 +1,14 @@
 require_dependency 'search'
 
-class SearchObserver < ActiveRecord::Observer
+class SearchIndexer
-  observe :topic, :post, :user, :category
+
+  def self.disable
+    @disabled = true
+  end
+
+  def self.enable
+    @disabled = false
+  end
 
   def self.scrub_html_for_search(html)
     HtmlScrubber.scrub(html)
@@ -72,17 +79,19 @@ class SearchObserver < ActiveRecord::Observer
   end
 
   def self.index(obj)
+    return if @disabled
+
     if obj.class == Post && obj.cooked_changed?
       if obj.topic
         category_name = obj.topic.category.name if obj.topic.category
-        SearchObserver.update_posts_index(obj.id, obj.cooked, obj.topic.title, category_name)
+        SearchIndexer.update_posts_index(obj.id, obj.cooked, obj.topic.title, category_name)
-        SearchObserver.update_topics_index(obj.topic_id, obj.topic.title, obj.cooked) if obj.is_first_post?
+        SearchIndexer.update_topics_index(obj.topic_id, obj.topic.title, obj.cooked) if obj.is_first_post?
       else
-        Rails.logger.warn("Orphan post skipped in search_observer, topic_id: #{obj.topic_id} post_id: #{obj.id} raw: #{obj.raw}")
+        Rails.logger.warn("Orphan post skipped in search_indexer, topic_id: #{obj.topic_id} post_id: #{obj.id} raw: #{obj.raw}")
       end
     end
     if obj.class == User && (obj.username_changed? || obj.name_changed?)
-      SearchObserver.update_users_index(obj.id, obj.username_lower || '', obj.name ? obj.name.downcase : '')
+      SearchIndexer.update_users_index(obj.id, obj.username_lower || '', obj.name ? obj.name.downcase : '')
     end
 
     if obj.class == Topic && obj.title_changed?
@@ -90,21 +99,17 @@ class SearchObserver < ActiveRecord::Observer
         post = obj.posts.find_by(post_number: 1)
         if post
           category_name = obj.category.name if obj.category
-          SearchObserver.update_posts_index(post.id, post.cooked, obj.title, category_name)
+          SearchIndexer.update_posts_index(post.id, post.cooked, obj.title, category_name)
-          SearchObserver.update_topics_index(obj.id, obj.title, post.cooked)
+          SearchIndexer.update_topics_index(obj.id, obj.title, post.cooked)
         end
       end
     end
 
     if obj.class == Category && obj.name_changed?
-      SearchObserver.update_categories_index(obj.id, obj.name)
+      SearchIndexer.update_categories_index(obj.id, obj.name)
     end
   end
 
-  def after_save(object)
-    SearchObserver.index(object)
-  end
-
 
   class HtmlScrubber < Nokogiri::XML::SAX::Document
     attr_reader :scrubbed

config/application.rb

@@ -85,7 +85,6 @@ module Discourse
         :user_email_observer,
         :user_action_observer,
         :post_alert_observer,
-        :search_observer
     ]
 
     # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
@@ -169,6 +168,22 @@ module Discourse
     end
 
     config.after_initialize do
+      # require common dependencies that are often required by plugins
+      # in the past observers would load them as side-effects
+      # correct behavior is for plugins to require stuff they need,
+      # however it would be a risky and breaking change not to require here
+      require_dependency 'category'
+      require_dependency 'post'
+      require_dependency 'topic'
+      require_dependency 'user'
+      require_dependency 'post_action'
+      require_dependency 'post_revision'
+      require_dependency 'notification'
+      require_dependency 'topic_user'
+      require_dependency 'group'
+      require_dependency 'user_field'
+      require_dependency 'post_action_type'
+
       # So open id logs somewhere sane
       OpenID::Util.logger = Rails.logger
       if plugins = Discourse.plugins

lib/search.rb

@@ -54,7 +54,7 @@ class Search
     posts.each do |post|
       # force indexing
       post.cooked += " "
-      SearchObserver.index(post)
+      SearchIndexer.index(post)
     end
 
     posts = Post.joins(:topic)
@@ -67,7 +67,7 @@ class Search
     posts.each do |post|
       # force indexing
       post.cooked += " "
-      SearchObserver.index(post)
+      SearchIndexer.index(post)
     end
 
     nil

lib/search/grouped_search_results.rb

@@ -49,7 +49,7 @@ class Search
 
 
     def self.blurb_for(cooked, term=nil, blurb_length=200)
-      cooked = SearchObserver::HtmlScrubber.scrub(cooked).squish
+      cooked = SearchIndexer::HtmlScrubber.scrub(cooked).squish
 
       blurb = nil
       if term

lib/tasks/search.rake

@@ -18,8 +18,8 @@ def reindex_search(db=RailsMultisite::ConnectionManagement.current_db)
     post_number = p["post_number"].to_i
     topic_id = p["topic_id"].to_i
 
-    SearchObserver.update_posts_index(post_id, cooked, title, category)
+    SearchIndexer.update_posts_index(post_id, cooked, title, category)
-    SearchObserver.update_topics_index(topic_id, title , cooked) if post_number == 1
+    SearchIndexer.update_topics_index(topic_id, title , cooked) if post_number == 1
 
     putc "."
   end
@@ -30,7 +30,7 @@ def reindex_search(db=RailsMultisite::ConnectionManagement.current_db)
     id = u["id"]
     name = u["name"]
     username = u["username"]
-    SearchObserver.update_users_index(id, username, name)
+    SearchIndexer.update_users_index(id, username, name)
 
     putc "."
   end
@@ -41,7 +41,7 @@ def reindex_search(db=RailsMultisite::ConnectionManagement.current_db)
   Category.exec_sql("select id, name from categories").each do |c|
     id = c["id"]
     name = c["name"]
-    SearchObserver.update_categories_index(id, name)
+    SearchIndexer.update_categories_index(id, name)
   end
 
   puts

spec/components/search_spec.rb

@@ -10,7 +10,7 @@ describe Search do
   end
 
   before do
-    ActiveRecord::Base.observers.enable :search_observer
+    SearchIndexer.enable
   end
 
   context 'post indexing observer' do

spec/controllers/search_controller_spec.rb

@@ -5,7 +5,7 @@ describe SearchController do
   context "integration" do
 
     before do
-      ActiveRecord::Base.observers.enable :search_observer
+      SearchIndexer.enable
     end
 
     it "can search correctly" do

spec/controllers/users_controller_spec.rb

@@ -1331,7 +1331,7 @@ describe UsersController do
     let(:user)  { Fabricate :user, username: "joecabot", name: "Lawrence Tierney" }
 
     before do
-      ActiveRecord::Base.observers.enable :all
+      SearchIndexer.enable
       Fabricate :post, user: user, topic: topic
     end
 

spec/models/topic_spec.rb

@@ -303,7 +303,7 @@ describe Topic do
 
     context 'with a similar topic' do
       let!(:topic) {
-        ActiveRecord::Base.observers.enable :search_observer
+        SearchIndexer.enable
         post = create_post(title: "Evil trout is the dude who posted this topic")
         post.topic
       }

spec/models/user_search_spec.rb

@@ -18,7 +18,7 @@ describe UserSearch do
   let(:staged)    { Fabricate :staged }
 
   before do
-    ActiveRecord::Base.observers.enable :all
+    SearchIndexer.enable
 
     Fabricate :post, user: user1, topic: topic
     Fabricate :post, user: user2, topic: topic2

spec/rails_helper.rb

@@ -107,6 +107,8 @@ Spork.prefork do
       # disable all observers, enable as needed during specs
       #
       ActiveRecord::Base.observers.disable :all
+      SearchIndexer.disable
+
       SiteSetting.provider.all.each do |setting|
         SiteSetting.remove_override!(setting.name)
       end

spec/services/search_indexer_spec.rb

@@ -1,13 +1,13 @@
 require 'rails_helper'
 
-describe SearchObserver do
+describe SearchIndexer do
 
   it 'correctly indexes chinese' do
     SiteSetting.default_locale = 'zh_CN'
     data = "你好世界"
     expect(data.split(" ").length).to eq(1)
 
-    SearchObserver.update_posts_index(99, "你好世界", "", nil)
+    SearchIndexer.update_posts_index(99, "你好世界", "", nil)
 
     raw_data = PostSearchData.where(post_id: 99).pluck(:raw_data)[0]
     expect(raw_data.split(' ').length).to eq(2)
@@ -16,13 +16,13 @@ describe SearchObserver do
   it 'correctly indexes a post' do
     data = "<a>This</a> is a test"
 
-    SearchObserver.update_posts_index(99, data, "", nil)
+    SearchIndexer.update_posts_index(99, data, "", nil)
 
     raw_data, locale = PostSearchData.where(post_id: 99).pluck(:raw_data, :locale)[0]
     expect(raw_data).to eq("This is a test")
     expect(locale).to eq("en")
 
-    SearchObserver.update_posts_index(99, "tester", "", nil)
+    SearchIndexer.update_posts_index(99, "tester", "", nil)
 
     raw_data = PostSearchData.where(post_id: 99).pluck(:raw_data)[0]
     expect(raw_data).to eq("tester")