From ed81601723b672af92ae50315460a926c168bc8d Mon Sep 17 00:00:00 2001
From: Gannon McGibbon <gannon.mcgibbon@gmail.com>
Date: Thu, 16 Jan 2020 14:14:28 -0500
Subject: [PATCH] Only enqueue analysis jobs when blob is analyzable

---
 activestorage/CHANGELOG.md                           |  4 ++++
 .../app/models/active_storage/blob/analyzable.rb     |  8 ++++++--
 activestorage/lib/active_storage/analyzer.rb         |  6 ++++++
 .../lib/active_storage/analyzer/null_analyzer.rb     |  4 ++++
 activestorage/test/models/attachment_test.rb         | 12 ++++++++++++
 5 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md
index dd71286588..f008f783e3 100644
--- a/activestorage/CHANGELOG.md
+++ b/activestorage/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Only enqueue analysis jobs for blobs with non-null analyzer classes.
+
+    *Gannon McGibbon*
+
 *   Previews are created on the same service as the original blob.
 
     *Peter Zhu*
diff --git a/activestorage/app/models/active_storage/blob/analyzable.rb b/activestorage/app/models/active_storage/blob/analyzable.rb
index 5bda6e6d73..fa58e814f6 100644
--- a/activestorage/app/models/active_storage/blob/analyzable.rb
+++ b/activestorage/app/models/active_storage/blob/analyzable.rb
@@ -29,12 +29,16 @@ module ActiveStorage::Blob::Analyzable
     update! metadata: metadata.merge(extract_metadata_via_analyzer)
   end
 
-  # Enqueues an ActiveStorage::AnalyzeJob which calls #analyze.
+  # Enqueues an ActiveStorage::AnalyzeJob which calls #analyze, or calls #analyze inline based on analyzer class configuration.
   #
   # This method is automatically called for a blob when it's attached for the first time. You can call it to analyze a blob
   # again (e.g. if you add a new analyzer or modify an existing one).
   def analyze_later
-    ActiveStorage::AnalyzeJob.perform_later(self)
+    if analyzer_class.analyze_later?
+      ActiveStorage::AnalyzeJob.perform_later(self)
+    else
+      analyze
+    end
   end
 
   # Returns true if the blob has been analyzed.
diff --git a/activestorage/lib/active_storage/analyzer.rb b/activestorage/lib/active_storage/analyzer.rb
index 26414ffbc2..a7dc7a7da8 100644
--- a/activestorage/lib/active_storage/analyzer.rb
+++ b/activestorage/lib/active_storage/analyzer.rb
@@ -12,6 +12,12 @@ module ActiveStorage
       false
     end
 
+    # Implement this method in concrete subclasses. It will determine if blob anlysis
+    # should be done in a job or performed inine. By default, analysis is enqueued in a job.
+    def self.analyze_later?
+      true
+    end
+
     def initialize(blob)
       @blob = blob
     end
diff --git a/activestorage/lib/active_storage/analyzer/null_analyzer.rb b/activestorage/lib/active_storage/analyzer/null_analyzer.rb
index 8ff7ce48e5..e7f8d83f7e 100644
--- a/activestorage/lib/active_storage/analyzer/null_analyzer.rb
+++ b/activestorage/lib/active_storage/analyzer/null_analyzer.rb
@@ -6,6 +6,10 @@ module ActiveStorage
       true
     end
 
+    def self.analyze_later?
+      false
+    end
+
     def metadata
       {}
     end
diff --git a/activestorage/test/models/attachment_test.rb b/activestorage/test/models/attachment_test.rb
index 16a9004ff1..814584126d 100644
--- a/activestorage/test/models/attachment_test.rb
+++ b/activestorage/test/models/attachment_test.rb
@@ -25,6 +25,18 @@ class ActiveStorage::AttachmentTest < ActiveSupport::TestCase
     assert_equal 2736, blob.metadata[:height]
   end
 
+  test "attaching a un-analyzable blob" do
+    blob = create_blob(filename: "blank.txt")
+
+    assert_not_predicate blob, :analyzed?
+
+    assert_no_enqueued_jobs do
+      @user.highlights.attach(blob)
+    end
+
+    assert_predicate blob.reload, :analyzed?
+  end
+
   test "mirroring a directly-uploaded blob after attaching it" do
     with_service("mirror") do
       blob = directly_upload_file_blob