feat(onedrive): add ref support #1435
Conversation
|
Please give some tests. And does this work after the reference's token refreshed? |
jyxjjj
added
enhancement
invalid
|
I've run it on my cluster, and it works properly. It should work after the reference token is refreshed, as it will use the reference token for each request. Some extra tests might be needed, but I'm not familiar with Go, so I'll wait for someone else to pick it up. |
jerrita
force-pushed
the
feat-onedrive-ref
branch
from
24cc5d1 to
91b625e
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds reference support for OneDrive drivers, allowing multiple OneDrive instances to share authentication tokens without requiring separate login processes. This enables sharing multiple paths on OneDrive efficiently.
Key changes:
- Added reference field to OneDrive struct to store shared authentication
- Modified token refresh logic to use reference tokens when available
- Added InitReference method to establish driver references
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| drivers/onedrive/driver.go | Adds ref field and InitReference method for sharing authentication between OneDrive instances |
| drivers/onedrive/util.go | Updates token refresh and request methods to use reference tokens when available |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
drivers/onedrive/util.go
Outdated
| d.mutex.Lock() | ||
| ref := d.ref | ||
| if ref != nil { | ||
| err := ref._refreshToken() | ||
| if err != nil { | ||
| d.mutex.Unlock() | ||
| return err | ||
| } | ||
| d.AccessToken = ref.AccessToken | ||
| d.RefreshToken = ref.RefreshToken | ||
| d.mutex.Unlock() | ||
| return nil | ||
| } | ||
| d.mutex.Unlock() |
There was a problem hiding this comment.
Potential race condition: The reference token refresh and token copying should be protected by both mutexes. Consider acquiring ref.mutex.Lock() before calling ref._refreshToken() to prevent concurrent modifications to the reference's tokens.
|
It is not extremely high concurrency; I think the lock just adds complexity and reduces performance. The initial commit might be sufficient. |
jerrita
force-pushed
the
feat-onedrive-ref
branch
from
292c42f to
24cc5d1
Compare
|
Reverted to the initial commit. |
4 participants
Description
Add reference support for OneDrive.
Motivation and Context
This feature is useful when you want to share multiple paths on OneDrive without having to log in repeatedly.
How Has This Been Tested?
Tested on my cluster; may require additional testing.
Checklist
我已阅读 CONTRIBUTING 文档。
go fmtor prettier.我已使用
go fmt或 prettier 格式化提交的代码。我已为此 PR 添加了适当的标签（如无权限或需要的标签不存在，请在描述中说明，管理员将后续处理）。
我已在适当情况下使用"Request review"功能请求相关代码作者进行审查。
我已相应更新了相关仓库（若适用）。