NoMethodError after upgrading to 0.3.0

[rbclark]

Hello,

My test suite is failing after upgrading to version 0.3.0 of this gem. It seems something goes wrong every time I try to save an object which has a postgresql_lo uploader mounted on it. The error is

NoMethodError: undefined method `path' for 26980:Integer
from /gempath/2.5.0/gems/carrierwave-1.2.2/lib/carrierwave/mounter.rb:146:in `block in remove_previous'

This did not previously happen with the 0.2.0 version of the gem. Let me know if there's anything else I can do to help debug and fix this, I'd imagine it is just a breaking change in the upgraded version of carrierwave.

[diogob]

@rbclark it seems that our test suite is missing something big. Would you be able to provide a spec that breaks under the new version?

[nononoy]

hi. got this errror too after bundle update. Just on instance update.

NoMethodError: undefined method `path' for 7873066:Integer

steps to reproduce

2.4.4 :017 > d = Document.new(title: 'title', pdf: 'pdf')
2.4.4 :018 > d.save!
 => true 
2.4.4 :019 > d.pdf = 'sample'
 => "sample" 
2.4.4 :020 > d.save!
   (0.6ms)  BEGIN
  SQL (0.9ms)  UPDATE "documents" SET "pdf" = $1, "updated_at" = $2 WHERE "documents"."id" = $3  [["pdf", 7873068], ["updated_at", "2018-07-27 12:11:44.459326"], ["id", 3]]
   (3.3ms)  COMMIT
NoMethodError: undefined method `path' for 7873068:Integer
        from (irb):20

Rails 5.1, Ruby 2.4.4

    carrierwave (1.2.3)
      activemodel (>= 4.0.0)
      activesupport (>= 4.0.0)
      mime-types (>= 1.16)
    carrierwave-aws (1.3.0)
      aws-sdk-s3 (~> 1.0)
      carrierwave (>= 0.7, < 2.0)
    carrierwave-postgresql (0.3.0)
      carrierwave (>= 0.10.0)

[nononoy]

i figured it out. DB field should be only string, not oid or integer. You can close the issue
Link to source

[rbclark]

It seems in my case the problem is due to rails fixtures. With the old version of this gem I was able to declare my fixtures without including my file fields at all and everything worked fine. Now with 0.3.0 any attempts to actually save my object to the database result in a undefined method `path' for 26980:Integer error. I've been stepping line by line though and I see the problem is basically due to a change in how remove_previous used to work, it seems to fix all of my problems if I change this line: https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/mounter.rb#L127 in carrierwave proper to return if before.blank? which seems more logical than return unless before since blank will check for nil and empty array. I have created a PR against regular carrierwave at carrierwaveuploader/carrierwave#2331 which will hopefully fix this issue for me, unless there's something else I missed that is causing this to happen in the carrierwave-postgresql gem.

[nononoy]

Even with your fix it's actually do not working. Checked on your patch-1 pull request carrierwave version. Problems with oid column type.

[rbclark]

@nononoy Is this happening in test fixtures for you or somewhere else? I have mainly seen the problem in my test fixtures I believe however if you are seeing it somewhere else it may be helpful for the maintainers to know.

[nononoy]

This is happening im my app during instance update, not during tests.

t.oid "front_image", null: false

----

NoMethodError (undefined method `path' for 8328440:Integer):

So version 0.3.0 only works with string table columns. It's not working even with your PR to carrierwave gem.
Downgrading to 0.2.0 helps to fix this

[rbclark]

@nononoy Is the undefined method error on the same line for you as it was for me?

[aronwolf90]

I have found the problem. The problem is that on carrierwave, they use saved_changes for rails 5.1 and highter (instead the deprectated changed method). This, Instead returning the casted typed, returns the orignal types of the database. This is the reason because they added the if value,is_a?(String)` condition, that is causing the problem.

Changing if value,is_a?(String) condition. to if value.respond_to?(:path) condition, solve the problem. I will submit a pull request on carrierwave. Until this, you can use this monkey path:

module CarrierWave
  class Mounter
    def remove_previous(before=nil, after=nil)
      after ||= []
      return unless before

      # both 'before' and 'after' can be string when 'mount_on' option is set
      before = before.reject(&:blank?).map do |value|
        if value.respond_to?(:path)
          value
        else
          uploader = blank_uploader
          uploader.retrieve_from_store!(value)
          uploader
        end
      end
      after_paths = after.reject(&:blank?).map do |value|
        if value.respond_to?(:path)
          value
        else
          uploader = blank_uploader
          uploader.retrieve_from_store!(value)
          uploader
        end.path
      end
      before.each do |uploader|
        if uploader.remove_previously_stored_files_after_update and not after_paths.include?(uploader.path)
          uploader.remove!
        end
      end
    end
  end
end

[shtirlic]

Thank you, monkey patching helped.

[shlima]

I've fix this with the following patch:

CarrierWave::Mounter.prepend(Module.new do
  def remove_previous(before = nil, after = nil)
    before &&= Array.new(before).map(&:to_s)
    after &&= Array.new(after).map(&:to_s)
    super(before, after)
  end
end)