Allow updating file metadata without changing file etag (only change parent directory etag)

[ckamm]

This is based on owncloud/client#3235.

Sometimes a file's mtime changes without any content change, such as when using touch (this happens in the wild to potentially big .eml files). In that case, re-uploading the file doesn't make sense. Instead, the sync client should just update the server mtime.

That file metadata change should not update the file's etag (other clients should not re-download the file), but should update the parent directory's etag (to signal that a metadata update is required).

I've tried doing this with a PROPPATCH on the getlastmodified property against 8.2.0, but the file etag changes.

[ogoffart]

In general, i'd say the etag should change if and only if the contents of the file changes. It should not change for metadata like mtime or permissions.
However the etag of the parent directory recursively up to the root should change.

[MorrisJobke]

cc @icewind1991 @PVince81 @DeepDiver1975

[PVince81]

This was the expected behavior so far, so this would be a change of behavior to be discussed.

@ckamm I'm guessing that the purpose of this change is to prevent unneeded redownload of files when they were touched.

Does the client cache the server's mtime in its local database ? If yes, then if the behavior changes, how will the client update the local DB if the etag did not change ? Will it do a PROPFIND diff and find a different mtime ?

also CC @rperezb for the mobile team

[PVince81]

Ah yes, the eml problem. I also had the same thoughts as @ckamm regarding touching the file.

Please note that changing share information (permissions) will now change the etag. Permissions could be considered as metadata too. #16587.
But that will change the etag of the parent, not of the file directly.
Also slightly related #16589

[ckamm]

@PVince81 Yes, we want to prevent unneeded downloads.

I think @ogoffart put it nicely: the etag of a file should change if and only if the file contents change. If the file metadata changes, keep the file etag the same, but update the parent directories etag (recursively up to root).

As far as I know this is how permission metadata changes work too.

[PVince81]

Sounds good

[PVince81]

@icewind1991 @DeepDiver1975 any comments on this proposal ?

[PVince81]

00002839

[PVince81]

I thought there was a blue ticket but @MorrisJobke said the case was resolved already.

[MorrisJobke]

I thought there was a blue ticket but @MorrisJobke said the case was resolved already.

Only by a workaround to enter the files that cause issues to the ignore list of each client. A proper fix is still wanted.

[icewind1991]

This would probably require changing our touch behavior to only update the mtime in the database, not on the storage

[rullzer]

@icewind1991 mmm that seems like a dangerous solution as then the files on the server are no longer 'identical' to the ones on synces instances.

[guruz]

Would be nice to get this fixed at some point™.
owncloud/client#6629 (comment)

[PVince81]

In theory, in the scenario from the first post and now that we have checksums, we could even compare the checksum and not change the etag in case the file is the same as before, at the time we'd overwrite the checksum value. Or even go further, if the announced checksum from an upload matches the current file, don't bother uploading at all and discard the data and chunk assembly.
If the mtime changed, we'd still need to adjust it based on "OC-X-Mtime".

Now there's still the question whether we should trust the announced checksum in case of bugs: for example if a client would send different data with the same old checksum, in which case a checksum error would be expected instead of just saying "ok, I already had the file, bye".

Not sure if we should consider this related to the topic at hand or if it's a completely separate topic.

---

Regarding the actual topic of not changing metadata, I had a look in the code and it seems we'll need a bit of refactoring and/or tweaking the Updater API signature: this writeUpdate https://github.com/owncloud/core/blob/v10.0.8/lib/private/Files/View.php#L1204 actually calls the Cache\Updater class to set the new mtime to oc_filecache, and this class also implicitly calls the etag propagator. We'd need to extend its update() method to take a parameter $parentOnly = false to tell it to only propagate to parents.

let's go with the following approach:

• adjust Cache\Updater::update() with extra $parentOnly = false param
• pass along the param from View's basicOperation writeUpdate() call

Estimate: 1-2md

The max time is in case the tech concept is not acceptable and need to find another way around, because Cache\Updater is technically public API, so need to be careful with changing the method signatures.

Any objections @butonic @jvillafanez to the concept ?

[guruz]

@PVince81 Hm I think what you are writing in your first part of the comment is a different topic. From the client side we want to use PROPPATCH. There is no "other checksum" in this case, there only is a new MTime. (And maybe an If-Match with the client perceived ETag)
What @ckamm described in his post is a client-side decision (detecting if the contents actually updated)

However you could still do what you described, in a multi-client or multi-user scenario it makes sense. But it's not what we from client side wanted now..

[stale]

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.