[Owned] entities does not trigger hooks

[cjblomqvist]

A is an owned entity. There are 2 other entities B and C which both use A. E.g.

[Owned]
public class A {
  // ...
}

public class B {
  public A MyA { get; set; }
  // ...
}

public class C {
  public A MyA { get; set; }
  // ...
}

Current behavior: You can't add triggers to A (reasonable). Whenever any change on A happens, no triggeres are triggered (e.g. on B or C).

Expected behavior: Whenever anything changes on B.MyA, any trigger for B is triggered. Whenever anything changes on C.MyA, any trigger for C is triggered.

[cjblomqvist]

After digging a little bit into this, turns out it's probably equally easy (albeit probably with some perf implications) do just add a trigger that always triggers first, and do the same thing that would have happen in Triggered itself. It would be nice with native support, but not critical. For anyone ending up with the same issue as us, then you can use below trigger code (def. needs some cleanup, but you'll get the gist):

using EntityFrameworkCore.Triggered;
using Microsoft.EntityFrameworkCore;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace API.Models.Triggers
{
    public class CascadeOwnedChangesTrigger : ITriggerPriority, IBeforeSaveTrigger<object>
    {
        public int Priority => CommonTriggerPriority.Earlier - 1;

        private readonly MainContext _context;

        public CascadeOwnedChangesTrigger(
            MainContext context
        )
        {
            _context = context;
        }

        public async Task BeforeSave(ITriggerContext<object> context, CancellationToken cancellationToken)
        {
            var entity = context.Entity;
            var entry = _context.Entry(entity);

            if(entry.Metadata.IsOwned())
            {
                var ownership = entry.Metadata.FindOwnership()!;
                var navigation = ownership.GetNavigation(false)!;
                var type = navigation.DeclaringType.ClrType;
                var propertyName = navigation.Name;
                var property = type.GetProperty(propertyName)!;

                var newEntries = _context.ChangeTracker.Entries(); 

                var ownerTypeEntries = newEntries.Where(e => e.Entity.GetType() == type);
                var ownerEntry = ownerTypeEntries.FirstOrDefault(e => property.GetValue(e.Entity) == entry.Entity);

                if (ownerEntry is not null && ownerEntry.State == EntityState.Unchanged)
                {
                    ownerEntry.State = EntityState.Modified;
                }
            }
        }
    }
}

[cjblomqvist]

(one could also argue whether it makes sense to trigger on the principle entity or not - since the definition of owned is DDD Aggregate then I think there's quite a clear case for that, but anyhow)

[koenbeuk]

You can't add triggers to A (reasonable).

You can add triggers to A. triggers will fire regardless of the entity being an owned entity or not. You are actually using this functionality to get your workaround working :), I also think this is the preferred approach as it allows for specifying behavior attached with A regardless of who owns A. (e.g. it's perfectly reasonable to implement A BeforeSaveTrigger on an hypothetical owned Address entity which does some verification).

Whenever any change on A happens, no triggeres are triggered (e.g. on B or C).

That is correct. I understand the need to fire Triggers for the owner of an owned entity however this is not always desired. (e.g. in relation to the Address entity example, we probably do not want to fire triggers on the Owner if the address changes).

Whenever anything changes on B.MyA, any trigger for B is triggered. Whenever anything changes on C.MyA, any trigger for C is triggered.

This is a reasonable request for some scenarios (e.g. auditing). Not so much for others. Your workaround is indeed problematic as it marks the parent as modified. If you known the Owners of your type then perhaps consider a more subtle approach, e.g. hardcoding knowledge of the owners.

If we want to move forth in enabling this feature in the library, we would probably want to allow the user to opt-into this feature, e.g. triggerOptions.EnableOwnedTypePropagation() (Not sure yet about the name but that is semantics). We would probably want to update the TriggerContextFactory to also create entries for the owners (similar as to how you have things setup).

EntityFrameworkCore.Triggered/src/EntityFrameworkCore.Triggered/Internal/TriggerContextTracker.cs

Line 97 in 546f640

var triggerContextDescriptor = new TriggerContextDescriptor(entry, changeType.Value);

Keep in mind that in your current solution, you still have to deal with hierarchies of owned entities (in case A is owned by B and B is owned by C then you will want to raise triggers on all 3).

[cjblomqvist]

You can add triggers to A.

Yes, you're of course absolutely right. As my workaround suggest (as it's based on triggers on A) :)

however this is not always desired. (e.g. in relation to the Address entity example, we probably do not want to fire triggers on the Owner if the address changes).

Yes, I can see that it's relatively opiniated to always trigger modified on the parent. I do think though that the DDD Aggregate-argument (on which Owned functionality are designed upon) makes it so that it's a reasonable default for most scenario. But I can really see opt-out being needed. Then, if opt-in or opt-out - not so critical imo. Specifically, I think it makes sense even in the Address case. But I guess that's the thing with opinions - they differ :)

An alternative to triggerOptions.EnableOwnedTypePropagation() could be a CascadeStrategy. E.g. EntityAndTypeWithAutoOwned?

An alternative to adding it to the library (although I think that makes sense) it could be nice to just document the workaround/have a sample in the project.

Your workaround is indeed problematic as it marks the parent as modified.

I lack to see why it's problematic? By DDD definition the Address is part of the Owner, so isn't the Owner modified if the Address is a part of it is modified? I guess Value Objects ( dotnet/efcore#26065 ) might be a more obvious case where the owner should be considered updated as well (I guess? or maybe not)

If you known the Owners of your type then perhaps consider a more subtle approach, e.g. hardcoding knowledge of the owners.

Yes, that's the reason why I have 2 different owners in my example (to highlight that it's not possible to do except a lookup like my workaround). I couldn't find any other way in EF Core either to find the owner for a specific owned isntance.

Keep in mind that in your current solution, you still have to deal with hierarchies of owned entities (in case A is owned by B and B is owned by C then you will want to raise triggers on all 3).

I think that's handled automatically using cascading as normal? A -> B -> C. Or am I missing something?

[koenbeuk]

I think that's handled automatically using cascading as normal? A -> B -> C. Or am I missing something?

You're absolutely right!

By DDD definition the Address is part of the Owner, so isn't the Owner modified if the Address is a part of it is modified?

The problem is not in relation to DDD but rather with perf and unintended side-effects. By marking the owner as Modified, we'll force EF to send an update statement (this will currently affect relational databases but might affect the cosmosdb provider in the future if partial document replacement ever gets implemented).

By implementing this feature in the library as I proposed, we can still raise triggers on the owners while not forcing the state of the owners to be Modified in EF.

[cjblomqvist]

The problem is not in relation to DDD but rather with perf and unintended side-effects. By marking the owner as Modified, we'll force EF to send an update statement (this will currently affect relational databases but might affect the cosmosdb provider in the future if partial document replacement ever gets implemented).

Thanks for pointing that out - I didn't know that! I guess for 1-1 owned entities it doesn't matter so much because the table is updated anyway (unless you've explicitly configured the owned to be a separate table, but I'd guess that's quite rare). Then of course, for 1-X owned entities, it's always a different table.

Anyway, is seems like a library solution is needed for it to be viable long-term (and my solution is more of a workaround).

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.