Merge pull request #3847 from r888888888/intelligent-autocomplete

Intelligent autocomplete

app/controllers/tags_controller.rb

@@ -10,6 +10,7 @@ class TagsController < ApplicationController
 
   def index
     @tags = Tag.search(search_params).paginate(params[:page], :limit => params[:limit], :search_count => params[:search])
+
     respond_with(@tags) do |format|
       format.xml do
         render :xml => @tags.to_xml(:root => "tags")
@@ -18,7 +19,13 @@ class TagsController < ApplicationController
   end
 
   def autocomplete
-    @tags = Tag.names_matches_with_aliases(params[:search][:name_matches])
+    if CurrentUser.is_builder?
+      # limit rollout
+      @tags = TagAutocomplete.search(params[:search][:name_matches])
+    else
+      @tags = Tag.names_matches_with_aliases(params[:search][:name_matches])
+    end
+
     expires_in params[:expiry].to_i.days if params[:expiry]
 
     respond_with(@tags) do |format|

app/javascript/src/javascripts/autocomplete.js.erb

@@ -307,6 +307,11 @@ Autocomplete.insert_completion = function(input, completion) {
   var regexp = new RegExp("(" + Autocomplete.TAG_PREFIXES + ")?\\S+$", "g");
   before_caret_text = before_caret_text.replace(regexp, "$1") + completion + " ";
 
+  if (Utility.meta('current-user-id') === '1') {
+    // is this actually better?
+    after_caret_text = after_caret_text.replace(/^\S+/, "");
+  }
+
   input.value = before_caret_text + after_caret_text;
   input.selectionStart = input.selectionEnd = before_caret_text.length;
 };

app/logical/tag_autocomplete.rb

@@ -0,0 +1,99 @@
+module TagAutocomplete
+  extend self
+
+  PREFIX_BOUNDARIES = "(_/:;-"
+
+  class Result < Struct.new(:name, :post_count, :category, :antecedent_name)
+    def to_xml(options = {})
+      to_h.to_xml(options)
+    end
+  end
+
+  def search(query)
+    candidates = count_sort(
+      query,
+      search_prefix(query, 3) + 
+      search_fuzzy(query, 5) +
+      search_exact(query, 3) +
+      search_aliases(query, 3)
+    )
+  end
+
+  def count_sort(query, words)
+    words.uniq.sort_by do |x|
+      x.post_count
+    end.reverse
+  end
+
+  def search_exact(query, n=3)
+    Tag
+      .where("name like ? escape e'\\\\'", query.to_escaped_for_sql_like + "%")
+      .where("post_count > 0")
+      .order("post_count desc")
+      .limit(n)
+      .pluck(:name, :post_count, :category)
+      .map {|row| Result.new(*row)}
+  end
+
+  def search_fuzzy(query, n=5)
+    if query.size <= 3
+      return []
+    end
+
+    Tag
+      .where("name % ?", query)
+      .where("name like ? escape E'\\\\'", query[0].to_escaped_for_sql_like + '%')
+      .where("post_count > 0")
+      .order(Arel.sql("similarity(name, #{Tag.connection.quote(query)}) * log(10, post_count + 1) DESC"))
+      .limit(n)
+      .pluck(:name, :post_count, :category)
+      .map {|row| Result.new(*row)}
+  end
+
+  def search_prefix(query, n=3)
+    if query.size >= 5
+      return []
+    end
+
+    if query.size <= 1
+      return []
+    end
+
+    if query =~ /[-_()]/
+      return []
+    end
+
+    if query.size >= 3
+      min_post_count = 0
+    else
+      min_post_count = 5_000
+      n += 2
+    end
+
+    anchors = "^" + query.split("").map {|x| Regexp.escape(x)}.join(".*[#{PREFIX_BOUNDARIES}]")
+    Tag
+      .where("name ~ ?", anchors)
+      .where("post_count > ?", min_post_count)
+      .where("post_count > 0")
+      .order("post_count desc")
+      .limit(n)
+      .pluck(:name, :post_count, :category)
+      .map {|row| Result.new(*row)}
+  end
+
+  def search_aliases(query, n=20)
+    wildcard_name = query + "*"
+    TagAlias
+      .select("tags.name, tags.post_count, tags.category, tag_aliases.antecedent_name")
+      .joins("INNER JOIN tags ON tags.name = tag_aliases.consequent_name")
+      .where("tag_aliases.antecedent_name LIKE ? ESCAPE E'\\\\'", wildcard_name.to_escaped_for_sql_like)
+      .active
+      .where("tags.name NOT LIKE ? ESCAPE E'\\\\'", wildcard_name.to_escaped_for_sql_like)
+      .where("tag_aliases.post_count > 0")
+      .order("tag_aliases.post_count desc")
+      .limit(n)
+      .pluck(:name, :post_count, :category, :antecedent_name)
+      .map {|row| Result.new(*row)}
+  end
+end
+

app/models/tag_implication.rb

@@ -1,8 +1,9 @@
 class TagImplication < TagRelationship
+  extend Memoist
+
   before_save :update_descendant_names
   after_save :update_descendant_names_for_parents
   after_destroy :update_descendant_names_for_parents
-  after_save :update_descendant_names_for_parents, if: ->(rec) { rec.is_retired? }
   after_save :create_mod_action
   validates_uniqueness_of :antecedent_name, :scope => :consequent_name
   validate :absence_of_circular_relation
@@ -17,6 +18,7 @@ class TagImplication < TagRelationship
 
   module DescendantMethods
     extend ActiveSupport::Concern
+    extend Memoist
 
     module ClassMethods
       # assumes names are normalized
@@ -32,17 +34,16 @@ class TagImplication < TagRelationship
     end
 
     def descendants
-      @descendants ||= begin
+      [].tap do |all|
-        [].tap do |all|
+        children = [consequent_name]
-          children = [consequent_name]
 
-          until children.empty?
+        until children.empty?
-            all.concat(children)
+          all.concat(children)
-            children = TagImplication.active.where(antecedent_name: children).pluck(:consequent_name)
+          children = TagImplication.active.where(antecedent_name: children).pluck(:consequent_name)
-          end
+        end
-        end.sort.uniq
+      end.sort.uniq
-      end
     end
+    memoize :descendants
 
     def descendant_names_array
       descendant_names.split(/ /)
@@ -53,7 +54,7 @@ class TagImplication < TagRelationship
     end
 
     def update_descendant_names!
-      clear_descendants_cache
+      flush_cache
       update_descendant_names
       update_attribute(:descendant_names, descendant_names)
     end
@@ -64,20 +65,15 @@ class TagImplication < TagRelationship
         parent.update_descendant_names_for_parents
       end
     end
-
-    def clear_descendants_cache
-      @descendants = nil
-    end
   end
 
   module ParentMethods
-    def parents
+    extend Memoist
-      @parents ||= self.class.where(["consequent_name = ?", antecedent_name])
-    end
 
-    def clear_parents_cache
+    def parents
-      @parents = nil
+      self.class.where("consequent_name = ?", antecedent_name)
     end
+    memoize :parents
   end
 
   module ValidationMethods
@@ -139,6 +135,8 @@ class TagImplication < TagRelationship
   end
 
   module ApprovalMethods
+    extend Memoist
+
     def process!(update_topic: true)
       unless valid?
         raise errors.full_messages.join("; ")
@@ -215,19 +213,18 @@ class TagImplication < TagRelationship
     end
 
     def forum_updater
-      @forum_updater ||= begin
+      post = if forum_topic
-        post = if forum_topic
+        forum_post || forum_topic.posts.where("body like ?", TagImplicationRequest.command_string(antecedent_name, consequent_name) + "%").last
-          forum_post || forum_topic.posts.where("body like ?", TagImplicationRequest.command_string(antecedent_name, consequent_name) + "%").last
+      else
-        else
+        nil
-          nil
-        end
-        ForumUpdater.new(
-          forum_topic, 
-          forum_post: post, 
-          expected_title: TagImplicationRequest.topic_title(antecedent_name, consequent_name)
-        )
       end
+      ForumUpdater.new(
+        forum_topic, 
+        forum_post: post, 
+        expected_title: TagImplicationRequest.topic_title(antecedent_name, consequent_name)
+      )
     end
+    memoize :forum_updater
   end
 
   include DescendantMethods
@@ -236,8 +233,7 @@ class TagImplication < TagRelationship
   include ApprovalMethods
 
   def reload(options = {})
+    flush_cache
     super
-    clear_parents_cache
-    clear_descendants_cache
   end
 end

test/models/tag_autocomplete_test.rb

@@ -0,0 +1,101 @@
+require 'test_helper'
+
+class TagAutocompleteTest < ActiveSupport::TestCase
+  subject { TagAutocomplete }
+
+  context "#search_exact" do
+    setup do
+      @tags = [
+        create(:tag, name: "abcdef", post_count: 1),
+        create(:tag, name: "abczzz", post_count: 2),
+        create(:tag, name: "abcyyy", post_count: 0),
+        create(:tag, name: "bbbbbb")
+      ]
+    end
+
+    should "find the tags" do
+      expected = [
+        @tags[1],
+        @tags[0]
+      ].map(&:name)
+      assert_equal(expected, subject.search_exact("abc", 3).map(&:name))
+    end
+  end
+
+  context "#search_fuzzy" do
+    setup do
+      @tags = [
+        create(:tag, name: "abcdef", post_count: 1),
+        create(:tag, name: "abcdzz", post_count: 2),
+
+        # one char mismatch
+        create(:tag, name: "abcezz", post_count: 2),
+
+        # too long
+        create(:tag, name: "abcdefghijk", post_count: 2),
+
+        # wrong prefix
+        create(:tag, name: "bbcdef", post_count: 2),
+
+        # zero post count
+        create(:tag, name: "abcdyy", post_count: 0),
+
+        # completely different
+        create(:tag, name: "bbbbbb")
+      ]
+    end
+
+    should "find the tags" do
+      expected = [
+        @tags[1],
+        @tags[2],
+        @tags[0]
+      ].map(&:name)
+      assert_equal(expected, subject.search_fuzzy("abcd", 3).map(&:name))
+    end
+  end
+
+  context "#search_prefix" do
+    setup do
+      @tags = [
+        create(:tag, name: "abcdef", post_count: 1),
+        create(:tag, name: "alpha_beta_cat", post_count: 2),
+        create(:tag, name: "alpha_beta_dat", post_count: 0),
+        create(:tag, name: "alpha_beta_(cane)", post_count: 2),
+        create(:tag, name: "alpha_beta/cane", post_count: 2)
+      ]
+    end
+
+    should "find the tags" do
+      expected = [
+        @tags[1],
+        @tags[3],
+        @tags[4]
+      ].map(&:name)
+      assert_equal(expected, subject.search_prefix("abc", 3).map(&:name))
+    end
+  end
+
+  context "#search_aliases" do
+    setup do
+      @user = create(:user)
+      @tags = [
+        create(:tag, name: "/abc", post_count: 0),
+        create(:tag, name: "abcdef", post_count: 1),
+        create(:tag, name: "zzzzzz", post_count: 1),
+      ]
+      as_user do
+        @aliases = [
+          create(:tag_alias, antecedent_name: "/abc", consequent_name: "abcdef", status: "active", post_count: 1)
+        ]
+      end
+    end
+
+    should "find the tags" do
+      results = subject.search_aliases("/abc", 3)
+      assert_equal(1, results.size)
+      assert_equal("abcdef", results[0].name)
+      assert_equal("/abc", results[0].antecedent_name)
+    end
+  end
+end

test/unit/tag_implication_test.rb

@@ -100,11 +100,11 @@ class TagImplicationTest < ActiveSupport::TestCase
     end
 
     should "update its descendants on save" do
-      ti1 = FactoryBot.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb")
+      ti1 = FactoryBot.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb", :status => "active")
-      ti2 = FactoryBot.create(:tag_implication, :antecedent_name => "ccc", :consequent_name => "ddd")
+      ti2 = FactoryBot.create(:tag_implication, :antecedent_name => "ccc", :consequent_name => "ddd", :status => "active")
       ti1.reload
       ti2.reload
-      ti2.update_attributes(
+      ti2.update(
         :antecedent_name => "bbb"
       )
       ti1.reload