Skip to content

Commit

Permalink
CV2-4597 rework how cached responses are returned to re-differentiate…
Browse files Browse the repository at this point in the history 
… 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
  • Loading branch information
@DGaffney
DGaffney authored May 16, 2024
1 parent dbf9775 commit 20c4d14
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 8 deletions.
1 change: 1 addition & 0 deletions app/models/bot/alegre.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
11 changes: 8 additions & 3 deletions app/models/concerns/alegre_v2.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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={})
Expand All @@ -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)
Expand Down
52 changes: 50 additions & 2 deletions test/controllers/reports_controller_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
132 changes: 129 additions & 3 deletions test/models/bot/alegre_v2_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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",
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 20c4d14

Please sign in to comment.