opendal [gdrive] potential regression of folder name handling

in my app, I found folders would have extra / for google drive

might be a regression recently

Jun 12 '24 20:06 imWildCat

20:18:04 [WARN] service=gdrive operation=stat path=apps/infoflow/default.ifvault/v1/data/48aaec2f-ed21-4f37-a3ca-90f9ac5d6872 -> NotFound (permanent) at stat, context: { service: gdrive, path: apps/infoflow/default.ifvault/v1/data/48aaec2f-ed21-4f37-a3ca-90f9ac5d6872 } => path not found: apps/infoflow/default.ifvault/v1/data/48aaec2f-ed21-4f37-a3ca-90f9ac5d6872
20:18:06 [WARN] service=gdrive operation=stat path=apps/infoflow/default.ifvault/v1/data/1dff98d4-a8c6-4b08-a9e3-78e45d4aa034 -> NotFound (permanent) at stat, context: { service: gdrive, path: apps/infoflow/default.ifvault/v1/data/1dff98d4-a8c6-4b08-a9e3-78e45d4aa034 } => path not found: apps/infoflow/default.ifvault/v1/data/1dff98d4-a8c6-4b08-a9e3-78e45d4aa034

yes. it is a regression. the path created by the same open dal client cannot be read.

Jun 12 '24 20:06 imWildCat

this regression is happening between v0.45.1 and v0.46.0

this is the last correct version: https://github.com/apache/opendal/blob/v0.45.1/core/src/services/gdrive/core.rs#L396

Jun 12 '24 20:06 imWildCat

Interesting. It also repros using v0.45.1

Jun 12 '24 20:06 imWildCat

https://github.com/apache/opendal/blob/27c6a776653b7671da8171dea8c36283713eeb45/core/src/services/gdrive/core.rs#L396-L405

Maybe we should trim the / suffix for dir?

Jun 13 '24 01:06 Xuanwo

let me try!

Jun 13 '24 02:06 imWildCat

it seems to be fixed by this change.

https://github.com/imWildCat/opendal/commits/main/

I also found another issue shown above: opendal now create folder with same names...

Jun 13 '24 03:06 imWildCat

I also found another issue shown above: opendal now create folder with same names...

We should already have checks for the folder exist before creating. Maybe some logic not adapated yet? Like creating abc folder but check with abc/?

Jun 13 '24 08:06 Xuanwo

Could it be related to cache? I'm not calling opendal concurrently.

Jun 13 '24 12:06 imWildCat

I'm not calling opendal concurrently.

The cache is thread-safe, so concurrent calls should not pose a problem. I believe this issue is related to how we handle paths.

Jun 13 '24 13:06 Xuanwo

after the fix you proposed above, the duplication problem still exists.

Jun 13 '24 14:06 imWildCat

anyway, let me create a PR to fix the naming issue first. let's see if the test can pass

Jun 13 '24 20:06 imWildCat

@Xuanwo any suggestion on the direction to fix the tests on main first?

I believe we have many regressions since https://github.com/apache/opendal/pull/3975

Jul 11 '24 20:07 imWildCat

@Xuanwo any suggestion on the direction to fix the tests on main first?

Hi, I've been quite busy recently. I plan to set aside some time next week to debug this issue.

Jul 12 '24 04:07 Xuanwo

@Xuanwo any suggestion on the direction to fix the tests on main first?

Hi, I've been quite busy recently. I plan to set aside some time next week to debug this issue.

Thanks!

Jul 12 '24 13:07 imWildCat

I found an issue about Google Drive folder too. I can stat /assets but can't stat /assets/images or any other path with / as separator. Does it cause by this too?

Jan 05 '25 08:01 Shylock-Hg

I found an issue about Google Drive folder too. I can stat /assets but can't stat /assets/images or any other path with / as separator. Does it cause by this too?

Not sure, please open a new issue instead. Thanks!

Jan 08 '25 02:01 Xuanwo