Duplicate a record with file using deterministic location

Hey!

I’m currently in the process of upgrading shrine from 2 to 3 and I am currently stuck at copying records.

I’m using shrine 3.4.0 with Rails 6.0.

Previously, I was able to copy records like this using the copy plugin.

duplicate = dup
# Exclude the current attachment data, otherwise the original will be deleted
# See https://github.com/shrinerb/shrine/issues/237#issuecomment-373007093
duplicate.file_data = nil
# With validation, we can't save because we validate that an image is present.
# Since we just deleted the `file_data`, this is no longer the case, so we
# disable validation during this save operation.
duplicate.save(validate: false)
duplicate.file_attacher.copy(file_attacher)

The upgrade guide suggests replacing the last line of code with:

attacher.set attacher.upload(other_attacher.file)
attacher.add_derivatives other_attacher.derivatives # if using derivatives 

Similar code can be found in another topic (“Duplicate A Record With An Image”, sorry, I’m a new user and am not allowed to post links), however it’s a little different since it use the original attacher in .set instead of the duplicated one.

duplication = self.dup
duplication.image_attacher.set image_attacher.upload(image)
duplication.save

My problem however is as follows: When trying to use any of these two versions, I get an argument error:

ArgumentError: same file: /path/to/original /path/to/duplicate

That’s because we use our own plugin to generate a location, which is deterministic and – as of now – depends on the record.id. And herein lies the problem, the attacher of the duplicated record points to the old record still.

# config/initializers/shrine.rb
# We use FileSystem as storage, so we have this in our initializer.
# Will be relevant later, since we used to use the `moving` plugin.
Shrine.plugin :upload_options, cache: { move: true }, store: { move: true }
# app/uploaders/image_uploader.rb
class ImageUploader < Shrine
...
  plugin :derivatives,
         create_on_promote:      true, # automatically create derivatives on promotion
         versions_compatibility: true  # handle versions column format
...
end
# lib/shrine/plugins/my_plugin.rb
class Shrine
  module Plugins
    module MyPlugin
    ...
      # _Heavily_ simplified, there's error handling, we check if `record` is `nil`,
      # don't hardcode the extension etc.
      def generate_location(io, record: nil, name: nil, **options)
        "#{name}-#{record.id}.jpg"
      end
    ...
  end
end

Code to reproduce:

company.id
=> 42
company.logo_attacher.record.id
=> 42

duplicate = company.dup

duplicate.id
=> nil
duplicate.logo_attacher.record.id
=> 42

# Tried setting `logo_data` to `nil` which didn't work.
# duplicate.logo_data = nil
# duplicate.save(validate: false)

duplicate.save

# Saved the duplicate, it has an ID now
duplicate.id
=> 43

# Still `42` after reloading
duplicate.reload.logo_attacher.record.id
=> 42

# This works
duplicate = Company.find(duplicate.id)
duplicate.logo_attacher.record.id
=> 43

# Original file still exists
company.logo.exists?
=> true

## Initially, I forgot to set `upload_options: { move: false }`

# Duplicating, using the code from the migration guide
duplicate.logo_attacher.set duplicate.logo_attacher.upload(company.logo)
duplicate.logo_attacher.add_derivatives company.logo_attacher.derivatives

# Original file is gone
company.logo.exists?
=> false

# Thumbnail is gone
company.logo(:thumb).exists?
=> false

## Using `upload_options: { move: false }`

duplicate.logo_attacher.set duplicate.logo_attacher.upload(company.logo, upload_options: { move: false }) 
duplicate.logo_attacher.add_derivatives company.logo_attacher.derivatives, upload_options: { move: false }

# Original file still exists
company.logo.exists?
=> true

# Original thumbnail still exists
company.logo(:thumb).exists?

So, in the end, I came up with the following code:

# /app/models/company.rb
def copy_with_attachment
  duplicate = dup
  duplicate.save
  duplicate = self.class.find(duplicate.id)
  duplicate.logo_attacher.set(duplicate.logo_attacher.upload(logo_attacher.file, upload_options: { move: false }))
  duplicate.logo_attacher.add_derivatives(logo_attacher.derivatives, upload_options: { move: false })
  duplicate.save
end

Which left me wondering if there is a way to duplicate a record and still have this work without having to .find the duplicate and possibly without having to specify upload_options? I don’t mind the last part that much, but I guess I’m still thinking in the implicit ways that shrine v2 did things using the moving and copy plugin.

But even now that things are explicit, it feels like a lot of code to “just” copy a record which includes a file. Especially since it seems I have to .save the record quite often and cannot keep a not-persistent record around until I need to save it.

Hi, thanks for the detailed report :pray:

Yeah, the main problem is that the attacher of a duplicated model instance keeps pointing to the original model instance. That’s definitely a bug.

Regarding the rest, based on what you’ve described, I’m reconsidering adding the copy plugin back in a lighter version of just providing Attacher#copy, and an :upload_options plugin option for passing options to the uploader. But having to write out the code definitely wouldn’t be as bad if copying the model instance worked correctly.

Regarding not being able to post links as a new user, I really don’t know how to fix that, because I’ve already updated the settings which were supposed to fix that :man_shrugging:

It shows that your trust level is “basic user”, which is one level above “new user”.

I’ve just pushed a fix for the duplicated attacher referencing the old model instance. I might release a new patch version soon.

Thank you for the timely reply! I wasn’t sure if it was a bug, since I am still making my way through the docs (some more hidden than others). I’ll be sure to try out the master branch during this week to see if I can reduce the amount of code needed. I guess if this was part of a “recipe” somewhere it might also help others.

I think I might have accidentally found another way of copying files, but I had a long pry session open and I might just have had a bad state in there. I’ll check it out again this week.

As for the basic user level, I got the new level after liking a post in another discussion. Guess I should have done that before. Thanks for checking that out as well!

@janko, thank you again. I’ve just tried the latest commit of the repo and was able to reduce the code to:

copy = dup
copy.save # Need to save the copy so a record ID exists for the filename.
copy.file_attacher.set(copy.file_attacher.upload(file_attacher.file, upload_options: { move: false }))
copy.file_attacher.add_derivatives(file_attacher.derivatives, upload_options: { move: false })

to get it working for our needs.

Since I’m still left with a .save call (since we require a record ID to exist so it can be in the filename. Might be a stupid requirement but a requirement nonetheless), I was wondering if – save for a potential copy plugin – it wouldn’t make sense for our specific use case to just .download the old attachment and use that temp file to create the duplicate?

I haven’t tried yet, but I believe it would save us from having to .save before copying the files and also, since we’re using automatic promotion, save us the call to .add_derivatives. I assume they’d be regenerated, but that’s a sacrifice we’d be willing to make as it takes very little time in our case and we don’t need derivatives to be exact duplicates in case a new libvips version did not produce the exact same thumbnails anymore.

A second thought occurred, but I haven’t found any APIs for it yet (still digging through the docs). Is there a way to have shrine regenerate the path after saving the record, so the record ID exists? Ideally not at the callers side.

I realize this is a very specific use case and I’m not asking for shrine to provide this out of the box, but while the way you described works perfectly, it doesn’t quite work for use because of the need for the record ID and the current way does not go through the same flow as a newly created record would.

Thank you again, I’ve learned a lot about shrine 3 this way already! :slight_smile: