FIX: Copying image markdown for secure media loading full image (#9488)

* When copying the markdown for an image between posts, we were not adding the srcset and data-small-image attributes which are done by calling optimize_image! in cooked post processor
* Refactored the code which was confusing in its current state (the consider_for_reuse method was super confusing) and fixed the issue

app/models/upload.rb

@@ -123,13 +123,26 @@ class Upload < ActiveRecord::Base
     "upload://#{short_url_basename}"
   end
 
+  def uploaded_before_secure_media_enabled?
+    original_sha1.blank?
+  end
+
+  def matching_access_control_post?(post)
+    access_control_post_id == post.id
+  end
+
+  def copied_from_other_post?(post)
+    return false if access_control_post_id.blank?
+    !matching_access_control_post?(post)
+  end
+
   def short_path
     self.class.short_path(sha1: self.sha1, extension: self.extension)
   end
 
   def self.consider_for_reuse(upload, post)
     return upload if !SiteSetting.secure_media? || upload.blank? || post.blank?
-    return nil if upload.access_control_post_id != post.id || upload.original_sha1.blank?
+    return nil if !upload.matching_access_control_post?(post) || upload.uploaded_before_secure_media_enabled?
     upload
   end
 

lib/cooked_post_processor.rb

@@ -339,12 +339,7 @@ class CookedPostProcessor
       width, height = ImageSizer.resize(width, height)
     end
 
-    # if the upload already exists and is attached to a different post,
+    upload = Upload.get_from_url(src)
-    # or the original_sha1 is missing meaning it was created before secure
-    # media was enabled. we want to re-thumbnail and re-optimize in this case
-    # to avoid using uploads linked to many other posts
-    upload = Upload.consider_for_reuse(Upload.get_from_url(src), @post)
-
     if upload.present?
       upload.create_thumbnail!(width, height, crop: crop)
 

spec/models/upload_spec.rb

@@ -288,53 +288,6 @@ describe Upload do
     end
   end
 
-  describe ".consider_for_reuse" do
-    let(:post) { Fabricate(:post) }
-    let(:upload) { Fabricate(:upload) }
-
-    it "returns nil when the provided upload is blank" do
-      expect(Upload.consider_for_reuse(nil, post)).to eq(nil)
-    end
-
-    it "returns the upload when secure media is disabled" do
-      expect(Upload.consider_for_reuse(upload, post)).to eq(upload)
-    end
-
-    context "when secure media enabled" do
-      before do
-        enable_secure_media
-      end
-
-      context "when the upload access control post is != to the provided post" do
-        before do
-          upload.update(access_control_post_id: Fabricate(:post).id)
-        end
-
-        it "returns nil" do
-          expect(Upload.consider_for_reuse(upload, post)).to eq(nil)
-        end
-      end
-
-      context "when the upload original_sha1 is blank (pre-secure-media upload)" do
-        before do
-          upload.update(original_sha1: nil, access_control_post: post)
-        end
-
-        it "returns nil" do
-          expect(Upload.consider_for_reuse(upload, post)).to eq(nil)
-        end
-      end
-
-      context "when the upload original_sha1 is present and access control post is correct" do
-        let(:upload) { Fabricate(:secure_upload_s3, access_control_post: post) }
-
-        it "returns the upload" do
-          expect(Upload.consider_for_reuse(upload, post)).to eq(upload)
-        end
-      end
-    end
-  end
-
   describe '.update_secure_status' do
     it "respects the secure_override_value parameter if provided" do
       upload.update!(secure: true)