moiristo / deep_cloneable

This gem gives every ActiveRecord::Base object the possibility to do a deep clone that includes user specified associations.
MIT License
786 stars 88 forks source link

Error: private method `open' called for #<ActiveStorage::Attached::Many #127

Open spacerobotTR opened 4 years ago

spacerobotTR commented 4 years ago

Not sure what I am doing wrong. I am using rails 6 and attempting to clone a record and it's attached uploads. Any ideas why this is generating this error?

Model:

class Compitem < ApplicationRecord
  belongs_to :user
  has_many_attached :uploads, dependent: :destroy
end

Copy method

  def copy
   @source = Compitem.find(params[:id])
   @source.deep_clone do |original, kopy|
      if kopy.is_a?(Compitem) && original.uploads.attached?
        original.uploads.open do |tempfile|
          kopy.uploads.attach({
            io: File.open(tempfile.path),
            filename: original.uploads.blob.filename,
            content_type: original.uploads.blob.content_type
          })
        end
      end
    end
   render 'new'
  end
moiristo commented 4 years ago

Hi, I'm actually not using activestorage myself, but as you have a many relatonship, I think you should just iterate over it, something like this:

  def copy
   @source = Compitem.find(params[:id])
   @source.deep_clone do |original, kopy|
      if kopy.is_a?(Compitem) && original.uploads.attached?
        original.uploads.each do |upload|
          upload.open do |tempfile|
            kopy.uploads.attach({
              io: File.open(tempfile.path),
              filename: upload.blob.filename,
              content_type: upload.blob.content_type
            })
          end
        end
      end
    end
   render 'new'
  end
moiristo commented 4 years ago

One more thing; I assume here that this is a simplified example. When you're not including associations in your copy, you might as well just use ActiveRecord's dup method.

spacerobotTR commented 4 years ago

Thanks for getting back to me. The above code works to clone the record, but I still do not see the file attachments linked to the new record created. I also do not see entries in the active storage tables in the DB. I do see these entries in the log:

08:00:24 web.1    |   ActiveStorage::Attachment Exists? (1.5ms)  SELECT 1 AS one FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_id" = $1 AND "active_storage_attachments"."record_type" = $2 AND "active_storage_attachments"."name" = $3 LIMIT $4  [["record_id", 19], ["record_type", "Compitem"], ["name", "uploads"], ["LIMIT", 1]]
08:00:24 web.1    |   ↳ app/controllers/compitems_controller.rb:23:in `block in copy'
08:00:24 web.1    |   ActiveStorage::Attachment Load (1.2ms)  SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_id" = $1 AND "active_storage_attachments"."record_type" = $2 AND "active_storage_attachments"."name" = $3  [["record_id", 19], ["record_type", "Compitem"], ["name", "uploads"]]
08:00:24 web.1    |   ↳ app/controllers/compitems_controller.rb:24:in `block in copy'
08:00:24 web.1    |   ActiveStorage::Blob Load (0.2ms)  SELECT "active_storage_blobs".* FROM "active_storage_blobs" WHERE "active_storage_blobs"."id" = $1 LIMIT $2  [["id", 2], ["LIMIT", 1]]
08:00:24 web.1    |   ↳ app/controllers/compitems_controller.rb:25:in `block (2 levels) in copy'
08:00:24 web.1    |   Disk Storage (0.6ms) Downloaded file from key: dgfcdm3dc4iqjeq9srryicysdcag
08:00:24 web.1    |   ActiveStorage::Blob Load (0.2ms)  SELECT "active_storage_blobs".* FROM "active_storage_blobs" WHERE "active_storage_blobs"."id" = $1 LIMIT $2  [["id", 3], ["LIMIT", 1]]
08:00:24 web.1    |   ↳ app/controllers/compitems_controller.rb:25:in `block (2 levels) in copy'
08:00:24 web.1    |   Disk Storage (0.4ms) Downloaded file from key: 2yvwmvl8lob04jrk36o0w7mdds5h

Anything else I am missing here?

Also yes I was using dup at first which produced the same results. The record was cloned fine, but the attachments were not.

moiristo commented 4 years ago

When you're not including any associations, this gem does exactly the same as dup, therefore I don't think you really need it. I think I would approach this by doing some debugging on the new record.

The ActiveStorage tests assign new blobs as follows. Looks about the same as my example, only my io object is a file IO instead of a StringIO. Maybe the IO object should be built differently? See also this comment, it might help. Could you let me know if you find out what the issue was?

moiristo commented 4 years ago

Looking at the code I would already rewrite it a little. Moreover, after reading this, I think using open might well be the issue, as the tempfile is closed after the block ends. So, I think download should be used (note that this does indeed download the whole thing to memory, be sure to have enough).

Resulting in this, I hope this works for you:

  def copy
   @source = Compitem.find(params[:id])
   @source.deep_clone do |original, kopy|
      if kopy.is_a?(Compitem) && original.uploads.attached?
        new_blobs = original.uploads.map do |upload|
          { io: StringIO.new(upload.download), filename: upload.filename, content_type: upload.content_type }
        end

        kopy.uploads.attach(*new_blobs)
      end
    end

   render 'new'
  end
spacerobotTR commented 4 years ago

Thanks for the help! I am playing around with this now.

On the above code I am getting the error: ActiveStorage::FileNotFoundError on { io: StringIO.new(upload.download), filename: upload.filename, content_type: upload.content_type }

moiristo commented 4 years ago

@spacerobotTR In that case it looks like one or more of the original uploads are already broken somehow?

spacerobotTR commented 4 years ago

I think I am close. It runs without error but now I am getting an error trying to display the form:

undefined method `errors' for nil:NilClass

<%= form_with(model: compitem, local: true) do |form| %>
  <% if compitem.errors.any? %>
moiristo commented 4 years ago

compitem should probably be @compitem? Or @source in the example.

spacerobotTR commented 4 years ago

The following goes through fine without error, clones the record, but still does not clone the attachments. I am using active storage that write files to <%= Rails.root.join("public/uploads") %>. Do I need to specify that path?

  def copy
   @source = Compitem.find(params[:id])
   @compitem = @source.deep_clone do |original, kopy|
      if kopy.is_a?(Compitem) && original.uploads.attached?
        new_blobs = original.uploads.map do |upload|
          { io: StringIO.new(upload.download), filename: upload.filename, content_type: upload.content_type }
        end

        kopy.uploads.attach(*new_blobs)
      end
    end

   render 'new'
  end
moiristo commented 4 years ago

I actually don't know how AR would be able to render copies in the form. Maybe you need to save the copy first and redirect to an edit form instead.

spacerobotTR commented 4 years ago

Down and dirty do you know how I would keep the files and just copy the records in active storage to point to the same files? Without actually creating new files?

moiristo commented 4 years ago

As far as I know, AS uses has_many through associations for the blobs. So in theory, you could just achieve that by calling @source.deep_clone(include: :uploads_blobs)

spacerobotTR commented 4 years ago

Strange. Same results. Record clones fine without error, but no attachment links.

moiristo commented 4 years ago

This also depends on how the form is rendered. You could also try to include via the direct association, for example @source.deep_clone(include: { uploads_attachments: :blob }). But I'm not sure. It is also a bit off-topic as it has not really anything to do with this gem..

spacerobotTR commented 4 years ago

Thanks I'll play around with it and see if I can get it to work. I appreciate the help! I know I went off topic on this one.

spacerobotTR commented 4 years ago

I got it working! Final answer. Had to save:

  def copy
   @source = Compitem.find(params[:id])
   @compitem = @source.deep_clone(include: :uploads_blobs)
   @compitem.save
   render 'new'
  end
dpaola2 commented 4 years ago

I'm having a similar issue that I'm wondering if you could shed some light on. I'm iterating over a collection and making a bunch of copies, like so:

def create_desks
    coordinates = [
      [0, 8],
      [0, 13],
      [5, 8],
      [5, 13],
      [10, 8],
      [10, 13]
    ]

    desk_prototype = Relic.find(73)
    coordinates.each do |coord|
      new_desk = desk_prototype.deep_clone include: [:sprite, :content], skip_missing_associations: true do |orig, kopy|
        if orig.sprite.attached?
          orig.sprite.open do |tempfile|
            kopy.sprite.attach(
              io: File.open(tempfile.path),
              filename: orig.sprite.blob.filename,
              content_type: orig.sprite.blob.content_type
            )
          end
        end

        if orig.image.attached?
          kopy.image.attach(
            io: File.open(tempfile.path),
            filename: orig.image.blob.filename,
            content_type: orig.image.blob.content_type
          )
        end  
      end
      new_desk.x = coord.first
      new_desk.y = coord.second
      new_desk.team = self
      new_desk.name = "Empty Desk"
      new_desk.save!
    end

  end

The issue: only the FIRST in the loop gets the attachments copied over. I've pulled my hair out and cannot for the life of me figure out why. Any ideas?

moiristo commented 4 years ago

You mean that only the first 'new_desk' with coordinates [0,8] gets the attachments and not the others? No clue actually :/

dpaola2 commented 4 years ago

Yes. It's bizarre.