neos / flow-development-collection

The unified repository containing the Flow core packages, used for Flow development.
https://flow.neos.io/
MIT License
137 stars 189 forks source link

TASK: Cleanup illegal dependency from Flow to Neos.Media #3272

Open mhsdesign opened 8 months ago

mhsdesign commented 8 months ago

The resource:clean command is intertwined with Neos.Media's AssetRepository and ThumbnailRepository. This is hard to type for phpstan and generally unpleasant to look at :D

Flow 9 should provide some kind of extensibility so that Neos.Media can hook in and do its job.

https://github.com/neos/flow-development-collection/blob/5dd18f11155e2701951002f328bd8184fa2849d7/Neos.Flow/Classes/Command/ResourceCommandController.php#L243

mhsdesign commented 8 months ago

But that doesnt turn out to be thaaat straight forward, at least not with ugly and hard to understand hooks/interfaces.

The functionality is as follows:

If the Neos.Media package is active, this command will also detect any assets referring to broken resources and will remove the respective Asset object from the database when the broken resource is removed.

This part is very transparent, also the related assets will be shown when one is asked to delete a persistent resource, and after a deletion of a single resource it is shown how many assest were also purged:

Removed %s asset object(s) from the database.

A similar hook is in place for the thumbnails, but its a little less obvious, as its not documented in the command and only a deletion with related thumbnails will show:

Removed %s thumbnail object(s) from the database.

Im not sure what the best solution to approach this is, and if we can leverage doctrine lifetime cycle's or the like ...

kitsunet commented 8 months ago

Can't we simply introduce a signal ala PersistentResourceNoLongerExists with the object? then an implementation in the media package can take care of handling anything on that signal

mhsdesign commented 8 months ago

yes sure, buuut we wont be able to log it to the console no?

image
kitsunet commented 8 months ago

There is ways I guess, but nothing too beautiful indeed. We could pass the output to the signal but that would bind us to CLI there, or equally dirty but a bit more standalone-ish, we pass somehting like a Messages object and the slot code just writes into that and we read the messages after the signal

$messages = new Messages();
$this->emitPersistentResourceNotLongerExists($persistentResource, $messages);
$this->output($messages->toString());

pseudo code but you get my drift....

Alternatively, what if we defined an interface for "plugins" for this, that defines a method that gets a resource and can return a message of some shape and don't define a signal but just go over all implementations of this interface and call the method.

mhsdesign commented 8 months ago

Hmm yes sure, but to 100% reflect the current state we need a "beforePossibleRemove" hook as well, to display the related assets that would possibly be removed.

I think that we should rather have a specialised command sitting in neos.media and doing this job?

I fear all those signals will make the code less easy to grasp. Now its at least all in one place.

kitsunet commented 8 months ago

separate command is perfeclty fine for me

mhsdesign commented 8 months ago

so

flow resource:clean

will only remove the persistent resources, but ignore all assets, that still might be attached to them.

And

flow media:cleanresources (inspired by media:importresources) will remove the persistent resources AND keeping assets in mind.?

So using the original flow resource:clean would of course cause a little confusion in the migration period, we should probably warn them like this:

flow's resource:clean is only safe to be used in a pure flow installation without Neos.Media. To also cleanup assets you must use media:cleanresources. If you still want to continue and only want to cleanup resources press Y.

Aaand additionally the media cleanup could also cleanup orphaned assets, if the resource was removed via flow's resource:clean.