Skip to content

Fix/fix for string ids #651

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 4 commits into from
Dec 23, 2019
Merged

Fix/fix for string ids #651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
242583d
chore: allow obfuscatedModelIds
wisepotato Dec 4, 2019
8e1c590
feat: looser constraints on the ids
wisepotato Dec 4, 2019
325137c
chore: remove obfuscated idemodel
wisepotato Dec 4, 2019
0a324db
chore: use 3.0.* instead of 3.*
wisepotato Dec 11, 2019
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
12 changes: 6 additions & 6 deletions Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
<PropertyGroup>
<NetCoreAppVersion>netcoreapp3.0</NetCoreAppVersion>
<NetStandardVersion>netstandard2.1</NetStandardVersion>
<AspNetCoreVersion>3.*</AspNetCoreVersion>
<MicrosoftLoggingVersion>3.*</MicrosoftLoggingVersion>
<MicrosoftConfigurationVersion>3.*</MicrosoftConfigurationVersion>
<MicrosoftOptionsVersion>3.*</MicrosoftOptionsVersion>
<EFCoreVersion>3.*</EFCoreVersion>
<EFCoreToolsVersion>3.*</EFCoreToolsVersion>
<AspNetCoreVersion>3.0</AspNetCoreVersion>
<MicrosoftLoggingVersion>3.0.*</MicrosoftLoggingVersion>
<MicrosoftConfigurationVersion>3.0.*</MicrosoftConfigurationVersion>
<MicrosoftOptionsVersion>3.0.*</MicrosoftOptionsVersion>
<EFCoreVersion>3.0.*</EFCoreVersion>
<EFCoreToolsVersion>3.0.*</EFCoreToolsVersion>
<NpgsqlVersion>4.1.1</NpgsqlVersion>
<NpgsqlPostgreSQLVersion>3.0.1</NpgsqlPostgreSQLVersion>
<TuplesVersion>4.5.0</TuplesVersion>
Expand Down
54 changes: 10 additions & 44 deletions src/JsonApiDotNetCore/Middleware/CurrentRequestMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,18 @@ public async Task Invoke(HttpContext httpContext,

private string GetBaseId()
{
var resource = _currentRequest.GetRequestResource();
var individualComponents = SplitCurrentPath();
if (individualComponents.Length < 2)
if (_routeValues.TryGetValue("id", out object stringId))
{
return null;
if ((string)stringId == string.Empty)
{
throw new JsonApiException(400, "No empty string as id please.");
}
return (string)stringId;
}
var indexOfResource = individualComponents.ToList().FindIndex(c => c == resource.ResourceName);
var baseId = individualComponents.ElementAtOrDefault(indexOfResource + 1);
if (baseId == null)
else
{
return null;
}
CheckIdType(baseId, resource.IdentityType);
return baseId;
}
private string GetRelationshipId()
{
Expand All @@ -93,7 +91,6 @@ private string GetRelationshipId()
var relType = _currentRequest.RequestRelationship.RightType;
var relResource = _resourceGraph.GetResourceContext(relType);
var relIdentityType = relResource.IdentityType;
CheckIdType(toReturn, relIdentityType);
return toReturn;
}
private string[] SplitCurrentPath()
Expand All @@ -106,37 +103,6 @@ private string[] SplitCurrentPath()
return individualComponents;
}


private void CheckIdType(string value, Type idType)
{
try
{
var converter = TypeDescriptor.GetConverter(idType);
if (converter != null)
{
if (!converter.IsValid(value))
{
throw new JsonApiException(500, $"We could not convert the id '{value}'");
}
else
{
if (idType == typeof(int))
{
if ((int)converter.ConvertFromString(value) < 0)
{
throw new JsonApiException(500, "The base ID is an integer, and it is negative.");
}
}
}
}
}
catch (NotSupportedException)
{

}

}

private string GetBasePath(string resourceName = null)
{
var r = _httpContext.Request;
Expand All @@ -147,7 +113,7 @@ private string GetBasePath(string resourceName = null)
var ns = GetNameSpace(resourceName);
var customRoute = GetCustomRoute(r.Path.Value, resourceName);
var toReturn = $"{r.Scheme}://{r.Host}/{ns}";
if(customRoute != null)
if (customRoute != null)
{
toReturn += $"/{customRoute}";
}
Expand All @@ -159,9 +125,9 @@ private object GetCustomRoute(string path, string resourceName)
var ns = GetNameSpace();
var trimmedComponents = path.Trim('/').Split('/').ToList();
var resourceNameIndex = trimmedComponents.FindIndex(c => c == resourceName);
var newComponents = trimmedComponents.Take(resourceNameIndex ).ToArray();
var newComponents = trimmedComponents.Take(resourceNameIndex).ToArray();
var customRoute = string.Join('/', newComponents);
if(customRoute == ns)
if (customRoute == ns)
{
return null;
}
Expand Down
80 changes: 34 additions & 46 deletions test/UnitTests/Middleware/CurrentRequestMiddlewareTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,78 +23,67 @@ namespace UnitTests.Middleware
public class CurrentRequestMiddlewareTests
{
[Fact]
public async Task ParseUrlBase_UrlHasBaseIdSet_ShouldSetCurrentRequestWithSaidId()
public async Task ParseUrlBase_ObfuscatedIdClass_ShouldSetIdCorrectly()
{
// Arrange
var id = "123";
var configuration = GetConfiguration($"/users/{id}");
var id = "ABC123ABC";
var configuration = GetConfiguration($"/obfuscatedIdModel/{id}", action: "GetAsync", id: id);
var currentRequest = configuration.CurrentRequest;

// Act
await RunMiddlewareTask(configuration);

// Assert
Assert.Equal(id, currentRequest.BaseId);
}

}
[Fact]
public async Task ParseUrlBase_UrlHasNoBaseIdSet_ShouldHaveBaseIdSetToNull()
public async Task ParseUrlBase_UrlHasBaseIdSet_ShouldSetCurrentRequestWithSaidId()
{
// Arrange
var configuration = GetConfiguration("/users");
var id = "123";
var configuration = GetConfiguration($"/users/{id}", id: id);
var currentRequest = configuration.CurrentRequest;

// Act
await RunMiddlewareTask(configuration);

// Assert
Assert.Null(currentRequest.BaseId);
Assert.Equal(id, currentRequest.BaseId);
}

[Fact]
public async Task ParseUrlRel_UrlHasRelationshipIdSet_ShouldHaveBaseIdAndRelationshipIdSet()
public async Task ParseUrlBase_UrlHasNoBaseIdSet_ShouldHaveBaseIdSetToNull()
{
// Arrange
var baseId = "5";
var relId = "23";
var configuration = GetConfiguration($"/users/{baseId}/relationships/books/{relId}", relType: typeof(TodoItem), relIdType: typeof(int));
var configuration = GetConfiguration("/users");
var currentRequest = configuration.CurrentRequest;

// Act
await RunMiddlewareTask(configuration);

// Assert
Assert.Equal(baseId, currentRequest.BaseId);
Assert.Equal(relId, currentRequest.RelationshipId);
Assert.Null(currentRequest.BaseId);
}

[Fact]
public async Task ParseUrlBase_UrlHasNegativeBaseIdAndTypeIsInt_ShouldThrowJAException()
public async Task ParseUrlBase_UrlHasNegativeBaseIdAndTypeIsInt_ShouldNotThrowJAException()
{
// Arrange
var configuration = GetConfiguration("/users/-5/");

// Act
var task = RunMiddlewareTask(configuration);

// Assert
var exception = await Assert.ThrowsAsync<JsonApiException>(async () =>
{
await task;
});
Assert.Equal(500, exception.GetStatusCode());
Assert.Contains("negative", exception.Message);
// Act / Assert
await RunMiddlewareTask(configuration);
}

[Theory]
[InlineData("12315K", typeof(int), true)]
[InlineData("12315K", typeof(int), false)]
[InlineData("5", typeof(Guid), true)]
[InlineData("5", typeof(Guid), false)]
public async Task ParseUrlBase_UrlHasIncorrectBaseIdSet_ShouldThrowException(string baseId, Type idType, bool addSlash)
[InlineData("", false)]
[InlineData("", true)]
public async Task ParseUrlBase_UrlHasIncorrectBaseIdSet_ShouldThrowException(string baseId, bool addSlash)
{
// Arrange
var url = addSlash ? $"/users/{baseId}/" : $"/users/{baseId}";
var configuration = GetConfiguration(url, idType: idType);
var configuration = GetConfiguration(url, id: baseId);

// Act
var task = RunMiddlewareTask(configuration);
Expand All @@ -104,7 +93,7 @@ public async Task ParseUrlBase_UrlHasIncorrectBaseIdSet_ShouldThrowException(str
{
await task;
});
Assert.Equal(500, exception.GetStatusCode());
Assert.Equal(400, exception.GetStatusCode());
Assert.Contains(baseId, exception.Message);
}

Expand All @@ -126,34 +115,29 @@ private Task RunMiddlewareTask(InvokeConfiguration holder)
var resourceGraph = holder.ResourceGraph.Object;
return holder.MiddleWare.Invoke(context, controllerResourceMapping, options, currentRequest, resourceGraph);
}
private InvokeConfiguration GetConfiguration(string path, string resourceName = "users", Type idType = null, Type relType = null, Type relIdType = null)
private InvokeConfiguration GetConfiguration(string path, string resourceName = "users", string action = "", string id =null, Type relType = null)
{
if((relType != null) != (relIdType != null))
{
throw new ArgumentException("Define both reltype and relidType or dont.");
}
if (path.First() != '/')
{
throw new ArgumentException("Path should start with a '/'");
}
idType ??= typeof(int);
var middleware = new CurrentRequestMiddleware((context) =>
{
return Task.Run(() => Console.WriteLine("finished"));
});
var forcedNamespace = "api/v1";
var mockMapping = new Mock<IControllerResourceMapping>();
Mock<IJsonApiOptions> mockOptions = CreateMockOptions(forcedNamespace);
var mockGraph = CreateMockResourceGraph(idType, resourceName, relIdType : relIdType);
var mockGraph = CreateMockResourceGraph(resourceName, includeRelationship: relType != null);
var currentRequest = new CurrentRequest();
if (relType != null && relIdType != null)
if (relType != null)
{
currentRequest.RequestRelationship = new HasManyAttribute
{
RightType = relType
};
}
var context = CreateHttpContext(path, isRelationship: relType != null);
var context = CreateHttpContext(path, isRelationship: relType != null, action: action, id: id);
return new InvokeConfiguration
{
MiddleWare = middleware,
Expand All @@ -172,33 +156,37 @@ private static Mock<IJsonApiOptions> CreateMockOptions(string forcedNamespace)
return mockOptions;
}

private static DefaultHttpContext CreateHttpContext(string path, bool isRelationship = false)
private static DefaultHttpContext CreateHttpContext(string path, bool isRelationship = false, string action = "", string id =null)
{
var context = new DefaultHttpContext();
context.Request.Path = new PathString(path);
context.Response.Body = new MemoryStream();
var feature = new RouteValuesFeature();
feature.RouteValues["controller"] = "fake!";
feature.RouteValues["action"] = isRelationship ? "relationships" : "noRel";
feature.RouteValues["action"] = isRelationship ? "GetRelationship" : action;
if(id != null)
{
feature.RouteValues["id"] = id;
}
context.Features.Set<IRouteValuesFeature>(feature);
return context;
}

private Mock<IResourceGraph> CreateMockResourceGraph(Type idType, string resourceName, Type relIdType = null)
private Mock<IResourceGraph> CreateMockResourceGraph( string resourceName, bool includeRelationship = false)
{
var mockGraph = new Mock<IResourceGraph>();
var resourceContext = new ResourceContext
{
ResourceName = resourceName,
IdentityType = idType
IdentityType = typeof(string)
};
var seq = mockGraph.SetupSequence(d => d.GetResourceContext(It.IsAny<Type>())).Returns(resourceContext);
if (relIdType != null)
if (includeRelationship)
{
var relResourceContext = new ResourceContext
{
ResourceName = "todoItems",
IdentityType = relIdType
IdentityType = typeof(string)
};
seq.Returns(relResourceContext);
}
Expand Down