Model sub-classes, with Uploader sub-classes

So I have an app that does some pretty complex/generic file handling – so Shrine’s flexibility is nice.

I end up with a model inheritance hieararchy. This isn’t exactly from my app, but gets the point across, there might be models like:

class FileObject  < ApplicationRecord
   include FileUploader::Attachment.new(:bytestream)
end

class Audio < FileObject ; end
class Image < FileObject ; end
class ImageThumbnail < Image ; end

I happen to be using Rails/ActiveRecord, with “single-table inheritance”, but I think this kind of model inheritance hieararchy is a pretty reasonable thing to do regardless of any/no ORM, it’s a kind of classic inheritance example. And it’s working out well for me.

OK, but. I’d really like the different sub-classes to have slightly different attachment configuration. For instance, there might be different metadata extraction for different file types, as a clear example. There also could be different validations, or different derivatives, etc.

Shrine already supports sub-classing Shrine/Uploader classes, that works out pretty well. To be good OO citizens and respect the Liskoff substitution principle we should probably make our extra uploaders have a matching inheritance hieararchy, no problem, although that isn’t necessarily important for the discussion, let’s just assume it.

class FileUploader < Shrine; ... ; end
class ImageUploader < FileUploader ; ... ; end
class ImageThumbnailUploader < ImageUploader ; ... ; end

OK, great, and maybe ImageUploader has additional add_metadata to add on to what FileUploader had, etc. Works great.

And I’d like my Image class to use the ImageUploader for the same column that FileObject uses the FileUploader. HERE’s where we run into some trouble.

We can try just including the shrine attachment module again…

class FileObject  < ApplicationRecord
   include FileUploader::Attachment.new(:bytestream)
end

class Image < FileObject
   include ImageUploader::Attachment.new(:bytestream)
end

The problem here is that this “double module inclusion” has some odd effects. Now the Image class has in it’s ancestors an ImageUploader::Attachment and also later a FileUploader::Attachment.

All the ActiveRecord callbacks end up called double: before_save, after_commit, all get registered twice. It doesn’t seem to cause any problems (they are all idempotent I guess), but it makes me nervous that there might be edge cases where it would, or performance implications where it may sometimes actually be doing a DB query double etc. Similar reload gets overridden twice, so will do it’s override code twice on every call. None of this might cause problems at least currently, but it seems kind of fragile and convoluted to me.

There is one aspect that DOES cause problems, the AR validation block is also registered twice, so if you have any shrine validations, you get double error messages for them all. :frowning:

We could try doing something crazy like having the ImageUploader sub-class re-include the activerecord plugin without validations… plugin :activerecord, validations: false. That sort of solves that problem, but do we have the activerecord plugin included twice still doubling up other methods? callbacks: false too maybe.

But this is all just seeming very hacky and fragile, you know?

Solutions?

What if we had a way to change the Attachment class without actually including another module in the ancestor chain.

Looking at the code, I think there are actually only two places that reference a particular Attachment class.

The (in this case) bytestream_attacher instance method on the model – everything else apepars to refer to/call that to get an attacher, doesn’t assume the attachment class itself. The implementation is defined in the entity plugin

AND the bytestream_attacher class method, so for instance as Image.bytestream_attacher. Which also seems to be implemented in the entity plugin

So what if we just override those methods in the model sub-class to call the Uploader sub-class we want?

class Image < FileObject
  def self.bytestream_attacher(**options)
    ImageUploader::Attachment.new(:file).send(:class_attacher, **options)
  end

  def bytestream_attacher(**options)
    ImageUploader::Attachment.new(:file).send(:attacher, self, **options)
  end

This APPEARS to work. Our model sub-class uses our uploader sub-class. No need to double-include a module, no double-include of any callbacks, or double-definition of methods in ancestors, etc.

Does it seem like a safe reliable thing to do? Is it a bad idea? Good idea?

Should this be “officially” supported by shrine in any way, with any kind of doc or test?

Note: it could be slightly more DRY

What if we didn’t have to override two methods, but only one?

If this line in the model plugin’s #attacher method were changed from attacher = class_attacher(**options) to instead be:

attacher = record.class.send("#{@name}_attacher", **options)

Then you only have to override the class method in the model subclass, the instance method will automatically use it.

(You’d have to do something similar in the entity plugin for uploaders that only use entity not model, since they have different attacher implementations).

But overriding two methods in such a relatively simple way doesn’t seem so bad.