[Search 2.0] Add Search::Postgres::Username behind a feature flag (fo…

…rem#12975)

* Add PostgreSQL FTS for usernames

* Change profile_image_90 logic

* Make search_users private

* Add tsvector index on usernames

* Limit the number of search results

* Update index name

app/controllers/search_controller.rb

@@ -87,9 +87,13 @@ def users
   end
 
   def usernames
-    usernames = Search::User.search_usernames(params[:username])
+    result = if FeatureFlag.enabled?(:search_2_usernames)
+               Search::Postgres::Username.search_documents(params[:username])
+             else
+               Search::User.search_usernames(params[:username])
+             end
 
-    render json: { result: usernames }
+    render json: { result: result }
   rescue Search::Errors::Transport::BadRequest
     render json: { result: [] }
   end

app/models/user.rb

@@ -5,6 +5,11 @@ class User < ApplicationRecord
   include Searchable
   include Storext.model
 
+  include PgSearch::Model
+  pg_search_scope :search_by_username,
+                  against: :username,
+                  using: { tsearch: { prefix: true } }
+
   # @citizen428 Preparing to drop profile columns from the users table
   PROFILE_COLUMNS = %w[
     available_for

app/services/search/postgres/username.rb

@@ -0,0 +1,32 @@
+module Search
+  module Postgres
+    class Username
+      MAX_RESULTS = 6
+
+      ATTRIBUTES = %i[
+        id
+        name
+        profile_image
+        username
+      ].freeze
+
+      def self.search_documents(term)
+        users = search_users(term)
+
+        users.map do |user|
+          user.as_json(only: %i[id name username])
+            .merge("profile_image_90" => user.profile_image_90)
+        end
+      end
+
+      def self.search_users(term)
+        ::User
+          .search_by_username(term)
+          .limit(MAX_RESULTS)
+          .select(*ATTRIBUTES)
+      end
+
+      private_class_method :search_users
+    end
+  end
+end

app/services/search/user.rb

@@ -29,7 +29,8 @@ def username_query(username)
               analyze_wildcard: true,
               allow_leading_wildcard: false
             }
-          }
+          },
+          size: Search::Postgres::Username::MAX_RESULTS # Limit the number of results we return to the frontend
         }
       end
 

db/migrate/20210312191925_add_username_tsvector_index_to_users.rb

@@ -0,0 +1,24 @@
+class AddUsernameTsvectorIndexToUsers < ActiveRecord::Migration[6.0]
+  disable_ddl_transaction!
+
+  INDEX = "to_tsvector('simple'::regconfig, COALESCE((username)::text, ''::text))"
+  private_constant :INDEX
+
+  def up
+    return if index_exists?(:users, INDEX, name: "index_users_on_username_as_tsvector")
+
+    add_index :users,
+              INDEX,
+              using: :gin,
+              algorithm: :concurrently,
+              name: "index_users_on_username_as_tsvector"
+  end
+
+  def down
+    return unless index_exists?(:users, INDEX, name: "index_users_on_username_as_tsvector")
+
+    remove_index :users,
+                 name: "index_users_on_username_as_tsvector",
+                 algorithm: :concurrently
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_03_10_154630) do
+ActiveRecord::Schema.define(version: 2021_03_12_191925) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
@@ -1348,6 +1348,7 @@
     t.boolean "welcome_notifications", default: true, null: false
     t.datetime "workshop_expiration"
     t.string "youtube_url"
+    t.index "to_tsvector('simple'::regconfig, COALESCE((username)::text, ''::text))", name: "index_users_on_username_as_tsvector", using: :gin
     t.index ["apple_username"], name: "index_users_on_apple_username"
     t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
     t.index ["created_at"], name: "index_users_on_created_at"

spec/requests/search_spec.rb

@@ -96,22 +96,45 @@
   end
 
   describe "GET /search/usernames" do
-    let(:authorized_user) { create(:user) }
-    let(:result) do
-      {
-        "username" => "molly",
-        "name" => "Molly",
-        "profile_image_90" => "something"
-      }
+    before do
+      sign_in authorized_user
     end
 
-    it "returns json" do
-      sign_in authorized_user
-      allow(Search::User).to receive(:search_usernames).and_return(
-        result,
-      )
-      get "/search/usernames"
-      expect(response.parsed_body).to eq("result" => result)
+    context "when using Elasticsearch" do
+      let(:result) do
+        {
+          "username" => "molly",
+          "name" => "Molly",
+          "profile_image_90" => "something"
+        }
+      end
+
+      it "returns json" do
+        allow(Search::User).to receive(:search_usernames).and_return(
+          result,
+        )
+        get "/search/usernames"
+        expect(response.parsed_body).to eq("result" => result)
+      end
+    end
+
+    context "when using PostgreSQL" do
+      before do
+        allow(FeatureFlag).to receive(:enabled?).with(:search_2_usernames).and_return(true)
+      end
+
+      it "returns nothing if there is no username parameter" do
+        get search_usernames_path
+        expect(response.parsed_body["result"]).to be_empty
+      end
+
+      it "finds a username by a partial name" do
+        user = create(:user, username: "Sloan")
+
+        get search_usernames_path(username: "slo")
+
+        expect(response.parsed_body["result"].first).to include("username" => user.username)
+      end
     end
   end
 

spec/services/search/postgres/username_spec.rb

@@ -0,0 +1,58 @@
+require "rails_helper"
+
+RSpec.describe Search::Postgres::Username, type: :service do
+  it "defines necessary constants" do
+    expect(described_class::ATTRIBUTES).not_to be_nil
+    expect(described_class::MAX_RESULTS).not_to be_nil
+  end
+
+  describe "::search_documents" do
+    it "returns data in the expected format" do
+      user = create(:user)
+
+      result = described_class.search_documents(user.username)
+
+      expect(result.first.keys).to match_array(
+        %w[id name profile_image_90 username],
+      )
+    end
+
+    it "finds a user by their username" do
+      user = create(:user)
+
+      expect(described_class.search_documents(user.username)).to be_present
+    end
+
+    it "finds a user by a partial username" do
+      user = create(:user)
+
+      expect(described_class.search_documents(user.username.first(1))).to be_present
+    end
+
+    it "finds multiple users whose names have common parts", :aggregate_failures do
+      alex = create(:user, username: "alex")
+      alexsmith = create(:user, username: "alexsmith")
+      rhymes = create(:user, username: "rhymes")
+
+      result = described_class.search_documents("ale")
+      usernames = result.map { |r| r["username"] }
+
+      expect(usernames).to include(alex.username)
+      expect(usernames).to include(alexsmith.username)
+      expect(usernames).not_to include(rhymes.username)
+    end
+
+    it "limits the number of results to the value of MAX_RESULTS" do
+      max_results = 1
+      stub_const("#{described_class}::MAX_RESULTS", max_results)
+
+      alex = create(:user, username: "alex")
+      alexsmith = create(:user, username: "alexsmith")
+
+      results = described_class.search_documents("alex")
+
+      expect(results.size).to eq(max_results)
+      expect([alex.username, alexsmith.username]).to include(results.first["username"])
+    end
+  end
+end

spec/services/search/user_spec.rb

@@ -86,5 +86,14 @@
     it "does not allow leading wildcards" do
       expect { described_class.search_usernames("*star") }.to raise_error(Search::Errors::Transport::BadRequest)
     end
+
+    it "limits the number of results to the value of MAX_RESULTS" do
+      max_results = 1
+      stub_const("Search::Postgres::Username::MAX_RESULTS", max_results)
+
+      usernames = described_class.search_usernames("star*")
+      expect(usernames.size).to eq(max_results)
+      expect([user1.username, user2.username]).to include(usernames.first["username"])
+    end
   end
 end