[gdrive] potential regression of folder name handling

[imWildCat]

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

might be a regression recently

[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.

[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

[imWildCat]

Interesting. It also repros using v0.45.1

[Xuanwo]

opendal/core/src/services/gdrive/core.rs

Lines 396 to 405 in 27c6a77

	async fn create_dir(&self, parent_id: &str, name: &str) -> Result<String> {
	let url = "https://www.googleapis.com/drive/v3/files";
	let content = serde_json::to_vec(&json!({
	"name": name,
	"mimeType": "application/vnd.google-apps.folder",
	// If the parent is not provided, the folder will be created in the root folder.
	"parents": [parent_id],
	}))
	.map_err(new_json_serialize_error)?;

Maybe we should trim the / suffix for dir?

[imWildCat]

let me try!

[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...

[Xuanwo]

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/?

[imWildCat]

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

[Xuanwo]

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.

[imWildCat]

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

[imWildCat]

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

[imWildCat]

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

I believe we have many regressions since #3975

[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.

[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.

Thanks!