From a622954d6ae7020b8b7c28bb6440920bee4f7521 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Wed, 2 Mar 2022 10:59:56 +0100 Subject: [PATCH 1/3] Bugfix: hide Self link in included resources when there's no registered controller for it. Before, it would use the controller URL from the current request in the Self link, which is wrong. --- .../Serialization/Response/LinkBuilder.cs | 8 ++++++++ .../IntegrationTests/Links/LinkInclusionTests.cs | 1 + .../UnitTests/Links/LinkInclusionTests.cs | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/JsonApiDotNetCore/Serialization/Response/LinkBuilder.cs b/src/JsonApiDotNetCore/Serialization/Response/LinkBuilder.cs index 38e63ea9cb..000c75bc80 100644 --- a/src/JsonApiDotNetCore/Serialization/Response/LinkBuilder.cs +++ b/src/JsonApiDotNetCore/Serialization/Response/LinkBuilder.cs @@ -255,6 +255,14 @@ private bool ShouldIncludeResourceLink(LinkTypes linkType, ResourceType resource private string? GetLinkForResourceSelf(ResourceType resourceType, IIdentifiable resource) { string? controllerName = _controllerResourceMapping.GetControllerNameForResourceType(resourceType); + + if (controllerName == null) + { + // When passing null to RenderLinkForAction, it uses the controller for the current endpoint. This is incorrect for + // included resources of a different resource type: it should hide their Self links when there's no controller for them. + return null; + } + IDictionary routeValues = GetRouteValues(resource.StringId!, null); return RenderLinkForAction(controllerName, GetPrimaryControllerActionName, routeValues); diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionTests.cs index 128a19efb9..c23bed2b6b 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionTests.cs @@ -17,6 +17,7 @@ public LinkInclusionTests(IntegrationTestContext testContext.UseController(); testContext.UseController(); + testContext.UseController(); } [Fact] diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Links/LinkInclusionTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/Links/LinkInclusionTests.cs index 6c0e9fe1a0..5effda921f 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/Links/LinkInclusionTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/Links/LinkInclusionTests.cs @@ -1,4 +1,5 @@ using FluentAssertions; +using Humanizer; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Middleware; using JsonApiDotNetCore.Queries; @@ -386,7 +387,7 @@ public ResourceType GetResourceTypeForController(Type? controllerType) public string? GetControllerNameForResourceType(ResourceType? resourceType) { - return null; + return resourceType == null ? null : $"{resourceType.PublicName.Pascalize()}Controller"; } } From 473d53b76faa19f69556a5ff0168a631808fc0db Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Wed, 2 Mar 2022 11:28:08 +0100 Subject: [PATCH 2/3] Added test --- .../Links/LinkInclusionIncludeTests.cs | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionIncludeTests.cs diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionIncludeTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionIncludeTests.cs new file mode 100644 index 0000000000..8c8b8e4ec4 --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionIncludeTests.cs @@ -0,0 +1,61 @@ +using System.Net; +using FluentAssertions; +using JsonApiDotNetCore.Serialization.Objects; +using TestBuildingBlocks; +using Xunit; + +namespace JsonApiDotNetCoreTests.IntegrationTests.Links; + +public sealed class LinkInclusionIncludeTests : IClassFixture, LinksDbContext>> +{ + private readonly IntegrationTestContext, LinksDbContext> _testContext; + private readonly LinksFakers _fakers = new(); + + public LinkInclusionIncludeTests(IntegrationTestContext, LinksDbContext> testContext) + { + _testContext = testContext; + + testContext.UseController(); + } + + [Fact] + public async Task Hides_Self_link_in_included_resources_for_unregistered_controllers() + { + // Arrange + PhotoLocation location = _fakers.PhotoLocation.Generate(); + location.Photo = _fakers.Photo.Generate(); + location.Album = _fakers.PhotoAlbum.Generate(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.PhotoLocations.Add(location); + await dbContext.SaveChangesAsync(); + }); + + string route = $"/photoLocations/{location.StringId}?include=photo,album"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.Included.ShouldHaveCount(2); + + responseDocument.Included.Should().ContainSingle(resource => resource.Type == "photos").Subject.With(resource => + { + resource.Links.Should().BeNull(); + + resource.Relationships.ShouldContainKey("location").With(value => + { + value.ShouldNotBeNull(); + value.Links.ShouldNotBeNull(); + }); + }); + + responseDocument.Included.Should().ContainSingle(resource => resource.Type == "photoAlbums").Subject.With(resource => + { + resource.Links.Should().BeNull(); + }); + } +} From ab67c860a525c4d8269abba70bb5693bcc2106dd Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Tue, 8 Mar 2022 13:10:28 +0100 Subject: [PATCH 3/3] Fixed: also hide links for missing controllers on deeply nested relationships from includes --- .../Serialization/Response/LinkBuilder.cs | 15 +++++++-------- .../Links/LinkInclusionIncludeTests.cs | 18 +++++++++++------- ...CreateResourceWithToOneRelationshipTests.cs | 1 + .../ReadWrite/Fetching/FetchResourceTests.cs | 1 + .../ReplaceToManyRelationshipTests.cs | 1 + .../Resources/UpdateToOneRelationshipTests.cs | 1 + .../IntegrationTests/ReadWrite/WorkTag.cs | 1 + .../UnitTests/Links/LinkInclusionTests.cs | 5 ++++- 8 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/JsonApiDotNetCore/Serialization/Response/LinkBuilder.cs b/src/JsonApiDotNetCore/Serialization/Response/LinkBuilder.cs index 000c75bc80..0bad02066b 100644 --- a/src/JsonApiDotNetCore/Serialization/Response/LinkBuilder.cs +++ b/src/JsonApiDotNetCore/Serialization/Response/LinkBuilder.cs @@ -255,14 +255,6 @@ private bool ShouldIncludeResourceLink(LinkTypes linkType, ResourceType resource private string? GetLinkForResourceSelf(ResourceType resourceType, IIdentifiable resource) { string? controllerName = _controllerResourceMapping.GetControllerNameForResourceType(resourceType); - - if (controllerName == null) - { - // When passing null to RenderLinkForAction, it uses the controller for the current endpoint. This is incorrect for - // included resources of a different resource type: it should hide their Self links when there's no controller for them. - return null; - } - IDictionary routeValues = GetRouteValues(resource.StringId!, null); return RenderLinkForAction(controllerName, GetPrimaryControllerActionName, routeValues); @@ -320,6 +312,13 @@ private bool ShouldIncludeResourceLink(LinkTypes linkType, ResourceType resource protected virtual string? RenderLinkForAction(string? controllerName, string actionName, IDictionary routeValues) { + if (controllerName == null) + { + // When passing null to LinkGenerator, it uses the controller for the current endpoint. This is incorrect for + // included resources of a different resource type: it should hide its links when there's no controller for them. + return null; + } + return _options.UseRelativeLinks ? _linkGenerator.GetPathByAction(HttpContext, actionName, controllerName, routeValues) : _linkGenerator.GetUriByAction(HttpContext, actionName, controllerName, routeValues); diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionIncludeTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionIncludeTests.cs index 8c8b8e4ec4..f93ea357d4 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionIncludeTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Links/LinkInclusionIncludeTests.cs @@ -19,7 +19,7 @@ public LinkInclusionIncludeTests(IntegrationTestContext // Assert httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + responseDocument.Data.SingleValue.ShouldNotBeNull(); + + responseDocument.Data.SingleValue.Relationships.ShouldContainKey("photo").With(value => + { + value.ShouldNotBeNull(); + value.Links.ShouldNotBeNull(); + }); + responseDocument.Included.ShouldHaveCount(2); responseDocument.Included.Should().ContainSingle(resource => resource.Type == "photos").Subject.With(resource => { resource.Links.Should().BeNull(); - - resource.Relationships.ShouldContainKey("location").With(value => - { - value.ShouldNotBeNull(); - value.Links.ShouldNotBeNull(); - }); + resource.Relationships.Should().BeNull(); }); responseDocument.Included.Should().ContainSingle(resource => resource.Type == "photoAlbums").Subject.With(resource => { resource.Links.Should().BeNull(); + resource.Relationships.Should().BeNull(); }); } } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Creating/CreateResourceWithToOneRelationshipTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Creating/CreateResourceWithToOneRelationshipTests.cs index 377edfc6a3..b70e53eaf0 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Creating/CreateResourceWithToOneRelationshipTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Creating/CreateResourceWithToOneRelationshipTests.cs @@ -22,6 +22,7 @@ public CreateResourceWithToOneRelationshipTests(IntegrationTestContext(); testContext.UseController(); testContext.UseController(); + testContext.UseController(); var options = (JsonApiOptions)testContext.Factory.Services.GetRequiredService(); options.AllowClientGeneratedIds = true; diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Fetching/FetchResourceTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Fetching/FetchResourceTests.cs index 5f2e13d3e4..ac9d9fb872 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Fetching/FetchResourceTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Fetching/FetchResourceTests.cs @@ -17,6 +17,7 @@ public FetchResourceTests(IntegrationTestContext(); testContext.UseController(); + testContext.UseController(); } [Fact] diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/ReplaceToManyRelationshipTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/ReplaceToManyRelationshipTests.cs index 158ad4cc10..0c44fb6995 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/ReplaceToManyRelationshipTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/ReplaceToManyRelationshipTests.cs @@ -19,6 +19,7 @@ public ReplaceToManyRelationshipTests(IntegrationTestContext(); + testContext.UseController(); testContext.ConfigureServicesAfterStartup(services => { diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateToOneRelationshipTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateToOneRelationshipTests.cs index 8c1a5a4ee8..1d0b689713 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateToOneRelationshipTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Resources/UpdateToOneRelationshipTests.cs @@ -20,6 +20,7 @@ public UpdateToOneRelationshipTests(IntegrationTestContext(); testContext.UseController(); testContext.UseController(); + testContext.UseController(); testContext.ConfigureServicesAfterStartup(services => { diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/WorkTag.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/WorkTag.cs index 585690fb77..f2408bd022 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/WorkTag.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/WorkTag.cs @@ -5,6 +5,7 @@ namespace JsonApiDotNetCoreTests.IntegrationTests.ReadWrite; [UsedImplicitly(ImplicitUseTargetFlags.Members)] +[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.ReadWrite")] public sealed class WorkTag : Identifiable { [Attr] diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Links/LinkInclusionTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/Links/LinkInclusionTests.cs index 5effda921f..81bd4a6ef6 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/Links/LinkInclusionTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/Links/LinkInclusionTests.cs @@ -69,7 +69,10 @@ public void Applies_cascading_settings_for_top_level_links(LinkTypes linksInReso PrimaryId = "1", IsCollection = true, Kind = EndpointKind.Relationship, - Relationship = new HasOneAttribute() + Relationship = new HasOneAttribute + { + LeftType = exampleResourceType + } }; var paginationContext = new PaginationContext