Consider supporting the ObjectFactory attribute on child types

[yvansigouin]

As part of improving the data portal factory model in CSLA 6, it might make sense to allow the ObjectFactory attribute on child types. This would probably mean adding behaviors to the ObjectFactory base type so it can help create/fetch/update child objects, much like the UpdateChildren method in BusinessBase.

The train of thought is that a parent factory could call a base method asking to update children, and the data portal would use the child type's ObjectFactory attribute to find and invoke the child factory.

Original post:

Describe the bug
Hi,
I am currently migrating a solution that includes several classes from CSLA 4.5.501 (which worked fine) to version 5.4.0 and I having issues with child entities. All my business entities use factories. The problem occurs during the Save() operation. It seems that CSLA can't find the factory method of the child entities and throw an exception.

Version and Platform
CSLA version: 5.4.0 (and 5.4.1 preview)
OS: Windows 10
Platform: .NET 5 And Visual Studio 2019.

Code that Fails
Here a test exemple to add in csla.netcore.test project:

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Text;
using Csla.Server;
#if !NUNIT
using Microsoft.VisualStudio.TestTools.UnitTesting;
#else
using NUnit.Framework;
using TestClass = NUnit.Framework.TestFixtureAttribute;
using TestInitialize = NUnit.Framework.SetUpAttribute;
using TestCleanup = NUnit.Framework.TearDownAttribute;
using TestMethod = NUnit.Framework.TestAttribute;
#endif

namespace Csla.Test.BasicModern
{
  [TestClass]
  public class TestInsertChild
  {
    [TestMethod]
    public void Test_Save_Child_Entity()
    { 
      // Create the graph
      var rootEntity = RootEntityWithFactory.NewRoot("Root entity");
      var childEntityWithoutFactory = ChildEntityWithoutFactory.NewEntity("Child without factory");
      rootEntity.ChildEntityWithoutFactory = childEntityWithoutFactory;
      var childEntityWithFactory = ChildEntityWithFactory.NewEntity("Child with factory");
      rootEntity.ChildEntityWithFactory = childEntityWithFactory;
      
      // Save the entity graph - Throw exception when saving ChildEntityWithFactory
      var savedRootEntity = rootEntity.Save();
    }
  }

  [Serializable]
  [Csla.Server.ObjectFactory("Csla.Test.BasicModern.RootEntityFactory, csla.netcore.test, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null")]
  public class RootEntityWithFactory : BusinessBase<RootEntityWithFactory>
  {
    public static readonly PropertyInfo<string> NameProperty = RegisterProperty<string>(nameof(Name));
    public string Name
    {
      get { return GetProperty(NameProperty); }
      set { SetProperty(NameProperty, value); }
    }

    public static readonly PropertyInfo<ChildEntityWithoutFactory> ChildWithoutFactoryProperty = RegisterProperty<ChildEntityWithoutFactory>(nameof(ChildEntityWithoutFactory));
    public ChildEntityWithoutFactory ChildEntityWithoutFactory
    {
      get { return GetProperty(ChildWithoutFactoryProperty); }
      set { LoadProperty(ChildWithoutFactoryProperty, value); }
    }

    public static readonly PropertyInfo<ChildEntityWithFactory> ChildWithFactoryProperty = RegisterProperty<ChildEntityWithFactory>(nameof(ChildEntityWithFactory));
    public ChildEntityWithFactory ChildEntityWithFactory
    {
      get { return GetProperty(ChildWithFactoryProperty); }
      set { LoadProperty(ChildWithFactoryProperty, value); }
    }

    public static RootEntityWithFactory NewRoot(string name)
    {
      var entity = Csla.DataPortal.Create<RootEntityWithFactory>();
      entity.Name = name;
      return entity;
    }

    protected override void OnSaved(RootEntityWithFactory newObject, Exception e, object userState)
    {
      // This method is execute
      base.OnSaved(newObject, e, userState);
      // Save ChildEntityWithoutFactory and ChildEntityWithFactory
      FieldManager.UpdateChildren();
    }
  }


  [Serializable]
  [Csla.Server.ObjectFactory("Csla.Test.BasicModern.ChildEntityFactory, csla.netcore.test, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null")]
  public class ChildEntityWithFactory : BusinessBase<ChildEntityWithFactory>
  {
    public static readonly PropertyInfo<string> NameProperty = RegisterProperty<string>(nameof(Name));
    public string Name
    {
      get { return GetProperty(NameProperty); }
      set { SetProperty(NameProperty, value); }
    }

    public static ChildEntityWithFactory NewEntity(string name)
    {
      var entity = Csla.DataPortal.Create<ChildEntityWithFactory>();
      entity.Name = name;
      return entity;
    }

    private void Child_Insert()
    {
      // This method is never executed because this entity use factory?
    }
  }


  [Serializable]
  public class ChildEntityWithoutFactory : BusinessBase<ChildEntityWithoutFactory>
  {
    public static readonly PropertyInfo<string> NameProperty = RegisterProperty<string>(nameof(Name));
    public string Name
    {
      get { return GetProperty(NameProperty); }
      set { SetProperty(NameProperty, value); }
    }

    public static ChildEntityWithoutFactory NewEntity(string name)
    {
      var entity = Csla.DataPortal.Create<ChildEntityWithoutFactory>();
      entity.Name = name;
      return entity;
    }

    [RunLocal]
    protected override void DataPortal_Create()
    {
      // This method is executed
      base.DataPortal_Create();
    }
    
    private void Child_Insert()
    {
      // This method is executed
    }
  }

  public class RootEntityFactory : ObjectFactory
  {
    public RootEntityWithFactory Create()
    {
      // Method is executed
      return new RootEntityWithFactory();
    }

    public RootEntityWithFactory Update(RootEntityWithFactory entity)
    {
      // This method is executed
      return entity;
    }
  }

  public class ChildEntityFactory : ObjectFactory
  {
    public ChildEntityWithFactory Create()
    {
      // This method is executed
      return new ChildEntityWithFactory();
    }

    public ChildEntityWithFactory Update(ChildEntityWithFactory entity)
    {
      // ERROR! This method is never executed
      return entity;
    }
  }
}

Stack Trace or Exception Detail

BusinessErrorInfo' threw an exception of type 'System.NullReferenceException'	
BusinessException	{"Csla.Test.BasicModern.ChildEntityWithFactory.[InsertChild]()"}	System.Exception {System.Reflection.TargetParameterCountException}
   at Csla.Reflection.ServiceProviderMethodCaller.FindDataPortalMethod[T](Type targetType, Object[] criteria, Boolean throwOnError) in C:\csla\csla-5.4.1\csla-5.x\Source\Csla.Shared\Reflection\ServiceProviderMethodCaller.cs:line 249
   at Csla.Reflection.ServiceProviderMethodCaller.FindDataPortalMethod[T](Object target, Object[] criteria) in C:\csla\csla-5.4.1\csla-5.x\Source\Csla.Shared\Reflection\ServiceProviderMethodCaller.cs:line 44
   at Csla.Reflection.LateBoundObject.CallMethodTryAsyncDI[T](Boolean isSync, Object[] parameters) in C:\csla\csla-5.4.1\csla-5.x\Source\Csla.Shared\Reflection\LateBoundObject.cs:line 171
   at Csla.Server.DataPortalTarget.UpdateChildAsync(Object[] parameters) in C:\csla\csla-5.4.1\csla-5.x\Source\Csla.Shared\Server\DataPortalTarget.cs:line 261
   at Csla.Server.ChildDataPortal.Update(Object obj, Boolean hasParameters, Boolean bypassIsDirtyTest, Object[] parameters) in C:\csla\csla-5.4.1\csla-5.x\Source\Csla.Shared\Server\ChildDataPortal.cs:line 326
   at Csla.BusinessBase`1.Save() in C:\csla\csla-5.4.1\csla-5.x\Source\Csla.Shared\BusinessBase.cs:line 132
   at Csla.Test.BasicModern.TestInsertChild.Test_Save_Child_Entity() in C:\csla\csla-5.4.1\csla-5.x\Source\csla.netcore.test\BasicModern\TestInsertChild.cs:line 33

Additional context

[rockfordlhotka]

I'll be honest, I didn't think child objects could or would save through factory types, and I'm surprised that this ever worked. The concept behind factories is that the factory manages the root and all child types. If this worked in 4.5 it is purely by accident 😄

I notice that your factory code doesn't do anything to manage metastate (marknew, markold, etc.), which is fine for a test, but is a problem in production of course.

I'll need to trace through the UpdateChildren code flow to see if it updates metastate for you, because that'd be weird - mixing models where the factory model expects no automatic metastate management.

Also I see that you are trigger the child save operation in OnSaved, so after the root has been completely saved. That's really odd, and will prevent the use of any transactions, and I suspect that's a bug in your code. Certainly it is outside the design parameters of the data portal.

[yvansigouin]

Thanks Rockford for your response.

To clarify with CSLA 4.5.x. The FieldManager.UpdateChildren () method of CSLA 4.5 executed Child_XYZ even if the child entity had the Csla.Server.ObjectFactory [...] attribute. CSLA 5.4 throws an exception when the child entity is linked to a factory (I have many entities that are used as both root and child entity).

(In my opinion, if the child entity has the attribute of using a factory, why not use it?)

Thank you
Yvan

[rockfordlhotka]

It is not a bad idea @yvansigouin - it is just outside the original design parameters of the factory model for the data portal.

I wouldn't have it work using OnSaved though, because that's absolutely not a supported scenario, as it isn't the intent of that event. That event is raised in the logical client-side, and so the result with a remote data portal would be a lot of extra round-trips over the network.

I looked at the code, and it is not a trivial thing to enable the use of a factory for a child object the correct way, because it was never part of the original design. It is something I might consider for CSLA 6, because part of the roadmap already includes some fairly big updates to the factory model.

[rockfordlhotka]

So @yvansigouin let's discuss how to get you moving forward.

Having child factory types is not the problem. The problem is that the data portal only automatically invokes a factory for the root type, not for child types. The philosophy being that the data portal hands the responsibility of persisting the entire object graph (and of managing all metastate) off to you.

Someone might implement this by having all root and child persistence behaviors in a single factory class. For a complex object graph, this would result in a lot of code in a single class.

I think it is better to do something more like what you've done, where you have multiple factory types corresponding to the different domain types. However, the factories need to call each other: you can't rely on the data portal to invoke the child factories. That needs to be done by the parent factory.

So I think you should be able to use your existing factory types, it is just that you need to change it so your child factory types are called from the parent factory, not from OnSaved.