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 API to let apps register their own Sabre plugins #26195

Closed
PVince81 opened this Issue Sep 23, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@PVince81
Member

PVince81 commented Sep 23, 2016

Considering that we want Webdav to be the main API in ownCloud, we need a way to let apps register their own Sabre plugins.

@DeepDiver1975 @butonic @mrow4a

@PVince81 PVince81 added the app:dav label Sep 23, 2016

@PVince81 PVince81 added this to the backlog milestone Sep 23, 2016

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 23, 2016

Member

This is a prerequisite for trashbin and versions to be able to provide their own endpoints: #10571 #15646

Member

PVince81 commented Sep 23, 2016

This is a prerequisite for trashbin and versions to be able to provide their own endpoints: #10571 #15646

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Sep 23, 2016

Member

Considering that we want Webdav to be the main API in ownCloud, we need a way to let apps register their own Sabre plugins.

I'd add this to the info.xml

<sabre>
   <plugins>
       <plugin>\OCA\TrashBin\Sabre\VersionPlugin</plugin>
   </plugins>
   <rootcollection>
       <collection>\OCA\TrashBin\Sabre\VersionsRootCollection</collection>
   </rootcollection>
</sabre>
Member

DeepDiver1975 commented Sep 23, 2016

Considering that we want Webdav to be the main API in ownCloud, we need a way to let apps register their own Sabre plugins.

I'd add this to the info.xml

<sabre>
   <plugins>
       <plugin>\OCA\TrashBin\Sabre\VersionPlugin</plugin>
   </plugins>
   <rootcollection>
       <collection>\OCA\TrashBin\Sabre\VersionsRootCollection</collection>
   </rootcollection>
</sabre>
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Sep 23, 2016

Member

How about dependency injection ?

With this format it means the given classes must have a constructor without arguments.

Maybe we should rather expect a facade class SabrePluginsProvider which itself would receive the DAV server and register plugins as needed.

Member

PVince81 commented Sep 23, 2016

How about dependency injection ?

With this format it means the given classes must have a constructor without arguments.

Maybe we should rather expect a facade class SabrePluginsProvider which itself would receive the DAV server and register plugins as needed.

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Sep 23, 2016

Member

These classes can be used in the server container to be resolves - we do this for repairsteps and jobs as well

Member

DeepDiver1975 commented Sep 23, 2016

These classes can be used in the server container to be resolves - we do this for repairsteps and jobs as well

@PVince81 PVince81 modified the milestones: 9.2, backlog Nov 23, 2016

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Nov 23, 2016

Member

As we're writing more apps and move more to be in the market, we'll need this mechanism.

Member

PVince81 commented Nov 23, 2016

As we're writing more apps and move more to be in the market, we'll need this mechanism.

@PVince81 PVince81 referenced this issue Nov 29, 2016

Closed

Basic implementation #2

9 of 9 tasks complete
@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Dec 2, 2016

Member

Now the interesting part is that now we could move the comments app code (Sabre plugins + managers + DB structure) completely to the comments app. Same for system tags...

Member

PVince81 commented Dec 2, 2016

Now the interesting part is that now we could move the comments app code (Sabre plugins + managers + DB structure) completely to the comments app. Same for system tags...

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Dec 2, 2016

Member

PR here: #26761

I need to do more actual testing to see if everything fits correctly.
Some concerns:

  • when should these app plugins be registered ?
    • inside the beforeMethod block that waits for auth ?
    • at the very end ? (this is what the PR does now)
    • what about auth ?
  • where should the collection be registered ?
    • at the very end ? (this is what the PR does now)
    • what about auth ?
    • potential naming conflict as the collection provides its own name, maybe we should namespace it to the name of the app ?

For the auth part one idea is to have an attribute <plugin needs-auth=true>...</plugin> and same for collections. This would move their registration into the authenticated block.
Or we'd expect the plugins + collections to always implement the ACL schemes. Not sure how to do that though and not sure if it's compatible with the way we do auth now.

@DeepDiver1975 thoughts ?

Member

PVince81 commented Dec 2, 2016

PR here: #26761

I need to do more actual testing to see if everything fits correctly.
Some concerns:

  • when should these app plugins be registered ?
    • inside the beforeMethod block that waits for auth ?
    • at the very end ? (this is what the PR does now)
    • what about auth ?
  • where should the collection be registered ?
    • at the very end ? (this is what the PR does now)
    • what about auth ?
    • potential naming conflict as the collection provides its own name, maybe we should namespace it to the name of the app ?

For the auth part one idea is to have an attribute <plugin needs-auth=true>...</plugin> and same for collections. This would move their registration into the authenticated block.
Or we'd expect the plugins + collections to always implement the ACL schemes. Not sure how to do that though and not sure if it's compatible with the way we do auth now.

@DeepDiver1975 thoughts ?

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Dec 2, 2016

Member

Interestingly this also means that CalDAV/CardDAV could be moved to their own separate apps and even be disable-able...

Member

PVince81 commented Dec 2, 2016

Interestingly this also means that CalDAV/CardDAV could be moved to their own separate apps and even be disable-able...

@PVince81 PVince81 referenced this issue Dec 3, 2016

Closed

Webdav API for groups management #3

27 of 28 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment