FIX: Use correct version when generating file path for optimized image (#6871)

approved

#1

FIX: Use correct version when generating file path for optimized image (#6871)

diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb
index 9043205..2c8b416 100644
--- a/lib/file_store/base_store.rb
+++ b/lib/file_store/base_store.rb
@@ -116,7 +116,8 @@ module FileStore
 
     def get_path_for_optimized_image(optimized_image)
       upload = optimized_image.upload
-      extension = "_#{OptimizedImage::VERSION}_#{optimized_image.width}x#{optimized_image.height}#{optimized_image.extension}"
+      version = optimized_image.version || 1
+      extension = "_#{version}_#{optimized_image.width}x#{optimized_image.height}#{optimized_image.extension}"
       get_path_for("optimized".freeze, upload.id, upload.sha1, extension)
     end
 
diff --git a/spec/components/file_store/base_store_spec.rb b/spec/components/file_store/base_store_spec.rb
index 20d898e..3de0ff2 100644
--- a/spec/components/file_store/base_store_spec.rb
+++ b/spec/components/file_store/base_store_spec.rb
@@ -19,4 +19,19 @@ RSpec.describe FileStore::BaseStore do
       end
     end
   end
+
+  describe '#get_path_for_optimized_image' do
+    let(:upload) { Fabricate(:upload) }
+    let(:optimized_path) { "optimized/1X/#{upload.sha1}_1_100x200.png" }
+
+    it 'should return the right path' do
+      optimized = Fabricate(:optimized_image, upload: upload, version: 1)
+      expect(FileStore::BaseStore.new.get_path_for_optimized_image(optimized)).to eq(optimized_path)
+    end
+
+    it 'should return the right path for `nil` version' do
+      optimized = Fabricate(:optimized_image, upload: upload, version: nil)
+      expect(FileStore::BaseStore.new.get_path_for_optimized_image(optimized)).to eq(optimized_path)
+    end
+  end
 end
diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb
index 21f7776..e18a092 100644
--- a/spec/components/file_store/s3_store_spec.rb
+++ b/spec/components/file_store/s3_store_spec.rb
@@ -164,6 +164,24 @@ describe FileStore::S3Store do
         store.remove_upload(upload)
       end
 
+      it "removes the optimized image from s3 with the right paths" do
+        optimized = Fabricate(:optimized_image, version: 1)
+        upload = optimized.upload
+        path = "optimized/1X/#{upload.sha1}_#{optimized.version}_#{optimized.width}x#{optimized.height}.png"
+
+        store.expects(:get_depth_for).with(upload.id).returns(0)
+        s3_helper.expects(:s3_bucket).returns(s3_bucket).at_least_once
+        optimized.update_attributes!(url: "//s3-upload-bucket.s3.dualstack.us-west-1.amazonaws.com/#{path}")
+        s3_object = stub
+
+        s3_bucket.expects(:object).with("tombstone/#{path}").returns(s3_object)
+        s3_object.expects(:copy_from).with(copy_source: "s3-upload-bucket/#{path}")
+        s3_bucket.expects(:object).with(path).returns(s3_object)
+        s3_object.expects(:delete)
+
+        store.remove_optimized_image(optimized)
+      end
+
       describe "when s3_upload_bucket includes folders path" do
         before do
           SiteSetting.s3_upload_bucket = "s3-upload-bucket/discourse-uploads"
diff --git a/spec/fabricators/optimized_image_fabricator.rb b/spec/fabricators/optimized_image_fabricator.rb
index 30fafd2..2ac3b8a 100644
--- a/spec/fabricators/optimized_image_fabricator.rb
+++ b/spec/fabricators/optimized_image_fabricator.rb
@@ -5,4 +5,5 @@ Fabricator(:optimized_image) do
   width 100
   height 200
   url "138569_100x200.png"
+  version OptimizedImage::VERSION
 end

GitHub sha: f94c0283


#2

Excellent, this is a big one!


#3