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

Indicate explicitly in constructor binding exception that entity type instances cannot be injected #17123

Closed
AgentTheGreat opened this issue Aug 13, 2019 · 13 comments · Fixed by #26398
Labels
area-docs area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@AgentTheGreat
Copy link

AgentTheGreat commented Aug 13, 2019

If you don't include a private, parameter-less constructor in your entity classes, but do provide a proper parametrized constructor with parameters having the same name as entity properties, EF Core mistakenly points to your properly defined constructor with:

No suitable constructor found for entity type 'MyClass'. The following constructors had parameters that could not be bound to properties of the entity type: cannot bind 'valueObject1', 'valueObject2' in 'MyClass(ValueObject valueObject1, ValueObject valueObject2)'.

Steps to reproduce

Consider these two classes:

public class MyClass
    {
       public Guid Id {get;}
       public ValueObject ValueObject1 {get; private set;}
       public ValueObject ValueObject2 {get; private set;}
       public MyClass(ValueObject valueObject1, ValueObject valueObject2) 
       {
          ValueObject1 = valueObject1;
          ValueObject2 = valueObject2;
       }
       private MyClass(){} // FORGETTING THIS LEADS TO AMBIGUOUS ERROR MESSAGE
    }

    public class ValueObject
    {
       public string Value {get; private set;}
       public ValueObject(string value) 
       {
          Value = value;
       }
       private ValueObject(){}
    }

and this configuration for the DBContext:

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<MyClass>(b => b.HasKey(mc => mc.Id));
            modelBuilder.Entity<MyClass>(b => b.OwnsOne(mc => mc.ValueObject1, rb =>
            {
                rb.Property(vo => vo.Value);
            }));
            modelBuilder.Entity<MyClass>(b => b.OwnsOne(mc => mc.ValueObject2, rb =>
            {
                rb.Property(vo => vo.Value);
            }));

            base.OnModelCreating(modelBuilder);
        }

If you forget to include the private constructor, EF Core mistakenly says there is something wrong with your properly defined, parametrized constructor. It wasted two days of my time in a project with big domain model when I went ahead with creating my new database.

Further technical details

EF Core version: 2.2.6
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10 1809
IDE: Visual Studio 2019 Community Edition V. 16.0.3

@ajcvickers
Copy link
Member

@AgentTheGreat EF is saying that it can't use that constructor to create instances. It may be valid from the perspective of your code, but it's not a constructor that EF can use.

@AgentTheGreat
Copy link
Author

@AgentTheGreat EF is saying that it can't use that constructor to create instances. It may be valid from the perspective of your code, but it's not a constructor that EF can use.

So there is no way to detect if a requirement is present or not and warn the developer about it?

@ajcvickers
Copy link
Member

@AgentTheGreat What do you mean by "if a requirement is present?"

@AgentTheGreat
Copy link
Author

@AgentTheGreat What do you mean by "if a requirement is present?"

Forgive me if I am not understanding this issue correctly.
EF Core requires a private, parameter-less constructor to be included if constructor binding to properties with private setters is going to happen, correct? That's how my issue got solved.
What I am asking is, would it be possible for EF Core to tell me I haven't included the private, parameter-less constructor, instead of telling me that it can't use the public, parametrized constructor? That particular message led me to try and figure out what was wrong with the parametrized constructor. There was nothing wrong with it. So "the requirement" would be the inclusion of a private, parameter-less constructor.

@ajcvickers
Copy link
Member

@AgentTheGreat EF is telling you it can't use that constructor. There is no requirement for the constructor it does use be a private parameterless constructor. However, EF needs to have a constructor for which it can figure out how the parameters of the constructor map to mapped properties of the entity type. If the constructor has parameters that it can't handle--in this case other entity types--then EF won't use that constructor, and will keep looking. If EF can't find any constructor it can use, then EF tells you this. See https://docs.microsoft.com/en-us/ef/core/modeling/constructors

@AgentTheGreat
Copy link
Author

AgentTheGreat commented Aug 13, 2019

@AgentTheGreat EF is telling you it can't use that constructor. There is no requirement for the constructor it does use be a private parameterless constructor. However, EF needs to have a constructor for which it can figure out how the parameters of the constructor map to mapped properties of the entity type. If the constructor has parameters that it can't handle--in this case other entity types--then EF won't use that constructor, and will keep looking. If EF can't find any constructor it can use, then EF tells you this. See https://docs.microsoft.com/en-us/ef/core/modeling/constructors

How is it that when I include that private constructor, it realizes it now can properly use the parametrized constructor? Basically I am asking why EF thinks it can't use

public MyClass(ValueObject valueObject1, ValueObject valueObject2) 
       {
          ValueObject1 = valueObject1;
          ValueObject2 = valueObject2;
       }

When it doesn't have

private MyClass(){}

Maybe there is something I'm missing here, but from your explanation it seems the first, parametrized constructor should be enough.
Tell me when this becomes more of a tutoring session than a bug discussion :D

@ajcvickers
Copy link
Member

@AgentTheGreat EF is not using the parameterized constructor. It needs a constructor it can use. The parameterless constructor is just that, so it uses it.

It can't use the parameterized constructor because it contains references to ValueObject which is an entity type. The confusion might be if you are expecting owned types to behave like DDD value objects--they don't. They are entity types. EF is not able to inject entity type instances into other entity type instances using a constructor.

@paiden
Copy link

paiden commented Aug 14, 2019

I completely agree with @AgentTheGreat here. It just took me 2 hours of stepping around in EF core code and finally giving up and searching here (luckily this issue here already exists) to find out how to fix this thing.

Please try to view this from an API consumer side. I do not - and do not want to - know how EF works internally (in such detail). But from and API user side I would expect EF to do something like this if I provide a parameterized constructor with entity types for my entity.

// My Code
public class Entity {
    public Entity(DepEntityA a, DepEntityB b) { ... }
    ...
}

// Pseudo code of what I think EF would do internally for such an entity
var a = db.Materialize(...);
var b = db.Materialize(...);
return new Entity(a, b);

it seems for whatever reason EF cannot do this. All I know is, that Newtonsoft.JSON (yes I know that one has nothing to do with DBs) can do things like this. So I somewhat expected it to be similar here, or parameterized constructors not work at all.

I do not expect that EF now changes its implementation in the background. I'm sure there are valid reasons that EF cannot inject related entities.

But at least I would like to see a much more useful error message like this, that would have saved me 2 hours of frustrating pseudo debugging of release NuGet packages and guessing around:

EntityFramework cannot handle entities without a default constructor. If you are using
parameterized constructors, please also provide a (private) default constructor for entity 
type 'Entity'.

Adding such a check in 'ProcessModelFinalized' should IMO not be a big problem.

@smitpatel
Copy link
Member

But that error is message is not entirely true.

@ajcvickers ajcvickers changed the title Confusing error message when no private, prameter-less constructor is provided and constructor binding is expected Indicate explicitly in constructor binding exception that entity type instances cannot be injected Aug 14, 2019
@ajcvickers ajcvickers added this to the Backlog milestone Aug 14, 2019
@ajcvickers ajcvickers self-assigned this Aug 14, 2019
@ajcvickers
Copy link
Member

@paiden re-opened to add a note that explicitly states that entity type instances cannot be injected, since this seems to be the main confusion here. See also #12078

@ajcvickers ajcvickers reopened this Aug 14, 2019
@AgentTheGreat
Copy link
Author

@paiden re-opened to add a note that explicitly states that entity type instances cannot be injected, since this seems to be the main confusion here. See also #12078

That WAS the main confusion! I didn't know EF can't inject entity type instances, until @paiden made his case. Now your comments makes sense to me. I basically gave up trying to understand what was going on.

So seeing as conforming with DDD principles we need our domain objects to be set through the Aggregate Root and not be directly accessible from outside, what is the workaround here if you don't mind me asking?

@ajcvickers
Copy link
Member

@AgentTheGreat Usually by using property with a private setter for EF to call or with no setter such that EF will set the backing field directly.

@AgentTheGreat
Copy link
Author

@AgentTheGreat Usually by using property with a private setter for EF to call or with no setter such that EF will set the backing field directly.

So everything was working almost perfectly fine already. (almost since I still have to change my domain model to not include read-only properties to accommodate EF. Hopefully we'll get even closer to having a fully DDD compatible ORM through EF.)

Thanks for the clarification and great work.

ajcvickers added a commit that referenced this issue Oct 18, 2021
Fixes #19383
Fixes #17123

> No suitable constructor was found for entity type 'BlogNone'. The following constructors had parameters that could not be bound to properties of the entity type: cannot bind 'did' in 'BlogNone(string title, int did)'; cannot bind 'notTitle' in 'BlogNone(string notTitle, Guid? shadow, int id)'; cannot bind 'dummy' in 'BlogNone(string title, Guid? shadow, bool dummy, int id)'; cannot bind 'dummy', 'description' in 'BlogNone(string title, Guid? shadow, bool dummy, int id, string description)'. Note that only mapped properties can be bound to constructor parameters. Navigations to related entities, including references to owned types, cannot be bound.

Fixes #26341

> The 'DateOnly' property 'Blog.PostedOn' could not be mapped because the database provider does not support this type. Consider converting the property value to a type supported by the database using a value converter. See https://aka.ms/efcore-docs-value-converters for more information. Alternately, exclude the property from the model using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.

Or if the store type is specified:

> The 'DateOnly' property 'Blog.PostedOn' could not be mapped to the database type 'datetime2' because the database provider does not support mapping 'DateOnly' properties to 'datetime2' columns. Consider mapping to a different database type or converting the property value to a type supported by the database using a value converter. See https://aka.ms/efcore-docs-value-converters for more information. Alternately, exclude the property from the model using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.

Fixes #21954

> Cannot save instance of 'Skinner' because it is an owned entity without any reference to its owner. Owned entities can only be saved as part of an aggregate also including the owner entity.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 18, 2021
@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 Oct 18, 2021
ajcvickers added a commit that referenced this issue Oct 19, 2021
Fixes #19383
Fixes #17123

> No suitable constructor was found for entity type 'BlogNone'. The following constructors had parameters that could not be bound to properties of the entity type: cannot bind 'did' in 'BlogNone(string title, int did)'; cannot bind 'notTitle' in 'BlogNone(string notTitle, Guid? shadow, int id)'; cannot bind 'dummy' in 'BlogNone(string title, Guid? shadow, bool dummy, int id)'; cannot bind 'dummy', 'description' in 'BlogNone(string title, Guid? shadow, bool dummy, int id, string description)'. Note that only mapped properties can be bound to constructor parameters. Navigations to related entities, including references to owned types, cannot be bound.

Fixes #26341

> The 'DateOnly' property 'Blog.PostedOn' could not be mapped because the database provider does not support this type. Consider converting the property value to a type supported by the database using a value converter. See https://aka.ms/efcore-docs-value-converters for more information. Alternately, exclude the property from the model using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.

Or if the store type is specified:

> The 'DateOnly' property 'Blog.PostedOn' could not be mapped to the database type 'datetime2' because the database provider does not support mapping 'DateOnly' properties to 'datetime2' columns. Consider mapping to a different database type or converting the property value to a type supported by the database using a value converter. See https://aka.ms/efcore-docs-value-converters for more information. Alternately, exclude the property from the model using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.

Fixes #21954

> Cannot save instance of 'Skinner' because it is an owned entity without any reference to its owner. Owned entities can only be saved as part of an aggregate also including the owner entity.
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-preview1 Feb 14, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview1, 7.0.0 Nov 5, 2022
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-docs area-model-building closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants