shrine: backgrounding plugin breaks manual call of `promote`

You may sometimes want to manually call promote, and you ought to be able to. One use case is if the ordinary promotion failed for some reason, perhaps a reason related to external environment; maybe a disk filled up. Now the model is an unpromoted state and will never change unless you promote it; maybe you want to manually call promote to do so.

Backgrounding changes the nature of Attacher#promote, so it no longer works when called without any arguments. Worse, it fails silently leaving you no way to know it did not actually save the record with promoted data (file on store, any processing or metadata extraction you have configured to happen during promotion), etc.

This confused me greatly and took me all day to figure out, so I figured I’d report it!

To verify, we can look at an existing attacher_test, and just copy it the backgrounding_test, to be tested in the context of backgrounding. If you run that “#promote uploads the cached file to store” test with backgrounding loaded – it will fail. I can supply a branch or PR with this failing test if this isn’t clear.

Why? Well, that’s what took me a while to figure out.

A) In a plain shrine uploader, on a model with an attachment, if you have configured things for refresh_metadata on :store (as is one suggested way to do things with backgrounding), calling promote will trigger metadata extraction, which will result in a mutated in-memory model. The original in-memory model now has that new extracted metadata. But then promotion goes on to call update, with, which the activerecord (or other ORM) plugin, just saves it to disk, along with that new metadata.

B) The backgrounding plugin, however, will silently refuse to save it to disk. Because it looks at the changed in-memory data, and sees that the thing stored in the db does not match, and therefore decides to abort promotion entirely, in it’s overridden swap

C) So how does backgrounding normally work at all? Well, backgrounding only breaks the no-argument use of promote – or specifically, no first argument given, so the default value is used, calling method get. But ordinarily under backgrounding, a no-arg promote is never called. Instead backgrounding calls promote with an explicit first arg, deserialized from a bg job argument. Since the explicit arg was created by deserializing some JSON, when passing it, refresh/extract_metadata during storage does not mutate the in-memory model, so it passes backgrounding’s check in overridden “swap”.

So it only breaks promote manually called with no arguments, whcih doesn’t ordinarily happen in backgrounding “happy path”.

I suppose this could be considered not a bug – why would you want to call promote manually with no args? I think I’ve given a use case, but also it’s just confusing if backgrounding changes the semantics of existing methods (to make them stop working when they used to work). And led to me spending a day figuring out why things weren’t working like I expected – in this case in some of my local test setup code, but still. Adding a plugin probably shouldn’t change the semantics of an existing method, to make something that would have worked fine before, now fail silently. Such gotchas make shrine seem a lot less predictable/reliable.