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

Lock and Unlock does not provide the new etag value for a file in question #162

Closed
allexzander opened this issue Sep 5, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@allexzander
Copy link

When we lock or unlock a file its etag gets modified and is later synced by the desktop client. The problem is, in case we also modify a file locally after we lock it, the etag we are storing in the desktop client is still the one before calling lock/unlock, and, since we've modified a file as well as its etag got changed on the server - we are getting a false positive file conflict. The etag is used by desktop client to detect changes made to the file on the server.

Possible solutions (not sure why the etag is chosen to mark file lock state change, could be that we need a more exclusive way of handling this not involving the etag):

  • in the XML body (example below) we do not have any mention of the new etag value, so it would allow me fix the conflict issue on desktop client if it was included in the XML reply
  • why do we use etag? can not we have a separate metadata field for that? someone suggested to have "etagForMetadata"/"etagMetadata" at some point

Example of XML response I am getting:

<?xml version="1.0"?>
<d:prop xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns" xmlns:nc="http://nextcloud.org/ns">
 <nc:lock>1</nc:lock>
 <nc:lock-owner-type>0</nc:lock-owner-type>
 <nc:lock-owner>admin</nc:lock-owner>
 <nc:lock-owner-displayname>admin</nc:lock-owner-displayname>
 <nc:lock-owner-editor>admin</nc:lock-owner-editor>
 <nc:lock-time>1693899473</nc:lock-time>
 <nc:lock-timeout>0</nc:lock-timeout>
 <nc:lock-token>files_lock/cf140436-7691-4cc9-b4d5-33ad1daf52cb</nc:lock-token>
</d:prop>
@allexzander allexzander added the enhancement New feature or request label Sep 5, 2023
@jospoortvliet
Copy link
Member

FYI this can cause a ton of issues, especially with larger files. If your workflow is:

  1. Lock file
  2. edit file
  3. save
  4. unlock

and the file is, say, 100mb or more, you risk causing conflicts, besides the 3x inflation of network traffic.

@AndyScherzinger
Copy link
Member

looping in @juliushaertl

@juliusknorr
Copy link
Member

We can expose the changed etag with the lock response, however having a separate one is not really feasible as we would need that separation for a metadata tag as described in #147 / nextcloud/server#8477

A quick implementation draft for returning the etag property on the user LOCK/UNLOCK webdav requests, so maybe that already helps for this specific case #163

@allexzander
Copy link
Author

@juliushaertl

A quick implementation draft for returning the etag property on the user LOCK/UNLOCK webdav requests, so maybe that already helps for this specific case #163

This will help, I would appreciate it if this gets merged/deployed (at least to one of try.nextcloud.com instances).

@allexzander
Copy link
Author

@juliushaertl So I tested it and the etag is returned but is still the old one

@max-nextcloud
Copy link
Collaborator

@allexzander could you try again? Julius updated the PR.

@allexzander
Copy link
Author

@max-nextcloud Yes, changes on desktop client's side already merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants