-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Dynamic share providers / share types in Nextcloud #29215
Comments
Yet sharing has such wide influence on lots of software components in Nextcloud. Like anything changed there also likely requires changes in the front-end, the connected clients and so on, right? |
I think it would make sense to make the share provider registration more dynamic but mainly for introducing a better separation in the code. I'd still prefer to have possible share providers listed and registered within server. Also the oc_share table is not really flexible in that regard, so we'd at least need to keep the mapping constants for the share type there. And as @ChristophWurst mentioned frontend and clients might also need special handling of course. |
For an implementation, we were thinking along the lines of leaving the constants as-is and adding a mechanism through which other types could be dynamicly added. Changes to the front-end would be small, if any, as the majority of the existing logic can already be (re-)used. The connected clients would most likely only be the plugins that introduce custom types (at least to begin with). We had an idea for registering (and listing) providers without using the database as well, to help keep code changes to a minimum. Would it be helpfull if we open an MR to demonstrate what the impact of our current thinking would be on the code? |
Sure, it would make sense to discuss the code in question and a draft pull request might be good for that. I agree that modularising the share provider code is generally a good idea. |
might be interesting to grep for the share type email and circles and see where it's used, and whether any additional types would require to be there as well or not ? or opt-in ? |
My colleague @ylebre is working on a project that makes it possible to connect Nextcloud servers to Sciencemesh.
For this project we need to add custom handling for File Sharing. This also means we need a custom sharing type.
For our custom install, we can just add another class constant to the
lib/public/Share/IShare.php
interface, and custom handling logic for this type.But when we were looking at the code, we saw a comment in the Share20
Manager
class that says:Hmmm... "yet"? 🤔
And in the Share20
ProviderFactory
, the latest addition (for share typeIShare::TYPE_DECK
)uses
$this->getProvider('deck')
instead of$this->getDeckShareProvider()
.This got us thinking that Nextcloud might benfit from an implementation that makes is possible to dynamically add share providers (types and logic). This could be very useful for Nextcloud plugins.
So my question is:
Is there any interest in merging such functionality, if provided? (Including any docs or tests deemed necessary)
From what we can see, we think this would be a relatively small change.
Files that we can see that need to be touched are:
apps/files_sharing/lib/Controller/ShareAPIController.php
and related testlib/private/Share20/Manager.php
and related testlib/private/Share20/ProviderFactory.php
(no tests seem to be available?)Is the information I've shared here complete and correct? Am I missing anything?
And if there's an interest, which steps would we need to take in order to make this happen?
How to use GitHub
Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
Describe the solution you'd like
A clear and concise description of what you want to happen.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: