-
Notifications
You must be signed in to change notification settings - Fork 189
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
Upload into a moved folder fails (at least on ocis.owncloud.works) #975
Comments
Reproduced with Android app 2.15.3 |
desktop client 2.7.1 stops all sync down activity, due to this:
|
so it looks like the following holds with the ocis storage:
expected: This can be seen in this screenshot |
Cannot reproduce on ocis-1.0.0-rc5 on eos. connect desktop client-2.7.1 as user einstein, wait until initial sync finishes.
Syncs just fine. No errors! |
So, we found out that and because of that the extended attributes are not written to the correct node, but to the Trying to read the node after the |
As you may have seen there are two codepaths in the ocis move(). The first is for renaming a file in the same folder. The second one is for moving a node to a new parent and optionally changing the name at the same time. |
The target node in that case is the result of the lookup in https://github.com/cs3org/reva/blob/e4fc511d5fac467cd1425077eb676c5af7f20441/pkg/storage/fs/ocis/ocis.go#L342 Even if the target path does not exist the call returns a note struct that should be filled with the correct parent I'd. |
NodeFromResourco might take a path or an id reference. Let me check what ocs sends... |
Yep, took us way too long but we arrived there as well. The issue is that we need a valid ID to build the target path for where we want to write the extended attributes. While the parentid indeed exists and is valid, the ID is empty because the node didn't exist when we read it. |
The ocs move checks if the destination exists. If not it checks if the parent exists. And it will always use a path based reference: https://github.com/cs3org/reva/blob/e4fc511d5fac467cd1425077eb676c5af7f20441/internal/http/services/owncloud/ocdav/move.go#L181 |
Ok so NodeFromPath uses Walk with() from a node: https://github.com/cs3org/reva/blob/e4fc511d5fac467cd1425077eb676c5af7f20441/pkg/storage/fs/ocis/lookup.go#L63 |
Actually walkPath is implemented in lookup.go https://github.com/cs3org/reva/blob/4a9be347ac29706e429d47525815e638f9770f29/pkg/storage/fs/ocis/lookup.go#L126 😞 needs cleanup ... as I said the Interface is not yet done ... |
Anyway ... The code finally relies on a nodes Child() to return a node https://github.com/cs3org/reva/blob/e4fc511d5fac467cd1425077eb676c5af7f20441/pkg/storage/fs/ocis/node.go#L199 |
Child() creates a new node struct and sets the parentID to the parent nodes ID property. https://github.com/cs3org/reva/blob/e4fc511d5fac467cd1425077eb676c5af7f20441/pkg/storage/fs/ocis/node.go#L202 AFAICT the target does not exist so the new child node has no ID set. Which is what I would expect. |
The code in move should handle that case correctly, because it needs to reuse the node I'd from the source... |
@kulmann ok that is why https://github.com/cs3org/reva/blob/e4fc511d5fac467cd1425077eb676c5af7f20441/pkg/storage/fs/ocis/tree.go#L177 has no ID. We need to copy it from the src node. That should do the trick. Need to take care of the kids now... |
related to #719 ? |
Upload into a moved folder fails
Steps to reproduce:
1: create folder
/test
2: create folder
/test-moved
3: move folder
/test-moved
into/test
, so that its path is/test/test-moved
4: navigate into
/test/test-moved
5: upload a file (tus) - upload fails with error message
File upload failed… tus: unexpected response while creating upload, originated from request (response code: 500, response text: )
Then:
6: create folder
/test/test-created
7: navigate into
/test/test-created
8: upload a file (tus) - upload succeeds.
Server logs:
The text was updated successfully, but these errors were encountered: