Index ClassifiedListings to Elasticsearch (#6132) [deploy]

* Add ClassifiedListingEsIndexWorker & spec

* Update ClassifiedListing model

- Add es index methods (inline and async)
- Index ClassifiedListing to es after_commit
- Add serialized_search_hash
- Update model specs for new methods
- Add serializers referenced in new model method

* Update specs

* Create DataUpdateScript to index ClassifiedListings

* Fix formatting

* Fix codeclimate :/

* Use match_array instead of keys.sort

* Change find_by! to find

* Update spec

* Update mappings for ClassifiedListing

* Update specs

* Implement refactor with Searchable module

app/models/classified_listing.rb

@@ -1,5 +1,10 @@
 class ClassifiedListing < ApplicationRecord
   include AlgoliaSearch
+  include Searchable
+
+  SEARCH_INDEX_WORKER = Search::ClassifiedListingEsIndexWorker
+  SEARCH_SERIALIZER = Search::ClassifiedListingSerializer
+  SEARCH_CLASS = Search::ClassifiedListing
 
   CATEGORIES_AVAILABLE = {
     cfp: { cost: 1, name: "Conference CFP", rules: "Currently open for proposals, with link to form." },
@@ -22,6 +27,7 @@ class ClassifiedListing < ApplicationRecord
   before_save :evaluate_markdown
   before_create :create_slug
   before_validation :modify_inputs
+  after_commit :index_to_elasticsearch
   acts_as_taggable_on :tags
   has_many :credits, as: :purchase, inverse_of: :purchase, dependent: :nullify
 

app/serializers/search/classified_listing_author_serializer.rb

@@ -0,0 +1,7 @@
+module Search
+  class ClassifiedListingAuthorSerializer
+    include FastJsonapi::ObjectSerializer
+
+    attributes :username, :name, :profile_image_90
+  end
+end

app/serializers/search/classified_listing_serializer.rb

@@ -0,0 +1,25 @@
+module Search
+  class ClassifiedListingSerializer
+    include FastJsonapi::ObjectSerializer
+
+    attributes :id,
+               :body_markdown,
+               :bumped_at,
+               :category,
+               :contact_via_connect,
+               :expires_at,
+               :location,
+               :processed_html,
+               :slug,
+               :title,
+               :user_id
+
+    attribute :tags, &:tag_list
+
+    attribute :author do |cl|
+      ClassifiedListingAuthorSerializer.new(cl.author).
+        serializable_hash.
+        dig(:data, :attributes)
+    end
+  end
+end

app/workers/search/classified_listing_es_index_worker.rb

@@ -0,0 +1,12 @@
+module Search
+  class ClassifiedListingEsIndexWorker
+    include Sidekiq::Worker
+
+    sidekiq_options queue: :high_priority
+
+    def perform(classified_listing_id)
+      classified_listing = ::ClassifiedListing.find(classified_listing_id)
+      classified_listing.index_to_elasticsearch_inline
+    end
+  end
+end

config/elasticsearch/mappings/classified_listings.json

@@ -19,7 +19,8 @@
       }
     },
     "body_markdown": {
-      "type": "text"
+      "type": "text",
+      "copy_to": "classified_listing_search"
     },
     "bumped_at": {
       "type": "date"
@@ -27,6 +28,9 @@
     "category": {
       "type": "keyword"
     },
+    "classified_listing_search": {
+      "type": "text"
+    },
     "contact_via_connect": {
       "type": "boolean"
     },
@@ -35,6 +39,7 @@
     },
     "location": {
       "type": "text",
+      "copy_to": "classified_listing_search",
       "fields": {
         "raw": {
           "type": "keyword"
@@ -46,6 +51,7 @@
     },
     "slug": {
       "type": "text",
+      "copy_to": "classified_listing_search",
       "fields": {
         "raw": {
           "type": "keyword"
@@ -53,10 +59,12 @@
       }
     },
     "tags": {
-      "type": "keyword"
+      "type": "keyword",
+      "copy_to": "classified_listing_search"
     },
     "title": {
       "type": "text",
+      "copy_to": "classified_listing_search",
       "fields": {
         "raw": {
           "type": "keyword"

lib/data_update_scripts/20200217215802_index_classified_listings_to_elasticsearch.rb

@@ -0,0 +1,9 @@
+module DataUpdateScripts
+  class IndexClassifiedListingsToElasticsearch
+    def run
+      # Choose to do inline so development envs are ready immediately after
+      # this is run
+      ClassifiedListing.find_each(&:index_to_elasticsearch_inline)
+    end
+  end
+end

spec/lib/data_update_scripts/index_classified_listings_to_elasticsearch_spec.rb

@@ -0,0 +1,11 @@
+require "rails_helper"
+require Rails.root.join("lib/data_update_scripts/20200217215802_index_classified_listings_to_elasticsearch.rb")
+
+describe DataUpdateScripts::IndexClassifiedListingsToElasticsearch, elasticsearch: true do
+  it "indexes classified_listings to Elasticsearch" do
+    classified_listing = FactoryBot.create(:classified_listing)
+    expect { classified_listing.elasticsearch_doc }.to raise_error(Elasticsearch::Transport::Transport::Errors::NotFound)
+    described_class.new.run
+    expect(classified_listing.elasticsearch_doc).not_to be_nil
+  end
+end

spec/models/classified_listing_spec.rb

@@ -71,4 +71,49 @@ RSpec.describe ClassifiedListing, type: :model do
       expect(credit.reload.purchase).to be_nil
     end
   end
+
+  describe "#index_to_elasticsearch" do
+    it "enqueues job to index classified_listing to elasticsearch" do
+      sidekiq_assert_enqueued_with(job: Search::ClassifiedListingEsIndexWorker, args: [classified_listing.id]) do
+        classified_listing.index_to_elasticsearch
+      end
+    end
+  end
+
+  describe "#index_to_elasticsearch_inline" do
+    it "indexed classified_listing to elasticsearch inline" do
+      allow(Search::ClassifiedListing).to receive(:index)
+      classified_listing.index_to_elasticsearch_inline
+      expect(Search::ClassifiedListing).to have_received(:index).with(classified_listing.id, hash_including(:id, :body_markdown))
+    end
+  end
+
+  describe "#after_commit" do
+    it "enqueues worker to index tag to elasticsearch" do
+      classified_listing.save
+
+      sidekiq_assert_enqueued_with(job: Search::ClassifiedListingEsIndexWorker, args: [classified_listing.id]) do
+        classified_listing.save
+      end
+    end
+  end
+
+  describe "#serialized_search_hash" do
+    it "creates a valid serialized hash to send to elasticsearch" do
+      # classified_listing_search is a copy_to field and will never be included
+      # in the searialized_search_hash, so we skip it
+      mapping_keys = Search::ClassifiedListing::MAPPINGS.dig(:properties).except(:classified_listing_search).keys
+      search_hash_keys = classified_listing.serialized_search_hash.symbolize_keys.keys
+
+      expect(search_hash_keys).to match_array(mapping_keys)
+    end
+  end
+
+  describe "#elasticsearch_doc" do
+    it "finds document in elasticsearch", elasticsearch: true do
+      allow(Search::ClassifiedListing).to receive(:find_document)
+      classified_listing.elasticsearch_doc
+      expect(Search::ClassifiedListing).to have_received(:find_document)
+    end
+  end
 end

spec/services/search/classified_listing_spec.rb

@@ -5,7 +5,7 @@ RSpec.describe Search::ClassifiedListing, type: :service, elasticsearch: true do
     it "indexes a classified_listing to elasticsearch" do
       classified_listing = FactoryBot.create(:classified_listing)
       expect { described_class.find_document(classified_listing.id) }.to raise_error(Elasticsearch::Transport::Transport::Errors::NotFound)
-      described_class.index(classified_listing.id, id: classified_listing.id)
+      described_class.index(classified_listing.id, classified_listing.serialized_search_hash)
       expect(described_class.find_document(classified_listing.id)).not_to be_nil
     end
   end
@@ -13,7 +13,7 @@ RSpec.describe Search::ClassifiedListing, type: :service, elasticsearch: true do
   describe "::find_document" do
     it "fetches a document for a given ID from elasticsearch" do
       classified_listing = FactoryBot.create(:classified_listing)
-      described_class.index(classified_listing.id, id: classified_listing.id)
+      described_class.index(classified_listing.id, classified_listing.serialized_search_hash)
       expect { described_class.find_document(classified_listing.id) }.not_to raise_error
     end
   end
@@ -77,9 +77,14 @@ RSpec.describe Search::ClassifiedListing, type: :service, elasticsearch: true do
       initial_mapping = SearchClient.indices.get_mapping(index: other_name).dig(other_name, "mappings")
       expect(initial_mapping).to be_empty
 
+      # This might look a little strange...it's because es_mapping_keys returns
+      # certain fields like copy_to as an Array and our mappings don't have it
+      # in that format. As a result, we're just comparing keys instead.
       described_class.update_mappings(index_alias: other_name)
-      mapping = SearchClient.indices.get_mapping(index: other_name).dig(other_name, "mappings")
-      expect(mapping.deep_stringify_keys).to include(described_class::MAPPINGS.deep_stringify_keys)
+      es_mapping_keys = SearchClient.indices.get_mapping(index: other_name).dig(other_name, "mappings", "properties").symbolize_keys.keys
+      mapping_keys = described_class::MAPPINGS.dig(:properties).keys
+
+      expect(mapping_keys).to match_array(es_mapping_keys)
 
       # Have to cleanup index since it wont automatically be handled by our cluster class bc of the unexpected name
       described_class.delete_index(index_name: other_name)

spec/workers/search/classified_listing_es_index_worker_spec.rb

@@ -0,0 +1,24 @@
+require "rails_helper"
+
+RSpec.describe Search::ClassifiedListingEsIndexWorker, type: :worker, elasticsearch: true do
+  let(:worker) { subject }
+
+  include_examples "#enqueues_on_correct_queue", "high_priority", [1]
+
+  it "raises an error if record is not found" do
+    expect { worker.perform(1234) }.to raise_error(ActiveRecord::RecordNotFound)
+  end
+
+  it "indexes classified_listing" do
+    classified_listing = FactoryBot.create(:classified_listing)
+    expect { classified_listing.elasticsearch_doc }.to raise_error(Elasticsearch::Transport::Transport::Errors::NotFound)
+    worker.perform(classified_listing.id)
+
+    elasticsearch_doc = classified_listing.elasticsearch_doc.dig("_source").deep_symbolize_keys
+    cl_serialized_search_hash = classified_listing.serialized_search_hash.deep_symbolize_keys
+
+    expect(elasticsearch_doc[:id]).to eq(cl_serialized_search_hash[:id])
+    expect(elasticsearch_doc[:author][:name]).to eq(cl_serialized_search_hash[:author][:name])
+    expect(elasticsearch_doc[:body_markdown]).to eq(cl_serialized_search_hash[:body_markdown])
+  end
+end