Skip to content
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.

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

Support for attribute-based route names #44

Open
joaopbnogueira opened this issue Feb 18, 2017 · 10 comments
Open

Support for attribute-based route names #44

joaopbnogueira opened this issue Feb 18, 2017 · 10 comments
Labels

Comments

@joaopbnogueira
Copy link

I am using attribute routing in my controllers, using the [Route] attribute with route names, and I noticed that Hyprlinkr always assumes that there's a route named 'DefaultApi' to generate the Uris, which does not work with attribute routing.

Is that correct, or am I missing something?

Thank you

@ploeh
Copy link
Owner

ploeh commented Feb 18, 2017

IIRC, that's what IRouteDispatcher is for. There's a bit of a discussion in #30.

@ploeh ploeh added the question label Feb 18, 2017
@joaopbnogueira
Copy link
Author

Hi, yes, I believe that you are right, but I meant for Controller-level attribute routing.

Like so :

[Route("/controllerCustomRoute", Name = ControllerRouteName)] 
public class RouteAttributeController : ApiController

I forked your code and did some modifications but the build validation step is failing with the following error:

c:\CloudStation\MyDocuments\Projects\Hyprlinkr\Hyprlinkr\DefaultRouteDispatcher.cs(138): error CA1308: Microsoft.Globalization : In method 'DefaultRouteDispatcher.AddControllerNameAndMethodToRouteValues(MethodCallExpression, IDictionary<string, object>)', replace the call to 'string.ToLowerInvariant()' with String.ToUpperInvariant(). [C:\CloudStation\MyDocuments\Projects\Hyprlinkr\Hyprlinkr\Hyprlinkr.csproj]

Any idea why? The existing code base is using ToLowerInvariant() ...

The offending code is:

var newRouteValues = new Dictionary<string, object>(routeValues);
var controllerName = callExpression.Object.Type.Name.ToLowerInvariant().Replace("controller", "");
newRouteValues["controller"] = controllerName;
newRouteValues["action"] = callExpression.Method.Name.ToLowerInvariant();

@ploeh
Copy link
Owner

ploeh commented Feb 21, 2017

It could be a tool versioning issue. CA is getting more and more fucked up, because different versions of Visual Studio come with different CA profiles. Which version of VS are you using?

@joaopbnogueira
Copy link
Author

I'm using Visual Studio 2015 Enterprise Update 3, the IDE builds the code & runs the tests just fine.

@ploeh
Copy link
Owner

ploeh commented Feb 21, 2017

Sounds like my setup as well...

@ploeh
Copy link
Owner

ploeh commented Feb 21, 2017

I dug a bit deeper, and there's only one case of ToLowerInvariant in the existing code base, and it has a [SuppressMessage] attribute that explicitly suppresses CA1308, and also explains the reason. Now that I look at it, though, I think that the reason given there is incorrectly worded, but I'll have to look closer into that...

@ploeh
Copy link
Owner

ploeh commented Feb 21, 2017

It turns out that that suppression reason is fairly accurate after all. This is easy to discover if you remove that .ToLowerInvariant() line of code. When I do that, I get 14 failing tests. Here's one example:

Test 'Ploeh.Hyprlinkr.UnitTest.RouteLinkerTests.GetUriForGetMethodWithParameters(request: Method: methodfe5f3ab3-364a-4a99-b0f7-f2cca0aebc78, RequestUri: 'http://2f358f48-300e-479c-9449-67ac460d20ea/', Version: 0.0, Content: Castle.Proxies.HttpContentProxy, Headers:
{
}, sut: Ploeh.Hyprlinkr.RouteLinker, id: 233)' failed:
	Assert.Equal() Failure
Expected: http://2f358f48-300e-479c-9449-67ac460d20ea/api/foo/233
Actual:   http://2f358f48-300e-479c-9449-67ac460d20ea/api/FooController/233
	RouteLinkerTests.cs(91,0): at Ploeh.Hyprlinkr.UnitTest.RouteLinkerTests.GetUriForGetMethodWithParameters(HttpRequestMessage request, RouteLinker sut, Int32 id)

ASP.NET Web API wants the controller route value to be lower-cased, it seems. If you replace .ToLowerInvariant() with .ToUpperInvariant(), you have the same sort of problem.

It's been years since I wrote that, so I can't remember all of the details, so I wonder if the controller URL segment always has to be lower-case, or if that's only an artefact of the way I wrote the tests...

@joaopbnogueira
Copy link
Author

You're right, there was a suppress attribute that I missed when I moved that particular bit of code to its own method.

How important are the F# tests? I don't have F# installed so I only added a test in the c# test project.

@ploeh
Copy link
Owner

ploeh commented Feb 22, 2017

The F# tests aren't that important; they're mostly there in order to verify that the GetUri method also works from F#, against Controller classes written in F#.

I can always add one or two for new methods, if I feel like it.

@joaopbnogueira
Copy link
Author

Thanks, I just submitted a pull request (#45)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants