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

Inline metadata for tags and sharing in PROPFIND response #33503

Closed
felix-schwarz opened this issue Nov 13, 2018 · 15 comments
Closed

Inline metadata for tags and sharing in PROPFIND response #33503

felix-schwarz opened this issue Nov 13, 2018 · 15 comments

Comments

@felix-schwarz
Copy link

felix-schwarz commented Nov 13, 2018

Motivation and status quo

I'm currently working on the iOS file provider for the new ownCloud ios-sdk and ios-app and found the current support incomplete for its purposes:

  • the iOS Files app shows tags alongside the items and therefore expects that information to be immediately available
  • currently, the ownCloud server doesn't seem to provide a way to retrieve the tags as part of a PROPFIND, instead requiring one request per item, which:
    • does not scale well
    • can significantly prolong the time needed until the client has all data
    • may lead to inconsistencies, since changes could be made between the PROPFIND that provided the list of items, and the server answering the Tags API requests for these items. There's no guarantee that item 1 from the PROPFIND still has the same tags 100 Tags API request later.
    • makes the process of gathering a consistent, complete view of the items in a folder significantly more complex and error-prone

Proposal

What I'd like to see is

  • a way to retrieve tags metadata (and really all other, too) for the items with a single PROPFIND
  • and the ability to modify the metadata of an item via a single PROPPATCH
  • the ability to retrieve and set tags in cleartext (rather than via their IDs) as it would save a ton of work in the client, too

Benefits:

  • atomic "snapshots" of and updates to an item's metadata
  • a single request to fetch all metadata at once

What should be covered, in order of importance:

  1. tags
  2. shares
  3. comments

Possible optimization:

Allow formulating PROPFINDs in a way that metadata that doesn't exist for an item (f.ex. no tags, no shares, no comments) is omitted from the multi status response, including the 404 Not Found section for the item(s).

@ownclouders
Copy link
Contributor

GitMate.io thinks the contributor most likely able to help you is @PVince81.

Possibly related issues are #2846 (Slow PROPFIND on Shared folders), #16427 (Tags and flexible sharing), #18450 (PROPFIND on federated share takes long), #2552 (Temporary 500 response for PROPFIND with multiple users), and #13168 (Ability to share by tag).

@PVince81
Copy link
Contributor

Tags have different scopes: so setting them in clear text is not possible currently. This is why they require ids. In each scope (visible, invisible, restricted) a tag with a similar name could exist. (this was an old design decision which I find regrettable, but this is how it is now)

Due to this, if we want to be able to create tags with PROPPATCH, one would need to pass all this information and the backend would need to match against existing tag or create it if needed.

On the XML level I guess it should be possible to add additional sub-fields to make room for this.

I wonder if in OC 11 we should rework the tags and merge all the scopes which would simplify a lot.

cc @pmaier1

@PVince81 PVince81 added this to the backlog milestone Nov 13, 2018
@PVince81
Copy link
Contributor

and with "merging the scope" I just mean "have a single list of unique tag names with attached scopes" instead of have "three separate lists of tag names, one per scope with duplication possible".

@felix-schwarz
Copy link
Author

felix-schwarz commented Nov 16, 2018

Due to this, if we want to be able to create tags with PROPPATCH, one would need to pass all this information and the backend would need to match against existing tag or create it if needed.

On the XML level I guess it should be possible to add additional sub-fields to make room for this.

Yeah, that's what I was thinking of.

I wonder if in OC 11 we should rework the tags and merge all the scopes which would simplify a lot.

and with "merging the scope" I just mean "have a single list of unique tag names with attached scopes" instead of have "three separate lists of tag names, one per scope with duplication possible".

I wasn't aware that tag names could exist multiple times, in different scopes. Making this impossible in the future certainly sounds like a goal well worth pursuing. A migration path for installations using the same tags in different scopes could be to rename the affected tags by pre-pending the name of the scope to them.

Zooming out and looking at the big picture, something I'd really love to see is:

  • a generalized form of storing metadata for an item (much like extended attributes on filesystems)
  • a single API around it (PROPFIND and PROPPATCH look good as vehicles for this to me)
  • tags, shares, comments, favorites, checksums, etc. be represented, accessible and modifiable through the metadata API
  • the ability for apps to store custom meta data (like the "favorite ranks" set by the iOS Files app, or bookmarked pages in a PDF, …)

I believe such a metadata API could also open up ownCloud to many new uses that it can't address as directly right now.

/cc @PVince81 @DeepDiver1975 @felixboehm @michaelstingl @pmaier1

@phil-davis
Copy link
Contributor

phil-davis commented Nov 16, 2018

I wasn't aware that tag names could exist multiple times, in different scopes

One of the issues with having just a single namespace for all names (no name duplication) is that as soon as you make features that restrict tag visibility, then the single namespace means:

  1. Users with restricted tag visibility may try to create tags and be given some message about "tag name already exists" and wonder why, because they cannot see any such tag - that's just a UI message design issue.

  2. The people in charge of creating the restricted tags will sometimes find that some other regular user has already created a tag with the name they wanted to use.

  3. Some installations that restrict tag visibility will do so because the restricted tags are about secret-squirrel business, and even the tag name is "classified". e.g. tag tesla-self-drive-nuclear-fusion-vehicle-project client-John-Smith-UK etc. With a single tag namespace, other users can find out that a tag name exists by trying to create it. If they get a "tag name already exists" type of error then the existence of the tag name has been revealed.

@PVince81
Copy link
Contributor

a generalized form of storing metadata for an item (much like extended attributes on filesystems)

You can already use custom Webdav attributes for that: PROPPATCH for setting, PROPFIND for reading. However it only works correctly with single string values, not sub-tags.

@PVince81
Copy link
Contributor

to clarify, you can invent an attribute name like "oc:myattr" and set it on a file with PROPPATCH. It will stay attached to the file.

@PVince81
Copy link
Contributor

If they get a "tag name already exists" type of error then the existence of the tag name has been revealed.

This was the main reason for providing these different scopes/namespaces.

But in hindsight I'm not sure there is much value nor criticality in such ability for "guessing" when compared to the advantage and reduced complexity of having a single namespace.

@felix-schwarz
Copy link
Author

felix-schwarz commented Nov 16, 2018

@phil-davis:

I wasn't aware that tag names could exist multiple times, in different scopes

One of the issues with having just a single namespace for all names (no name duplication) is that as soon as you make features that restrict tag visibility, then the single namespace means:

  1. Users with restricted tag visibility may try to create tags and be given some message about "tag name already exists" and wonder why, because they cannot see any such tag - that's just a UI message design issue.
  2. The people in charge of creating the restricted tags will sometimes find that some other regular user has already created a tag with the name they wanted to use.
  3. Some installations that restrict tag visibility will do so because the restricted tags are about secret-squirrel business, and even the tag name is "classified". e.g. tag tesla-self-drive-nuclear-fusion-vehicle-project client-John-Smith-UK etc. With a single tag namespace, other users can find out that a tag name exists by trying to create it. If they get a "tag name already exists" type of error then the existence of the tag name has been revealed.

What could solve all three, improve clarity and present a migration strategy would be for scopes to become part of the tag name, like f.ex. classified:elvis-presley:

  1. Regular users could still use elvis-presley and wouldn't be confronted with an error message.
  2. People in charge wouldn't run into issues either. But they would now have a shortcut to creating tags in a specific scope, like creating a new classified x-files tag simply by entering classified:x-files.
  3. Unauthorized users trying to enter would get the same error message for classified:elvis-presley as for classified:does-not-exist: "You're not authorized to use classified tags.". The existence of a tag name could no longer be revealed through guessing.

I don't know how/if the scope of a tag is currently expressed visually in ownCloud. But this might be an opportunity to do a "split" presentation like this in the UI, preserving both legibility and visually representing the "scope:tag" formatting:
bildschirmfoto 2018-11-16 um 11 38 53

Here's the HTML code if anybody wants to play around with it ;)

<html>
<body style="font-family: -apple-system, BlinkMacSystemFont, sans-serif;">

<div style="border-radius: 5px 0px 0px 5px; background: red; padding: 2px 5px; font-size: 13px; color: white; display: inline;">classified</div><div style="border-radius: 0px 5px 5px 0px; background: #fbb; padding: 2px 5px; font-size: 13px; display: inline;">elvis-presley</div>

<div style="border-radius: 5px; background: #bdf; padding: 2px 6px; font-size: 13px; display: inline;">invoices</div>

</body>
</html>

@PVince81:

to clarify, you can invent an attribute name like "oc:myattr" and set it on a file with PROPPATCH. It will stay attached to the file.

Awesome! Is there any size limitation to what can be stored?

@felix-schwarz
Copy link
Author

@PVince81:

to clarify, you can invent an attribute name like "oc:myattr" and set it on a file with PROPPATCH. It will stay attached to the file.

Awesome! Is there any size limitation to what can be stored?

Is it also possible to store data on a per-user basis (f.ex. for bookmarks in PDFs, which will be different for different users)?

Does changing custom metadata change the eTag of the item, or the collection its stored in?

@PVince81
Copy link
Contributor

The custom properties are global for all users, there is no per-user file metadata currently.

Size limit is the one from the DB table:

MariaDB [owncloud]> describe oc_properties;
+---------------+---------------------+------+-----+---------+----------------+
| Field         | Type                | Null | Key | Default | Extra          |
+---------------+---------------------+------+-----+---------+----------------+
| id            | bigint(20)          | NO   | PRI | NULL    | auto_increment |
| fileid        | bigint(20) unsigned | YES  | MUL | NULL    |                |
| propertyname  | varchar(255)        | NO   |     |         |                |
| propertyvalue | varchar(255)        | NO   |     | NULL    |                |
+---------------+---------------------+------+-----+---------+----------------+

Maybe we should change that propertyvalue column to be a BLOB/CLOB... @DeepDiver1975

@PVince81
Copy link
Contributor

raised #33554 to blobify propertyvalue.

@PVince81
Copy link
Contributor

cc @tomneedham and @micbar who have been using this table for some app, in case there is further input

@felix-schwarz
Copy link
Author

felix-schwarz commented Nov 19, 2018

@PVince81:

to clarify, you can invent an attribute name like "oc:myattr" and set it on a file with PROPPATCH. It will stay attached to the file.

The custom properties are global for all users, there is no per-user file metadata currently.

Just got an idea how per-user attributes could be implemented rather cheaply and thought I'd share it: by adding special prefixes to attribute names and checking for permission on the server side:

F.ex. an attribute x-customattrib for user felix could look like oc:_user.felix.x-customattrib:

  • the server could check attributes for the _user. prefix
  • if one matches, extract the user name and check if it's the current user
  • if it's the current user: serve the value and allow its modification
  • if it's not the current user: return an access denied status code for the attribute

Possible even cheaper:

The attribute could also just be named oc:_user.x-customattrib and the server then dynamically insert the connected user's name into the attribute (so it becomes oc:_user.felix.x-customattrib again) before hitting the oc_properties table. Might be even less effort (incl. on the client side!).

/cc @DeepDiver1975

@stale
Copy link

stale bot commented Sep 20, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

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

5 participants