CV2-4126 revert video round 2 (#1875)

app/models/bot/alegre.rb

@@ -151,7 +151,7 @@ def self.run(body)
       if body.dig(:event) == 'create_project_media' && !pm.nil?
         Rails.logger.info("[Alegre Bot] [ProjectMedia ##{pm.id}] This item was just created, processing...")
         self.get_language(pm)
-        if ['audio', 'image', 'video'].include?(self.get_pm_type(pm))
+        if ['audio', 'image'].include?(self.get_pm_type(pm))
           self.relate_project_media_async(pm)
         else
           Bot::Alegre.send_to_media_similarity_index(pm)

app/models/concerns/alegre_similarity.rb

@@ -286,6 +286,18 @@ def similar_texts_from_api_conditions(text, models, fuzzy, team_id, fields, thre
       params
     end
 
+    def get_items_with_similar_media(media_url, threshold, team_id, path)
+      self.get_similar_items_from_api(
+        path,
+        self.similar_media_content_from_api_conditions(
+          team_id,
+          media_url,
+          threshold
+        ),
+        threshold
+      )
+    end
+
     def similar_media_content_from_api_conditions(team_id, media_url, threshold, match_across_content_types=true)
       {
         url: media_url,

app/models/concerns/alegre_v2.rb

@@ -169,14 +169,6 @@ def generic_package_media(project_media, params)
       ).merge(params)
     end
 
-    def generic_package_video(project_media, params)
-      generic_package_media(project_media, params)
-    end
-
-    def delete_package_video(project_media, _field, params)
-      generic_package_video(project_media, params)
-    end
-
     def generic_package_image(project_media, params)
       generic_package_media(project_media, params)
     end
@@ -214,10 +206,6 @@ def get_context(project_media, field=nil)
       context
     end
 
-    def store_package_video(project_media, _field, params)
-      generic_package_video(project_media, params)
-    end
-
     def store_package_image(project_media, _field, params)
       generic_package_image(project_media, params)
     end
@@ -413,16 +401,13 @@ def relate_project_media_callback(project_media, field=nil)
       self.add_relationships(project_media, get_similar_items_v2_callback(project_media, field)) unless project_media.is_blank?
     end
 
-    def is_cached_data_not_good(cached_data)
-      cached_data.values.collect{|x| x.to_a.empty?}.include?(true)
-    end
     def get_items_with_similar_media_v2(args={})
       media_url = args[:media_url]
       project_media = args[:project_media]
       threshold = args[:threshold]
       team_ids = args[:team_ids]
       type = args[:type]
-      if ['audio', 'image', 'video'].include?(type)
+      if ['audio', 'image'].include?(type)
         if project_media.nil?
           project_media = TemporaryProjectMedia.new
           project_media.url = media_url
@@ -432,16 +417,18 @@ def get_items_with_similar_media_v2(args={})
         end
         get_similar_items_v2_async(project_media, nil)
         cached_data = get_cached_data(get_required_keys(project_media, nil))
-        timeout = args[:timeout] || 60
+        timeout = 60
         start_time = Time.now
-        while start_time + timeout > Time.now && is_cached_data_not_good(cached_data) #more robust for any type of null response
+        while start_time + timeout > Time.now && cached_data.values.include?([])
           sleep(1)
           cached_data = get_cached_data(get_required_keys(project_media, nil))
         end
-        CheckSentry.notify(AlegreTimeoutError.new('Timeout when waiting for async response from Alegre'), params: args.merge({ cached_data: cached_data })) if start_time + timeout > Time.now
+        CheckSentry.notify(AlegreTimeoutError.new('Timeout when waiting for async response from Alegre'), params: args.merge({ cached_data: cached_data })) if cached_data.values.include?([])
         response = get_similar_items_v2_callback(project_media, nil)
         delete(project_media, nil) if project_media.is_a?(TemporaryProjectMedia)
         return response
+      else
+        self.get_items_with_similar_media("#{media_url||media_file_url(project_media)}?hash=#{SecureRandom.hex}", threshold, team_ids, "/#{type}/similarity/search/")
       end
     end
 
@@ -461,7 +448,7 @@ def process_alegre_callback(params)
       field = params.dig('data', 'item', 'raw', 'context', 'field')
       access_key = confirmed ? :confirmed_results : :suggested_or_confirmed_results
       key = get_required_keys(project_media, field)[access_key]
-      response = cache_items_via_callback(project_media, field, confirmed, params.dig('data', 'results', 'result').dup) #dup so we can better debug when playing with this in a repl
+      response = cache_items_via_callback(project_media, field, confirmed, params.dig('data', 'results', 'result'))
       redis.set(key, response.to_yaml)
       redis.expire(key, 1.day.to_i)
       relate_project_media_callback(project_media, field) if should_relate

app/models/concerns/alegre_webhooks.rb

@@ -10,34 +10,22 @@ def valid_request?(request)
       !token.blank? && token == CheckConfig.get('alegre_token')
     end
 
-    def is_from_alegre_search_result_callback(params)
-      params.dig('data', 'is_shortcircuited_search_result_callback') || params.dig('data', 'is_search_result_callback')
-    end
-
-    def parse_body(request)
-      JSON.parse(request.body.read)
-    end
-
     def webhook(request)
       key = nil
-      body = parse_body(request)
-      redis = Redis.new(REDIS_CONFIG)
       begin
-        doc_id = body.dig('data', 'requested', 'id')
-        # search for doc_id on completed full-circuit callbacks
-        doc_id = body.dig('data', 'item', 'id') if doc_id.nil?
-        # search for doc_id on completed short-circuit callbacks (i.e. items already known to Alegre but added context TODO make these the same structure)
-        doc_id = body.dig('data', 'item', 'raw', 'doc_id') if doc_id.nil?
-        CheckSentry.notify(AlegreCallbackError.new("Tracing Webhook NOT AN ERROR"), params: {is_raised_from_error: false, doc_id: doc_id, is_not_a_bug_is_a_temporary_log_to_sentry: true, alegre_response: request.params, body: body })
+        doc_id = request.params.dig('data', 'requested', 'id')
+        doc_id = request.params.dig('data', 'item', 'id') if doc_id.nil?
+        is_from_alegre_callback = request.params.dig('data', 'item', 'callback_url').to_s.include?("/presto/receive/add_item")
         raise 'Unexpected params format' if doc_id.blank?
-        if is_from_alegre_search_result_callback(body)
-          Bot::Alegre.process_alegre_callback(body)
+        if is_from_alegre_callback
+          Bot::Alegre.process_alegre_callback(request.params)
         else
+          redis = Redis.new(REDIS_CONFIG)
           key = "alegre:webhook:#{doc_id}"
-          redis.lpush(key, body.to_json)
+          redis.lpush(key, request.params.to_json)
         end
       rescue StandardError => e
-        CheckSentry.notify(AlegreCallbackError.new(e.message), params: { is_raised_from_error: true, alegre_response: request.params })
+        CheckSentry.notify(AlegreCallbackError.new(e.message), params: { alegre_response: request.params })
       ensure
         redis.expire(key, 1.day.to_i) if !key.nil?
       end

app/resources/api/v2/report_resource.rb

@@ -27,7 +27,7 @@ class ReportResource < BaseResource
       attribute :report_text_content
 
       def score
-        (RequestStore.store[:scores] && @model && RequestStore.store[:scores][@model.id]) ? RequestStore.store[:scores][@model.id][:score].to_f : nil
+        RequestStore.store[:scores] ? RequestStore.store[:scores][@model.id][:score].to_f : nil
       end
 
       def self.records(options = {})

test/controllers/webhooks_controller_test.rb

@@ -234,42 +234,32 @@ def setup
   end
 
   test "should process Alegre webhook" do
-    CheckSentry.expects(:notify).once
+    CheckSentry.expects(:notify).never
     redis = Redis.new(REDIS_CONFIG)
-    redis.del('alegre:webhook:foo')
+    redis.del('foo')
     id = random_number
     payload = { 'action' => 'audio', 'data' => {'requested' => { 'id' => 'foo', 'context' => { 'project_media_id' => id } }} }
     assert_nil redis.lpop('alegre:webhook:foo')
 
-    post :index, params: { name: :alegre, token: CheckConfig.get('alegre_token') }, body: payload.to_json
+    post :index, params: { name: :alegre, token: CheckConfig.get('alegre_token') }.merge(payload)
     response = JSON.parse(redis.lpop('alegre:webhook:foo'))
     assert_equal 'foo', response.dig('data', 'requested', 'id')
-    expectation = payload
+    expectation = {"action"=>"index", "data"=>{"requested"=>{"context"=>{"project_media_id"=>id.to_s}, "id"=>"foo"}}, "token"=>"test", "name"=>"alegre", "controller"=>"api/v1/webhooks"}
     assert_equal expectation, response
   end
 
-  test "should process Alegre callback webhook with is_shortcircuited_search_result_callback" do
-    CheckSentry.expects(:notify).once
-    id = random_number
-    payload = { 'action' => 'audio', 'data' => {'is_shortcircuited_search_result_callback' => true, 'item' => { 'callback_url' => '/presto/receive/add_item', 'id' => id.to_s }} }
-    Bot::Alegre.stubs(:process_alegre_callback).returns({})
-    post :index, params: { name: :alegre, token: CheckConfig.get('alegre_token') }, body: payload.to_json
-    assert_equal '200', response.code
-    assert_match /success/, response.body
-  end
-
-  test "should process Alegre callback webhook with is_search_result_callback" do
-    CheckSentry.expects(:notify).once
+  test "should process Alegre callback webhook" do
+    CheckSentry.expects(:notify).never
     id = random_number
-    payload = { 'action' => 'audio', 'data' => {'is_search_result_callback' => true, 'item' => { 'callback_url' => '/presto/receive/add_item', 'id' => id.to_s }} }
+    payload = { 'action' => 'audio', 'data' => {'item' => { 'callback_url' => '/presto/receive/add_item', 'id' => id.to_s }} }
     Bot::Alegre.stubs(:process_alegre_callback).returns({})
-    post :index, params: { name: :alegre, token: CheckConfig.get('alegre_token') }, body: payload.to_json
+    post :index, params: { name: :alegre, token: CheckConfig.get('alegre_token') }.merge(payload)
     assert_equal '200', response.code
     assert_match /success/, response.body
   end
 
   test "should report error if can't process Alegre webhook" do
-    CheckSentry.expects(:notify).twice
-    post :index, params: { name: :alegre, token: CheckConfig.get('alegre_token') }, body: {foo: "bar"}.to_json
+    CheckSentry.expects(:notify).once
+    post :index, params: { name: :alegre, token: CheckConfig.get('alegre_token') }.merge({ foo: 'bar' })
   end
 end

test/models/bot/alegre_2_test.rb

@@ -32,38 +32,34 @@ def teardown
     pm1 = create_project_media team: @team, media: create_uploaded_video
     pm2 = create_project_media team: @team, media: create_uploaded_video
     pm3 = create_project_media team: @team, media: create_uploaded_video
-    params = {:doc_id => Bot::Alegre.item_doc_id(pm3), :context => {:team_id => pm3.team_id, :project_media_id => pm3.id, :has_custom_id => true, :temporary_media => false}, :url => @media_path}
-    Bot::Alegre.stubs(:request).with('post', '/similarity/async/video', params.merge({ threshold: 0.9, confirmed: false })).returns(true)
-    Bot::Alegre.stubs(:request).with('post', '/similarity/async/video', params.merge({ threshold: 0.9, confirmed: true })).returns(true)
-    Redis.any_instance.stubs(:get).returns({
-      pm1.id => {
-        score: 0.971234,
-        context: { team_id: pm1.team_id, project_media_id: pm1.id, temporary: false },
-        model: "video",
-        source_field: "video",
-        target_field: "video",
-        relationship_type: Relationship.confirmed_type
-      },
-      pm2.id => {
-        score: 0.983167,
-        context: { team_id: pm2.team_id, project_media_id: pm2.id, temporary: false, content_type: 'video'  },
-        model: "video",
-        source_field: "video",
-        target_field: "video",
-        relationship_type: Relationship.confirmed_type
-      }
-    }.to_yaml)
+    Bot::Alegre.stubs(:request).with('post', '/video/similarity/search/', @params.merge(context: { has_custom_id: true, team_id: @team.id})).returns({
+      result: [
+        {
+          context: [
+            { team_id: @team.id.to_s, project_media_id: pm1.id.to_s }
+          ],
+          score: 0.971234,
+          filename: '/app/persistent_disk/blah/12342.tmk'
+        },
+        {
+          context: [
+            { team_id: @team.id.to_s, project_media_id: pm2.id.to_s }
+          ],
+          score: 0.983167,
+          filename: '/app/persistent_disk/blah/12343.tmk'
+        }
+      ]
+    }.with_indifferent_access)
     Bot::Alegre.stubs(:media_file_url).with(pm3).returns(@media_path)
     assert_difference 'Relationship.count' do
       Bot::Alegre.relate_project_media_to_similar_items(pm3)
     end
+    Bot::Alegre.unstub(:request)
     Bot::Alegre.unstub(:media_file_url)
-    Redis.any_instance.unstub(:get)
     r = Relationship.last
     assert_equal pm3, r.target
     assert_equal pm2, r.source
     assert_equal r.weight, 0.983167
-    Bot::Alegre.unstub(:request)
   end
 
   test "should match similar audios" do

test/models/bot/alegre_3_test.rb

@@ -232,12 +232,22 @@ def teardown
     end
   end
 
-  test "should extract project medias from context as dict" do
-    assert_equal Bot::Alegre.extract_project_medias_from_context({"_score" => 2, "_source" => {"context" => {"project_media_id" => 1}}}), {1=>{:score=>2, :context=>{"project_media_id"=>1}, :model=>nil}}
+  def self.extract_project_medias_from_context(search_result)
+    # We currently have two cases of context:
+    # - a straight hash with project_media_id
+    # - an array of hashes, each with project_media_id
+    context = search_result.dig('_source', 'context')
+    pms = []
+    if context.kind_of?(Array)
+      context.each{ |c| pms.push(c.with_indifferent_access.dig('project_media_id')) }
+    elsif context.kind_of?(Hash)
+      pms.push(context.with_indifferent_access.dig('project_media_id'))
+    end
+    Hash[pms.flatten.collect{|pm| [pm.to_i, search_result.with_indifferent_access.dig('_score')]}]
   end
 
-  test "should extract project medias from context as array" do
-    assert_equal Bot::Alegre.extract_project_medias_from_context({"_score" => 2, "_source" => {"context" => [{"project_media_id" => 1}]}}), {1=>{:score=>2, :context=>[{"project_media_id"=>1}], :model=>nil}}
+  test "should extract project medias from context" do
+    assert_equal Bot::Alegre.extract_project_medias_from_context({"_score" => 2, "_source" => {"context" => {"project_media_id" => 1}}}), {1=>{:score=>2, :context=>{"project_media_id"=>1}, :model=>nil}}
   end
 
   test "should update on alegre" do

test/models/bot/alegre_test.rb

@@ -101,7 +101,6 @@ def teardown
   end
 
   test "should unarchive item after running" do
-    WebMock.stub_request(:delete, 'http://alegre/text/similarity/').to_return(body: {success: true}.to_json)
     stub_configs({ 'alegre_host' => 'http://alegre', 'alegre_token' => 'test' }) do
       WebMock.stub_request(:delete, 'http://alegre/text/similarity/').to_return(status: 200, body: '{}')
       pm = create_project_media