json-api-dotnet / JsonApiDotNetCore

A framework for building JSON:API compliant REST APIs using ASP.NET and Entity Framework Core.
https://www.jsonapi.net
MIT License
679 stars 159 forks source link

Is it possible to automatically include entity models and fields? #1290

Closed matthewblott closed 1 year ago

matthewblott commented 1 year ago

Currently I have to decorate each model and their fields like so:

[Resource]
public class Category : Identifiable<int>
{
  [Attr]
  public string CategoryName { get; set; }
  [Attr]
  public string Description { get; set; }
}

As I'm going to be adding decorating most classes and fields that's a lot of boilerplate I'd like to avoid. I've spent the morning trying to find a way of attaching attributes at runtime but I've hit a brick wall so thought I'd ask the question here! I guess this could be a feature request, so you could do something like the following:

builder.Services.AddJsonApi<DbContext>(
    options =>
    {
      options.AutoIncludeModels = true;
      options.AutoIncludeFields = true;
    },
    discovery => discovery.AddCurrentAssembly()
);

I can fill out a feature request if required but if there's a quicker solution anyone is able to provide I'd be grateful. Thanks in advance :-)

bkoelman commented 1 year ago

Hi @matthewblott, thanks for asking.

JsonApiDotNetCore does not support this by design. Too many times (before we used JADNC), we experienced a developer added a database column/table that was accidentally exposed and put in production. Once deployed, we were stuck with it forever or had to break existing consumers.

We believe it's good practice to explicitly define which endpoints, resources and fields are exposed. If you want, you can try implementing your own version of ResourceGraphBuilder. However we don't intend to add such support to the framework.

matthewblott commented 1 year ago

Hi @bkoelman thanks for the prompt reply. I've been using DreamFactory which does a lot out the box (but is a slightly different product) and I was looking for something in the .NET space. I'd be interested in doing something with ResourceGraphBuilder but I'm flying blind somewhat here. Is it a class I can inherit from and 'plug in' when setting the services and endpoints?

bkoelman commented 1 year ago

I pointed you to our implementation that builds the resource graph merely for inspiration. It's not a pluggable component, but the resource graph it produces is. You'll probably need to do something similar, I suppose, based on some kind of convention that decides when a property is a JSON:API attribute, to-one/to-many relationship, or neither. You could inherit, but there are no virtual methods to override. Even if we changed GetAttributes/GetRelationships to protected virtual, you'd still need to use reflection to set ResourceFieldAttribute.Property because that's internal (it is designed to be immutable after the graph has been built). Either way, you'll ultimately need to register your own resource graph instance as a singleton in the IoC containter.

As documented here, there are existing ways to get the resource classes into the graph. But because your classes don't have [Resource], our source generator that produces ASP.NET controllers does not run. To overcome that, you could hook into the ASP.NET pipeline and generate your own controllers at runtime, based on yet another kind of convention.

Lastly, as mentioned here, you may want to think of a way to narrow down Capabilities, based on some kind of convention, in order to prevent denial of service attacks.

matthewblott commented 1 year ago

Thanks again for the detailed response. That sounds like a lot of effort, I think I'd prefer writing a macro for a code generator. I understand the reasons for not wanting to auto include entities but what is the case against having it 'opt-in' or is it just not a requirement that comes up very much?

bkoelman commented 1 year ago

It hasn’t been asked before, as far as I'm aware. But more importantly, I've given several reasons why I think it's a bad idea, which you haven't responded to. Therefore I'm not motivated to spend time on implementing it. I'm not too concerned that applying anti-patterns isn't easy to do.

matthewblott commented 1 year ago

It seemed a reasonable request include all models in a given folder without decorating everything. If a new table or field is added to the database it won't be exposed unless someone adds an accompanying POCO object or field. I didn't expand further because you were adamant it wasn't something you were interested in doing. I didn't want to annoy you by pestering you about it further. I was trying to be polite. I appreciate the work you've done so far, it's a great piece of software.

bkoelman commented 1 year ago

Thanks. I didn't mean to come across that strong. I just tried to express my point of view, hoping you would challenge my reasoning and try to convince me otherwise. Please feel free to elaborate on why your project benefits from it and make your case.

Usually developers pick JADNC because they need a standardized, long-term contract to expose their API publicly or to third parties. They often share the models in a separate project, consumed by background jobs, reporting and other code. It is therefore uncommon to publicly expose everything. Such models may also contain calculated properties, intended for internal consumption only. Or contain entities used for authorization/auditing or broker queues (transactional outbox pattern) that aren't meant to be exposed.

Assuming there are substantial use cases, I still don't see how JADNC can decide which JSON:API field type to choose for a given property. When using EF Core, it could query its model, but that wouldn't work for other data sources.

bkoelman commented 1 year ago

Closing due to inactivity. Please let me know if it needs to stay open.