Merge pull request #215 from carrierwaveuploader/carrierwave-3

Support CarrierWave 3.x

Author: rmm5t

.github/workflows/ci.yml

@@ -8,8 +8,8 @@ jobs:
         mongodb: [4.4]
         ruby: [2.7, "3.0", 3.1, 3.2, 3.3]
         gemfile:
-          - carrierwave-2.1
           - carrierwave-2.2
+          - carrierwave-3.1
           - mongoid-7
           - mongoid-8
           - mongoid-9
@@ -20,6 +20,8 @@ jobs:
           - { mongodb: "4.4", ruby: "2.6", gemfile: "carrierwave-1.2" }
           - { mongodb: "4.4", ruby: "2.6", gemfile: "carrierwave-1.3" }
           - { mongodb: "4.4", ruby: "2.6", gemfile: "carrierwave-2.0" }
+          - { mongodb: "4.4", ruby: "2.6", gemfile: "carrierwave-2.1" }
+          - { mongodb: "4.4", ruby: "2.6", gemfile: "carrierwave-3.0" }
           - { mongodb: "4.4", ruby: "2.6", gemfile: "mongoid-3" }
           - { mongodb: "4.4", ruby: "2.6", gemfile: "mongoid-4" }
           - { mongodb: "4.4", ruby: "2.6", gemfile: "mongoid-5" }
@@ -32,6 +34,8 @@ jobs:
         uses: supercharge/mongodb-github-action@1.3.0
         with:
           mongodb-version: ${{ matrix.mongodb }}
+      - name: Install ImageMagick
+        run: sudo apt-get -y install imagemagick
       - uses: actions/checkout@v4
       - name: Set up Ruby
         uses: ruby/setup-ruby@v1

.gitignore

@@ -3,6 +3,7 @@ pkg/*
 *.sw*
 .bundle
 spec/public
+spec/tmp
 .rvmrc
 .ruby-version
 Gemfile.lock

carrierwave-mongoid.gemspec

@@ -20,7 +20,7 @@ Gem::Specification.new do |s|
   s.executables   = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) }
   s.require_paths = ["lib"]
 
-  s.add_dependency "carrierwave", [">= 0.8", "< 3"]
+  s.add_dependency "carrierwave", [">= 0.8", "< 4"]
   s.add_dependency "mongoid", [">= 3.0", "< 10.0"]
   s.add_dependency "mongoid-grid_fs", [">= 1.3", "< 3.0"]
   s.add_dependency "mime-types", "< 3" if RUBY_VERSION < "2.0" # mime-types 3+ doesn't support ruby 1.9

gemfiles/carrierwave-3.0.gemfile

@@ -0,0 +1,5 @@
+source "https://rubygems.org"
+
+gem "carrierwave", "~> 3.0.0"
+
+gemspec path: "../"

gemfiles/carrierwave-3.1.gemfile

@@ -0,0 +1,5 @@
+source "https://rubygems.org"
+
+gem "carrierwave", "~> 3.1.0"
+
+gemspec path: "../"

lib/carrierwave/mongoid.rb

@@ -140,6 +140,7 @@ def self.model_name
 
       describe 'model#remove_uploaders=' do
         before do
+          model.images = [stub_file('test.jpg')]
           model.save
         end
 
@@ -208,7 +209,7 @@ def self.model_name
 
             before do
               model.save!
-              model.collection.update_one({ _id: model.id }, { images: identifiers })
+              model.collection.update_one({ _id: model.id }, '$set' => { images: identifiers })
             end
 
             it 'returns an array of uploaders' do
@@ -239,10 +240,6 @@ def self.model_name
                 expect(model.images.map(&:identifier)).to match_array(identifiers)
               end
 
-              it 'does not write anything to the database, in order to prevent overridden filenames to fail because of unassigned attributes' do
-                expect(model[:images]).to match_array([])
-              end
-
               it 'copies a file into into the cache directory' do
                 expect(model.images.first.current_path).to match(/^#{Regexp.escape(public_path('uploads/tmp'))}/)
               end
@@ -260,10 +257,6 @@ def self.model_name
                 expect(model.images.map(&:identifier)).to match_array(['portrait.jpg', 'test.jpeg'])
               end
 
-              it 'does not write anything to the database, in order to prevent overridden filenames to fail because of unassigned attributes' do
-                expect(model[:images]).to match_array(['portrait.jpg'])
-              end
-
               it 'copies a file into into the cache directory' do
                 expect(model.images.map(&:current_path)).to all(match(/^#{Regexp.escape(public_path('uploads/tmp'))}/))
               end
@@ -301,10 +294,6 @@ def self.model_name
             expect(model.images).to all(be_an_instance_of(uploader_class))
           end
 
-          it 'does not write to the database' do
-            expect(model[:images]).to be_empty
-          end
-
           it 'copies a file into into the cache directory' do
             expect(model.images.map(&:current_path)).to all(start_with(public_path('uploads/tmp')))
           end
@@ -393,15 +382,20 @@ def munge
 
         before do
           model_class.create!(images: files)
-        end
-
-        it 'replaced it by a file with the same name' do
           record.update!(images: [stub_file('test.jpeg')])
-
           record.reload
+        end
 
-          expect(record[:images]).to match_array(['test.jpeg'])
-          expect(record.images_identifiers).to match_array(['test.jpeg'])
+        if Gem::Version.new(CarrierWave::VERSION) >= Gem::Version.new("3.0.beta")
+          it "performs deduplication" do
+            expect(record[:images]).to match_array(['test(2).jpeg'])
+            expect(record.images_identifiers).to match_array(['test(2).jpeg'])
+          end
+        else
+          it 'replaced it by a file with the same name' do
+            expect(record[:images]).to match_array(['test.jpeg'])
+            expect(record.images_identifiers).to match_array(['test.jpeg'])
+          end
         end
       end
 
@@ -492,12 +486,14 @@ def munge
             expect(model.save).to be_truthy
             expect(File).to exist(public_path('uploads/new.jpeg'))
             expect(File).to exist(public_path('uploads/old.jpeg'))
+          ensure
+            uploader_class.remove_previously_stored_files_after_update = true
           end
 
-          it 'does not remove file if old file had the same path' do
+          it 'does not remove new file if both of files had the same path' do
             model.images = [stub_file('old.jpeg')]
             expect(model.save).to be_truthy
-            expect(File).to exist(public_path('uploads/old.jpeg'))
+            expect(File).to exist(model.images[0].path)
           end
 
           it 'does not remove file if validations fail on save' do
@@ -519,13 +515,13 @@ def filename
 
           let!(:model) { model_class.create!(name: 'Mike', images: [stub_file('old.jpeg')]) }
 
-          it 'does not remove file if old file had the same dynamic path' do
+          it 'does not remove new file if both of files had the same path' do
             expect(File).to exist(public_path('uploads/Mike.jpeg'))
             expect(model.images.first.read).to eq 'this is stuff'
 
             model.update!(images: [stub_file('test.jpeg')])
 
-            expect(File).to exist(public_path('uploads/Mike.jpeg'))
+            expect(File).to exist(model.images[0].path)
           end
 
           it 'removes old file if old file had a different dynamic path' do
@@ -553,12 +549,14 @@ def filename
             expect(embedded_model.save).to be_truthy
             expect(File).to exist(public_path('uploads/new.jpeg'))
             expect(File).to exist(public_path('uploads/old.jpeg'))
+          ensure
+            uploader_class.remove_previously_stored_files_after_update = true
           end
 
-          it 'does not remove file if old file had the same path' do
+          it 'should not remove new file if both of files had the same path' do
             embedded_model.images = [stub_file('old.jpeg')]
             expect(embedded_model.save).to be_truthy
-            expect(File).to exist(public_path('uploads/old.jpeg'))
+            expect(File).to exist(embedded_model.images[0].path)
           end
 
           it 'does not remove file if validations fail on save' do
@@ -592,12 +590,14 @@ def filename
             expect(double_embedded_model.save).to be_truthy
             expect(File).to exist(public_path('uploads/new.jpeg'))
             expect(File).to exist(public_path('uploads/old.jpeg'))
+          ensure
+            uploader_class.remove_previously_stored_files_after_update = true
           end
 
-          it 'does not remove file if old file had the same path' do
+          it 'should not remove new file if both of files had the same path' do
             double_embedded_model.images = [stub_file('old.jpeg')]
             expect(double_embedded_model.save).to be_truthy
-            expect(File).to exist(public_path('uploads/old.jpeg'))
+            expect(File).to exist(double_embedded_model.images[0].path)
           end
 
           it 'does not remove file if validations fail on save' do
@@ -878,14 +878,14 @@ def filename
             expect(File).not_to exist(public_path('uploads/thumb_old.jpeg'))
           end
 
-          it 'does not remove file if old file had the same path' do
+          it 'does not remove new file if both of files had the same path' do
             expect(File).to exist(public_path('uploads/old.jpeg'))
             expect(File).to exist(public_path('uploads/thumb_old.jpeg'))
 
             model.update!(images: [stub_file('old.jpeg')])
 
-            expect(File).to exist(public_path('uploads/old.jpeg'))
-            expect(File).to exist(public_path('uploads/thumb_old.jpeg'))
+            expect(File).to exist(model.images[0].path)
+            expect(File).to exist(model.images[0].thumb.path)
           end
         end
 
@@ -926,25 +926,25 @@ def self.model_name
             expect(File).not_to exist(public_path('uploads/old.txt'))
           end
 
-          it 'removes old file1 but not file2 if old file1 had a different path but old file2 has the same path' do
+          it 'removes old file1 but not new file2 if old file1 had a different path but old file2 has the same path' do
             expect(File).to exist(public_path('uploads/old.jpeg'))
             expect(File).to exist(public_path('uploads/old.txt'))
 
             model.update!(images: [stub_file('new.jpeg')], textfiles: [stub_file('old.txt')])
 
             expect(File).to exist(public_path('uploads/new.jpeg'))
             expect(File).not_to exist(public_path('uploads/old.jpeg'))
-            expect(File).to exist(public_path('uploads/old.txt'))
+            expect(File).to exist(model.textfiles[0].path)
           end
 
-          it 'does not remove file1 or file2 if file1 and file2 have the same paths' do
+          it 'does not remove new files if each pair of files has the same paths' do
             expect(File).to exist(public_path('uploads/old.jpeg'))
             expect(File).to exist(public_path('uploads/old.txt'))
 
             model.update!(images: [stub_file('old.jpeg')], textfiles: [stub_file('old.txt')])
 
-            expect(File).to exist(public_path('uploads/old.jpeg'))
-            expect(File).to exist(public_path('uploads/old.txt'))
+            expect(File).to exist(model.images[0].path)
+            expect(File).to exist(model.textfiles[0].path)
           end
         end
 
@@ -981,12 +981,12 @@ def self.model_name
             expect(File).not_to exist(public_path('uploads/old.jpeg'))
           end
 
-          it 'does not remove file if old file had the same path' do
+          it 'does not remove new file if both of files had the same path' do
             expect(File).to exist(public_path('uploads/old.jpeg'))
 
             model.update!(avatars: [stub_file('old.jpeg')])
 
-            expect(File).to exist(public_path('uploads/old.jpeg'))
+            expect(File).to exist(model.avatars[0].path)
           end
         end
       end