Disable ext storage provider app must keep mount point

[PVince81]

Steps

1. Setup WND from EE (or any other app that provides a custom external storage backend/storage)
2. Setup a mount point "/mount" with WND (or the other storage)
3. Access the mount point "/mount" and see that files appear
4. Disable the WND app (or other app)
5. Access the mount point again

Expected result

Mount point exists but returns "storage not available" when accessing or syncing.

Actual result

Mount point disappears completely. This would cause sync clients to delete the local files for every user in the event that an app got disabled by mistake. This can happen also at update time where third party apps get disabled.

To avoid this kind of needless data loss through syncing, the mount point must stay visible but any access to it must throw StorageNotAvailableException.

Versions

Tested on 9.2 prealpha / git master.

Since we want to move many storage backends from files_external to separate apps, the probability that one of the separate app is not enabled on time gets higher and people might see their mount points disappear. Fixing this issue would help ease the migration in case the admin has not enabled the separate apps yet.

medium because there is a workaround: set OC to maintenance mode after an update and make sure to enable the missing storage apps before exiting maintenance mode.

@owncloud/qa @DeepDiver1975 @butonic

[PVince81]

@owncloud/filesystem

[PVince81]

Looks like the storage config even disappears from the UI, not good.

This is tricky, because the code expects a StorageConfig to be created in StoragesService::getStorageConfigFromDBMount() but if one of Backend or AuthMechanism cannot be instantiated, it discards the config.

Some ideas:

• return the StorageConfig but with null as backend and auth mechanism
• return the StorageConfig with a dummy FailedBackend and FailedAuthMechanism instances
• in any of these cases, display the config in the external storage admin page but make it read-only and deletable only.

[jvillafanez]

To give a second thought, if I disable and remove the app on purpose, the files that are in the desktop client will stay even though they shouldn't be available (probably) and it's possible to keep trying to upload files there. There will be leftovers everywhere (files in the desktop client, unused data in the filecache, unused data in mobiles...)

If we're talking about upgrades, the server should return a proper error if anyone tries to access the file, something like "ownCloudUpgradingException", so clients know the server is being upgraded and can respond accordingly.

There is the additional topic of what to do if the app is incompatible with the new version...

The problem of using the "StorageNotAvailableException" is that the clients won't know what is the real reason of the error: is there a temporal network failure between ownCloud and the backend? did the connector disappear? is ownCloud being updated?

If there is a temporal network failure, clients can try another folder, but if ownCloud is being updated is pointless to keep syncing.

[butonic]

Maybe using the Retry-After header makes sense. Hm we could use it to tell the clients how often to do a PROPFIND. Even dynamically based on server load. And I agree that something like a 'MaintenanceException' in a 503 Service Unavailable with the Retry After header makes sense.

[PVince81]

To give a second thought, if I disable and remove the app on purpose, the files that are in the desktop client will stay even though they shouldn't be available (probably) and it's possible to keep trying to upload files there. There will be leftovers everywhere (files in the desktop client, unused data in the filecache, unused data in mobiles...)

If that's what the admin decided, then the admin can also manually delete the matching storages from the config.

If we're talking about upgrades, the server should return a proper error if anyone tries to access the file, something like "ownCloudUpgradingException", so clients know the server is being upgraded and can respond accordingly.

Not directly. This is mostly for the case where for example the FTP app was available in OC < 9.2 with respective storages but then when upgrading to 9.2 it has been moved to the market, so the admin might need to manually reinstall it. (unless we can do this automatically, somehow)

The problem of using the "StorageNotAvailableException" is that the clients won't know what is the real reason of the error: is there a temporal network failure between ownCloud and the backend? did the connector disappear? is ownCloud being updated?

Yeah, I guess in general (and separately) we could add more subclasses of StorageNotAvailableException which specify what kind of error it is with a proper exception message. And then depending on it also incorporate @butonic's Retry-After idea. But this is out of the scope of this ticket as it's not the problem I'm trying to solve.

[PVince81]

Estimate: 0.5-2 md as I need some time to dive into this again. Shortest estimate is if we keep the current approach and if it still works after rebase.

[PVince81]

moving to triage to reconsider later...

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.