Allow to store all files in storage

[qwerty287]

Change storage paths etc. to only require the storage directory to be writable to the web user by default.

Especially migrating them is a bit special and might result in various issues.

public/{uploads,sym} is now in storage/app/public/{uploads,sym}, user.css is in storage/app/public/user.css. These files are linked with a symbolic link to public/storage/ which is the recommended way of Laravel.

TODO:

• new path for uploads, sym, user.css
• rename old files
• link bootstrap/cache to storage
• change default sqlite location to storage/app/database.sqlite

Advantages

This allows you to only make storage/ writable to the web user which enhances security. In addition, it allows you to easily backup the whole instance: copy storage/app/. Currently, you have to copy multiple paths which makes this pretty hard.

Breaking: the DB path changed, and this must be renamed by the user. Otherwise Laravel won't find the new database file and can't migrate the old one. So, the release notes should contains something like: If you're using SQLite with an empty DB_DATABASE configuration (using default values), please run the following command in your shell to move the database file to the new default location: mv database/database.sqlite storage/app/database.sqlite

[codecov]

Codecov Report

Merging #1650 (8bafa8f) into master (ecef28a) will decrease coverage by 0.22%.
The diff coverage is 100.00%.

Additional details and impacted files

[d7415]

Especially migrating them is a bit special and might result in various issues.

Yes. This is likely to break a lot of installations.

This allows you to only make storage/ writable to the web user which enhances security.

This is plainly false, though I get your point.

If you're using SQLite with an empty DB_DATABASE configuration (using default values), please run the following command in your shell to move the database file to the new default location: mv database/database.sqlite storage/app/database.sqlite

I'm not convinced a shell command is a good instruction here. I'd just say "move..."

But yeah, my main concern here is breaking lots. Well, that and cluttering up the Docker scripts with things that are only needed once but will need to be run every time because we don't know the state. It feels like a very small improvement for new installations at the cost of a high risk operation (admittedly of breakage, rather than loss) for all existing users.

[qwerty287]

Especially migrating them is a bit special and might result in various issues.

Yes. This is likely to break a lot of installations.

I actually tried to prevent issues, this includes a migration which moves the files. I wasn't sure if there are special setups like S3 storage or something like this which was the main reason for me to write this.

In general, I definitely understand your concerns. My main issue is that Lychee stores everything somewhere else. Maybe, a good idea would be some kind of option in .env that allows you to set all paths to storage by default. This is easy to do and will not break installations: if you want your data in storage, you can enable the option, otherwise, the old paths are used. If you want to migrate, you can move the files and enable the option. I mainly want to make it more consistent where data is stored. Some parts are easy to change here (e.g. database path), others are pretty complex (e.g. user.css).
What do you think about an option in .env that changes all storage paths to the locations I suggested in this PR, but keeps the old ones if it's not enabled?

What you would mainly need to change to support this are symlinks to storage and changed paths to the DB and the filesystem disks.

[d7415]

What do you think about an option in .env that changes all storage paths to the locations I suggested in this PR, but keeps the old ones if it's not enabled?

That sounds messier. if we're going to do it, we should just do it, but we need to accept that it's a big change, likely a breaking change and needs to be handled accordingly (including being sure that we're not going to do this again). And it needs to be worth it.

I think it's probably a worthwhile change to simplify things, I'm just not sure it's worth the upheaval. I'd be interested how the others feel.

[ildyria]

I am not convinced by that change. :( Backup currently is basically just the upload folder in public and the database.
Furthermore why storage/app/public instead of just... storage/public ? :)

[d7415]

Furthermore why storage/app/public instead of just... storage/public ? :)

I think that was so that all of the backup items (photos, db, etc) were in one place away from unimportant Laravel stuff in storage.

[qwerty287]

Yes, also storage/app/ is Laravel's default. storage/app/public is for public stuff which is symlinked to the public folder.

As for the backup: this works as long as you don't use the public/sym folder or user.css. They must be backed up separately, and then we already have 4 folders/files you need to store.

[ildyria]

As for the backup: this works as long as you don't use the public/sym folder.

I would not back up those. Does not make much sense, and you can just regenerate them on first use, just need to clean the DB on sym_links. :)

[qwerty287]

I see that this will be a very problematic change, so let me suggest a simpler changing only a few files and being fully optional:

1. let git ignore public/storage to allow using the artisan storage:link command
2. allow changing storage locations of user.css and public/sym
3. symlink bootstrap/cache to storage/cache

This will make all moving of files to storage optional while being able to still store everything there. In addition, it's a clean solution.

[ildyria]

Quickly tested. LGTM.