From 20c4d14888002087b0074d3339c5280a91f5f512 Mon Sep 17 00:00:00 2001 From: Devin Gaffney Date: Thu, 16 May 2024 11:42:33 -0700 Subject: [PATCH] CV2-4597 rework how cached responses are returned to re-differentiate between empty responses and non-responses (#1883) * CV2-4126 rework how cached responses are returned to re-differentiate between empty responses and non-responses * fix typo * fix fixture * fix broken test with proper query events * fix test fixtures * resolve uncovered lines --- app/models/bot/alegre.rb | 1 + app/models/concerns/alegre_v2.rb | 11 +- test/controllers/reports_controller_test.rb | 52 +++++++- test/models/bot/alegre_v2_test.rb | 132 +++++++++++++++++++- 4 files changed, 188 insertions(+), 8 deletions(-) diff --git a/app/models/bot/alegre.rb b/app/models/bot/alegre.rb index 7a22aa9b8d..2276b900fe 100644 --- a/app/models/bot/alegre.rb +++ b/app/models/bot/alegre.rb @@ -401,6 +401,7 @@ def self.transcribe_audio(pm) def self.media_file_url(pm) # FIXME Ugly hack to get a usable URL in docker-compose development environment. if pm.is_a?(TemporaryProjectMedia) + print("HELLO") url = pm.url else url = (ENV['RAILS_ENV'] != 'development' ? pm.media.file.file.public_url : "#{CheckConfig.get('storage_endpoint')}/#{CheckConfig.get('storage_bucket')}/#{pm.media.file.file.path}") diff --git a/app/models/concerns/alegre_v2.rb b/app/models/concerns/alegre_v2.rb index 69514e1028..aeb6904607 100644 --- a/app/models/concerns/alegre_v2.rb +++ b/app/models/concerns/alegre_v2.rb @@ -383,14 +383,18 @@ def get_required_keys(project_media, field) } end + def get_parsed_cached_data_for_key(key) + value = Redis.new(REDIS_CONFIG).get(key) + Hash[YAML.load(value).collect{|kk,vv| [kk.to_i, vv]}] if value + end + def get_cached_data(required_keys) - redis = Redis.new(REDIS_CONFIG) # For a given project media, we expect a set of keys to be set by the webhook callbacks sent from alegre back to check-api. # For each callback response (which is set via #process_alegre_callback), we store the value as serialized YAML to persist # the data such that symbolized keys return as symbols (as opposed to JSON, which loses the distinction). Here, in effect, # we check to see if all the responses we expect from Alegre have been sent - downstream of this, we check to see if all # responses are non-empty before proceeding to creating relationships. - Hash[required_keys.collect{|k,v| [k, (Hash[YAML.load(redis.get(v)).collect{|kk,vv| [kk.to_i, vv]}] rescue [])]}] + Hash[required_keys.collect{|k,v| [k, get_parsed_cached_data_for_key(v)]}] end def get_similar_items_v2_callback(project_media, field) @@ -420,7 +424,7 @@ def relate_project_media_callback(project_media, field=nil) end def is_cached_data_not_good(cached_data) - cached_data.values.collect{|x| x.to_a.empty?}.include?(true) + cached_data.values.collect{|x| x.nil?}.include?(true) end def get_items_with_similar_media_v2(args={}) @@ -431,6 +435,7 @@ def get_items_with_similar_media_v2(args={}) type = args[:type] if ['audio', 'image', 'video'].include?(type) if project_media.nil? + print("hello") project_media = TemporaryProjectMedia.new project_media.url = media_url project_media.id = Digest::MD5.hexdigest(project_media.url).to_i(16) diff --git a/test/controllers/reports_controller_test.rb b/test/controllers/reports_controller_test.rb index 32e197a704..0af43ffdd5 100644 --- a/test/controllers/reports_controller_test.rb +++ b/test/controllers/reports_controller_test.rb @@ -40,8 +40,56 @@ def from_alegre(pm) pm5 = create_project_media team: @t, media: create_uploaded_video create_project_media team: @t - Bot::Alegre.stubs(:request).returns({ 'result' => [from_alegre(@pm), from_alegre(pm), from_alegre(pm2), from_alegre(pm3), from_alegre(pm4), from_alegre(pm5)] }) - + Bot::Alegre.stubs(:get_items_with_similar_media_v2).returns({ + @pm.id => { + score: 1.0, + context: {"team_id" => @pm.team_id, "project_media_id" => @pm.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(@pm), + source_field: Bot::Alegre.get_type(@pm), + target_field: Bot::Alegre.get_type(@pm), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm.id => { + score: 1.0, + context: {"team_id" => pm.team_id, "project_media_id" => pm.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm), + source_field: Bot::Alegre.get_type(pm), + target_field: Bot::Alegre.get_type(pm), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm2.id => { + score: 1.0, + context: {"team_id" => pm2.team_id, "project_media_id" => pm2.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm2), + source_field: Bot::Alegre.get_type(pm2), + target_field: Bot::Alegre.get_type(pm2), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm3.id => { + score: 1.0, + context: {"team_id" => pm3.team_id, "project_media_id" => pm3.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm3), + source_field: Bot::Alegre.get_type(pm3), + target_field: Bot::Alegre.get_type(pm3), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm4.id => { + score: 1.0, + context: {"team_id" => pm4.team_id, "project_media_id" => pm4.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm4), + source_field: Bot::Alegre.get_type(pm4), + target_field: Bot::Alegre.get_type(pm4), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + pm5.id => { + score: 1.0, + context: {"team_id" => pm5.team_id, "project_media_id" => pm5.id, "temporary_media" => false}, + model: Bot::Alegre.get_type(pm5), + source_field: Bot::Alegre.get_type(pm5), + target_field: Bot::Alegre.get_type(pm5), + relationship_type: {:source=>"confirmed_sibling", :target=>"confirmed_sibling"} + }, + }) post :index, params: {} assert_response :success assert_equal 7, json_response['data'].size diff --git a/test/models/bot/alegre_v2_test.rb b/test/models/bot/alegre_v2_test.rb index 07c119917f..174bf7aac2 100644 --- a/test/models/bot/alegre_v2_test.rb +++ b/test/models/bot/alegre_v2_test.rb @@ -48,6 +48,15 @@ def teardown assert_equal Bot::Alegre.media_file_url(pm1).class, String end + test "should generate media file url for temporary object" do + project_media = TemporaryProjectMedia.new + project_media.url = "http://example.com" + project_media.id = Digest::MD5.hexdigest(project_media.url).to_i(16) + project_media.team_id = [1,2,3] + project_media.type = "audio" + assert_equal Bot::Alegre.media_file_url(project_media).class, String + end + test "should generate item_doc_id" do pm1 = create_project_media team: @team, media: create_uploaded_audio assert_equal Bot::Alegre.item_doc_id(pm1).class, String @@ -816,8 +825,57 @@ def teardown } } } + unconfirmed_params = { + "model_type": "image", + "data": { + "item": { + "id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "callback_url": "http://alegre:3100/presto/receive/add_item/image", + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "text": nil, + "raw": { + "doc_id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "context": { + "team_id": pm1.team_id, + "project_media_id": pm1.id, + "has_custom_id": true, + "temporary_media": false, + }, + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "threshold": 0.63, + "confirmed": false, + "created_at": "2024-03-14T22:05:47.588975", + "limit": 200, + "requires_callback": true, + "final_task": "search" + }, + "hash_value": "1110101010001011110100000011110010101000000010110101101010100101101111110101101001011010100001011111110101011010010000101010010110101101010110100000001010100101101010111110101000010101011100001110101010101111100001010101001011101010101011010001010101010010" + }, + "results": { + "result": [ + { + "id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "doc_id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "pdq": "1110101010001011110100000011110010101000000010110101101010100101101111110101101001011010100001011111110101011010010000101010010110101101010110100000001010100101101010111110101000010101011100001110101010101111100001010101001011101010101011010001010101010010", + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "context": [ + { + "team_id": pm2.team_id, + "has_custom_id": true, + "project_media_id": pm2.id, + "temporary_media": false, + } + ], + "score": 1.0, + "model": "image/pdq" + } + ] + } + } + } assert_difference 'Relationship.count' do # Simulate the webhook hitting the server and being executed.... + Bot::Alegre.process_alegre_callback(JSON.parse(unconfirmed_params.to_json)) relationship = Bot::Alegre.process_alegre_callback(JSON.parse(params.to_json)) #hack to force into stringed keys end assert_equal relationship.source, pm2 @@ -833,7 +891,7 @@ def teardown params = { "model_type": "image", "data": { - "is_shortcircuited_callback": true, + "is_shortcircuited_search_result_callback": true, "item": { "callback_url": "http://alegre:3100/presto/receive/add_item/image", "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", @@ -847,7 +905,7 @@ def teardown "temporary_media": false, }, "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", - "threshold": 0.73, + "threshold": 0.85, "confirmed": true, "created_at": "2024-03-14T22:05:47.588975", "limit": 200, @@ -878,8 +936,57 @@ def teardown } } } + unconfirmed_params = { + "model_type": "image", + "data": { + "is_shortcircuited_search_result_callback": true, + "item": { + "callback_url": "http://alegre:3100/presto/receive/add_item/image", + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "text": nil, + "raw": { + "doc_id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "context": { + "team_id": pm1.team_id, + "project_media_id": pm1.id, + "has_custom_id": true, + "temporary_media": false, + }, + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "threshold": 0.73, + "confirmed": false, + "created_at": "2024-03-14T22:05:47.588975", + "limit": 200, + "requires_callback": true, + "final_task": "search" + }, + "hash_value": "1110101010001011110100000011110010101000000010110101101010100101101111110101101001011010100001011111110101011010010000101010010110101101010110100000001010100101101010111110101000010101011100001110101010101111100001010101001011101010101011010001010101010010" + }, + "results": { + "result": [ + { + "id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "doc_id": "Y2hlY2stcHJvamVjdF9tZWRpYS0yMTQt", + "pdq": "1110101010001011110100000011110010101000000010110101101010100101101111110101101001011010100001011111110101011010010000101010010110101101010110100000001010100101101010111110101000010101011100001110101010101111100001010101001011101010101011010001010101010010", + "url": "http://minio:9000/check-api-dev/uploads/uploaded_image/55/09572dedf610aad68090214303c14829.png", + "context": [ + { + "team_id": pm2.team_id, + "has_custom_id": true, + "project_media_id": pm2.id, + "temporary_media": false, + } + ], + "score": 1.0, + "model": "image/pdq" + } + ] + } + } + } assert_difference 'Relationship.count' do # Simulate the webhook hitting the server and being executed.... + Bot::Alegre.process_alegre_callback(JSON.parse(unconfirmed_params.to_json)) #hack to force into stringed keys relationship = Bot::Alegre.process_alegre_callback(JSON.parse(params.to_json)) #hack to force into stringed keys end assert_equal relationship.source, pm2 @@ -946,9 +1053,28 @@ def teardown end end + test "should handle a call to get_items_with_similar_media_v2 with a temporary request" do + keys = { + confirmed_results: "alegre:async_results:blah_nil_true", + suggested_or_confirmed_results: "alegre:async_results:blah_nil_false" + } + sequence = sequence('get_cached_data_sequence') + Bot::Alegre.stubs(:get_similar_items_v2_async).returns(true) + Bot::Alegre.stubs(:get_cached_data).in_sequence(sequence).returns({blah: nil}).then.returns({}) + Bot::Alegre.stubs(:get_required_keys).returns(keys) + Bot::Alegre.stubs(:get_similar_items_v2_callback).returns({}) + Bot::Alegre.stubs(:delete).returns(true) + assert_equal Bot::Alegre.get_items_with_similar_media_v2(type: "audio", media_url: "http://example.com", timeout: 1), {} + Bot::Alegre.unstub(:get_similar_items_v2_async) + Bot::Alegre.unstub(:get_cached_data) + Bot::Alegre.unstub(:get_required_keys) + Bot::Alegre.unstub(:get_similar_items_v2_callback) + Bot::Alegre.unstub(:delete) + end + test "should get_cached_data with right fallbacks" do pm1 = create_project_media team: @team, media: create_uploaded_audio - assert_equal Bot::Alegre.get_cached_data(Bot::Alegre.get_required_keys(pm1, nil)), {confirmed_results: [], suggested_or_confirmed_results: []} + assert_equal Bot::Alegre.get_cached_data(Bot::Alegre.get_required_keys(pm1, nil)), {confirmed_results: nil, suggested_or_confirmed_results: nil} end test "should relate project media for audio" do