Use IncludeGraphOperationBuilder in combination with global PostConfiguration option

[meinsiedler]

Description

This is a follow-up issue/question to #542 .

We set up a column mappping for shadow properties which are automatically set when performing any bulk operation like BulkInsert, BulkMerge. This setup is performed globally using the EntityFrameworkManager.BulkOperationBuilder. This now works great also with the new IncludeGraph option from the new version 7.100.0.

But when we use the global setup in combination with the IncludeGraphOperationBuilder, we receive the following exception. When we don't use the IncludeGraphOperationBuilder, our code works even when using the new IncludeGraph option.

Is this expected behavior or should it be possible to setup the PostConfiguration globally even when using the IncludeGraphOerationBuilder?

Please see the linked Fiddle for more details and a repro of the issue.

Exception

If you are seeing an exception, include the full exceptions details (message and stack trace).

Unhandled exception. System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.Exception: When using `IncludeGraph`, columns related options CANNOT be specified outside the `IncludeGraphOperationBuilder` since they depend on the type (See: https://entityframework-extensions.net/include-graph). The following options must be specified inside `IncludeGraphOperationBuilder`: _postConfiguration
   at Z.BulkOperations.BulkOperation`1.(BulkOperation )
   at Z.EntityFramework.Extensions.PublicInternalBulkOperationManager.`2.(BulkOperation`1 )
   at .BulkMerge[T](DbContext this, IEntityType entityType, IEnumerable`1 list, Action`1 options, SavingSelector savingSelector, Boolean forceSpecificTypeMapping)
   at Z.EntityFramework.Extensions.PublicInternalBulkOperationManager.BulkMergeCastWithSavingSelector[T,OldT](BulkOperation`1 this, DbContext context, IEntityType entityType, List`1 list, SavingSelector savingSelector, Boolean forceSpecificTypeMapping)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
   --- End of inner exception stack trace ---
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
   at System.Reflection.RuntimeMethodInfo.InvokeWithManyArguments(RuntimeMethodInfo mi, Int32 argCount, Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at .BulkMerge[T](DbContext this, IEntityType entityType, IEnumerable`1 list, Action`1 options, SavingSelector savingSelector, Boolean forceSpecificTypeMapping)
   at .[](DbContext , SavingSelector , Action`1 )
   at .[](DbContext ,  , BulkOperationActionType , Action`1 )
   at .BulkMerge[T](DbContext this, IEnumerable`1 entities, Action`1 options, Boolean isBulkSaveChanges)
   at DbContextExtensions.BulkMerge[T](DbContext this, IEnumerable`1 entities, Action`1 options)
   at DbContextExtensions.`1.()
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
--- End of stack trace from previous location ---
   at DbContextExtensions.BulkMergeAsync[T](DbContext this, IEnumerable`1 entities, Action`1 options, CancellationToken cancellationToken)
   at Program.<Main>$(String[] args)
   at Program.<Main>(String[] args)
Command terminated by signal 6

Fiddle or Project (Optional)

https://dotnetfiddle.net/KEpQtr

Further technical details

• EF version: 7
• EF Extensions version: 7.100.0
• Database Provider: Sql Server

[JonathanMagnan]

Hello @meinsiedler ,

At this moment, it is indeed impossible to set a PostConfiguration globally the way you are doing it.

It's more for security purposes than anything else, as we could easily support it.

Here is a solution that add this code in the builder instead: https://dotnetfiddle.net/6eX0X4

await context.Invoices.BulkMergeAsync(invoices, opts =>
{
    opts.IncludeGraph = true;
    opts.IncludeGraphOperationBuilder = bulkOperation =>
    {
        // Just for testing: Assume we have some special IncludeGraphOperationBuilder config for one specific call to BulkMergeAsync
        if (bulkOperation is BulkOperation<InvoiceItem> invoiceItemBulkOperation)
        {
            invoiceItemBulkOperation.IgnoreOnMergeUpdateExpression = invoiceItem => new
            {
                invoiceItem.Description
            };
        }
    	bulkOperation.PostConfiguration = postConfigurationBulkOperation => ConfigureModifiedByUserIdColumnMapping(postConfigurationBulkOperation);	
    };
});

You can ask for it if you really need to set it globally. If that's the case, I will ask my architect how we could potentially allow this kind of scenario, such as allowing it when UnsafeMode is enabled.

Let me also know if that shows correctly the cause and solution of this issue.

Best Regards,

Jon

[meinsiedler]

Hello @JonathanMagnan ,

thank you for your response and for your efforts regarding this topic.

Ideally, we would really like to set up the configuration of the ModifiedByUserId column globally. With that, we wouldn't need to touch every call to BulkMergeAsync and BulkInsertAsync and developers don't need to think about passing the additional options on every call. This was the idea of setting up the PostConfiguration globally in the first place.

It's more for security purposes than anything else, as we could easily support it.

I don't quite understand, why the exeption throwing behaves differently when just passing IncludeGraph = true opposed to passing both IncludeGraph = true and IncludeGraphOperationBuilder.

(This different behavior of both calls to BulkMergeAsync is shown in my original Fiddle.)

[JonathanMagnan]

Hello @meinsiedler ,

Sure, we will look into this to allow this options to be global.

I will try to remember to give you the reason after the internal discussion we will have about this issue next Monday why they behave differently.

[meinsiedler]

Hello @JonathanMagnan ,

do you have any updates regarding this issue?

Thank you.

[JonathanMagnan]

Hello @meinsiedler ,

We made some progress on this issue, such as better understanding our current different behaviors.

One problem with IncludeGraphOperationBuilder is it copy action from some previous bulk operations which can cause some actions to be executed more than once. On your case, it's fine as you are only modifying columns but the security has been added in case someone was adding columns for example which will be very bad if executed multiple times.

We will investigate more this week about what can be done (We have been a little bit to busy past 2 weeks).

[meinsiedler]

Hello @JonathanMagnan ,

do you have any further updates regarding this issue? We were investigating different options we have when I created this issue and now we would like to bring this feature to production. Is is foreseeable that something will be changed to support our use case? If not we need to find another, non-generic way to set the modified columns, e.g. via custom extension methods. This wouldn't be our preferred option.

Thank you.