CommunityToolkit / Datasync

A collection of libraries that implement a client-server system used for synchronizing data from a cloud-based database.
https://communitytoolkit.github.io/Datasync/
Other
68 stars 13 forks source link

Uri's malformed when using a colon to pack partition keys. #124

Closed richard-einfinity closed 1 month ago

richard-einfinity commented 1 month ago

Describe the bug

Uri's are malformed when the id contains a colon.

To Reproduce

Steps to reproduce the behavior:

Generate client side id's containing colons and attempt to update or delete the entity's after the initial sync.

Expected behavior

Entity's update and delete after pushing changes.

What platforms?

Note: Any bug or feature request that is opened for an unsupported environment will be automatically closed.

Additional context

We we're previously discussing strategy's for dealing with partition keys etc on the previous version of the sdk. I opted for packing the id field with the client generated id and the partition key separated by a colon.

I've updated the client library to the latest version and it looks as though the mechanism for constructing the request uri's is now causing the uri's to become malformed. It would seem that if the string passed as the relative part of the Uri constructor contains a colon the constructor assumes its an absolute uri containing a schema and returns only this portion of the uri.

So here https://github.com/CommunityToolkit/Datasync/blob/fb3018dfb9918310910b4ae2697d173c9e8cd7ed/src/CommunityToolkit.Datasync.Client/Offline/Operations/DeleteOperation.cs#L25 and here https://github.com/CommunityToolkit/Datasync/blob/fb3018dfb9918310910b4ae2697d173c9e8cd7ed/src/CommunityToolkit.Datasync.Client/Offline/Operations/ReplaceOperation.cs#L25

Where endpoint might be https://dummysyncapi.azurewebsites.net/tables/item and id is 93AE2D56-CA9B-4397-B59E-EAB245DDE9E8:AF1E3CBC-A58D-4165-A18F-4C2B35021D02

new Uri(endpoint, id) returns 93AE2D56-CA9B-4397-B59E-EAB245DDE9E8:AF1E3CBC-A58D-4165-A18F-4C2B35021D02

The previous implementation combined the path of the endpoint and the id as a string before combining it with the base uri to make the absolute uri.

richard-einfinity commented 1 month ago

Possible solution. Update MakeAbsoluteUri signature to take optional item id parameter with default value of string empty and combine the id inside that method after the trailing slash.

https://github.com/CommunityToolkit/Datasync/blob/fb3018dfb9918310910b4ae2697d173c9e8cd7ed/src/CommunityToolkit.Datasync.Client/Offline/Operations/ExecutableOperation.cs#L26

adrianhall commented 1 month ago

Good catch - I suspect that there are other areas where this occurs. Is this also a problem in the "online" operations?

richard-einfinity commented 1 month ago

hi @adrianhall

Just looking at the online operations and the Uri is built by concatenating the string so it avoids this issue.

https://github.com/CommunityToolkit/Datasync/blob/fb3018dfb9918310910b4ae2697d173c9e8cd7ed/src/CommunityToolkit.Datasync.Client/Service/DatasyncServiceClient.cs#L450

Happy to submit a PR if your agreed with the suggested solution.

Cheers

Rich

adrianhall commented 1 month ago

Sounds like a reasonable fix to me. It was pretty much what I was going to do as well. Submit the PR - make sure all the tests pass.