Search code now uses ActiveRecord instead of SQL.

lib/search.rb

@@ -1,3 +1,7 @@
+require_dependency 'search/search_result'
+require_dependency 'search/search_result_type'
+require_dependency 'search/grouped_search_results'
+
 class Search
 
   def self.per_facet
@@ -8,7 +12,7 @@ class Search
     %w(topic category user)
   end
 
-  def self.current_locale_long
+  def self.long_locale
     case I18n.locale         # Currently-present in /conf/locales/* only, sorry :-( Add as needed
       when :da then 'danish'
       when :de then 'german'
@@ -24,299 +28,145 @@ class Search
   end
 
   def initialize(term, opts=nil)
-    @term = term.to_s if term.present?
+
+    if term.present?
+      @term = term.to_s
+      @original_term = PG::Connection.escape_string(@term)
+    end
+
     @opts = opts || {}
     @guardian = @opts[:guardian] || Guardian.new
+    @limit = Search.per_facet * Search.facets.size
+    @results = GroupedSearchResults.new(@opts[:type_filter])
   end
 
   # Query a term
   def execute
-
+    return nil if @term.blank? || @term.length < (@opts[:min_search_term_length] || SiteSetting.min_search_term_length)
-    return nil if @term.blank?
-
-    # really short terms are totally pointless
-    return nil if @term.length < (@opts[:min_search_term_length] || SiteSetting.min_search_term_length)
 
     # If the term is a number or url to a topic, just include that topic
-    if @opts[:type_filter] == 'topic'
+    if @results.type_filter == 'topic'
-
       begin
         route = Rails.application.routes.recognize_path(@term)
-        return single_topic(route[:topic_id]) if route[:topic_id].present?
+        return single_topic(route[:topic_id]).as_json if route[:topic_id].present?
       rescue ActionController::RoutingError
       end
 
-      return single_topic(@term.to_i) if @term =~ /^\d+$/
+      return single_topic(@term.to_i).as_json if @term =~ /^\d+$/
     end
 
-    # We are stripping only symbols taking place in FTS and simply sanitizing the rest.
+    find_grouped_results.as_json
-    @term = PG::Connection.escape_string(@term.gsub(/[:()&!]/,''))
-
-    query_string
   end
 
   private
 
-    # Search for a string term
+    def find_grouped_results
-    def query_string
 
-      args = {orig: @term,
+      if @results.type_filter.present?
-              query: @term.split.map {|t| "#{t}:*"}.join(" & "),
+        raise Discourse::InvalidAccess.new("invalid type filter") unless Search.facets.include?(@results.type_filter)
-              locale: Search.current_locale_long}
+        send("#{@results.type_filter}_search")
-
-      results = GroupedSearchResults.new(@opts[:type_filter])
-      type_filter = @opts[:type_filter]
-
-      if type_filter.present?
-        raise Discourse::InvalidAccess.new("invalid type filter") unless Search.facets.include?(type_filter)
-        args.merge!(limit: Search.per_facet * Search.facets.size)
-        case type_filter.to_s
-        when 'topic'
-          results.add(post_query(type_filter.to_sym, args))
-        when 'category'
-          results.add(category_query(args))
-        when 'user'
-          results.add(user_query(args))
-        end
       else
-        args.merge!(limit: (Search.per_facet + 1))
+        @limit = Search.per_facet + 1
-        results.add(user_query(args).to_a)
+        user_search
-        results.add(category_query(args).to_a)
+        category_search
-        results.add(post_query(:topic, args).to_a)
+        topic_search
       end
 
+      add_more_topics_if_expected
+      @results
+    end
+
+    # Add more topics if we expected them
+    def add_more_topics_if_expected
       expected_topics = 0
-      expected_topics = Search.facets.size unless type_filter.present?
+      expected_topics = Search.facets.size unless @results.type_filter.present?
-      expected_topics = Search.per_facet * Search.facets.size if type_filter == 'topic'
+      expected_topics = Search.per_facet * Search.facets.size if @results.type_filter == 'topic'
-
+      expected_topics -= @results.topic_count
-
-      # Subtract how many topics we have
-      expected_topics -= results.topic_count
-
       if expected_topics > 0
-        extra_topics = post_query(:post, args.merge(limit: expected_topics * 3)).to_a
+        topic_ids = @results.topic_ids
-
+        posts_query(expected_topics * 3).where("post_number > 1").each do |p|
-        topic_ids = results.topic_ids
+          if (expected_topics > 0) && (!topic_ids.include?(p.topic_id))
-        extra_topics.reject! do |i|
+            @results.add_result(SearchResult.from_post(p))
-          new_topic_id = i['id'].to_i
+            topic_ids << p.topic_id
-          if topic_ids.include?(new_topic_id)
+            expected_topics -= 1
-            true
-          else
-            topic_ids << new_topic_id
-            false
           end
         end
-        results.add(extra_topics[0..expected_topics-1])
       end
-
-      results.as_json
     end
 
-
     # If we're searching for a single topic
     def single_topic(id)
       topic = Topic.where(id: id).first
       return nil unless @guardian.can_see?(topic)
 
-      results = GroupedSearchResults.new(@opts[:type_filter])
+      @results.add_result(SearchResult.from_topic(topic))
-      results.add('type' => 'topic',
+      @results
-                  'id' => topic.id,
-                  'url' => topic.relative_url,
-                  'title' => topic.title)
-      results.as_json
     end
 
-    def add_allowed_categories(builder)
+    def secure_category_ids
-      allowed_categories = nil
+      return @secure_category_ids unless @secure_category_ids.nil?
-      allowed_categories = @guardian.secure_category_ids
+      @secure_category_ids = @guardian.secure_category_ids
-      if allowed_categories.present?
+    end
-        builder.where("(c.id IS NULL OR c.secure OR c.id in (:category_ids))", category_ids: allowed_categories)
+
+    def category_search
+      categories = Category.includes(:category_search_data)
+                           .where("category_search_data.search_data @@ #{ts_query}")
+                           .order("topics_month DESC")
+                           .secured(@guardian)
+                           .limit(@limit)
+
+      categories.each do |c|
+        @results.add_result(SearchResult.from_category(c))
+      end
+    end
+
+    def user_search
+      users = User.includes(:user_search_data)
+                  .where("user_search_data.search_data @@ #{ts_query}")
+                  .order("CASE WHEN username_lower = '#{@original_term.downcase}' THEN 0 ELSE 1 END")
+                  .order("last_posted_at DESC")
+                  .limit(@limit)
+
+      users.each do |u|
+        @results.add_result(SearchResult.from_user(u))
+      end
+    end
+
+    def posts_query(limit)
+      posts = Post.includes(:post_search_data, {:topic => :category})
+                  .where("post_search_data.search_data @@ #{ts_query}")
+                  .where("topics.deleted_at" => nil)
+                  .where("topics.visible")
+                  .where("topics.archetype <> ?", Archetype.private_message)
+                  .order("TS_RANK_CD(TO_TSVECTOR(#{query_locale}, topics.title), #{ts_query}) DESC")
+                  .order("TS_RANK_CD(post_search_data.search_data, #{ts_query}) DESC")
+                  .order("topics.bumped_at DESC")
+                  .limit(limit)
+
+      if secure_category_ids.present?
+        posts = posts.where("(categories.id IS NULL) OR (NOT categories.secure) OR (categories.id IN (?))", secure_category_ids)
       else
-        builder.where("(c.id IS NULL OR (NOT c.secure))")
+        posts = posts.where("(categories.id IS NULL) OR (NOT categories.secure)")
+      end
+      posts
+    end
+
+    def query_locale
+      @query_locale ||= Post.sanitize(Search.long_locale)
+    end
+
+    def ts_query
+      @ts_query ||= begin
+        escaped_term = PG::Connection.escape_string(@term.gsub(/[:()&!]/,''))
+        query = Post.sanitize(escaped_term.split.map {|t| "#{t}:*"}.join(" & "))
+        "TO_TSQUERY(#{query_locale}, #{query})"
       end
     end
 
-
+    def topic_search
-    def category_query(args)
+      posts_query(@limit).where(post_number: 1).each do |p|
-      builder = SqlBuilder.new <<SQL
+        @results.add_result(SearchResult.from_post(p))
-    SELECT 'category' AS type,
-            c.name AS id,
-            '/category/' || c.slug AS url,
-            c.name AS title,
-            NULL AS email,
-            c.color,
-            c.text_color
-    FROM categories AS c
-    JOIN category_search_data s on s.category_id = c.id
-    /*where*/
-    ORDER BY topics_month desc
-    LIMIT :limit
-SQL
-
-      builder.where "s.search_data @@ TO_TSQUERY(:locale, :query)"
-      add_allowed_categories(builder)
-
-      builder.exec(args)
       end
-
-    def user_query(args)
-      sql = "SELECT 'user' AS type,
-                    u.username_lower AS id,
-                    '/users/' || u.username_lower AS url,
-                    u.username AS title,
-                    u.email
-            FROM users AS u
-            JOIN user_search_data s on s.user_id = u.id
-            WHERE s.search_data @@ TO_TSQUERY(:locale, :query)
-            ORDER BY CASE WHEN u.username_lower = lower(:orig) then 0 else 1 end,  last_posted_at desc
-            LIMIT :limit"
-      ActiveRecord::Base.exec_sql(sql, args)
-    end
-
-    def post_query(type, args)
-      builder = SqlBuilder.new <<SQL
-      /*select*/
-      FROM topics AS ft
-      /*join*/
-        JOIN post_search_data s on s.post_id = p.id
-        LEFT JOIN categories c ON c.id = ft.category_id
-      /*where*/
-      ORDER BY
-              TS_RANK_CD(TO_TSVECTOR(:locale, ft.title), TO_TSQUERY(:locale, :query)) desc,
-              TS_RANK_CD(search_data, TO_TSQUERY(:locale, :query)) desc,
-              bumped_at desc
-      LIMIT :limit
-SQL
-
-      builder.select "'topic' AS type"
-      builder.select("CAST(ft.id AS VARCHAR)")
-
-      if type == :topic
-        builder.select "'/t/slug/' || ft.id AS url"
-      else
-        builder.select "'/t/slug/' || ft.id || '/' || p.post_number AS url"
-      end
-
-      builder.select "ft.title, NULL AS email, NULL AS color, NULL AS text_color"
-
-      if type == :topic
-        builder.join "posts AS p ON p.topic_id = ft.id AND p.post_number = 1"
-      else
-        builder.join "posts AS p ON p.topic_id = ft.id AND p.post_number > 1"
-      end
-
-      builder.where <<SQL
-  s.search_data @@ TO_TSQUERY(:locale, :query)
-        AND ft.deleted_at IS NULL
-        AND p.deleted_at IS NULL
-        AND ft.visible
-        AND ft.archetype <> '#{Archetype.private_message}'
-SQL
-
-      add_allowed_categories(builder)
-
-      builder.exec(args)
-    end
-
-    class SearchResult
-      attr_accessor :type, :id
-
-      def initialize(row)
-        @type = row['type'].to_sym
-        @url, @id, @title = row['url'], row['id'].to_i, row['title']
-
-        case @type
-        when :topic
-          # Some topics don't have slugs. In that case, use 'topic' as the slug.
-          new_slug = Slug.for(row['title'])
-          new_slug = "topic" if new_slug.blank?
-          @url.gsub!('slug', new_slug)
-        when :user
-          @avatar_template = User.avatar_template(row['email'])
-        when :category
-          @color, @text_color = row['color'], row['text_color']
-        end
-      end
-
-      def as_json
-        json = {id: @id, title: @title, url: @url}
-        json[:avatar_template] = @avatar_template if @avatar_template.present?
-        json[:color] = @color if @color.present?
-        json[:text_color] = @text_color if @text_color.present?
-        json
-      end
-    end
-
-    class SearchResultType
-
-      attr_accessor :more, :results
-
-      def initialize(type)
-        @type = type
-        @results = []
-        @more = false
-      end
-
-      def size
-        @results.size
-      end
-
-      def add(result)
-        @results << result
-      end
-
-      def as_json
-        { type: @type.to_s,
-          name: I18n.t("search.types.#{@type.to_s}"),
-          more: @more,
-          results: @results.map(&:as_json) }
-      end
-    end
-
-    class GroupedSearchResults
-
-      attr_reader :topic_count
-
-      def initialize(type_filter)
-        @type_filter = type_filter
-        @by_type = {}
-        @topic_count = 0
-      end
-
-      def add(results)
-        results = [results] if results.is_a?(Hash)
-
-        results.each do |r|
-          add_result(SearchResult.new(r))
-        end
-      end
-
-      def add_result(result)
-        grouped_result = @by_type[result.type] || (@by_type[result.type] = SearchResultType.new(result.type))
-
-        # Limit our results if there is no filter
-        if @type_filter.present? or (grouped_result.size < Search.per_facet)
-          @topic_count += 1 if (result.type == :topic)
-
-          grouped_result.add(result)
-        else
-          grouped_result.more = true
-        end
-      end
-
-      def topic_ids
-        topic_results = @by_type[:topic]
-        return Set.new if topic_results.blank?
-
-        Set.new(topic_results.results.map(&:id))
-      end
-
-      def as_json
-        @by_type.values.map do |grouped_result|
-          grouped_result.as_json
-        end
-      end
-
     end
 
 end

lib/search/grouped_search_results.rb

@@ -0,0 +1,39 @@
+class Search
+
+  class GroupedSearchResults
+    attr_reader :topic_count, :type_filter
+
+    def initialize(type_filter)
+      @type_filter = type_filter
+      @by_type = {}
+      @topic_count = 0
+    end
+
+    def topic_ids
+      topic_results = @by_type[:topic]
+      return Set.new if topic_results.blank?
+      Set.new(topic_results.results.map(&:id))
+    end
+
+    def as_json
+      @by_type.values.map do |grouped_result|
+        grouped_result.as_json
+      end
+    end
+
+    def add_result(result)
+      grouped_result = @by_type[result.type] || (@by_type[result.type] = SearchResultType.new(result.type))
+
+      # Limit our results if there is no filter
+      if @type_filter.present? or (grouped_result.size < Search.per_facet)
+        @topic_count += 1 if (result.type == :topic)
+
+        grouped_result.add(result)
+      else
+        grouped_result.more = true
+      end
+    end
+
+  end
+
+end

lib/search/search_result.rb

@@ -0,0 +1,49 @@
+class Search
+
+  class SearchResult
+    attr_accessor :type, :id
+
+    # Category attributes
+    attr_accessor :color, :text_color
+
+    # User attributes
+    attr_accessor :avatar_template
+
+    def initialize(row)
+      row.symbolize_keys!
+      @type = row[:type].to_sym
+      @url, @id, @title = row[:url], row[:id].to_i, row[:title]
+    end
+
+    def as_json
+      json = {id: @id, title: @title, url: @url}
+      json[:avatar_template] = @avatar_template if @avatar_template.present?
+      json[:color] = @color if @color.present?
+      json[:text_color] = @text_color if @text_color.present?
+      json
+    end
+
+    def self.from_category(c)
+      SearchResult.new(type: :category, id: c.id, title: c.name, url: "/category/#{c.slug}").tap do |r|
+        r.color = c.color
+        r.text_color = c.text_color
+      end
+    end
+
+    def self.from_user(u)
+      SearchResult.new(type: :user, id: u.username_lower, title: u.username, url: "/users/#{u.username_lower}").tap do |r|
+        r.avatar_template = User.avatar_template(u.email)
+      end
+    end
+
+    def self.from_topic(t)
+      SearchResult.new(type: :topic, id: t.id, title: t.title, url: t.relative_url)
+    end
+
+    def self.from_post(p)
+      SearchResult.from_topic(p.topic)
+    end
+
+  end
+
+end

lib/search/search_result_type.rb

@@ -0,0 +1,28 @@
+class Search
+
+  class SearchResultType
+    attr_accessor :more, :results
+
+    def initialize(type)
+      @type = type
+      @results = []
+      @more = false
+    end
+
+    def size
+      @results.size
+    end
+
+    def add(result)
+      @results << result
+    end
+
+    def as_json
+      { type: @type.to_s,
+        name: I18n.t("search.types.#{@type.to_s}"),
+        more: @more,
+        results: @results.map(&:as_json) }
+    end
+  end
+
+end