Varying id declaration in Controller Attributes

[mluepkes]

While revamping Localization, I stumbled across the following problem:

Controller methods are are decorated with different styles of attributes in regards to the declaration of the id. While e.g. the BrandsController uses [HttpGet("{id:guid}")], the UsersController uses [HttpGet("{id}")].

https://github.com/fullstackhero/dotnet-webapi-boilerplate/blob/a6143ebb0f21ba6d092ec2cffafa80c488c437c7/src/Host/Controllers/Catalog/BrandsController.cs#L15

https://github.com/fullstackhero/dotnet-webapi-boilerplate/blob/a6143ebb0f21ba6d092ec2cffafa80c488c437c7/src/Host/Controllers/Identity/UsersController.cs#L20

The problem with using [HttpGet("{id:guid}")] the way the API is written currently is that calling the corresponding endpoint with a malformed GUID will let the request fail without an error message, just the 404 header. The call to Mediator never gets sent.

Suggestions: a) convert all id:guid to a simple id and let the exception at the request handler level get thrown. The server will answer with "X not found", or b) Introduce some kind of new middleware that throws a MalformedParameterException or some such when the id is not a proper guid.

[fretje]

I don't really see the problem. When you don't supply a valid guid, then there is no endpoint which can handle that request, so a standard 404 is returned which means "this endpoint doesn't exist". Which is the right response imo.

You're right that there's an inconsistency here with how users are handled. But the thing is that the userId is actually defined as a string on the entity, and not a Guid like it is with brands and products, so the inconsistency already starts there... Not sure this actually needs solving, but if it does, I wouldn't start here (at the api level).

This is exactly the kind of messages I was talking about in a discussion earlier (probably on discord) where I was saying "I'm not sure we need translation of error messages like this from the server"... these are messages which should never be shown to the user in the UI anyway, as there should be other measures in place which make sure that kind of "malformed" calls are not possible. If the api starts throwing that kind of messages, there's something else wrong on anther level. Translating those messages at that point won't help the end user...

[mluepkes]

If we leave the guid part in, the server should reply with Error 400 - Bad Request instead, so the end user knows nothing has been found because the request was bad. 404 in and of itself is correct, too, but it's Not Found primarily because the request already was bad.

Also, /api/users/{id} take a id as string, it's stored as a GUID though AFAIK.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.