[#732] Attribute Routing: Implement ~/ for overriding a prefix
#761
Conversation
|
@rynowak @yishaigalatzer This is the right place to comment. |
|
|
||
| if (trimmedLeft == string.Empty) | ||
| { | ||
| return trimmedRight; | ||
| return right; |
There was a problem hiding this comment.
This seems broken. Consider Combine("~/", "Blog/Index/"). The right side won't get trimmed.
There was a problem hiding this comment.
fixed it and added a test case for it.
There was a problem hiding this comment.
you're returning right here not trimmedRight
There was a problem hiding this comment.
Make sure you add the testcase that I included above
|
|
| public async Task AttributeRoutedAction_ActionLevelRouteWithTildeSlash_OverridesControllerLevelRoute() | ||
| { | ||
| // Arrange | ||
| var server = TestServer.Create(_services, _app); |
There was a problem hiding this comment.
Could you add a test where the base path of the application has a VDir path...example: the request url is like http://localhost/MyApp/Manager/5?
There was a problem hiding this comment.
Open a bug, that's a good thing to have, but unrelated to this work.
There was a problem hiding this comment.
@javiercn : can you open a bug for this...
|
|
| @@ -22,16 +24,25 @@ public static string Combine(string left, string right) | |||
| } | |||
| else if (left == null) | |||
| { | |||
| return right.Trim('/'); | |||
| return right.TrimStart('~').Trim('/'); | |||
There was a problem hiding this comment.
don't do this. You create two extra strings in the process. Make a single helper to trim ~/ or / and use it
This change allows a user to override a route prefix set using
[Route("...")] on the controller by providing a route template
on the action that starts with "~/" or "/". For example,
[HttpGet("~/...")] or [HttpGet("/...")]
If the user specifies a template in [Route] that starts with "~/"
or "/", we will just strip the prefix from the template and use
the remaining part of the template.
The reason to do this is that there's a reasonable extensibility
scenario where a user can implement a global prefix for routes as
a convention (using IReflectedApplicationModelConvention), and use
~/ to escape that prefix (just like we support with action-level routes).
1) Updated StartsWith calls to use StringComparer.OrdinalIgnoreCase. 2) Trim the end of the right part of a template when it doesn't start with "/" or "~/" and added tests to cover both scenarios. 3) Fixed the format of tests to move all asserts before an // Assert comment after it, and remove aditional // Assert comments from the tests.
… calls and substituted the cleanup for removing starting "/" and "~/" prefixes and "/" suffix from the combined template.
|
|
|
commit 87c430a |
This change allows a user to override a route prefix set using
[Route("...")] on the controller by providing a route template
on the action that starts with "
/" or "/". For example,/...")] or [HttpGet("/...")][HttpGet("
If the user specifies a template in [Route] that starts with "~/"
or "/", we will just strip the prefix from the template and use
the remaining part of the template.
The reason to do this is that there's a reasonable extensibility
scenario where a user can implement a global prefix for routes as
a convention (using IReflectedApplicationModelConvention), and use
~/ to escape that prefix (just like we support with action-level routes).