Hyprlinkr icon indicating copy to clipboard operation
Hyprlinkr copied to clipboard

Support for attribute-based route names

Open joaopbnogueira opened this issue 8 years ago • 10 comments

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

joaopbnogueira avatar Feb 18 '17 01:02 joaopbnogueira

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

ploeh avatar Feb 18 '17 10:02 ploeh

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();

joaopbnogueira avatar Feb 20 '17 21:02 joaopbnogueira

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?

ploeh avatar Feb 21 '17 06:02 ploeh

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

joaopbnogueira avatar Feb 21 '17 20:02 joaopbnogueira

Sounds like my setup as well...

ploeh avatar Feb 21 '17 20:02 ploeh

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 avatar Feb 21 '17 20:02 ploeh

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...

ploeh avatar Feb 21 '17 21:02 ploeh

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.

joaopbnogueira avatar Feb 22 '17 08:02 joaopbnogueira

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.

ploeh avatar Feb 22 '17 15:02 ploeh

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

joaopbnogueira avatar Feb 22 '17 17:02 joaopbnogueira