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

ocdav: use reference instead of path in PROPFIND #778

Closed
wants to merge 1 commit into from
Closed

ocdav: use reference instead of path in PROPFIND #778

wants to merge 1 commit into from

Conversation

PVince81
Copy link
Contributor

PROPFIND recursion needs to use the reference object instead of the path
to make sure cross-storage exploration works correctly.

Co-authored-by: A.Unger zyxancf@gmail.com

Fix for #681 (comment)

@refs
Copy link
Member

refs commented May 27, 2020

Navigating into a file from Phoenix using this solution seems not to work:

2020-05-27T11:42:53+02:00 WRN resource not found path=/home/oc/feynman/test pkg=rhttp service=reva traceid=b104d79720ffadf4cf29ea90d1371bd3

with a folder named test existing:

image

@PVince81
Copy link
Contributor Author

fixed using a07b579

@PVince81
Copy link
Contributor Author

seems I failed committing 🤕

PROPFIND recursion needs to use the reference object instead of the path
to make sure cross-storage exploration works correctly.

Co-authored-by: A.Unger <zyxancf@gmail.com>
@PVince81
Copy link
Contributor Author

seems this implementation is still not correct, it returns some results twice:

    <d:href>/remote.php/webdav/test/</d:href>
    <d:href>/remote.php/webdav/test/a/</d:href>
    <d:href>/remote.php/webdav/test/b/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/a/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/b/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/a/a1/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/a/a2/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/a/a1/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/a/a2/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/a/a1/a11/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/a/a1/a11/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/b/b1/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/b/x1</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/b/b1/</d:href>
    <d:href>/remote.php/webdav/oc/einstein/test/b/x1</d:href>

@PVince81
Copy link
Contributor Author

even worse, the returned path seems to be the internal path!

@PVince81
Copy link
Contributor Author

it looks like when we query using the Resource_Id instead of the path, the path embedded into the result is absolute

according to the doc in owncloud.go the path returned by resolve() should be internal/relative.
it seems the logic is different for the resolution by resource id, I'll have a look...

@PVince81
Copy link
Contributor Author

I added some logging locally:

Resolved by path:

2020-05-27T15:45:18+02:00 DBG resolve() by path path=/var/tmp/reva/data/einstein/files/test pkg=rgrpc ref.GetPath()=/test service=reva traceid=2eb86ed1e459b43de1ebe32e3c625361
2020-05-27T15:45:18+02:00 DBG ocfs: unwrap: internal=/var/tmp/reva/data/einstein/files/test external=/test pkg=rgrpc service=reva traceid=2eb86ed1e459b43de1ebe32e3c625361
2020-05-27T15:45:18+02:00 DBG convertToResourceInfo fn=/test id=ee8ee0ee-7b89-4f8e-88ee-893787ebd3f9 np=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=2eb86ed1e459b43de1ebe32e3c625361

Resolve by id

2020-05-27T15:45:18+02:00 DBG resolve() by id np=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=2eb86ed1e459b43de1ebe32e3c625361
2020-05-27T15:45:18+02:00 DBG ocfs: unwrap: internal=/einstein/files/test external=/einstein/test pkg=rgrpc service=reva traceid=2eb86ed1e459b43de1ebe32e3c625361
2020-05-27T15:45:18+02:00 DBG convertToResourceInfo fn=/einstein/test id=ee8ee0ee-7b89-4f8e-88ee-893787ebd3f9 np=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=2eb86ed1e459b43de1ebe32e3c625361

so when resolving by id in the OC storage the path is "/einstein/test" instead of "/test"

@PVince81
Copy link
Contributor Author

oh, apparently the internal path passed in unwrap() is "/einstein/files/test" instead of "/var/tmp/reva/data/einstein/files/test", so the prefix trimming is not working correctly

@PVince81
Copy link
Contributor Author

more logging:

2020-05-27T16:01:30+02:00 DBG resolve() by path path=/var/tmp/reva/data/einstein/files/test pkg=rgrpc ref.GetPath()=/test service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG unwrap() beginning internal=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG unwrap() EnableHome block internal=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG ocfs: unwrap: internal=/var/tmp/reva/data/einstein/files/test external=/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG convertToResourceInfo fn=/test id=ee8ee0ee-7b89-4f8e-88ee-893787ebd3f9 np=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG resolve() by id np=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG unwrap() beginning internal=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG unwrap() non-EnableHome block internal=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG unwrap() data dir check DataDirectory=/var/tmp/reva/data pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG unwrap() new internal internal=/einstein/files/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG unwrap() parts: [ einstein files test] pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG ocfs: unwrap: internal=/einstein/files/test external=/einstein/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113
2020-05-27T16:01:30+02:00 DBG convertToResourceInfo fn=/einstein/test id=ee8ee0ee-7b89-4f8e-88ee-893787ebd3f9 np=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=df656e2c4e93f6d8158e9ddc11eb2113

it looks like for the "by path" resolution, the fs.c.EnableHome is set. But by id is it not set.

why would the config change based on how the request is done ?!

@PVince81
Copy link
Contributor Author

so this is the config for the "resolve by path":

2020-05-27T16:08:06+02:00 DBG unwrap() beginning fs.c={"DataDirectory":"/var/tmp/reva/data","EnableHome":true,"Redis":":6379","Scan":true,"UploadInfoDir":"/var/tmp/reva/uploadinfo","UserLayout":"{{.Username}}"} internal=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=30d91de330fc3956942eae5866b99d33

and with "resolve by id":

2020-05-27T16:08:06+02:00 DBG unwrap() beginning fs.c={"DataDirectory":"/var/tmp/reva/data","EnableHome":false,"Redis":":6379","Scan":false,"UploadInfoDir":"/var/tmp/reva/uploadinfo","UserLayout":"{{.Username}}"} internal=/var/tmp/reva/data/einstein/files/test pkg=rgrpc service=reva traceid=30d91de330fc3956942eae5866b99d33

so it seems only EnableHome has the wrong value. I haven't yet found and understood where that value should be set.

@PVince81
Copy link
Contributor Author

ok, it looks like it's connecting to different services.

when we want to resolve by path, it connects to "reva-storage-home" but by id it connects to "reva-storage-oc", so it's normal that the response contains different paths...

I don't think we can easily rewrite the paths in the ocdav layer as we'd need to know about the prefix to remove

@PVince81
Copy link
Contributor Author

I added logging in the gateway's "FindProvider" but apparently it never gets called for the "by path" case. It only gets call when using id:

reva-gateway.log:2020-05-27T16:34:34+02:00 DBG FindProvider() rules=map[: /:localhost:9152 /eos:localhost:9158 /home:localhost:9154 /oc:localhost:9162 /public/:localhost:10054 1284d238-aa92-42ce-bdc4-0b0000009152:localhost:9152 1284d238-aa92-42ce-bdc4-0b0000009158:localhost:9158 1284d238-aa92-42ce-bdc4-0b0000009162:localhost:9162 e1a73ede-549b-4226-abdf-40e69ca8230d:localhost:10054] address=localhost:9162 fn= id={"opaque_id":"ee8ee0ee-7b89-4f8e-88ee-893787ebd3f9","storage_id":"1284d238-aa92-42ce-bdc4-0b0000009162"} pkg=rgrpc service=reva traceid=838a341628f937ac4b5642a63f7d482f

@PVince81
Copy link
Contributor Author

in any case I suspect that the storage id that is stored in the response is always pointing at "/oc:localhost:9162" but the resolution by path would contain "/home" so would resolve to the other service.

now I wonder why the home service would return results with storage ids pointing at another storage...

Maybe it's the cross-storage scenario. Somehow we'd need a way to resolve by resource id but still transform the path to be in the view of a different storage instead of the path from the original storage.

@PVince81
Copy link
Contributor Author

I'm abandoning this attempt for now as I need a better understanding of the framework.

I've raised https://github.com/owncloud/ocis-reva/issues/231 to look into a proper solution for cross-storage recursion, which likely also affects the COPY operation additionally to PROPFIND.

Here is an alternative workaround to unblock the public share work: #779

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

Successfully merging this pull request may close these issues.

2 participants