Skip to content

Feature request: support resources with injected DbContext #657

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
bart-degreed opened this issue Dec 17, 2019 · 2 comments · Fixed by #731
Closed

Feature request: support resources with injected DbContext #657

bart-degreed opened this issue Dec 17, 2019 · 2 comments · Fixed by #731

Comments

@bart-degreed
Copy link
Contributor

Description

Getting a resource by ID with sparse fieldsets fails when that resource has a constructor to inject the DbContext, as described here: https://docs.microsoft.com/en-us/ef/core/modeling/constructors.

Example:

public sealed class User : Identifiable
{
    public User(AppDbContext context)
    {
    }
}

This fails because IQueryableExtensions.CallGenericSelectMethod (and a few other places) assumes the entity has a parameterless constructor, causing an exception to be thrown at the statement:

Expression.New(sourceType)

Environment

latest master branch

@bart-degreed
Copy link
Contributor Author

This also fails when trying to delete a resource, due to the usage of:

Activator.CreateInstance(typeof(TResource))

in DefaultResourceService.

@maurei
Copy link
Member

maurei commented Dec 23, 2019

That's an interesting feature of EF Core feature that I hadn't seen yet. I think it would be important to investigate if assuming parameterless constructors will or will not lead to JADNC being more tightly coupled with EF Core, because I feel such "EF Core parameter injection" is very EF-specific. But at the same time, having constructors with parameters in general to set read-only properties for models is not necessarily EF-Core specific, so it might/should be OK.

Either way I don't expect I will be able to work on this in the near future as I am currently low on development resources for JADNC (which is why I've been late in replying to your latest issues, my apologies for that)

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.

2 participants