Skip to content

Commit 585bf84

Browse files
wisepotatomaurei
wisepotato
authored and
maurei
committed
Fix/fix for string ids (#651)
* chore: allow obfuscatedModelIds * feat: looser constraints on the ids * chore: remove obfuscated idemodel * chore: use 3.0.* instead of 3.*
1 parent 54a76a7 commit 585bf84

File tree

3 files changed

+50
-96
lines changed

3 files changed

+50
-96
lines changed

‎Directory.Build.props‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
<PropertyGroup>
33
<NetCoreAppVersion>netcoreapp3.0</NetCoreAppVersion>
44
<NetStandardVersion>netstandard2.1</NetStandardVersion>
5-
<AspNetCoreVersion>3.*</AspNetCoreVersion>
6-
<MicrosoftLoggingVersion>3.*</MicrosoftLoggingVersion>
7-
<MicrosoftConfigurationVersion>3.*</MicrosoftConfigurationVersion>
8-
<MicrosoftOptionsVersion>3.*</MicrosoftOptionsVersion>
9-
<EFCoreVersion>3.*</EFCoreVersion>
10-
<EFCoreToolsVersion>3.*</EFCoreToolsVersion>
5+
<AspNetCoreVersion>3.0</AspNetCoreVersion>
6+
<MicrosoftLoggingVersion>3.0.*</MicrosoftLoggingVersion>
7+
<MicrosoftConfigurationVersion>3.0.*</MicrosoftConfigurationVersion>
8+
<MicrosoftOptionsVersion>3.0.*</MicrosoftOptionsVersion>
9+
<EFCoreVersion>3.0.*</EFCoreVersion>
10+
<EFCoreToolsVersion>3.0.*</EFCoreToolsVersion>
1111
<NpgsqlVersion>4.1.1</NpgsqlVersion>
1212
<NpgsqlPostgreSQLVersion>3.0.1</NpgsqlPostgreSQLVersion>
1313
<TuplesVersion>4.5.0</TuplesVersion>

‎src/JsonApiDotNetCore/Middleware/CurrentRequestMiddleware.cs‎

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -61,20 +61,18 @@ public async Task Invoke(HttpContext httpContext,
6161

6262
private string GetBaseId()
6363
{
64-
var resource = _currentRequest.GetRequestResource();
65-
var individualComponents = SplitCurrentPath();
66-
if (individualComponents.Length < 2)
64+
if (_routeValues.TryGetValue("id", out object stringId))
6765
{
68-
return null;
66+
if ((string)stringId == string.Empty)
67+
{
68+
throw new JsonApiException(400, "No empty string as id please.");
69+
}
70+
return (string)stringId;
6971
}
70-
var indexOfResource = individualComponents.ToList().FindIndex(c => c == resource.ResourceName);
71-
var baseId = individualComponents.ElementAtOrDefault(indexOfResource + 1);
72-
if (baseId == null)
72+
else
7373
{
7474
return null;
7575
}
76-
CheckIdType(baseId, resource.IdentityType);
77-
return baseId;
7876
}
7977
private string GetRelationshipId()
8078
{
@@ -93,7 +91,6 @@ private string GetRelationshipId()
9391
var relType = _currentRequest.RequestRelationship.RightType;
9492
var relResource = _resourceGraph.GetResourceContext(relType);
9593
var relIdentityType = relResource.IdentityType;
96-
CheckIdType(toReturn, relIdentityType);
9794
return toReturn;
9895
}
9996
private string[] SplitCurrentPath()
@@ -106,37 +103,6 @@ private string[] SplitCurrentPath()
106103
return individualComponents;
107104
}
108105

109-
110-
private void CheckIdType(string value, Type idType)
111-
{
112-
try
113-
{
114-
var converter = TypeDescriptor.GetConverter(idType);
115-
if (converter != null)
116-
{
117-
if (!converter.IsValid(value))
118-
{
119-
throw new JsonApiException(500, $"We could not convert the id '{value}'");
120-
}
121-
else
122-
{
123-
if (idType == typeof(int))
124-
{
125-
if ((int)converter.ConvertFromString(value) < 0)
126-
{
127-
throw new JsonApiException(500, "The base ID is an integer, and it is negative.");
128-
}
129-
}
130-
}
131-
}
132-
}
133-
catch (NotSupportedException)
134-
{
135-
136-
}
137-
138-
}
139-
140106
private string GetBasePath(string resourceName = null)
141107
{
142108
var r = _httpContext.Request;
@@ -147,7 +113,7 @@ private string GetBasePath(string resourceName = null)
147113
var ns = GetNameSpace(resourceName);
148114
var customRoute = GetCustomRoute(r.Path.Value, resourceName);
149115
var toReturn = $"{r.Scheme}://{r.Host}/{ns}";
150-
if(customRoute != null)
116+
if (customRoute != null)
151117
{
152118
toReturn += $"/{customRoute}";
153119
}
@@ -159,9 +125,9 @@ private object GetCustomRoute(string path, string resourceName)
159125
var ns = GetNameSpace();
160126
var trimmedComponents = path.Trim('/').Split('/').ToList();
161127
var resourceNameIndex = trimmedComponents.FindIndex(c => c == resourceName);
162-
var newComponents = trimmedComponents.Take(resourceNameIndex ).ToArray();
128+
var newComponents = trimmedComponents.Take(resourceNameIndex).ToArray();
163129
var customRoute = string.Join('/', newComponents);
164-
if(customRoute == ns)
130+
if (customRoute == ns)
165131
{
166132
return null;
167133
}

‎test/UnitTests/Middleware/CurrentRequestMiddlewareTests.cs‎

Lines changed: 34 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -23,78 +23,67 @@ namespace UnitTests.Middleware
2323
public class CurrentRequestMiddlewareTests
2424
{
2525
[Fact]
26-
public async Task ParseUrlBase_UrlHasBaseIdSet_ShouldSetCurrentRequestWithSaidId()
26+
public async Task ParseUrlBase_ObfuscatedIdClass_ShouldSetIdCorrectly()
2727
{
2828
// Arrange
29-
var id = "123";
30-
var configuration = GetConfiguration($"/users/{id}");
29+
var id = "ABC123ABC";
30+
var configuration = GetConfiguration($"/obfuscatedIdModel/{id}", action: "GetAsync", id: id);
3131
var currentRequest = configuration.CurrentRequest;
3232

3333
// Act
3434
await RunMiddlewareTask(configuration);
3535

3636
// Assert
3737
Assert.Equal(id, currentRequest.BaseId);
38-
}
3938

39+
}
4040
[Fact]
41-
public async Task ParseUrlBase_UrlHasNoBaseIdSet_ShouldHaveBaseIdSetToNull()
41+
public async Task ParseUrlBase_UrlHasBaseIdSet_ShouldSetCurrentRequestWithSaidId()
4242
{
4343
// Arrange
44-
var configuration = GetConfiguration("/users");
44+
var id = "123";
45+
var configuration = GetConfiguration($"/users/{id}", id: id);
4546
var currentRequest = configuration.CurrentRequest;
4647

4748
// Act
4849
await RunMiddlewareTask(configuration);
4950

5051
// Assert
51-
Assert.Null(currentRequest.BaseId);
52+
Assert.Equal(id, currentRequest.BaseId);
5253
}
54+
5355
[Fact]
54-
public async Task ParseUrlRel_UrlHasRelationshipIdSet_ShouldHaveBaseIdAndRelationshipIdSet()
56+
public async Task ParseUrlBase_UrlHasNoBaseIdSet_ShouldHaveBaseIdSetToNull()
5557
{
5658
// Arrange
57-
var baseId = "5";
58-
var relId = "23";
59-
var configuration = GetConfiguration($"/users/{baseId}/relationships/books/{relId}", relType: typeof(TodoItem), relIdType: typeof(int));
59+
var configuration = GetConfiguration("/users");
6060
var currentRequest = configuration.CurrentRequest;
6161

6262
// Act
6363
await RunMiddlewareTask(configuration);
6464

6565
// Assert
66-
Assert.Equal(baseId, currentRequest.BaseId);
67-
Assert.Equal(relId, currentRequest.RelationshipId);
66+
Assert.Null(currentRequest.BaseId);
6867
}
6968

7069
[Fact]
71-
public async Task ParseUrlBase_UrlHasNegativeBaseIdAndTypeIsInt_ShouldThrowJAException()
70+
public async Task ParseUrlBase_UrlHasNegativeBaseIdAndTypeIsInt_ShouldNotThrowJAException()
7271
{
7372
// Arrange
7473
var configuration = GetConfiguration("/users/-5/");
7574

76-
// Act
77-
var task = RunMiddlewareTask(configuration);
78-
79-
// Assert
80-
var exception = await Assert.ThrowsAsync<JsonApiException>(async () =>
81-
{
82-
await task;
83-
});
84-
Assert.Equal(500, exception.GetStatusCode());
85-
Assert.Contains("negative", exception.Message);
75+
// Act / Assert
76+
await RunMiddlewareTask(configuration);
8677
}
8778

8879
[Theory]
89-
[InlineData("12315K", typeof(int), true)]
90-
[InlineData("12315K", typeof(int), false)]
91-
[InlineData("5", typeof(Guid), true)]
92-
[InlineData("5", typeof(Guid), false)]
93-
public async Task ParseUrlBase_UrlHasIncorrectBaseIdSet_ShouldThrowException(string baseId, Type idType, bool addSlash)
80+
[InlineData("", false)]
81+
[InlineData("", true)]
82+
public async Task ParseUrlBase_UrlHasIncorrectBaseIdSet_ShouldThrowException(string baseId, bool addSlash)
9483
{
9584
// Arrange
9685
var url = addSlash ? $"/users/{baseId}/" : $"/users/{baseId}";
97-
var configuration = GetConfiguration(url, idType: idType);
86+
var configuration = GetConfiguration(url, id: baseId);
9887

9988
// Act
10089
var task = RunMiddlewareTask(configuration);
@@ -104,7 +93,7 @@ public async Task ParseUrlBase_UrlHasIncorrectBaseIdSet_ShouldThrowException(str
10493
{
10594
await task;
10695
});
107-
Assert.Equal(500, exception.GetStatusCode());
96+
Assert.Equal(400, exception.GetStatusCode());
10897
Assert.Contains(baseId, exception.Message);
10998
}
11099

@@ -126,34 +115,29 @@ private Task RunMiddlewareTask(InvokeConfiguration holder)
126115
var resourceGraph = holder.ResourceGraph.Object;
127116
return holder.MiddleWare.Invoke(context, controllerResourceMapping, options, currentRequest, resourceGraph);
128117
}
129-
private InvokeConfiguration GetConfiguration(string path, string resourceName = "users", Type idType = null, Type relType = null, Type relIdType = null)
118+
private InvokeConfiguration GetConfiguration(string path, string resourceName = "users", string action = "", string id =null, Type relType = null)
130119
{
131-
if((relType != null) != (relIdType != null))
132-
{
133-
throw new ArgumentException("Define both reltype and relidType or dont.");
134-
}
135120
if (path.First() != '/')
136121
{
137122
throw new ArgumentException("Path should start with a '/'");
138123
}
139-
idType ??= typeof(int);
140124
var middleware = new CurrentRequestMiddleware((context) =>
141125
{
142126
return Task.Run(() => Console.WriteLine("finished"));
143127
});
144128
var forcedNamespace = "api/v1";
145129
var mockMapping = new Mock<IControllerResourceMapping>();
146130
Mock<IJsonApiOptions> mockOptions = CreateMockOptions(forcedNamespace);
147-
var mockGraph = CreateMockResourceGraph(idType, resourceName, relIdType : relIdType);
131+
var mockGraph = CreateMockResourceGraph(resourceName, includeRelationship: relType != null);
148132
var currentRequest = new CurrentRequest();
149-
if (relType != null && relIdType != null)
133+
if (relType != null)
150134
{
151135
currentRequest.RequestRelationship = new HasManyAttribute
152136
{
153137
RightType = relType
154138
};
155139
}
156-
var context = CreateHttpContext(path, isRelationship: relType != null);
140+
var context = CreateHttpContext(path, isRelationship: relType != null, action: action, id: id);
157141
return new InvokeConfiguration
158142
{
159143
MiddleWare = middleware,
@@ -172,33 +156,37 @@ private static Mock<IJsonApiOptions> CreateMockOptions(string forcedNamespace)
172156
return mockOptions;
173157
}
174158

175-
private static DefaultHttpContext CreateHttpContext(string path, bool isRelationship = false)
159+
private static DefaultHttpContext CreateHttpContext(string path, bool isRelationship = false, string action = "", string id =null)
176160
{
177161
var context = new DefaultHttpContext();
178162
context.Request.Path = new PathString(path);
179163
context.Response.Body = new MemoryStream();
180164
var feature = new RouteValuesFeature();
181165
feature.RouteValues["controller"] = "fake!";
182-
feature.RouteValues["action"] = isRelationship ? "relationships" : "noRel";
166+
feature.RouteValues["action"] = isRelationship ? "GetRelationship" : action;
167+
if(id != null)
168+
{
169+
feature.RouteValues["id"] = id;
170+
}
183171
context.Features.Set<IRouteValuesFeature>(feature);
184172
return context;
185173
}
186174

187-
private Mock<IResourceGraph> CreateMockResourceGraph(Type idType, string resourceName, Type relIdType = null)
175+
private Mock<IResourceGraph> CreateMockResourceGraph( string resourceName, bool includeRelationship = false)
188176
{
189177
var mockGraph = new Mock<IResourceGraph>();
190178
var resourceContext = new ResourceContext
191179
{
192180
ResourceName = resourceName,
193-
IdentityType = idType
181+
IdentityType = typeof(string)
194182
};
195183
var seq = mockGraph.SetupSequence(d => d.GetResourceContext(It.IsAny<Type>())).Returns(resourceContext);
196-
if (relIdType != null)
184+
if (includeRelationship)
197185
{
198186
var relResourceContext = new ResourceContext
199187
{
200188
ResourceName = "todoItems",
201-
IdentityType = relIdType
189+
IdentityType = typeof(string)
202190
};
203191
seq.Returns(relResourceContext);
204192
}

0 commit comments

Comments
 (0)