Skip to content

Conversation

dkocher
Copy link
Contributor

@dkocher dkocher commented Aug 7, 2025

Extract UUID from ETag formatted as "{D7751A14-FEA5-4BDA-969E-F8022E58B7E7},9" which remains constant when moving files.

@dkocher dkocher added this to the 9.2 milestone Aug 7, 2025
@dkocher dkocher requested a review from a team as a code owner August 7, 2025 11:09
@dkocher dkocher added onedrive OneDrive Protocol Implementation sharepoint labels Aug 7, 2025
Copilot

This comment was marked as outdated.

@dkocher dkocher requested a review from Copilot August 7, 2025 11:17
Copilot

This comment was marked as outdated.

@AliveDevil
Copy link
Contributor

We have Hashes for Files available already:
https://github.com/iterate-ch/onedrive-java-client/blob/develop/src/main/java/org/nuxeo/onedrive/client/types/File.java#L9-L11

quickXorHash is guaranteed to be available.

@AliveDevil
Copy link
Contributor

AliveDevil commented Aug 7, 2025

Alternatively, use the cTag1-property:

An eTag for the content of the item. This eTag isn't changed if only the metadata is changed. Note This property isn't returned if the item is a folder. Read-only.

Relying on a implementation detail of the eTag is dangerous.

Footnotes

  1. https://learn.microsoft.com/en-us/graph/api/resources/driveitem?view=graph-rest-1.0#properties

@dkocher dkocher marked this pull request as draft August 7, 2025 11:52
@dkocher dkocher marked this pull request as ready for review August 10, 2025 15:35
@dkocher dkocher requested a review from Copilot August 10, 2025 15:35
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements UUID-based checksum comparison for OneDrive/SharePoint by extracting UUIDs from ETag values that remain constant when moving files, enhancing file synchronization accuracy.

  • Modifies Checksum class to support UUID extraction from ETag values formatted as "{UUID},version"
  • Updates OneDrive features to use checksum-based comparison instead of timestamp-only comparison
  • Adds comprehensive test coverage for checksum behavior during file operations

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

File Description
core/src/main/java/ch/cyberduck/core/io/Checksum.java Enhanced checksum parsing to support Base64 and improved hex string handling with proper validation
onedrive/src/main/java/ch/cyberduck/core/onedrive/features/GraphAttributesFinderFeature.java Added checksum extraction from OneDrive file hashes (QuickXorHash)
onedrive/src/main/java/ch/cyberduck/core/onedrive/GraphProtocol.java Updated comparison service to use checksum-based comparison with fallback to timestamp
onedrive/src/test/java/ch/cyberduck/core/onedrive/*Test.java Added comprehensive test assertions for checksum behavior during write and move operations

@dkocher dkocher changed the title Set UUID as checksum to compare file changes Set QuickXorHash as checksum to compare file changes Aug 10, 2025
if(file != null) {
final Hashes hashes = file.getHashes();
if(hashes != null) {
attributes.setChecksum(Checksum.parse(hashes.getQuickXorHash()));
Copy link
Contributor

Choose a reason for hiding this comment

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

This relies on quickXorHash always being 20 Bytes, and is misrepresented as SHA1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Full implementation requires us to implement quickXorHash as a ChecksumCompute feature.

@dkocher dkocher marked this pull request as draft August 11, 2025 12:48
@dkocher dkocher removed this from the 9.2 milestone Aug 11, 2025
@dkocher dkocher closed this Aug 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
onedrive OneDrive Protocol Implementation sharepoint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants