Issue #9 solved. Bubbling up of changed state is done with EF Core me…

…chanisms

Backend.Fx.sln

@@ -1,7 +1,7 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
 # Visual Studio 15
-VisualStudioVersion = 15.0.26430.15
+VisualStudioVersion = 15.0.26730.12
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{6B64354E-D95B-4711-BAF6-B32049C90CD9}"
 	ProjectSection(SolutionItems) = preProject
@@ -24,6 +24,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Backend.Fx.Tests", "tests\B
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Backend.Fx.Bootstrapping.Tests", "tests\Backend.Fx.Bootstrapping.Tests\Backend.Fx.Bootstrapping.Tests.csproj", "{C348AD1C-E928-4B11-8A88-EFCA70667B1D}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Backend.Fx.EfCorePersistence.Tests", "tests\Backend.Fx.EfCorePersistence.Tests\Backend.Fx.EfCorePersistence.Tests.csproj", "{4BB72B85-61F2-4C7F-9079-EA43492FCD44}"
+EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Backend.Fx.Testing", "src\Backend.Fx.Testing\Backend.Fx.Testing.csproj", "{E84BCF31-6BF8-4A96-99DB-F22158660F96}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -54,6 +58,14 @@ Global
 		{C348AD1C-E928-4B11-8A88-EFCA70667B1D}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{C348AD1C-E928-4B11-8A88-EFCA70667B1D}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{C348AD1C-E928-4B11-8A88-EFCA70667B1D}.Release|Any CPU.Build.0 = Release|Any CPU
+		{4BB72B85-61F2-4C7F-9079-EA43492FCD44}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{4BB72B85-61F2-4C7F-9079-EA43492FCD44}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{4BB72B85-61F2-4C7F-9079-EA43492FCD44}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{4BB72B85-61F2-4C7F-9079-EA43492FCD44}.Release|Any CPU.Build.0 = Release|Any CPU
+		{E84BCF31-6BF8-4A96-99DB-F22158660F96}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{E84BCF31-6BF8-4A96-99DB-F22158660F96}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{E84BCF31-6BF8-4A96-99DB-F22158660F96}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{E84BCF31-6BF8-4A96-99DB-F22158660F96}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -65,5 +77,10 @@ Global
 		{16D81037-F0D0-4647-BA4E-CA5518F6846D} = {53D4501E-953C-4A7C-97C4-1F9DE04BD092}
 		{3706F748-43F6-41BD-8875-81FA679220C7} = {C7885592-A4B8-4BA8-8D3A-1EDA4025D17A}
 		{C348AD1C-E928-4B11-8A88-EFCA70667B1D} = {C7885592-A4B8-4BA8-8D3A-1EDA4025D17A}
+		{4BB72B85-61F2-4C7F-9079-EA43492FCD44} = {C7885592-A4B8-4BA8-8D3A-1EDA4025D17A}
+		{E84BCF31-6BF8-4A96-99DB-F22158660F96} = {53D4501E-953C-4A7C-97C4-1F9DE04BD092}
+	EndGlobalSection
+	GlobalSection(ExtensibilityGlobals) = postSolution
+		SolutionGuid = {45648557-C751-44AD-9C87-0F12EB673969}
 	EndGlobalSection
 EndGlobal

src/Backend.Fx.EfCorePersistence/DbContextExtensions.cs

@@ -10,6 +10,7 @@
     using Logging;
     using Microsoft.EntityFrameworkCore;
     using Microsoft.EntityFrameworkCore.ChangeTracking;
+    using Microsoft.EntityFrameworkCore.Metadata;
 
     public static class DbContextExtensions
     {
@@ -53,9 +54,24 @@ public static void UpdateTrackingProperties(this DbContext dbContext, string use
             userId = userId ?? "anonymous";
             var isTraceEnabled = Logger.IsTraceEnabled();
             int count = 0;
+
+            // Modifying an entity (also removing an entity from an aggregate) should leave the aggregate root as modified
+            dbContext.ChangeTracker
+                     .Entries<Entity>()
+                     .Where(entry => entry.State == EntityState.Added || entry.State == EntityState.Modified || entry.State == EntityState.Deleted)
+                     .Where(entry => !(entry.Entity is AggregateRoot))
+                     .ForAll(entry =>
+                     {
+                         EntityEntry aggregateRootEntry = FindAggregateRootEntry(dbContext.ChangeTracker, entry);
+                         if (aggregateRootEntry.State == EntityState.Unchanged)
+                         {
+                             aggregateRootEntry.State = EntityState.Modified;
+                         }
+                     });
+
             dbContext.ChangeTracker
                 .Entries<Entity>()
-                .Where(entry => entry.State == EntityState.Added || entry.State == EntityState.Modified || entry.State == EntityState.Deleted)
+                .Where(entry => entry.State == EntityState.Added || entry.State == EntityState.Modified)
                 .ForAll(entry =>
                 {
                     try
@@ -79,14 +95,6 @@ public static void UpdateTrackingProperties(this DbContext dbContext, string use
                             }
                             entity.SetModifiedProperties(userId, utcNow);
                         }
-                        else if (entry.State == EntityState.Deleted)
-                        {
-                            if (isTraceEnabled)
-                            {
-                                Logger.Trace("tracking that {0}[{1}] was deleted by {2} at {3:T} UTC", entity.GetType().Name, entity.Id, userId, utcNow);
-                            }
-                            entity.SetDeleted(userId, utcNow);
-                        }
                     }
                     catch (Exception ex)
                     {
@@ -100,6 +108,54 @@ public static void UpdateTrackingProperties(this DbContext dbContext, string use
             }
         }
 
+        /// <summary>
+        /// This method finds the EntityEntry&lt;AggregateRoot&gt; of an EntityEntry&lt;Entity&gt;
+        /// assuming it has been loaded and is being tracked by the change tracker.
+        /// </summary>
+        private static EntityEntry FindAggregateRootEntry(ChangeTracker changeTracker, EntityEntry entry)
+        {
+            foreach (var navigation in entry.Navigations)
+            {
+                var navTargetTypeInfo = navigation.Metadata.GetTargetType().ClrType.GetTypeInfo();
+                int navigationTargetForeignKeyValue;
+
+                if (navigation.CurrentValue == null)
+                {
+                    // orphaned entity, original value contains the foreign key value
+                    if (navigation.Metadata.ForeignKey.Properties.Count > 1)
+                    {
+                        throw new InvalidOperationException("Foreign Keys with multiple properties are not supported.");
+                    }
+
+                    IProperty property = navigation.Metadata.ForeignKey.Properties[0];
+                    navigationTargetForeignKeyValue = (int)entry.OriginalValues[property];
+                }
+                else
+                {
+                    // added or modified entity, current value contains the foreign key value
+                    navigationTargetForeignKeyValue = ((Entity)navigation.CurrentValue).Id;
+                }
+
+                // assumption: an entity cannot be loaded on its own. Everything on the navigation path starting from the 
+                // aggregate root must have been loaded before, therefore we can find it using the change tracker
+                EntityEntry<Entity> navigationTargetEntry = changeTracker
+                        .Entries<Entity>()
+                        .Single(ent => Equals(ent.Entity.GetType().GetTypeInfo(), navTargetTypeInfo)
+                                       && ent.Property(nameof(Entity.Id)).CurrentValue.Equals(navigationTargetForeignKeyValue));
+
+                // if the target is AggregateRoot, no (further) recursion is needed
+                if (typeof(AggregateRoot).GetTypeInfo().IsAssignableFrom(navTargetTypeInfo))
+                {
+                    return navigationTargetEntry;
+                }
+
+                // recurse in case of "Entity -> Entity -> AggregateRoot"
+                return FindAggregateRootEntry(changeTracker, navigationTargetEntry);
+            }
+
+            return null;
+        }
+
         public static void TraceChangeTrackerState(this DbContext dbContext)
         {
             if (Logger.IsTraceEnabled())
@@ -143,7 +199,7 @@ public static void TraceChangeTrackerState(this DbContext dbContext)
 
         public static TDbContext CreateDbContext<TDbContext>(this DbContextOptions options) where TDbContext : DbContext
         {
-            return (TDbContext) Activator.CreateInstance(typeof(TDbContext), options);
+            return (TDbContext)Activator.CreateInstance(typeof(TDbContext), options);
         }
 
         private static string GetPrimaryKeyValue(EntityEntry entry)

src/Backend.Fx.EfCorePersistence/EfUnitOfWork.cs

@@ -41,7 +41,7 @@ protected override void UpdateTrackingProperties(string userId, DateTime utcNow)
 
         protected override void Commit()
         {
-            Flush();
+            DbContext.SaveChanges();
             currentTransaction.Commit();
             currentTransaction.Dispose();
             currentTransaction = null;

src/Backend.Fx.Testing/Backend.Fx.Testing.csproj

@@ -5,6 +5,12 @@
     <Version>1.3.0</Version>
   </PropertyGroup>
 
+  <ItemGroup>
+    <Compile Remove="TestData\**" />
+    <EmbeddedResource Remove="TestData\**" />
+    <None Remove="TestData\**" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="FakeItEasy" Version="3.3.2" />
     <PackageReference Include="NLog" Version="5.0.0-beta09" />
@@ -18,8 +24,4 @@
     <ProjectReference Include="..\Backend.Fx\Backend.Fx.csproj" />
   </ItemGroup>
 
-  <ItemGroup>
-    <Folder Include="TestData\" />
-  </ItemGroup>
-
 </Project>

src/Backend.Fx/BuildingBlocks/AggregateRoot.cs

@@ -12,10 +12,5 @@ protected AggregateRoot()
         protected AggregateRoot(int id) : base(id) { }
 
         public int TenantId { get; set; }
-
-        protected override AggregateRoot FindMyAggregateRoot()
-        {
-            return this;
-        }
     }
 }

src/Backend.Fx/BuildingBlocks/Entity.cs

@@ -4,6 +4,7 @@
     using System.ComponentModel.DataAnnotations;
     using System.Diagnostics;
     using JetBrains.Annotations;
+    using Logging;
 
     /// <summary>
     /// An object that is not defined by its attributes, but rather by a thread of continuity and its identity.
@@ -12,6 +13,8 @@
     [DebuggerDisplay("{" + nameof(DebuggerDisplay) + "}")]
     public abstract class Entity : IEquatable<Entity>
     {
+        private static readonly ILogger Logger = LogManager.Create<Entity>();
+
         protected Entity()
         { }
 
@@ -27,7 +30,7 @@ public string DebuggerDisplay
         }
 
         [Key]
-        public int Id { get; private set; }
+        public int Id { get; [UsedImplicitly] private set; }
 
         public DateTime CreatedOn { get; protected set; }
 
@@ -65,36 +68,14 @@ public void SetModifiedProperties([NotNull] string changedBy, DateTime changedOn
             }
             ChangedBy = changedBy.Length > 100 ? changedBy.Substring(0, 99) + "…" : changedBy;
             ChangedOn = changedOn;
-
-            // Modifying me results implicitly in a modification of the aggregate root, too.
-            AggregateRoot myAggregateRoot = FindMyAggregateRoot();
-            if (myAggregateRoot != this)
-            {
-                myAggregateRoot?.SetModifiedProperties(changedBy, changedOn);
-            }
         }
 
-        public void SetDeleted([NotNull] string changedBy, DateTime changedOn)
+        [Obsolete("Not used any more")]
+        protected virtual AggregateRoot FindMyAggregateRoot()
         {
-            if (changedBy == null)
-            {
-                throw new ArgumentNullException(nameof(changedBy));
-            }
-            if (changedBy == string.Empty)
-            {
-                throw new ArgumentException(nameof(changedBy));
-            }
-
-            // Deleting me results implicitly in a modification of the aggregate root.
-            AggregateRoot myAggregateRoot = FindMyAggregateRoot();
-            if (myAggregateRoot != this)
-            {
-                myAggregateRoot?.SetModifiedProperties(changedBy, changedOn);
-            }
+            return null;
         }
 
-        protected abstract AggregateRoot FindMyAggregateRoot();
-
         public bool Equals(Entity other)
         {
             if (other == null)
@@ -136,10 +117,13 @@ public override bool Equals(object obj)
         /// </returns>
         public override int GetHashCode()
         {
-            // ReSharper disable once NonReadonlyMemberInGetHashCode
             // id is practically readonly, only for framework reasons it can be set (because of EF, mostly)
+
+            // ReSharper disable once BaseObjectGetHashCodeCallInGetHashCode
+            // ReSharper disable once NonReadonlyMemberInGetHashCode
             return Id == default(int)
                 ? base.GetHashCode()
+                // ReSharper disable once NonReadonlyMemberInGetHashCode
                 : Id.GetHashCode();
         }
 

tests/Backend.Fx.EfCorePersistence.Tests/TheRepositoryOfComposedAggregate.cs

@@ -1,6 +1,7 @@
 namespace Backend.Fx.EfCorePersistence.Tests
 {
     using System;
+    using System.Collections.Generic;
     using System.Linq;
     using BuildingBlocks;
     using DummyImpl;
@@ -11,10 +12,12 @@
     using Microsoft.EntityFrameworkCore;
     using Patterns.Authorization;
     using Patterns.IdGeneration;
+    using Testing;
     using Xunit;
 
     public class TheRepositoryOfComposedAggregate : TestWithInMemorySqliteDbContext
     {
+        private readonly IEqualityComparer<DateTime?> tolerantDateTimeComparer = new TolerantDateTimeComparer(TimeSpan.FromMilliseconds(500));
         private readonly IEntityIdGenerator idGenerator = A.Fake<IEntityIdGenerator>();
         private int nextId = 1;
 
@@ -163,6 +166,30 @@ public void CanReplaceDependentCollection()
             Assert.Equal(5, count);
         }
 
+        [Fact]
+        public void UpdatesAggregateTrackingPropertiesOnDeleteOfDependant()
+        {
+            int id = CreateBlogWithPost(10);
+
+            var expectedModifiedOn = Clock.UtcNow.AddHours(1);
+            Clock.OverrideUtcNow(expectedModifiedOn);
+
+            using (var sut = new SystemUnderTest(DbContextOptions, Clock, TenantId))
+            {
+                var blog = sut.Repository.Single(id);
+                blog.Posts.Remove(blog.Posts.First());
+            }
+
+            Clock.OverrideUtcNow(Clock.UtcNow.AddHours(1));
+
+            using (var sut = new SystemUnderTest(DbContextOptions, Clock, TenantId))
+            {
+                var blog = sut.DbContext.Blogs.Find(id);
+                Assert.NotNull(blog.ChangedOn);
+                Assert.Equal(expectedModifiedOn, blog.ChangedOn.Value, tolerantDateTimeComparer);
+            }
+        }
+
         private int CreateBlogWithPost(int postCount = 1)
         {
             long blogId = nextId++;

tests/Backend.Fx.Tests/BuildingBlocks/TheAggregateRoot.cs

@@ -2,7 +2,6 @@
 {
     using System;
     using System.Collections.Generic;
-    using System.Linq;
     using Fx.BuildingBlocks;
     using RandomData;
     using Xunit;
@@ -36,10 +35,6 @@ public TestEntity(string name, TestAggregateRoot parent)
 
             public string Name { get; set; }
             public TestAggregateRoot Parent { get; set; }
-            protected override AggregateRoot FindMyAggregateRoot()
-            {
-                return Parent;
-            }
         }
 
         [Fact]
@@ -92,17 +87,6 @@ public void CreatedByPropertyIsChoppedAt100Chars()
             Assert.Equal(moreThanHundred.Substring(0, 99) + "…", sut.CreatedBy);
         }
 
-        [Fact]
-        public void ModifiedIsBubblingUpFromEntityToAggregateRoot()
-        {
-            DateTime now = DateTime.Now;
-            var sut = new TestAggregateRoot(nextId++, "gaga");
-            sut.Children.First().SetModifiedProperties("someone", now);
-
-            Assert.Equal("someone", sut.ChangedBy);
-            Assert.Equal(now, sut.ChangedOn);
-        }
-
         [Fact]
         public void ChangedByPropertyIsChoppedAt100Chars()
         {