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

WebDAV permission attribute wrong #10247

Closed
2 of 4 tasks
dragotin opened this issue Aug 7, 2014 · 23 comments
Closed
2 of 4 tasks

WebDAV permission attribute wrong #10247

dragotin opened this issue Aug 7, 2014 · 23 comments

Comments

@dragotin
Copy link
Contributor

dragotin commented Aug 7, 2014

Tested with stable7 on 7540a58 👍

  • directories that were shared by me do not have the "S" tag.
  • files in directories shared by me do not have the "S" tag.
  • directories that are shared with me do have the "S" tag, but also the "M" tag which they IMO should not have.
  • files in directories shared with me also have the "M" tag which they IMO should not have.

@DeepDiver1975 please help - thanks.

@DeepDiver1975
Copy link
Member

directories that were shared by me do not have the "S" tag.
files in directories shared by me do not have the "S" tag.

We have never though about shared by me.

I'll have a look regarding the 'M'

@DeepDiver1975 DeepDiver1975 added this to the 2014-sprint-01-current milestone Aug 7, 2014
@DeepDiver1975 DeepDiver1975 self-assigned this Aug 7, 2014
@dragotin
Copy link
Contributor Author

dragotin commented Aug 7, 2014

Oh, we discussed that one flag is enough for shared with and by which I still think is not sufficient, but that was the decision back than. Argument: In the web interface you also do have the same icons for both cases. Now with overlays, we only have icons for the shared with me which differes from the WI.

@DeepDiver1975
Copy link
Member

The 'M' can be fixed pretty quick - pr is coming ....

@DeepDiver1975
Copy link
Member

regarding the 'S' - maybe I remember the discussion wrong - but querying for the 'shared by me' information is expensive - we don't want to do this on every dav request

@dragotin
Copy link
Contributor Author

dragotin commented Aug 7, 2014

Ah ok, that might be the crucial detail. Hmm.

@DeepDiver1975
Copy link
Member

@schiesbn I need your expertise on the "shared by me" information. As explained above I'm a bit worried about the performance impact if we call some sharing function on each and every folder we return via webdav.

We might want to add some sql join trick to get the shared information into the FileInfo class? Just an idea .....

@icewind1991 for filesystem/cache and stuff 😉

@schiessle
Copy link
Contributor

shares with and shared by are both one \OC\Share::getItem() call. I don't know how it is implemented for the webdav call but both calls should have the same performance impact.

Generally: We could also think about separate this two operations. All the share information could also be fetched with OCS api call, that's also the way the mobile clients handle it. This way the client can decide when to update which information and we would no longer need to fetch all the information for every webdav call.

@dragotin
Copy link
Contributor Author

just as a sidenote, we saw today that stuff that is shared via link do not have the "S" in the permissions tag as well, which seems also buggy. @lmaestro will do a bug report about that. But maybe its related to this.

@icewind1991
Copy link
Contributor

Whether or not a file is shared with me can be determined quickly by looking at the storage, shared by me requires additional db queries

@schiessle
Copy link
Contributor

Whether or not a file is shared with me can be determined quickly by looking at the storage, shared by me requires additional db queries

Yes, but it is only there because we did the getItems() call before to set up the mount points. So the general statements stays true. For both information wen need exactly one getItems() call for each request. getItems() does exactly one DB query, at the moment I don't see how this could be optimized.

@DeepDiver1975
Copy link
Member

Yes, but it is only there because we did the getItems() call before to set up the mount points. So the general statements stays true. For both information wen need exactly one getItems() call for each request. getItems() does exactly one DB query, at the moment I don't see how this could be optimized.

well - the call of getItems() to setup the mount points is done anyways - if i want to get the 'shared-by-me' information it would be an additional call and it would be an additional call for EACH file and folder - right?

As stated above: from my understanding we should be in the position to join up t tables and get al information with the query on the file cache level.

@DeepDiver1975
Copy link
Member

just as a sidenote, we saw today that stuff that is shared via link do not have the "S" in the permissions tag as well, which seems also buggy.

share by link is a kind of 'shared-by-me' as well

@schiessle
Copy link
Contributor

if i want to get the 'shared-by-me' information it would be an additional call and it would be an additional call for EACH file and folder - right?

no, it would be one call to get a list of all files shared by you.

As stated above: from my understanding we should be in the position to join up t tables and get al information with the query on the file cache level.

fwiw, getItems() already does a join over the file cache table

@schiessle
Copy link
Contributor

share by link is a kind of 'shared-by-me' as well

they would be also included by the one getItems() call. So we are really talking about one additional function call and one additional db query to get all the information.

@DeepDiver1975
Copy link
Member

no, it would be one call to get a list of all files shared by you.

okay - the approach would then be to call it once and cache the data in RAM and set the dav property accordingly.

How does this perform in an environment with a hell of shared files?
😉

@DeepDiver1975
Copy link
Member

@dragotin we need to define a second property for shared-by-me - right?

@dragotin
Copy link
Contributor Author

@DeepDiver1975 IIRC we did not want to have the information in a special attrib but also have the "S" set as well. Not sure if thats good tough.

@MTRichards
Copy link
Contributor

The most important icon to get in here is the files or folders are shared WITH you. This is because the Shared folder disappeared, and there needs to be a replacement.

Shared by me is a nice to have attribute. Stability and robustness of Shared WITH you is far more important right now than Shared by you.

The second property to be able to offer up a feature set exactly the same as on the server web UI is secondary (or however it is determined to best implement this) - this can be done later.

@craigpg craigpg modified the milestones: 2014-sprint-03-next, 2014-sprint-02-current, ownCloud 7 backlog, 2014-sprint-03-current Sep 2, 2014
@DeepDiver1975 DeepDiver1975 removed this from the ownCloud 7 backlog milestone Jan 8, 2015
@DeepDiver1975 DeepDiver1975 modified the milestones: backlog, ownCloud 7 backlog Jan 8, 2015
@MorrisJobke MorrisJobke added triage and removed triage labels Feb 27, 2015
@PVince81
Copy link
Contributor

What's the next step ?

@PVince81
Copy link
Contributor

PVince81 commented Aug 3, 2015

Is this still an issue ? I've seen Webdav permission attributes set before, also the mobile and desktop clients are using them. Does it mean this is fixed or are there any outstanding issues ?

@guruz
Copy link
Contributor

guruz commented Aug 3, 2015

@PVince81 I think it's about shared TO me vs shared BY me..

@DeepDiver1975
Copy link
Member

@dragotin @guruz is this still of an issue? Please reopen if so - THX

@lock
Copy link

lock bot commented Aug 5, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants