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

[Question] Get details of Inherited instances #1138

Closed
Anthony-Michel opened this issue Feb 15, 2022 · 9 comments
Closed

[Question] Get details of Inherited instances #1138

Anthony-Michel opened this issue Feb 15, 2022 · 9 comments
Labels

Comments

@Anthony-Michel
Copy link

Anthony-Michel commented Feb 15, 2022

I have a structure with multiple resources inheriting from an abstract resource, serving as main entry point.
It's using EF Core Table-per-hierarchy and discriminator configuration.

a GetAll call give me only the common fields from the abstract resource.
I need to be able to retrieve the specific fields from each individual child types, and use them for filtering / sorting / selecting fieldsets.

I tried, as described in the documentation to configure specific Repositories or Services, without success.
Also saw an MR from 2018 describing how to add a mapping to the configuration, but it seems this logic is now removed.

Would you get any advice on how to best implement this ?

Some code sample:
EF Configuration:

        {
            builder.ToTable("LegalEntity");
            builder.HasDiscriminator(e => e.EntityType)
            .HasValue<IndividualEntity>(LegalEntityTypeEnum.Individual.ToString())
            .HasValue<CompanyEntity>(LegalEntityTypeEnum.Company.ToString())
            .HasValue<ListedEntity>(LegalEntityTypeEnum.PubliclyListed.ToString())
            .HasValue<GovernmentEntity>(LegalEntityTypeEnum.Government.ToString());
            ....

Main Entity:

public abstract class LegalEntity : Identifiable<long>
   {
       [Attr]
       public string DisplayName { get; set; }
       //discriminator for the different types
       [Attr]
       [Column(TypeName = "varchar(24)")]
       public string EntityType { get; set; }
       ...
    public class IndividualEntity : LegalEntity
    {
        //basic info
        [Attr]        
        public string Surname { get; set; }
        [Attr]        
        public string Firstname { get; set; }
        ...

Controller:

    public class LegalEntitiesController : JsonApiQueryController<LegalEntity, long>
    {
        public LegalEntitiesController(IJsonApiOptions options, 
                                    IResourceGraph resourceGraph, 
                                    ILoggerFactory loggerFactory, 
                                    IResourceService<LegalEntity, long> resourceService
                                    ) : base(options, resourceGraph, loggerFactory, resourceService)
        {
        }
    }

JsonApi Config

            services.AddJsonApi<TenantDbContext>(options =>
            {
                options.Namespace = "api";
                options.UseRelativeLinks = true;
                options.IncludeTotalResourceCount = true;
                options.SerializerOptions.WriteIndented = false;
                options.SerializerOptions.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull;
                options.SerializerOptions.ReferenceHandler = ReferenceHandler.IgnoreCycles;
                options.SerializerOptions.Converters.Add(new JsonStringEnumConverter());             
#if DEBUG
                options.IncludeExceptionStackTraceInErrors = true;
                options.IncludeRequestBodyInErrors = true;
#endif
            }, discovery => discovery.AddCurrentAssembly());
@bart-degreed
Copy link
Contributor

Hi @Anthony-Michel, thanks for asking.

Unfortunately, resource inheritance doesn't work very well in JADNC at the moment. It requires quite some work to provide a good and consistent experience, tracked in #844, which is on our roadmap. While we do have some basic tests here, #846 contains non-working scenarios we're aware of.

For now, I'd advise to not use inheritance. However, if you want to continue with that, note we welcome PRs with small patches to unblock existing scenarios. Another way to move this forward is to provide feedback and/or suggestions to what's proposed in #844.

@Anthony-Michel
Copy link
Author

Thank you for the quick answer.
I really just need it to work on readonly a the moment, even just GetAll :) .
Based on #844 I probably misunderstood this was actually working.

I don't really see a sustainable way i can get rid of the inheritance for this case to be honest, as the main use case is to be able to let Front end retrieve a list of LegalEntities and their specific details without knowing or filtering/sorting by type.
While keeping the control on object creation to be able to set only the fields that define the specific type created.

Eventually, for this specific endpoint, i could do a mapping from my EF entities to a flatten version of the resource, as similar to this 6097476
But it seems the code structure.
Is that still something supported ?

@bart-degreed
Copy link
Contributor

Support for entity-resource-separation no longer exists since v4. It was dropped during early development of v4, because there were just too many corner cases that we couldn't get to work.

You could try and remove abstract from your base class to see if that unblocks you.

Inheritance is a kind of relationship, so you can always model them to be JSON:API relationships instead. Unless you're willing to contribute fixes to JADNC, that sounds like the best way to move forward at this time.

@Anthony-Michel
Copy link
Author

Already tried without abstract before posting here :)
Didn't change much.
I updated slightly my model meanwhile.
Looking forward for the capability to be added. Will have a look to see if I can contribute when i'll have some free time.

@bart-degreed
Copy link
Contributor

Closing as duplicate of #844.

@bkoelman bkoelman closed this as completed Mar 8, 2022
@bart-degreed
Copy link
Contributor

@Anthony-Michel please note the work being done in #1142. If you feel things should work differently, please let me know.

@Anthony-Michel
Copy link
Author

@bart-degreed thank you for all the work that went into that, greatly appreciated.
It looks exactly like what we need and how i would have do it.
I'll be more than happy to integrate coming version with this change and test it.

@Anthony-Michel
Copy link
Author

As i was passing by, just realized i forgot to give a feedback, so just for the sake of it, we integrated the new version some time after the last comments and it's working perfectly since.

Once again thank you for all the work that went into it !

@bkoelman
Copy link
Member

bkoelman commented Sep 8, 2022

Glad to hear, thanks for the feedback!

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

No branches or pull requests

3 participants