Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(services/onedrive): add signer to utilize the refresh token #5733

Merged
merged 6 commits into from
Mar 11, 2025

Conversation

erickguan
Copy link
Contributor

Which issue does this PR close?

Closes #5653.
Part of #5677.
Part of #5702.

Rationale for this change

What changes are included in this PR?

This is a somewhat large PR. Reviewing it commit by commit is easier.

  1. This PR extracts the OneDrive service into the "core" structure. Minor changes to names, how to stat, etc.
  2. Uses context for an HTTP client.
  3. Adds a signer. This mostly follows the Google Drive service signer implementation.

Are there any user-facing changes?

I added a few more refresh token settings for the OneDrive service. I added the documentation.

@erickguan
Copy link
Contributor Author

Behavior tests
OPENDAL_TEST=onedrive cargo test behavior --features tests,services-onedrive -- --show-output
   Compiling opendal v0.52.0 (/home/erickg/Dev/opendal/core)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 10.76s
     Running unittests src/lib.rs (target/debug/deps/opendal-6a1470b1a6444062)

running 0 tests                                                                                                                                                                                                                                                                                                                                                         successes:                                                                                                                                                                          
successes:

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 136 filtered out; finished in 0.00s

     Running tests/behavior/main.rs (target/debug/deps/behavior-b979b2a41b98fab9)

running 104 tests
test behavior::test_delete_with_version                       ... ok
test behavior::test_delete_with_not_existing_version          ... ok
test behavior::test_batch_delete                              ... ok
test behavior::test_batch_delete_with_version                 ... ok
test behavior::test_list_with_start_after                     ... ok
test behavior::test_list_files_with_versions                  ... ok
test behavior::test_list_with_versions_and_limit              ... ok
test behavior::test_list_with_versions_and_start_after        ... ok
test behavior::test_list_files_with_deleted                   ... ok
test behavior::test_reader_with_if_none_match                 ... ok
test behavior::test_reader_with_if_match                      ... ok
test behavior::test_read_with_if_unmodified_since             ... ok
test behavior::test_read_with_if_modified_since               ... ok
test behavior::test_reader_with_if_modified_since             ... ok
test behavior::test_reader_with_if_unmodified_since           ... ok
test behavior::test_read_with_override_cache_control          ... ok
test behavior::test_read_with_override_content_type           ... ok
test behavior::test_read_with_override_content_disposition    ... ok
test behavior::test_read_with_not_existing_version            ... ok
test behavior::test_read_with_if_none_match                   ... ok
test behavior::test_read_with_version                         ... ok
test behavior::test_read_with_if_match                        ... ok
test behavior::test_delete_not_existing                       ... ok
test behavior::test_read_not_exist                            ... ok
test behavior::test_create_dir                                ... ok
test behavior::test_stat_with_if_match                        ... ok
test behavior::test_stat_with_if_none_match                   ... ok
test behavior::test_stat_with_if_modified_since               ... ok
test behavior::test_stat_with_if_unmodified_since             ... ok
test behavior::test_stat_with_override_cache_control          ... ok
test behavior::test_stat_with_override_content_disposition    ... ok
test behavior::test_stat_with_override_content_type           ... ok
test behavior::test_stat_root                                 ... ok
test behavior::test_stat_with_version                         ... ok
test behavior::stat_with_not_existing_version                 ... ok
test behavior::test_list_non_exist_dir                        ... ok
test behavior::test_write_with_empty_content                  ... ok
test behavior::test_write_with_dir_path                       ... ok
test behavior::test_delete_empty_dir                          ... ok
test behavior::test_write_with_cache_control                  ... ok
test behavior::test_write_with_content_type                   ... ok
test behavior::test_write_with_content_disposition            ... ok
test behavior::test_write_with_content_encoding               ... ok
test behavior::test_write_with_if_none_match                  ... ok
test behavior::test_write_with_if_not_exists                  ... ok
test behavior::test_write_with_if_match                       ... ok
test behavior::test_write_with_user_metadata                  ... ok
test behavior::test_check                                     ... ok
test behavior::test_writer_write                              ... ok
test behavior::test_read_with_dir_path                        ... ok
test behavior::test_writer_write_with_concurrent              ... ok
test behavior::test_writer_sink                               ... ok
test behavior::test_writer_sink_with_concurrent               ... ok
test behavior::test_stat_dir                                  ... ok
test behavior::test_stat_not_exist                            ... ok
test behavior::test_writer_futures_copy                       ... ok
test behavior::test_writer_futures_copy_with_concurrent       ... ok
test behavior::test_writer_return_metadata                    ... ok
test behavior::test_writer_abort                              ... ok
test behavior::test_read_range                                ... ok
test behavior::test_writer_abort_with_concurrent              ... ok
test behavior::test_stat_with_special_chars                   ... ok
test behavior::test_stat_not_cleaned_path                     ... ok
test behavior::test_write_returns_metadata                    ... ok
test behavior::test_create_dir_existing                       ... ok
test behavior::test_blocking_read_not_exist                   ... ok
test behavior::test_read_with_special_chars                   ... ok
test behavior::test_blocking_create_dir                       ... ok
test behavior::test_delete_with_special_chars                 ... ok
test behavior::test_stat_nested_parent_dir                    ... ok
test behavior::test_blocking_write_with_dir_path              ... ok
test behavior::test_stat_file                                 ... ok
test behavior::test_write_with_special_chars                  ... ok
test behavior::test_delete_file                               ... ok
test behavior::test_write_only                                ... ok
test behavior::test_list_file_with_recursive                  ... ok
test behavior::test_blocking_create_dir_existing              ... ok
test behavior::test_blocking_stat_not_exist                   ... ok
test behavior::test_read_full                                 ... ok
test behavior::test_blocking_stat_dir                         ... ok
test behavior::test_blocking_delete_file                      ... ok
test behavior::test_list_dir_with_file_path                   ... ok
test behavior::test_list_dir                                  ... ok
test behavior::test_blocking_stat_file                        ... ok
test behavior::test_blocking_read_range                       ... ok
test behavior::test_blocking_write_file                       ... ok
test behavior::test_list_prefix                               ... ok
test behavior::test_blocking_write_returns_metadata           ... ok
test behavior::test_blocking_remove_one_file                  ... ok
test behavior::test_blocking_stat_with_special_chars          ... ok
test behavior::test_blocking_read_full                        ... ok
test behavior::test_list_sub_dir                              ... ok
test behavior::test_remove_one_file                           ... ok
test behavior::test_reader                                    ... ok
test behavior::test_blocking_write_with_special_chars         ... ok
test behavior::test_writer_write_with_overwrite               ... ok
test behavior::test_list_nested_dir                           ... ok
test behavior::test_list_dir_with_recursive                   ... ok
test behavior::test_list_dir_with_recursive_no_trailing_slash ... ok
test behavior::test_list_root_with_recursive                  ... ok
test behavior::test_remove_all                                ... ok
test behavior::test_list_rich_dir                             ... ok
test behavior::test_list_empty_dir                            ... ok
test behavior::test_delete_stream                             ... ok

test result: ok. 104 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 128.37s

I know the OneDrive behavior test is flaky. Fixing tests will take a few PRs and love. But it's also a good time to add new features:

  • version
  • caching if the work on the other side goes well
  • etc

cc @emliunix Feel free to try this branch if you find time!

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other changes look great—thank you!

root: String,
access_token: String,
client: HttpClient,
pub struct OneDriveBackend {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming here needs to align with the scheme. Since our scheme is onedrive instead of one_drive, we must use Onedrive instead of OneDrivew.

Copy link
Contributor Author

@erickguan erickguan Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops, good catch. I didn't mean to change this struct name. Will push a fix.

I kept a few modules that are not "public interfaces" with OneDriveXXX because OneDrive is the product name. If you want to keep the same convention following the schema name, please ping me!

@erickguan erickguan requested a review from Xuanwo March 11, 2025 11:07
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I really love this change!

@Xuanwo Xuanwo merged commit 64a0f44 into apache:main Mar 11, 2025
94 checks passed
@erickguan erickguan deleted the onedrive-core branch March 11, 2025 12:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set up behavior tests for OneDrive
2 participants