Skip to content

Commit d21532d

Browse files
author
Bart Koelman
authored
Fixed invalid top-level self link. (#672)
Removed dependency on ResourceContext in the process, because it was no longer needed. Additionally I changed the decision to render which links to be taken from the related entity (from Article in a request like /people/1/articles instead of Person), because I believe that is more correct than taking it from the primary entity, which is the old behavior.
1 parent be4fc39 commit d21532d

File tree

9 files changed

+85
-55
lines changed

9 files changed

+85
-55
lines changed

src/JsonApiDotNetCore/Serialization/Client/RequestSerializer.cs

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Collections;
33
using System.Collections.Generic;
44
using System.Linq;
@@ -15,9 +15,10 @@ public class RequestSerializer : BaseDocumentBuilder, IRequestSerializer
1515
{
1616
private Type _currentTargetedResource;
1717
private readonly IResourceGraph _resourceGraph;
18+
1819
public RequestSerializer(IResourceGraph resourceGraph,
1920
IResourceObjectBuilder resourceObjectBuilder)
20-
: base(resourceObjectBuilder, resourceGraph)
21+
: base(resourceObjectBuilder)
2122
{
2223
_resourceGraph = resourceGraph;
2324
}
@@ -96,4 +97,4 @@ private List<RelationshipAttribute> GetRelationshipsToSerialize(IIdentifiable en
9697
return RelationshipsToSerialize.ToList();
9798
}
9899
}
99-
}
100+
}

src/JsonApiDotNetCore/Serialization/Common/DocumentBuilder.cs

+2-3
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,11 @@ namespace JsonApiDotNetCore.Serialization
1111
/// </summary>
1212
public abstract class BaseDocumentBuilder
1313
{
14-
protected readonly IResourceContextProvider _provider;
1514
protected readonly IResourceObjectBuilder _resourceObjectBuilder;
16-
protected BaseDocumentBuilder(IResourceObjectBuilder resourceObjectBuilder, IResourceContextProvider provider)
15+
16+
protected BaseDocumentBuilder(IResourceObjectBuilder resourceObjectBuilder)
1717
{
1818
_resourceObjectBuilder = resourceObjectBuilder;
19-
_provider = provider;
2019
}
2120

2221
/// <summary>

src/JsonApiDotNetCore/Serialization/Server/Builders/LinkBuilder.cs

+40-18
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Text;
12
using JsonApiDotNetCore.Configuration;
23
using JsonApiDotNetCore.Internal;
34
using JsonApiDotNetCore.Internal.Contracts;
@@ -27,17 +28,19 @@ public LinkBuilder(ILinksConfiguration options,
2728
}
2829

2930
/// <inheritdoc/>
30-
public TopLevelLinks GetTopLevelLinks(ResourceContext primaryResource)
31+
public TopLevelLinks GetTopLevelLinks()
3132
{
33+
ResourceContext resourceContext = _currentRequest.GetRequestResource();
34+
3235
TopLevelLinks topLevelLinks = null;
33-
if (ShouldAddTopLevelLink(primaryResource, Link.Self))
36+
if (ShouldAddTopLevelLink(resourceContext, Link.Self))
3437
{
35-
topLevelLinks = new TopLevelLinks { Self = GetSelfTopLevelLink(primaryResource.ResourceName) };
38+
topLevelLinks = new TopLevelLinks { Self = GetSelfTopLevelLink(resourceContext) };
3639
}
3740

38-
if (ShouldAddTopLevelLink(primaryResource, Link.Paging) && _pageService.CanPaginate)
41+
if (ShouldAddTopLevelLink(resourceContext, Link.Paging) && _pageService.CanPaginate)
3942
{
40-
SetPageLinks(primaryResource, topLevelLinks ??= new TopLevelLinks());
43+
SetPageLinks(resourceContext, topLevelLinks ??= new TopLevelLinks());
4144
}
4245

4346
return topLevelLinks;
@@ -48,50 +51,69 @@ public TopLevelLinks GetTopLevelLinks(ResourceContext primaryResource)
4851
/// configuration on the <see cref="ResourceContext"/>, and if not configured, by checking with the
4952
/// global configuration in <see cref="ILinksConfiguration"/>.
5053
/// </summary>
54+
/// <param name="resourceContext"></param>
5155
/// <param name="link"></param>
52-
private bool ShouldAddTopLevelLink(ResourceContext primaryResource, Link link)
56+
private bool ShouldAddTopLevelLink(ResourceContext resourceContext, Link link)
5357
{
54-
if (primaryResource.TopLevelLinks != Link.NotConfigured)
58+
if (resourceContext.TopLevelLinks != Link.NotConfigured)
5559
{
56-
return primaryResource.TopLevelLinks.HasFlag(link);
60+
return resourceContext.TopLevelLinks.HasFlag(link);
5761
}
5862

5963
return _options.TopLevelLinks.HasFlag(link);
6064
}
6165

62-
private void SetPageLinks(ResourceContext primaryResource, TopLevelLinks links)
66+
private void SetPageLinks(ResourceContext resourceContext, TopLevelLinks links)
6367
{
6468
if (_pageService.CurrentPage > 1)
6569
{
66-
links.Prev = GetPageLink(primaryResource, _pageService.CurrentPage - 1, _pageService.CurrentPageSize);
70+
links.Prev = GetPageLink(resourceContext, _pageService.CurrentPage - 1, _pageService.CurrentPageSize);
6771
}
6872

6973
if (_pageService.CurrentPage < _pageService.TotalPages)
7074
{
71-
links.Next = GetPageLink(primaryResource, _pageService.CurrentPage + 1, _pageService.CurrentPageSize);
75+
links.Next = GetPageLink(resourceContext, _pageService.CurrentPage + 1, _pageService.CurrentPageSize);
7276
}
7377

7478
if (_pageService.TotalPages > 0)
7579
{
76-
links.Self = GetPageLink(primaryResource, _pageService.CurrentPage, _pageService.CurrentPageSize);
77-
links.First = GetPageLink(primaryResource, 1, _pageService.CurrentPageSize);
78-
links.Last = GetPageLink(primaryResource, _pageService.TotalPages, _pageService.CurrentPageSize);
80+
links.Self = GetPageLink(resourceContext, _pageService.CurrentPage, _pageService.CurrentPageSize);
81+
links.First = GetPageLink(resourceContext, 1, _pageService.CurrentPageSize);
82+
links.Last = GetPageLink(resourceContext, _pageService.TotalPages, _pageService.CurrentPageSize);
7983
}
8084
}
8185

82-
private string GetSelfTopLevelLink(string resourceName)
86+
private string GetSelfTopLevelLink(ResourceContext resourceContext)
8387
{
84-
return $"{GetBasePath()}/{resourceName}";
88+
var builder = new StringBuilder();
89+
builder.Append(GetBasePath());
90+
builder.Append("/");
91+
builder.Append(resourceContext.ResourceName);
92+
93+
string resourceId = _currentRequest.BaseId;
94+
if (resourceId != null)
95+
{
96+
builder.Append("/");
97+
builder.Append(resourceId);
98+
}
99+
100+
if (_currentRequest.RequestRelationship != null)
101+
{
102+
builder.Append("/");
103+
builder.Append(_currentRequest.RequestRelationship.PublicRelationshipName);
104+
}
105+
106+
return builder.ToString();
85107
}
86108

87-
private string GetPageLink(ResourceContext primaryResource, int pageOffset, int pageSize)
109+
private string GetPageLink(ResourceContext resourceContext, int pageOffset, int pageSize)
88110
{
89111
if (_pageService.Backwards)
90112
{
91113
pageOffset = -pageOffset;
92114
}
93115

94-
return $"{GetBasePath()}/{primaryResource.ResourceName}?page[size]={pageSize}&page[number]={pageOffset}";
116+
return $"{GetBasePath()}/{resourceContext.ResourceName}?page[size]={pageSize}&page[number]={pageOffset}";
95117
}
96118

97119

src/JsonApiDotNetCore/Serialization/Server/Contracts/ILinkBuilder.cs

+1-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
using JsonApiDotNetCore.Internal;
21
using JsonApiDotNetCore.Models;
32
using JsonApiDotNetCore.Models.Links;
43

@@ -12,8 +11,7 @@ public interface ILinkBuilder
1211
/// <summary>
1312
/// Builds the links object that is included in the top-level of the document.
1413
/// </summary>
15-
/// <param name="primaryResource">The primary resource of the response body</param>
16-
TopLevelLinks GetTopLevelLinks(ResourceContext primaryResource);
14+
TopLevelLinks GetTopLevelLinks();
1715
/// <summary>
1816
/// Builds the links object for resources in the primary data.
1917
/// </summary>

src/JsonApiDotNetCore/Serialization/Server/ResponseSerializer.cs

+3-4
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,8 @@ public ResponseSerializer(IMetaBuilder<TResource> metaBuilder,
4040
ILinkBuilder linkBuilder,
4141
IIncludedResourceObjectBuilder includedBuilder,
4242
IFieldsToSerialize fieldsToSerialize,
43-
IResourceObjectBuilder resourceObjectBuilder,
44-
IResourceContextProvider provider) :
45-
base(resourceObjectBuilder, provider)
43+
IResourceObjectBuilder resourceObjectBuilder) :
44+
base(resourceObjectBuilder)
4645
{
4746
_fieldsToSerialize = fieldsToSerialize;
4847
_linkBuilder = linkBuilder;
@@ -158,7 +157,7 @@ private List<RelationshipAttribute> GetRelationshipsToSerialize(Type resourceTyp
158157
/// </summary>
159158
private void AddTopLevelObjects(Document document)
160159
{
161-
document.Links = _linkBuilder.GetTopLevelLinks(_provider.GetResourceContext<TResource>());
160+
document.Links = _linkBuilder.GetTopLevelLinks();
162161
document.Meta = _metaBuilder.GetMeta();
163162
document.Included = _includedBuilder.Build();
164163
}

test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingRelationshipsTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System.Net;
1+
using System.Net;
22
using System.Net.Http;
33
using System.Threading.Tasks;
44
using Bogus;
@@ -43,7 +43,7 @@ public async Task Request_UnsetRelationship_Returns_Null_DataObject()
4343
var server = new TestServer(builder);
4444
var client = server.CreateClient();
4545
var request = new HttpRequestMessage(httpMethod, route);
46-
var expectedBody = "{\"meta\":{\"copyright\":\"Copyright 2015 Example Corp.\",\"authors\":[\"Jared Nance\",\"Maurits Moeys\",\"Harro van der Kroft\"]},\"links\":{\"self\":\"http://localhost/api/v1/people\"},\"data\":null}";
46+
var expectedBody = "{\"meta\":{\"copyright\":\"Copyright 2015 Example Corp.\",\"authors\":[\"Jared Nance\",\"Maurits Moeys\",\"Harro van der Kroft\"]},\"links\":{\"self\":\"http://localhost" + route + "\"},\"data\":null}";
4747

4848
// Act
4949
var response = await client.SendAsync(request);

test/UnitTests/Builders/LinkBuilderTests.cs

+27-9
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using JsonApiDotNetCore.Configuration;
23
using JsonApiDotNetCore.Internal;
34
using JsonApiDotNetCore.Internal.Contracts;
@@ -18,6 +19,8 @@ public class LinkBuilderTests
1819
private readonly IPageService _pageService;
1920
private readonly Mock<IResourceGraph> _provider = new Mock<IResourceGraph>();
2021
private const string _host = "http://www.example.com";
22+
private const int _baseId = 123;
23+
private const string _relationshipName = "author";
2124
private const string _topSelf = "http://www.example.com/articles";
2225
private const string _resourceSelf = "http://www.example.com/articles/123";
2326
private const string _relSelf = "http://www.example.com/articles/123/relationships/author";
@@ -47,7 +50,7 @@ public void BuildResourceLinks_GlobalAndResourceConfiguration_ExpectedResult(Lin
4750
var builder = new LinkBuilder(config, GetRequestManager(), null, _provider.Object);
4851

4952
// Act
50-
var links = builder.GetResourceLinks("articles", "123");
53+
var links = builder.GetResourceLinks("articles", _baseId.ToString());
5154

5255
// Assert
5356
if (expectedResult == null)
@@ -96,7 +99,7 @@ public void BuildRelationshipLinks_GlobalResourceAndAttrConfiguration_ExpectedLi
9699
var attr = new HasOneAttribute(links: relationship) { RightType = typeof(Author), PublicRelationshipName = "author" };
97100

98101
// Act
99-
var links = builder.GetRelationshipLinks(attr, new Article { Id = 123 });
102+
var links = builder.GetRelationshipLinks(attr, new Article { Id = _baseId });
100103

101104
// Assert
102105
if (expectedSelfLink == null && expectedRelatedLink == null)
@@ -131,20 +134,32 @@ public void BuildRelationshipLinks_GlobalResourceAndAttrConfiguration_ExpectedLi
131134
[InlineData(Link.None, Link.Self, _topSelf, false)]
132135
[InlineData(Link.None, Link.Paging, null, true)]
133136
[InlineData(Link.None, Link.None, null, false)]
137+
[InlineData(Link.All, Link.Self, _resourceSelf, false)]
138+
[InlineData(Link.Self, Link.Self, _resourceSelf, false)]
139+
[InlineData(Link.Paging, Link.Self, _resourceSelf, false)]
140+
[InlineData(Link.None, Link.Self, _resourceSelf, false)]
141+
[InlineData(Link.All, Link.Self, _relRelated, false)]
142+
[InlineData(Link.Self, Link.Self, _relRelated, false)]
143+
[InlineData(Link.Paging, Link.Self, _relRelated, false)]
144+
[InlineData(Link.None, Link.Self, _relRelated, false)]
134145
public void BuildTopLevelLinks_GlobalAndResourceConfiguration_ExpectedLinks(Link global,
135146
Link resource,
136-
object expectedSelfLink,
147+
string expectedSelfLink,
137148
bool pages)
138149
{
139150
// Arrange
140151
var config = GetConfiguration(topLevelLinks: global);
141152
var primaryResource = GetResourceContext<Article>(topLevelLinks: resource);
142153
_provider.Setup(m => m.GetResourceContext<Article>()).Returns(primaryResource);
143154

144-
var builder = new LinkBuilder(config, GetRequestManager(), _pageService, _provider.Object);
155+
bool useBaseId = expectedSelfLink != _topSelf;
156+
string relationshipName = expectedSelfLink == _relRelated ? _relationshipName : null;
157+
ICurrentRequest currentRequest = GetRequestManager(primaryResource, useBaseId, relationshipName);
158+
159+
var builder = new LinkBuilder(config, currentRequest, _pageService, _provider.Object);
145160

146161
// Act
147-
var links = builder.GetTopLevelLinks(primaryResource);
162+
var links = builder.GetTopLevelLinks();
148163

149164
// Assert
150165
if (!pages && expectedSelfLink == null)
@@ -153,11 +168,11 @@ public void BuildTopLevelLinks_GlobalAndResourceConfiguration_ExpectedLinks(Link
153168
}
154169
else
155170
{
156-
Assert.True(CheckPages(links, pages));
171+
Assert.True(CheckLinks(links, pages, expectedSelfLink));
157172
}
158173
}
159174

160-
private bool CheckPages(TopLevelLinks links, bool pages)
175+
private bool CheckLinks(TopLevelLinks links, bool pages, string expectedSelfLink)
161176
{
162177
if (pages)
163178
{
@@ -167,13 +182,16 @@ private bool CheckPages(TopLevelLinks links, bool pages)
167182
&& links.Next == $"{_host}/articles?page[size]=10&page[number]=3"
168183
&& links.Last == $"{_host}/articles?page[size]=10&page[number]=3";
169184
}
170-
return links.First == null && links.Prev == null && links.Next == null && links.Last == null;
185+
186+
return links.Self == expectedSelfLink && links.First == null && links.Prev == null && links.Next == null && links.Last == null;
171187
}
172188

173-
private ICurrentRequest GetRequestManager(ResourceContext resourceContext = null)
189+
private ICurrentRequest GetRequestManager(ResourceContext resourceContext = null, bool useBaseId = false, string relationshipName = null)
174190
{
175191
var mock = new Mock<ICurrentRequest>();
176192
mock.Setup(m => m.BasePath).Returns(_host);
193+
mock.Setup(m => m.BaseId).Returns(useBaseId ? _baseId.ToString() : null);
194+
mock.Setup(m => m.RequestRelationship).Returns(relationshipName != null ? new HasOneAttribute(relationshipName) : null);
177195
mock.Setup(m => m.GetRequestResource()).Returns(resourceContext);
178196
return mock.Object;
179197
}

test/UnitTests/Serialization/Common/DocumentBuilderTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public BaseDocumentBuilderTests()
1818
{
1919
var mock = new Mock<IResourceObjectBuilder>();
2020
mock.Setup(m => m.Build(It.IsAny<IIdentifiable>(), It.IsAny<IEnumerable<AttrAttribute>>(), It.IsAny<IEnumerable<RelationshipAttribute>>())).Returns(new ResourceObject());
21-
_builder = new TestDocumentBuilder(mock.Object, _resourceGraph);
21+
_builder = new TestDocumentBuilder(mock.Object);
2222
}
2323

2424

test/UnitTests/Serialization/SerializerTestsSetup.cs

+5-12
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
using System;
1+
using System;
22
using System.Collections;
33
using System.Collections.Generic;
4-
using JsonApiDotNetCore.Internal;
54
using JsonApiDotNetCore.Internal.Contracts;
65
using JsonApiDotNetCore.Managers.Contracts;
76
using JsonApiDotNetCore.Models;
@@ -49,9 +48,8 @@ protected ResponseSerializer<T> GetResponseSerializer<T>(List<List<RelationshipA
4948
var included = GetIncludedRelationships(inclusionChains);
5049
var includedBuilder = GetIncludedBuilder();
5150
var fieldsToSerialize = GetSerializableFields();
52-
var provider = GetResourceContextProvider();
5351
ResponseResourceObjectBuilder resourceObjectBuilder = new ResponseResourceObjectBuilder(link, includedBuilder, included, _resourceGraph, GetSerializerSettingsProvider());
54-
return new ResponseSerializer<T>(meta, link, includedBuilder, fieldsToSerialize, resourceObjectBuilder, provider);
52+
return new ResponseSerializer<T>(meta, link, includedBuilder, fieldsToSerialize, resourceObjectBuilder);
5553
}
5654

5755
protected ResponseResourceObjectBuilder GetResponseResourceObjectBuilder(List<List<RelationshipAttribute>> inclusionChains = null, ResourceLinks resourceLinks = null, RelationshipLinks relationshipLinks = null)
@@ -74,11 +72,6 @@ protected IResourceObjectBuilderSettingsProvider GetSerializerSettingsProvider()
7472
return mock.Object;
7573
}
7674

77-
private IResourceGraph GetResourceContextProvider()
78-
{
79-
return _resourceGraph;
80-
}
81-
8275
protected IMetaBuilder<T> GetMetaBuilder<T>(Dictionary<string, object> meta = null) where T : class, IIdentifiable
8376
{
8477
var mock = new Mock<IMetaBuilder<T>>();
@@ -96,7 +89,7 @@ protected ICurrentRequest GetRequestManager<T>() where T : class, IIdentifiable
9689
protected ILinkBuilder GetLinkBuilder(TopLevelLinks top = null, ResourceLinks resource = null, RelationshipLinks relationship = null)
9790
{
9891
var mock = new Mock<ILinkBuilder>();
99-
mock.Setup(m => m.GetTopLevelLinks(It.IsAny<ResourceContext>())).Returns(top);
92+
mock.Setup(m => m.GetTopLevelLinks()).Returns(top);
10093
mock.Setup(m => m.GetResourceLinks(It.IsAny<string>(), It.IsAny<string>())).Returns(resource);
10194
mock.Setup(m => m.GetRelationshipLinks(It.IsAny<RelationshipAttribute>(), It.IsAny<IIdentifiable>())).Returns(relationship);
10295
return mock.Object;
@@ -131,7 +124,7 @@ protected IIncludeService GetIncludedRelationships(List<List<RelationshipAttribu
131124
/// </summary>
132125
protected class TestDocumentBuilder : BaseDocumentBuilder
133126
{
134-
public TestDocumentBuilder(IResourceObjectBuilder resourceObjectBuilder, IResourceContextProvider provider) : base(resourceObjectBuilder, provider) { }
127+
public TestDocumentBuilder(IResourceObjectBuilder resourceObjectBuilder) : base(resourceObjectBuilder) { }
135128

136129
public new Document Build(IIdentifiable entity, List<AttrAttribute> attributes = null, List<RelationshipAttribute> relationships = null)
137130
{
@@ -144,4 +137,4 @@ public TestDocumentBuilder(IResourceObjectBuilder resourceObjectBuilder, IResour
144137
}
145138
}
146139
}
147-
}
140+
}

0 commit comments

Comments
 (0)