Add file sync task

[bergice]

This task is intended to fix up the filesystem/db in version 4.

It will remove invalid db entries and create db entries for assets that are currently missing db entries.

[maxime-rainville]

Just did a high level code review. Didn't try running the code locally.

My main concern is that I think we should rely on the native FlySystem implementation we have in the Asset Store rather than rely on native PHP function. I would rather that part be fixed before spending more time testing it locally.

Also, a point to note is that some of the work I'm doing for the SS44 permalink job will probably help some of the things you are doing. I'm created generic classes that will be splitting up the URLs.

Reviewed

Reviewed.

[maxime-rainville]

Just did a deeper code review this time. There's still a lot of native file system code used, maybe confirm that there's no easy equivalent to flysystem for those.

The unit test looks very light. It's not testing with legacy_flenames and keep_archive_assets flags. I'll have a go at running it for real this afternoon.

[bergice]

Just did a deeper code review this time. There's still a lot of native file system code used, maybe confirm that there's no easy equivalent to flysystem for those.

The unit test looks very light. It's not testing with legacy_flenames and keep_archive_assets flags. I'll have a go at running it for real this afternoon.

I've added a couple of more tests for this now.

Looks like the namespaces for permalinks needs to be fixed up though:

There was 1 error:

1) SilverStripe\Assets\Tests\FilesystemSyncTaskTest::testSyncKeepArchivedAssets
SilverStripe\Core\Injector\InjectorNotFoundException: ReflectionException: Class SilverStripe\Assets\FilenameParsing\FileResolutionStrategy.public does not exist in /var/www/mysite/www/vendor/silverstripe/framework/src/Core/Injector/InjectionCreator.php:17

[sminnee]

I'm somewhat concerned about reintroducing this feature. We dropped it in the SS4 filesystem implementation because it was difficult to make robust and so caused follow on issues.

• If you're looking to update the database to re-list files that were previously uploaded to SilverStripe but the database records have been deleted, you risk data corruption. One example of why you might want to do this is if you're looking to restore filesystem assets but not the database. The source of corruption is that the IDs of the File records will be different, and so any other data objects that reference those files is going to be wrong. It would be more honest to say "if you want to restore assets you probably have to restore a database too".

• If you're looking to update the database to include files that were added to the assets/ folder out-of-band, I guess you're implementing a bulk upload feature. If that's the goal I question whether this is the best way of achieving this, especially for users who don't have FTP access to their servers.

So I'd ask the question: what problem are we trying to solve by re-adding this feature, and is this the best solution to the problem?

[sminnee]

If the goal is to bulk-add new assets to a locally-running project, something like this might be safer, simpler, and more useful:

vendor/bin/import-assets ~/Downloads/stuff-from-the-customer ./Uploads/StuffFromTheCustomer

Which would add the files to the filesystem underneath a root folder ./Uploads/StuffFromTheCustomer

[chillu]

Commented on the card: #175 (comment)

[emteknetnz]

This pull request hasn't had any activity for a while. Are you going to be doing further work on it, or would you prefer to close it now?

[maxime-rainville]

I think we decided to drop this approach in the end. I don't think there's anything that can be salvage from this PR.