Skip to content

Refactor URI construction and enhance test coverage #126

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 8, 2024
Merged

Refactor URI construction and enhance test coverage #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c73a68b
Refactor URI construction and enhance test coverage
richard-einfinity Oct 8, 2024
d10933d
Merge branch 'main' into bugfix/UriConcatenation
adrianhall Oct 8, 2024
fd2005d
Refactor URI handling and update related tests
richard-einfinity Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/CommunityToolkit.Datasync.Client/Offline/Operations/DeleteOperation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ internal class DeleteOperation(DatasyncOperation operation) : ExecutableOperatio
/// <returns>The result of the push operation (async).</returns>
internal override async Task<ServiceResponse> ExecuteAsync(EntityDatasyncOptions options, CancellationToken cancellationToken = default)
{
Uri endpoint = MakeAbsoluteUri(options.HttpClient.BaseAddress, options.Endpoint);
using HttpRequestMessage request = new(HttpMethod.Delete, new Uri(endpoint, operation.ItemId));
Uri endpoint = MakeAbsoluteUri(options.HttpClient.BaseAddress, options.Endpoint, operation.ItemId);
using HttpRequestMessage request = new(HttpMethod.Delete, endpoint);
if (!string.IsNullOrEmpty(operation.EntityVersion))
{
request.Headers.IfMatch.Add(new EntityTagHeaderValue($"\"{operation.EntityVersion}\""));
Expand Down
9 changes: 6 additions & 3 deletions src/CommunityToolkit.Datasync.Client/Offline/Operations/ExecutableOperation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,22 @@ internal abstract class ExecutableOperation
/// </summary>
/// <param name="baseAddress">The base address from the client.</param>
/// <param name="relativeOrAbsoluteUri">A relative or absolute URI</param>
/// <param name="itemId">The id of the item to append to the URI.</param>
/// <returns></returns>
internal static Uri MakeAbsoluteUri(Uri? baseAddress, Uri relativeOrAbsoluteUri)
internal static Uri MakeAbsoluteUri(Uri? baseAddress, Uri relativeOrAbsoluteUri, string? itemId = null)
{
itemId = string.IsNullOrWhiteSpace(itemId) ? string.Empty : $"/{itemId}";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adrianhall the separator is set here if the itemId is specified.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah - I get it. Although, use of IsNullOrWhiteSpace does change the semantics of the method. If someone submits " ", I believe it gets rejected before this point, so should be ok.

Assuming the tests pass, I'll merge this.


if (relativeOrAbsoluteUri.IsAbsoluteUri)
{
return new Uri($"{relativeOrAbsoluteUri.ToString().TrimEnd('/')}/");
return new Uri($"{relativeOrAbsoluteUri.ToString().TrimEnd('/')}{itemId}");
}

if (baseAddress != null)
{
if (baseAddress.IsAbsoluteUri)
{
return new Uri($"{new Uri(baseAddress, relativeOrAbsoluteUri).ToString().TrimEnd('/')}/");
return new Uri($"{new Uri(baseAddress, relativeOrAbsoluteUri).ToString().TrimEnd('/')}{itemId}");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we may need to trim the slash if it is at the end. In some scenarios, the ASP.NET Core routing middleware treats a http://server/foo/ as lexically different to http://server/foo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @adrianhall,

No problem. I'll update to remove the trailing slash and update the tests. I know in my current environment all the other operations work as expected against the server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adrianhall hopefully that's it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope - this definitely has an issue. See this one:

return new Uri($"{relativeOrAbsoluteUri.ToString().TrimEnd('/')}{itemId}");

If relativeOrAbsoluteUri ends with a / and itemId is set, then there is no separator.

Probably better to construct a string with the right URI in it. Then return new Uri(uri.TrimEnd('/')); at the end.

}

Expand Down
4 changes: 2 additions & 2 deletions src/CommunityToolkit.Datasync.Client/Offline/Operations/ReplaceOperation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ internal class ReplaceOperation(DatasyncOperation operation) : ExecutableOperati
/// <returns>The result of the push operation (async).</returns>
internal override async Task<ServiceResponse> ExecuteAsync(EntityDatasyncOptions options, CancellationToken cancellationToken = default)
{
Uri endpoint = MakeAbsoluteUri(options.HttpClient.BaseAddress, options.Endpoint);
using HttpRequestMessage request = new(HttpMethod.Put, new Uri(endpoint, operation.ItemId))
Uri endpoint = MakeAbsoluteUri(options.HttpClient.BaseAddress, options.Endpoint, operation.ItemId);
using HttpRequestMessage request = new(HttpMethod.Put, endpoint)
{
Content = new StringContent(operation.Item, JsonMediaType)
};
Expand Down
74 changes: 37 additions & 37 deletions tests/CommunityToolkit.Datasync.Client.Test/Offline/OfflineDbContext_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ public async Task PullAsync_List_Works_InitialSync()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");

DatasyncDeltaToken token = this.context.DatasyncDeltaTokens.Find(["CommunityToolkit.Datasync.TestCommon.Databases.ClientMovie"]);
token.Should().NotBeNull();
Expand Down Expand Up @@ -103,10 +103,10 @@ public async Task PullAsync_DbSet_Works_InitialSync()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");

DatasyncDeltaToken token = this.context.DatasyncDeltaTokens.Find(["CommunityToolkit.Datasync.TestCommon.Databases.ClientMovie"]);
token.Should().NotBeNull();
Expand Down Expand Up @@ -147,10 +147,10 @@ public async Task PullAsync_Configurator_Works_InitialSync_Ver1()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$filter=startswith%28title%2C%27abc%27%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$filter=startswith%28title%2C%27abc%27%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");

DatasyncDeltaToken token = this.context.DatasyncDeltaTokens.Find(["q-CommunityToolkit.Datasync.TestCommon.Databases.ClientMovie-abc"]);
token.Should().NotBeNull();
Expand Down Expand Up @@ -191,10 +191,10 @@ public async Task PullAsync_Configurator_Works_InitialSync_Ver2()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$filter=startswith%28title%2C%27abc%27%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$filter=startswith%28title%2C%27abc%27%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");

DatasyncDeltaToken token = this.context.DatasyncDeltaTokens.Find(["q-CommunityToolkit.Datasync.TestCommon.Databases.ClientMovie-a87ec01f71a5577199797b433e3bcc6b"]);
token.Should().NotBeNull();
Expand Down Expand Up @@ -242,10 +242,10 @@ public async Task PullAsync_Configurator_Works_InitialSync_Ver3()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$filter=startswith%28title%2C%27abc%27%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$filter=startswith%28title%2C%27abc%27%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");

DatasyncDeltaToken token = this.context.DatasyncDeltaTokens.Find(["q-CommunityToolkit.Datasync.TestCommon.Databases.ClientMovie-a87ec01f71a5577199797b433e3bcc6b"]);
token.Should().NotBeNull();
Expand Down Expand Up @@ -281,10 +281,10 @@ public async Task PullAsync_Configurator_Works_InitialSync_Ver4()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");

DatasyncDeltaToken token = this.context.DatasyncDeltaTokens.Find(["CommunityToolkit.Datasync.TestCommon.Databases.ClientMovie"]);
token.Should().NotBeNull();
Expand Down Expand Up @@ -321,10 +321,10 @@ public async Task PullAsync_List_Works_FollowonSync()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$filter=%28updatedAt gt cast%282024-08-23T20%3A22%3A54.291Z%2CEdm.DateTimeOffset%29%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$filter=%28updatedAt gt cast%282024-08-23T20%3A22%3A54.291Z%2CEdm.DateTimeOffset%29%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");
}

[Fact]
Expand Down Expand Up @@ -362,10 +362,10 @@ public async Task PullAsync_List_Works_DoesntAddDeletions()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$filter=%28updatedAt gt cast%282024-08-23T20%3A22%3A54.291Z%2CEdm.DateTimeOffset%29%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$filter=%28updatedAt gt cast%282024-08-23T20%3A22%3A54.291Z%2CEdm.DateTimeOffset%29%29&$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");
}

[Fact]
Expand Down Expand Up @@ -410,10 +410,10 @@ public async Task PullAsync_List_Works_DeletionsAndReplacements()
actual.Should().BeEquivalentTo(expected);

this.context.Handler.Requests.Should().HaveCount(4);
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/?$skip=15");
this.context.Handler.Requests[0].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$orderby=updatedAt&$count=true&__includedeleted=true");
this.context.Handler.Requests[1].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=5");
this.context.Handler.Requests[2].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=10");
this.context.Handler.Requests[3].RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies?$skip=15");
}

[Fact]
Expand All @@ -426,7 +426,7 @@ public async Task PullAsync_List_FailedRequest()
pullResult.IsSuccessful.Should().BeFalse();
pullResult.FailedRequests.Should().HaveCount(1);
KeyValuePair<Uri, ServiceResponse> kv = pullResult.FailedRequests.Single();
kv.Key.Should().Be("https://test.zumo.net/tables/movies/?$orderby=updatedAt&$count=true&__includedeleted=true");
kv.Key.Should().Be("https://test.zumo.net/tables/movies?$orderby=updatedAt&$count=true&__includedeleted=true");
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion tests/CommunityToolkit.Datasync.Client.Test/Offline/Operations/AddOperation_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public async Task AddOperation_ExecuteAsync()
HttpRequestMessage request = handler.Requests.SingleOrDefault();
request.Should().NotBeNull();
request.Method.Should().Be(HttpMethod.Post);
request.RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies/");
request.RequestUri.ToString().Should().Be("https://test.zumo.net/tables/movies");
(await request.Content.ReadAsStringAsync()).Should().Be(itemJson);

response.Should().NotBeNull();
Expand Down
Loading
Loading