Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide public APIs to register storage wrappers #26786

Closed
PVince81 opened this issue Dec 7, 2016 · 4 comments
Closed

Provide public APIs to register storage wrappers #26786

PVince81 opened this issue Dec 7, 2016 · 4 comments

Comments

@PVince81
Copy link
Contributor

PVince81 commented Dec 7, 2016

Currently storage wrappers can only be registered through the private preSetup hook on the Filesystem object, the Wrapper class itself is private and also the wrap method is: https://github.com/owncloud/core/blob/master/apps/files_trashbin/lib/Storage.php#L181

We should move these to the public namespace to make storage wrapper public.

That is, unless we want to get rid of storage wrappers altogether...

@DeepDiver1975 @jvillafanez @butonic

@ownclouders
Copy link
Contributor

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.

@jvillafanez
Copy link
Member

This will be problematic. Any app could register a wrapper and might insert the wrapper in any place of the wrapper chain. Controlling the wrapper order is something we mustn't delegate, and it should be responsability of whatever is creating the storage (which I guess is core).

  • Controlling the wrapper order via a priority value doesn't seem enough. Any app could mess up with the order and could cause a lot of issues
  • Adding the wrapper in the last position of the chain will also be problematic because it will depend on the app loading order (very undesirable)

The only thing that might fit is to somehow allow the admin to configure the chain via the config.php file or other mechanism, but that will also be problematic for the admin

@PVince81
Copy link
Contributor Author

To put it differently: we already have many apps that cheat and still use the private API to register storage wrappers to achieve their functionality. So far it works fine but as you say it could become problematic if the order of wrappers is determined by app loading order and not deterministic.

@jvillafanez
Copy link
Member

I guess the best compromise is to allow the apps to add the wrapper in specific sections of the chain, not exactly "beginning", "middle", "end" but kind of, based on the expected functionality that the wrapper will provide.
Some apps might want to add the wrapper to the beginning of the chain while others might want to add it to the end. Note that within each section the wrappers can be in any other.

This might work if we get a good naming scheme for each part of the chain based on the expected functionality that should be placed there. Note that we'll also need to deal with incompatibilities among the wrappers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants