FEATURE: Extensible followup after asset replacement

[kitsunet]

Allows implementing custom handlers to be called after an asset resource was replaced. This allows decoupling redirects for the replaced asset from the media package.

Also handles some unchecked couplings to asset methods via new interfaces and cleans up a bit of code.

Fixes: #4815

[kitsunet]

Note this will need another PR to the redirect package to (re)implement adding the redirect there.

[dlubitz]

Thanks for the new interfaces, that makes the code much better!

I have two concerns about the change to the asset replacement:

1. I assume there was a reason, why the URIs have been built before the actual replacement. As you changed that, the RedirectHandler package has to create the URIs on his own (which is fine), but is it still capable to do that after the replacement?
2. I wonder how to resolve the "responsible" service for handling the asset replacements. Consider you are using the RedirectPackage (for content page rewrites) together with a cloud asset package. Both would provide a service implementing the AssetResourceReplacementFollowUpInterface.

[kitsunet]

Yep good points both, the resources do still exist until we persist, so creating the URIs and redirects works fine (I have the actual handler ready here, just not made a PR yet).

The repsonsibility issues is indeed one. I first thought we should make it configurable per resource target and whatnot, but it felt super overengineered for the one usecase we have for it right now. I suppose you will rarely (ever?) use multiple (cloud) storages at once, so IMHO any cloud storage implementation could just overwrite the interface via settings and refer back to the default redirect one if the resource target is not the cloud target.

BUT we could introduce a configuration and figure out the right one to call (or even a chain?) instead of just grabbing the interface from the objectManager. I am open to both, just wanted to get this part of the code out there for now :)

[dlubitz]

Thanks for clarifing.

I think ONE service is fine without a chain. But I assume we need some configuration to decide, which one is to use. OR we remove the asset redirect from the RedirectHandler package and provide a separate package, but this would make the integration for most use cases more complex.

[kitsunet]

As said, I think any specific service like we would deliver for our google cloud package could check if the redirect handler is installed and use that handler as fallback if the resource is not actually stored in google cloud. Would break down if you had S3 + google cloud + redirect handler installed, at that point one of the cloud ones would be left out, but I think that's fine? But yeah if we want this I woudl say we have a configuration with a default handler and a map of resource target class name to handler as an override of the default one.

...
  default: ...Redirect\AssetRedirectAfterReplacement
  'Flownative\GoogleCloudStorage\GcsTarget': Flownative\GoogleCloudStorage\GcsAssetRedirectHandler

[dlubitz]

Ahh, you already thining about configuration per asset source. I thought we are talking about the current object manager approach.

[kitsunet]

Just choosing one implementation can be done via Object.yaml by overwriting the class for the interface. That would already work.

[dlubitz]

But AFAIK this could lead to flaky behavior, as we can't specifiy a loading order of the packages

[kitsunet]

requirement order is loading order, BUT you can always choose for your project in a global Objects.yaml with how the code is right now.

[mhsdesign]

Thank you so much for cleaning this up and introducing the fitting interfaces ❤️

[mhsdesign]

It seems there is a little place of a doc comment already anticipated and i would very much like this.

[kdambekalns]

Regarding:

remove the asset redirect from the RedirectHandler package and provide a separate package, but this would make the integration for most use cases more complex.

This could even be chance to be less cofusing. Currently redirects for assets only work if you adjust the default rewrite rules, to actually pass requests for resources to PHP. If that feature was moved to a separate package, maybe more people would actually read the documentation. 😇