Skip to content

PluralizedTypeName option when adding resource doesn't work for routing #786

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

Closed
vdaras opened this issue Jun 28, 2020 · 4 comments · Fixed by #805
Closed

PluralizedTypeName option when adding resource doesn't work for routing #786

vdaras opened this issue Jun 28, 2020 · 4 comments · Fixed by #805

Comments

@vdaras
Copy link

vdaras commented Jun 28, 2020

Description

Explicitly setting a pluralized type name for a resource doesn't work for routing purposes. Example:

var mvcBuilder = services.AddMvcCore();
services.AddJsonApi(options => {
    options.Namespace = "api/v1";
}, resources: resources => {
    resources.AddResource<TodoResource, Guid>(pluralizedTypeName: "todos");
});

services.AddScoped<IResourceService<TodoResource, Guid>, TodoResourceService>();

Controller is defined as follows

public sealed class TodoController : JsonApiController<TodoResource, Guid>
{
    public TodoController(
        IJsonApiOptions jsonApiOptions,
        ILoggerFactory loggerFactory,
        IResourceService<TodoResource, Guid> resourceService) 
    : base(jsonApiOptions, loggerFactory, resourceService)
    {
    }
}

But when trying to call the API it seems the respective controller is still registered at /api/v1/todoResources instead of /ap1/v1/todos

curl -I -X GET http://localhost:5001/api/v1/todos        
HTTP/1.1 404 Not Found
Date: Sun, 28 Jun 2020 19:14:46 GMT
Server: Kestrel
Content-Length: 0
curl -I -X GET http://localhost:5001/api/v1/todoResources
HTTP/1.1 200 OK
Date: Sun, 28 Jun 2020 19:13:15 GMT
Content-Type: application/vnd.api+json
Server: Kestrel
Transfer-Encoding: chunked

Environment

  • JsonApiDotNetCore Version: 4.0.0-alpha5
  • Other Relevant Package Versions:
@vdaras vdaras changed the title PluralizedTypeName option when adding resource doesn't work PluralizedTypeName option when adding resource doesn't work for routing Jun 29, 2020
@vdaras
Copy link
Author

vdaras commented Jun 29, 2020

The issue might be that the following code doesn't take into account pluralized type names when generating template for controller - but I'm not really sure since I'm not very familiar with the whole codebase

public void Apply(ApplicationModel application)

@bart-degreed
Copy link
Contributor

Hi @vdaras,

Thanks for reporting this. It is unclear to me whether this is a bug or by design, but adding the next line on your controller may solve the problem (see documentation here):

[Route("api/v1/todoResources"), DisableRoutingConvention]

@bart-degreed
Copy link
Contributor

bart-degreed commented Aug 15, 2020

Or would it help to put [Resource("todos")] on your resource type?

According to documentation pluralizedTypeName is not considered, I don't know why. Needs some investigation.

@maurei
Copy link
Member

maurei commented Aug 25, 2020

I looked into this, you're right @vdaras: the pluralizedTypeName option from your example is ignored for routing. This is not by design, but an error.

The default routing convention aims to create a route template for every controller that matches the resource name as in the json payload, i.e. the type: todoResources member (which should then match /todoResources).

Parallel to that, there is three ways to configure the resource name, in order of priority:

  1. the pluralizedTypeName option as you used in your example
  2. ResourceAttribute on the model class
  3. do nothing: the resource type name will be transformed with a camelCase formatter.

This means that in these 3 ways of configuring, the effects should ripple through to the URL accordingly. But right now configuration through (1) is currently not accessible by the routing convention because pluralizedTypeName is stored on the resource graph, which is not injected in the routing convention. Right now only a ResourceNameFormatter is injected in the routing convention, which is only enough to access configurations through (2) and (3), and it also leads to some code duplication because the ResourceNameFormatter is injected in multiple places doing the exact same thing. I'm going to fix this bug and duplication.

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

Successfully merging a pull request may close this issue.

3 participants