From 1146772debf861c5c8b6e4e3cebdba31a1138922 Mon Sep 17 00:00:00 2001 From: Erick Guan <fantasticfears@gmail.com> Date: Tue, 15 Aug 2017 17:46:57 +0200 Subject: [PATCH] Fix: unlinked topic search model (#5044) --- app/models/category.rb | 3 +- app/models/category_search_data.rb | 4 +- app/models/concerns/has_search_data.rb | 10 ++++ app/models/concerns/searchable.rb | 7 +++ app/models/post.rb | 2 +- app/models/post_search_data.rb | 4 +- app/models/topic.rb | 3 +- app/models/topic_search_data.rb | 18 ++++++ app/models/user.rb | 2 +- app/models/user_search_data.rb | 3 +- .../concern/has_search_data_spec.rb | 47 +++++++++++++++ spec/components/concern/searchable_spec.rb | 57 +++++++++++++++++++ 12 files changed, 147 insertions(+), 13 deletions(-) create mode 100644 app/models/concerns/has_search_data.rb create mode 100644 app/models/concerns/searchable.rb create mode 100644 app/models/topic_search_data.rb create mode 100644 spec/components/concern/has_search_data_spec.rb create mode 100644 spec/components/concern/searchable_spec.rb diff --git a/app/models/category.rb b/app/models/category.rb index 7939103cd0c..9450e44e962 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -1,7 +1,7 @@ require_dependency 'distributed_cache' class Category < ActiveRecord::Base - + include Searchable include Positionable include HasCustomFields include CategoryHashtag @@ -63,7 +63,6 @@ class Category < ActiveRecord::Base 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' has_many :subcategories, class_name: 'Category', foreign_key: 'parent_category_id' diff --git a/app/models/category_search_data.rb b/app/models/category_search_data.rb index bfbd1b316a4..f199887a366 100644 --- a/app/models/category_search_data.rb +++ b/app/models/category_search_data.rb @@ -1,7 +1,5 @@ class CategorySearchData < ActiveRecord::Base - belongs_to :category - - validates_presence_of :search_data + include HasSearchData end # == Schema Information diff --git a/app/models/concerns/has_search_data.rb b/app/models/concerns/has_search_data.rb new file mode 100644 index 00000000000..acf8f2cf53c --- /dev/null +++ b/app/models/concerns/has_search_data.rb @@ -0,0 +1,10 @@ +module HasSearchData + extend ActiveSupport::Concern + + included do + _asscoiated_record_name = self.name.sub('SearchData', '').underscore + self.primary_key = "#{_asscoiated_record_name}_id" + belongs_to _asscoiated_record_name.to_sym + validates_presence_of :search_data + end +end diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb new file mode 100644 index 00000000000..dd67af05474 --- /dev/null +++ b/app/models/concerns/searchable.rb @@ -0,0 +1,7 @@ +module Searchable + extend ActiveSupport::Concern + + included do + has_one "#{self.name.underscore}_search_data".to_sym, dependent: :destroy + end +end diff --git a/app/models/post.rb b/app/models/post.rb index 16e56e338ce..18b1bdf31dd 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -14,6 +14,7 @@ require 'digest/sha1' class Post < ActiveRecord::Base include RateLimiter::OnCreateRecord include Trashable + include Searchable include HasCustomFields include LimitedEdit @@ -40,7 +41,6 @@ class Post < ActiveRecord::Base has_many :post_uploads has_many :uploads, through: :post_uploads - has_one :post_search_data has_one :post_stat has_one :incoming_email diff --git a/app/models/post_search_data.rb b/app/models/post_search_data.rb index f9ee72b885a..7b82fca9bae 100644 --- a/app/models/post_search_data.rb +++ b/app/models/post_search_data.rb @@ -1,7 +1,5 @@ class PostSearchData < ActiveRecord::Base - belongs_to :post - - validates_presence_of :search_data + include HasSearchData end # == Schema Information diff --git a/app/models/topic.rb b/app/models/topic.rb index 7eb6e1c554e..e5787076900 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -15,6 +15,7 @@ class Topic < ActiveRecord::Base include RateLimiter::OnCreateRecord include HasCustomFields include Trashable + include Searchable include LimitedEdit extend Forwardable @@ -125,7 +126,7 @@ class Topic < ActiveRecord::Base has_one :user_warning has_one :first_post, -> { where post_number: 1 }, class_name: Post - + has_one :topic_search_data has_one :topic_embed, dependent: :destroy # When we want to temporarily attach some data to a forum topic (usually before serialization) diff --git a/app/models/topic_search_data.rb b/app/models/topic_search_data.rb new file mode 100644 index 00000000000..883ff5cf7a1 --- /dev/null +++ b/app/models/topic_search_data.rb @@ -0,0 +1,18 @@ +class TopicSearchData < ActiveRecord::Base + include HasSearchData +end + +# == Schema Information +# +# Table name: topic_search_data +# +# topic_id :integer not null, primary key +# raw_data :text +# locale :string not null +# search_data :tsvector +# core_version :integer default(0) +# +# Indexes +# +# idx_search_topic (search_data) +# diff --git a/app/models/user.rb b/app/models/user.rb index 51644f7cc5a..cc4f38a9e7b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,6 +12,7 @@ require_dependency 'letter_avatar' require_dependency 'promotion' class User < ActiveRecord::Base + include Searchable include Roleable include HasCustomFields @@ -66,7 +67,6 @@ class User < ActiveRecord::Base has_many :muted_user_records, class_name: 'MutedUser' has_many :muted_users, through: :muted_user_records - has_one :user_search_data, dependent: :destroy has_one :api_key, dependent: :destroy belongs_to :uploaded_avatar, class_name: 'Upload' diff --git a/app/models/user_search_data.rb b/app/models/user_search_data.rb index fab53372915..989df80169a 100644 --- a/app/models/user_search_data.rb +++ b/app/models/user_search_data.rb @@ -1,6 +1,5 @@ class UserSearchData < ActiveRecord::Base - belongs_to :user - validates_presence_of :search_data + include HasSearchData end # == Schema Information diff --git a/spec/components/concern/has_search_data_spec.rb b/spec/components/concern/has_search_data_spec.rb new file mode 100644 index 00000000000..9cc3089f585 --- /dev/null +++ b/spec/components/concern/has_search_data_spec.rb @@ -0,0 +1,47 @@ +require "rails_helper" + +describe HasSearchData do + context "belongs to its model" do + before do + Topic.exec_sql("create temporary table model_items(id SERIAL primary key)") + Topic.exec_sql("create temporary table model_item_search_data(model_item_id int primary key, search_data tsvector, raw_data text, locale text)") + + class ModelItem < ActiveRecord::Base + has_one :model_item_search_data, dependent: :destroy + end + + class ModelItemSearchData < ActiveRecord::Base + include HasSearchData + end + end + + after do + Topic.exec_sql("drop table model_items") + Topic.exec_sql("drop table model_item_search_data") + + # import is making my life hard, we need to nuke this out of orbit + des = ActiveSupport::DescendantsTracker.class_variable_get :@@direct_descendants + des[ActiveRecord::Base].delete(ModelItem) + des[ActiveRecord::Base].delete(ModelItemSearchData) + end + + let(:item) do + item = ModelItem.create! + item.create_model_item_search_data!( + model_item_id: item.id, + search_data: 'a', + raw_data: 'a', + locale: 'en') + item + end + + it 'sets its primary key into asscoiated model' do + expect(ModelItemSearchData.primary_key).to eq 'model_item_id' + end + + it 'can access the model' do + record_id = item.id + expect(ModelItemSearchData.find_by(model_item_id: record_id).model_item_id).to eq record_id + end + end +end diff --git a/spec/components/concern/searchable_spec.rb b/spec/components/concern/searchable_spec.rb new file mode 100644 index 00000000000..365f2b16f87 --- /dev/null +++ b/spec/components/concern/searchable_spec.rb @@ -0,0 +1,57 @@ +require "rails_helper" + +describe Searchable do + context "has search data" do + before do + Topic.exec_sql("create temporary table searchable_records(id SERIAL primary key)") + Topic.exec_sql("create temporary table searchable_record_search_data(searchable_record_id int primary key, search_data tsvector, raw_data text, locale text)") + + class SearchableRecord < ActiveRecord::Base + include Searchable + end + + class SearchableRecordSearchData < ActiveRecord::Base + self.primary_key = 'searchable_record_id' + belongs_to :test_item + end + end + + after do + Topic.exec_sql("drop table searchable_records") + Topic.exec_sql("drop table searchable_record_search_data") + + # import is making my life hard, we need to nuke this out of orbit + des = ActiveSupport::DescendantsTracker.class_variable_get :@@direct_descendants + des[ActiveRecord::Base].delete(SearchableRecord) + des[ActiveRecord::Base].delete(SearchableRecordSearchData) + end + + let(:item) { SearchableRecord.create! } + + it 'can build the data' do + expect(item.build_searchable_record_search_data).to be_truthy + end + + it 'can save the data' do + item.build_searchable_record_search_data( + search_data: '', + raw_data: 'a', + locale: 'en') + item.save + + loaded = SearchableRecord.find(item.id) + expect(loaded.searchable_record_search_data.raw_data).to eq 'a' + end + + it 'destroy the search data when the item is deprived' do + item.build_searchable_record_search_data( + search_data: '', + raw_data: 'a', + locale: 'en') + item.save + item_id = item.id + item.destroy + expect(SearchableRecordSearchData.find_by(searchable_record_id: item_id)).to be_nil + end + end +end