Skip to content

Commit

Permalink
CV2-5005: Sentry issue related to ES (#2057)
Browse files Browse the repository at this point in the history
* CV2-5005: limit ES date to updated fields only

* CV2-5005: fix tests

* CV2-5005: test coverage
  • Loading branch information
melsawy authored and DGaffney committed Oct 15, 2024
1 parent 7ac655d commit a244a38
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 61 deletions.
6 changes: 4 additions & 2 deletions app/lib/check_cached_fields.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ def index_and_pg_cached_field(options, value, name, target)
if should_update_cached_field?(options, target)
update_index = options[:update_es] || false
value = target.send(update_index, value) if update_index.is_a?(Symbol) && target.respond_to?(update_index)
field_name = options[:es_field_name] || name
es_options = { keys: [field_name], data: { field_name => value } }
field_name = options[:es_field_name].nil? ? [name] : [options[:es_field_name]]&.flatten
data = {}
field_name.each{ |f| data[f] = value }
es_options = { keys: field_name, data: data }
es_options[:pm_id] = target.id if target.class.name == 'ProjectMedia'
model = { klass: target.class.name, id: target.id }
ElasticSearchWorker.new.perform(YAML::dump(model), YAML::dump(es_options), 'update_doc')
Expand Down
37 changes: 16 additions & 21 deletions app/models/concerns/project_association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def belonged_to_project(objid, pid, tid)
validate :is_unique, on: :create, unless: proc { |p| p.is_being_copied }

after_commit :add_elasticsearch_data, on: :create
after_commit :update_elasticsearch_data, on: :update
after_update :update_elasticsearch_data
after_commit :destroy_elasticsearch_media , on: :destroy

def get_versions_log(event_types = nil, field_names = nil, annotation_types = nil, whodunnit = nil, include_related = false)
Expand All @@ -86,26 +86,21 @@ def add_elasticsearch_data

def update_elasticsearch_data
return if self.disable_es_callbacks || RequestStore.store[:disable_es_callbacks]
obj = self.class.find_by_id(self.id)
return if obj.nil?
data = {
'team_id' => obj.team_id,
'archived' => { method: 'archived', klass: obj.class.name, id: obj.id, type: 'int' },
'sources_count' => { method: 'sources_count', klass: 'ProjectMedia', id: obj.id, type: 'int' },
'user_id' => obj.user_id,
'read' => obj.read.to_i,
'media_published_at' => obj.media_published_at,
'source_id' => obj.source_id,
'source_name' => obj.source&.name,
'project_id' => obj.project_id,
'unmatched' => obj.unmatched,
'channel' => obj.channel.values.flatten.map(&:to_i),
'updated_at' => obj.updated_at.utc,
'title' => obj.title
}
options = { keys: data.keys, data: data, pm_id: obj.id }
model = { klass: obj.class.name, id: obj.id }
ElasticSearchWorker.perform_in(1.second, YAML::dump(model), YAML::dump(options), 'update_doc')
data = {}
['team_id', 'user_id', 'read', 'source_id', 'project_id', 'unmatched'].each do |fname|
data[fname] = self.send(fname).to_i if self.send("saved_change_to_#{fname}?")
end
['archived', 'sources_count'].each do |fname|
data[fname] = { method: fname, klass: 'ProjectMedia', id: self.id, type: 'int' } if self.send("saved_change_to_#{fname}?")
end
data['channel'] = self.channel.values.flatten.map(&:to_i) if self.send(:saved_change_to_channel?)
data['source_name'] = self.source&.name if self.send(:saved_change_to_source_id?)
unless data.blank?
data['updated_at'] = self.updated_at.utc
options = { keys: data.keys, data: data, pm_id: self.id }
model = { klass: self.class.name, id: self.id }
ElasticSearchWorker.perform_in(1.second, YAML::dump(model), YAML::dump(options), 'update_doc')
end
end

def destroy_elasticsearch_media
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/project_media_cached_fields.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def title_or_description_update

cached_field :title,
update_es: true,
es_field_name: :title_index,
es_field_name: [:title, :title_index],
recalculate: :recalculate_title,
update_on: title_or_description_update

Expand Down
4 changes: 2 additions & 2 deletions app/workers/elastic_search_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ class ElasticSearchWorker

include Sidekiq::Worker

sidekiq_options :queue => :esqueue, :retry => 5
sidekiq_options :queue => :esqueue, :retry => 3

sidekiq_retry_in { |_count, _e| 5 }
sidekiq_retry_in { |_count, _e| 3 }

def perform(model_data, options, type)
model_data = begin YAML::load(model_data) rescue nil end
Expand Down
28 changes: 15 additions & 13 deletions test/controllers/elastic_search_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,32 @@ def setup
test "should search media" do
u = create_user
@team = create_team
p = create_project team: @team
m1 = create_valid_media
pm1 = create_project_media project: p, media: m1, disable_es_callbacks: false
pm1 = create_project_media team: @team, media: m1, disable_es_callbacks: false
authenticate_with_user(u)
pender_url = CheckConfig.get('pender_url_private') + '/api/medias'
url = 'http://test.com'
response = '{"type":"media","data":{"url":"' + url + '/normalized","type":"item", "title": "title_a", "description":"search_desc"}}'
WebMock.stub_request(:get, pender_url).with({ query: { url: url } }).to_return(body: response)
m2 = create_media(account: create_valid_account, url: url)
pm2 = create_project_media project: p, media: m2, disable_es_callbacks: false
sleep 10
query = 'query Search { search(query: "{\"keyword\":\"title_a\",\"projects\":[' + p.id.to_s + ']}") { number_of_results, medias(first: 10) { edges { node { dbid } } } } }'
pm2 = create_project_media team: @team, media: m2, disable_es_callbacks: false
sleep 2
Team.stubs(:current).returns(@team)
query = 'query Search { search(query: "{\"keyword\":\"title_a\"}") { number_of_results, medias(first: 10) { edges { node { dbid } } } } }'
post :create, params: { query: query }
Team.unstub(:current)
assert_response :success
ids = []
JSON.parse(@response.body)['data']['search']['medias']['edges'].each do |id|
ids << id["node"]["dbid"]
end
assert_equal [pm2.id], ids
create_comment text: 'title_a', annotated: pm1, disable_es_callbacks: false
sleep 20
query = 'query Search { search(query: "{\"keyword\":\"title_a\",\"sort\":\"recent_activity\",\"projects\":[' + p.id.to_s + ']}") { medias(first: 10) { edges { node { dbid } } } } }'
sleep 2
Team.stubs(:current).returns(@team)
query = 'query Search { search(query: "{\"keyword\":\"title_a\",\"sort\":\"recent_activity\"}") { medias(first: 10) { edges { node { dbid } } } } }'
post :create, params: { query: query }
Team.unstub(:current)
assert_response :success
ids = []
JSON.parse(@response.body)['data']['search']['medias']['edges'].each do |id|
Expand All @@ -57,7 +60,7 @@ def setup
m2 = create_media(account: create_valid_account, url: url2)
pm = create_project_media project: p, media: m, disable_es_callbacks: false
pm2 = create_project_media project: p2, media: m2, disable_es_callbacks: false
sleep 10
sleep 2
query = 'query Search { search(query: "{\"keyword\":\"title_a\",\"projects\":[' + p.id.to_s + ',' + p2.id.to_s + ']}") { medias(first: 10) { edges { node { dbid } } } } }'
post :create, params: { query: query }
assert_response :success
Expand All @@ -67,7 +70,7 @@ def setup
end
assert_equal [pm.id, pm2.id], m_ids.sort
pm2.analysis = { content: 'new_description' }
sleep 10
sleep 2
query = 'query Search { search(query: "{\"keyword\":\"title_a\",\"projects\":[' + p.id.to_s + ',' + p2.id.to_s + ']}") { medias(first: 10) { edges { node { dbid, description } } } } }'
post :create, params: { query: query }
assert_response :success
Expand Down Expand Up @@ -104,7 +107,6 @@ def setup

test "should search with keyword" do
t = create_team
p = create_project team: t
pender_url = CheckConfig.get('pender_url_private') + '/api/medias'
url = 'http://test.com'
author_url = 'http://facebook.com/123456'
Expand All @@ -119,7 +121,7 @@ def setup
WebMock.stub_request(:get, pender_url).with({ query: { url: author_url } }).to_return(body: response)

m = create_media url: url, account_id: nil, user_id: nil, account: nil, user: nil
pm = create_project_media project: p, media: m, disable_es_callbacks: false
pm = create_project_media team: t, media: m, disable_es_callbacks: false
sleep 1
Team.current = t
result = CheckSearch.new({keyword: "non_exist_title"}.to_json)
Expand All @@ -143,13 +145,13 @@ def setup
response = '{"type":"media","data":' + data.to_json + '}'
WebMock.stub_request(:get, pender_url).with({ query: { url: media_url } }).to_return(body: response)
m2 = create_media url: media_url, account_id: nil, user_id: nil, account: nil, user: nil
pm2 = create_project_media project: p, media: m2, disable_es_callbacks: false
pm2 = create_project_media team: t, media: m2, disable_es_callbacks: false
sleep 1
result = CheckSearch.new({keyword: "search_desc"}.to_json)
assert_equal [pm.id, pm2.id].sort, result.medias.map(&:id).sort
# search in quote (with and operator)
m = create_claim_media quote: 'keyworda and keywordb'
pm = create_project_media project: p, media: m, disable_es_callbacks: false
pm = create_project_media team: t, media: m, disable_es_callbacks: false
sleep 1
result = CheckSearch.new({keyword: "keyworda and keywordb"}.to_json)
assert_equal [pm.id], result.medias.map(&:id)
Expand Down
14 changes: 6 additions & 8 deletions test/controllers/graphql_controller_3_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,17 +320,15 @@ def setup

test "should return updated offset from ES" do
RequestStore.store[:skip_cached_field_update] = false

u = create_user is_admin: true
authenticate_with_user(u)
t = create_team
p = create_project team: t
pm1 = create_project_media project: p
create_relationship source_id: pm1.id, target_id: create_project_media(project: p).id, relationship_type: Relationship.confirmed_type
pm2 = create_project_media project: p
create_relationship source_id: pm2.id, target_id: create_project_media(project: p).id, relationship_type: Relationship.confirmed_type
create_relationship source_id: pm2.id, target_id: create_project_media(project: p).id, relationship_type: Relationship.confirmed_type
sleep 10
pm1 = create_project_media team: t, disable_es_callbacks: false
create_relationship source_id: pm1.id, target_id: create_project_media(team: t).id, relationship_type: Relationship.confirmed_type
pm2 = create_project_media team: t, disable_es_callbacks: false
create_relationship source_id: pm2.id, target_id: create_project_media(team: t).id, relationship_type: Relationship.confirmed_type
create_relationship source_id: pm2.id, target_id: create_project_media(team: t).id, relationship_type: Relationship.confirmed_type
sleep 2
query = 'query CheckSearch { search(query: "{\"sort\":\"related\",\"id\":' + pm1.id.to_s + ',\"esoffset\":0,\"eslimit\":1}") {item_navigation_offset,medias(first:20){edges{node{dbid}}}}}'
post :create, params: { query: query, team: t.slug }
assert_response :success
Expand Down
2 changes: 1 addition & 1 deletion test/models/bot/fetch_2_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def setup
pm = ProjectMedia.last
result = $repository.find(get_es_id(pm))
assert_equal pm.media.type, result['associated_type']
assert_equal pm.original_title, result['title']
assert_equal pm.title, result['title']
assert_equal "Earth isn't flat", pm.title
end

Expand Down
28 changes: 15 additions & 13 deletions test/models/project_media_3_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,21 @@ def setup

test "should cache media published at" do
RequestStore.store[:skip_cached_field_update] = false
url = 'http://twitter.com/test/123456'
pender_url = CheckConfig.get('pender_url_private') + '/api/medias'
response = '{"type":"media","data":{"url":"' + url + '","type":"item","published_at":"1989-01-25 08:30:00"}}'
WebMock.stub_request(:get, pender_url).with({ query: { url: url } }).to_return(body: response)
pm = create_project_media media: nil, url: url
assert_queries(0, '=') { assert_equal 601720200, pm.media_published_at }
response = '{"type":"media","data":{"url":"' + url + '","type":"item","published_at":"1989-01-25 08:31:00"}}'
WebMock.stub_request(:get, pender_url).with({ query: { url: url, refresh: '1' } }).to_return(body: response)
pm = ProjectMedia.find(pm.id)
pm.refresh_media = true
pm.save!
pm = ProjectMedia.find(pm.id)
assert_queries(0, '=') { assert_equal 601720260, pm.media_published_at }
Sidekiq::Testing.inline! do
url = 'http://twitter.com/test/123456'
pender_url = CheckConfig.get('pender_url_private') + '/api/medias'
response = '{"type":"media","data":{"url":"' + url + '","type":"item","published_at":"1989-01-25 08:30:00"}}'
WebMock.stub_request(:get, pender_url).with({ query: { url: url } }).to_return(body: response)
pm = create_project_media media: nil, url: url
assert_queries(0, '=') { assert_equal 601720200, pm.media_published_at }
response = '{"type":"media","data":{"url":"' + url + '","type":"item","published_at":"1989-01-25 08:31:00"}}'
WebMock.stub_request(:get, pender_url).with({ query: { url: url, refresh: '1' } }).to_return(body: response)
pm = ProjectMedia.find(pm.id)
pm.refresh_media = true
pm.save!
pm = ProjectMedia.find(pm.id)
assert_queries(0, '=') { assert_equal 601720260, pm.media_published_at }
end
end

test "should cache number of related items" do
Expand Down
6 changes: 6 additions & 0 deletions test/models/project_media_6_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,12 @@ def setup
pm.save!
assert_match /^text-/, pm.get_title # Uncached
assert_match /^text-/, pm.title # Cached
# Verify save the title as a custom title
ProjectMedia.stubs(:get_title).returns(nil)
pm = create_project_media custom_title: 'Custom Title'
assert_equal 'Custom Title', pm.title
assert_equal 'custom_title', pm.reload.title_field
ProjectMedia.unstub(:get_title)
end

test "should avoid N + 1 queries problem when loading the team avatar of many items at once" do
Expand Down

0 comments on commit a244a38

Please sign in to comment.