Etag doesn't propagate across shared storage boundaries

[PVince81]

See #14596 (comment) for an analysis.
That ticket is about size propagation but in the results you'll see that etag is also not updated.

It used to work in OC 6: #14596 (comment)
And was broken since OC 7: #14596 (comment)

This is when a share recipient uploads a file, the etag must propagate to the owner's storage.
But since OC 7 the propagation stops at the shared folder itself.

Here are the steps again:

Steps to reproduce:

1. Create two users "user1" and "user2"
2. Login as "user1"
3. Create a folder "subdir/test"
4. Share "subdir/test" with "user2"
5. Check the etag of "user1"'s "files" entry in the database, write it down
6. Login as "user2"
7. Upload a big file (ex: 10MB) to "test" (as recipient)
8. Check the etag of "user1"'s "files" entry in the database, compare with the one from 5.

Expected result

Etag of share owner's root changed.

Actual result

Only the etag of "subdir/test" has changed. It wasn't propagated further.

Versions

OC 6: works correctly
OC 7 and OC 8: broken

@icewind1991 @schiesbn

[PVince81]

Need to find out how OC 6 did it.

From what I understand, every share recipient should have their shared folder etag changed and propagated to the root.

Note that the sync client currently contains a workaround to not only check the etag of the root but also of all children inside the root. This made the problem harder to detect if the shared folder is in the owner's root folder. (CC @guruz)

[icewind1991]

Reproduction steps in unit test form: a1d5922

[icewind1991]

I'll try and have a go at this

[PVince81]

Thanks a lot.

We might need to consider propagating etag for the other recipients too, not only owner and uploader.
Not sure how we did this in OC 6 ?

[PVince81]

I had a look at your unit test and what you do is have "user2" share with "user1" and then when "user2" uploads a file, you expect the recipient's root etag to change, but it doesn't. This is one case.

Another case is when "user1" the recipient uploads a file, then the etag of "user2"'s root must also change. We should test both cases.

[PVince81]

(also stealing your unit test for the size propagation test 😉)

[tessus]

Any update on this? Is there a fix I can test on my 7.0.4 install?

[PVince81]

There's this work in progress PR: #14764
It might work already but from the code it might cause performance issues, so if I were you I'd rather wait.

Also that fix is unfortunately quite big and intrusive, so not sure yet about backporting to OC 7.

[tessus]

@PVince81 wow, this is bad news. I'm still grateful you are trying to fix this, but your answer tells me 2 things:

• the architecture of ownCloud is less than optimal
• I can't use ownCloud for the main reason it exists (sharing and syncing files with other users)

Just to make this clear (since a lot of developers take opinions and facts as personal), this was not a personal attack. I'm just stunned that a product can fail so miserably in its basic function. I'm still using ownCloud and I think it is a great product overall, but I believe it needs better QA and a more streamlined architecture.

[PVince81]

Yes, I agree that the architecture of OC is not optimal. But it takes time to make it better, as we need to be careful to not break stuff while refactoring.
Also, @icewind1991 is the one working on a fix.

There is still hope for a backportable fix: #14764 (comment)

As for QA, this will get better over time, especially that there are now people writing end to end tests.

[PVince81]

Closing as fixed on master / 8.1.

Backport possibilities still need to be discussed though.