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

Allow same owned entity instance to be shared by many owners #12345

Closed
nicfahr opened this issue Jun 13, 2018 · 13 comments
Closed

Allow same owned entity instance to be shared by many owners #12345

nicfahr opened this issue Jun 13, 2018 · 13 comments
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported punted-for-2.2 punted-for-3.0 type-enhancement

Comments

@nicfahr
Copy link

nicfahr commented Jun 13, 2018

DDD style value object cannot be reused. Exception is thrown on context.SaveChanges().

Exception message: The entity of type 'Subscription' is sharing the table 'Subscriptions' with entities of type 'LocationValueObject', but there is no entity of this type with the same key value that has been marked as 'Added'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values
Stack trace:    at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.Validate(Dictionary`2 sharedTablesCommandsMap)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.CreateModificationCommands(IReadOnlyList`1 entries, Func`1 generateParameterName)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.BatchCommands(IReadOnlyList`1 entries)+MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(DbContext _, ValueTuple`2 parameters)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at ValueObjectIssue.Program.Main(String[] args) in E:\DEV\VS-SPIELWIESE\ValueObjectIssue\ValueObjectIssue\Program.cs:line 19

Steps to reproduce

    public class LocationValueObject : ValueObject
    {
        public int LocationId { get; private set; }
        public string Name { get; private set; }

        protected LocationValueObject() { }

        public LocationValueObject(int locationId, string name)
        {
            if (locationId <= 0)
            {
                throw new ArgumentException("locationId invalid.", "locationId");
            }

            this.LocationId = locationId;
            this.Name = name;
        }

        protected override IEnumerable<object> GetAtomicValues()
        {
            yield return LocationId;
            yield return Name;
        }
    }

    public class Subscription
    {
        private Subscription(LocationValueObject locationId)
        {
            this.Location = locationId;
            this.DateCreated = DateTime.Now;
        }

        protected Subscription() { /* ORM, see https://github.com/aspnet/EntityFrameworkCore/issues/11074 */ }

        public static Subscription Create(LocationValueObject locationId)
        {
            return new Subscription(locationId);
        }
        
        public int SubscriptionId { get; private set; }
        public LocationValueObject Location { get; private set; }
        public DateTime DateCreated { get; private set; }
    }

    public class Context : DbContext
    {
        public DbSet<Subscription> Subscriptions { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=SERVER;Database=value_object_issue;Trusted_Connection=True");
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.ApplyConfiguration<Subscription>(new SubscriptionConfiguration());
        }
    }

    internal class SubscriptionConfiguration : IEntityTypeConfiguration<Subscription>
    {
        public void Configure(EntityTypeBuilder<Subscription> builder)
        {
            builder.OwnsOne(p => p.Location);
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            using (var context = new Context())
            {
                var location = new LocationValueObject(4711, "IDENT-NAME");

                var subscription1 = Subscription.Create(location);
                var subscription2 = Subscription.Create(location);

                context.Subscriptions.Add(subscription1);
                context.Subscriptions.Add(subscription2);

                context.SaveChanges();
            }
        }
    }

Further technical details

EF Core version: 2.1.0
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Win 10
IDE: Visual Studio 2017 15.7.3

@ajcvickers
Copy link
Member

@elfrico Sharing owned entity type instances is not supported; I have created an issue to document this: dotnet/EntityFramework.Docs#764

Triage: we will attempt to make the exception message better in this case since there is some expectation that owned types will act like value objects, which is true in some ways, but in general not the best way to view them conceptually.

@ajcvickers ajcvickers added this to the 2.2.0 milestone Jun 13, 2018
@ajcvickers
Copy link
Member

Another note from triage: when looking at this consider how feasible it would be to attempt to support this.

@nicfahr
Copy link
Author

nicfahr commented Jun 14, 2018

@ajcvickers Thanks for investigation and clarification

@Tarig0
Copy link

Tarig0 commented Jun 14, 2018

Interesting looks like for value objects you would have to have the status be on the relationship instead of the entity, else you would have to copy the value object for each relationship.

@ajcvickers ajcvickers modified the milestones: 2.2.0-preview2, 2.2.0 Sep 11, 2018
@ajcvickers ajcvickers modified the milestones: 2.2.0, 3.0.0 Oct 3, 2018
@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog May 10, 2019
@ajcvickers ajcvickers changed the title Reuse Value Object fails Allow same owned entity instance to be shared by many owners Jul 7, 2019
@afranioce
Copy link

Attach same error #10578

@Cogax
Copy link

Cogax commented Nov 11, 2019

When I use DbContextOptionsBuilder.UseInMemoryDatabase(), this error doesn't occur. So if I test my application with that option, I don't get an Error. Shouldn't it behave the same way?

@ajcvickers
Copy link
Member

Closing as this is no longer something we intend to implement.

@ajcvickers ajcvickers added closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. and removed propose-close labels Nov 17, 2019
@ajcvickers ajcvickers removed this from the Backlog milestone Nov 17, 2019
@AndriySvyryd AndriySvyryd removed their assignment Mar 6, 2020
@PTatsky
Copy link

PTatsky commented Jul 5, 2020

How should we be implementing ValueObjects with Entity Framework Core? I am using 3.1.5 and currently I am unable to support a DDD model with ValueObjects as I cannot share instances of value objects as owned entities between many owners. This is a basic concept, I think. I for example have an object ControlPanel with a value object ControlPanelNodeId. I want to add a Zone to my ControlPanel. The Zone also has a ControlPanelNodeId property. When creating a Zone on the ControlPanel, the ControlPanelNodeId is populated with the value of the NodeId from the ControlPanel. This means the Control Panel and the new Zone are sharing the same ControlPanelNodeId value object.

However, I encounter an error as the owned entity ControlPanelNodeID cannot be shared.

Please can you point me to documentation which shows me how to implement ValueObjects for a DDD model?

@AndriySvyryd
Copy link
Member

Value object support will be implemented by #13947

@PTatsky
Copy link

PTatsky commented Jul 6, 2020

Thank you @AndriySvyryd.

For now I am having to implement a method on my value objects which allows me to clone them, effectively creating a new instance with the exact same value. So when I assign a value object to a property on another aggregate, within the backing state that I have on an aggregate I call the .Clone() method in the property setter.

This allows me to get around the issue where EF Core is tracking the same instance by assigning an shadow ID to it.

Hopefully I can engineer this out when a better solution is offered either through #13947 or #9906?

@AndriySvyryd
Copy link
Member

@PTatsky Yes, those two will likely be implemented together

@luisabreu
Copy link

I know this is closed, but is there any solution besides creating a clone method? I'm sorry, but coming from NH to EF is really a huge pain because EF seems to only understand tables...

@AndriySvyryd
Copy link
Member

@luisabreu See https://learn.microsoft.com/ef/core/what-is-new/ef-core-8.0/whatsnew#value-objects-using-complex-types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. customer-reported punted-for-2.2 punted-for-3.0 type-enhancement
Projects
None yet
Development

No branches or pull requests

8 participants