reva icon indicating copy to clipboard operation
reva copied to clipboard

Consult user shares provider for shares markers when doing propfinds

Open ishank011 opened this issue 2 years ago • 2 comments

When doing propfinds, we use the metadata returned by the storage providers to decide whether the resource has been shared or not

https://github.com/cs3org/reva/blob/623d181b5e5b26e4a47220cfa98321719bb0e742/internal/http/services/owncloud/ocdav/propfind/propfind.go#L1031-L1040

And call ListPublicShares to check if a public link has been created.

https://github.com/cs3org/reva/blob/623d181b5e5b26e4a47220cfa98321719bb0e742/internal/http/services/owncloud/ocdav/propfind/propfind.go#L306-L319

The storage provider might be operated by entities other than reva and thus might have inconsistent info; e.g., the users can set grants themselves without creating shares in reva. Can we also make the call to ListShares to return if the resource has been shared? @butonic

ishank011 avatar Jun 07 '22 09:06 ishank011

The problem is that listing shares and links for a folder with 1000 entries makes a request with a resource id filter for every resource. the share / link manager then has to find shares for 1000 resource ids.

While that is possible, it is far from optimal. We are seeing performance bottlenecks for folders with thousands of files because the ocdav service currently does not cache the public shares for a folder. We could cache that in ocdav, but the problem then becomes cache invalidation.

For the share manager I am quite happy that we can get the share indicators from the storage provider. If we had to query the share manager with a filter of all the resourceids in a folder it would have the same overhead.

The root cause of the problem is that oc flavored webdav implemented by the ocdav service has to support an oc:share-types property, only the web ui get everything in one propfind. IMO the web ui should get the bare minimum of necessary data in PROPFIND and then use additional requests to fetch shares and links only for files visible in the ui. So only 20 resourceids need to be queried.

Unfortunately,

  • [ ] the ocs api currently cannot fetch shares for multiple resourceids at once,
  • [ ] webdav PROPFINDs are not paginated, which would reduce the problem to a corner case when clients request all resources in a collection AND want to see 'expensive' properties like share indicators.
    • related: https://github.com/owncloud/web/issues/116
    • related: https://github.com/owncloud/ocis/issues/4542

butonic avatar Sep 08 '22 13:09 butonic

We could add an If-Modified-Since like header to the List(Public)Shares calls. Then the (public) share managers can take into account an aggregated mtime of all shares in a space and only return the requested ids if they changed.

hm or add an mtime per resourceid?

butonic avatar Sep 08 '22 15:09 butonic