Closed jmvermeulen closed 8 years ago
Hi jvm,
Sorry about that, I think the problem is that even though you set the value to null the framework isnt distinguishing between not found and setting something specifically to null.
I'm on vacation today away from my computer but I'll be back tomorrow and I think I can get this fixed pretty quickly and do a release this weekend.
If you need to get around this before tomorrow you could try instantiating a blank Administration object or an empty List
Hi @ipjohnson
Thanks for your response (even on your holiday)!
I think there are 2 possible issues here.
Personally I think the first issue has the highest prio since the second is more like a edge case. Some other Fixture framework's (like AutoFixture) have a solution for this, maybe you can investigate their solution?
Please let me know if I can do anything to help you.
HI @jmvermeulen
So after looking at AutoFixture it looks like it offers a couple different options on how to handle this problem.
1) OmitOnRecursionBehavior - this option essentially just skips recursive properties, in your case it returns an empty list for the Order property. 2) Customize and Skip the properties manually 3) Customize the fixture and plug an empty list of Order
That said it doesn't look like there is an easy way to automatically wire the objects together, even to the point where the author says he doesn't like it as a design pattern and isn't interested in supporting see issue 242.
So I can see a couple ways of solving this and I think taking an approach that allows the most flexibility will be best.
1) go the way autofixture does and just skip circular references 2) Offer a way to customize the fixture and wire the child object to the parent object manually 3) Support automatically wiring parent child relationship by walking up the object hierarchy
I'm inclined to try and support all three situations as an enumeration that can be provided during initialization and make the third option the default. That way out of the box it will work correctly for most situations.
For your situation I'm assuming you would want one Order object created, one Administration object created and the Order property object will be an ICollection with one Order instance that was created first.
Does that make sense to you?
Below is a unit test that I've put together to describe the AutoWire behavior. I've changed my mind on enabling AutoWire by default but I think the syntax is pretty straight forward. Let me know what you think
var fixture = new Fixture(DefaultFixtureConfiguration.AutoWire);
var order = fixture.Generate<Order>();
Assert.NotNull(order);
Assert.NotNull(order.Administration);
Assert.NotNull(order.Administration.Order);
Assert.Equal(1, order.Administration.Order.Count);
Assert.Same(order, order.Administration.Order.First());
Yes this totally makes sense, I think its a perfect solution for pretty much all use-cases.
What would happen if a Administration was generated fixture.Generate<Administration>();
?
Will it create N orders and handle the references the same way?
Correct. I added this test to cover what you are describing. I'll do a little more testing and do a release either today or tomorrow.
var fixture = new Fixture(DefaultFixtureConfiguration.AutoWire);
var administration = fixture.Generate<Administration>();
Assert.NotNull(administration);
Assert.NotNull(administration.Order);
Assert.True(administration.Order.Count > 0);
foreach(var order in administration.Order)
{
Assert.Same(administration, order.Administration);
}
I just released version 1.5.0 containing this fix.
@ipjohnson thanks alot!
Tested it successfully with AutoWire mode!
The OmitCircularReferences mode has a small issue I think. Indeed the Administration has 0 orders in a list, but the rest of the Administration properties is left in default values. I would expect that the administration name for example was still generated.
Do you agree on this?
Edit:
And maybe another improvement.
The order.administrationId
has a different value then the order.Administration.Id
(with EF it will contain the same value) although I think that quite hard to resolve this without extra metadata specification.
Edit 2: Maybe this can be fixed with some kind of AutoMapper syntax
cfg.CreateMap<Order, Administration>()
.ForMember(dest => dest.Id, opt => opt.MapFrom(src => src.AdministrationId))
Sorry not trying to make it much harder then it is already ;-)
To make my point more clear I created this two (failing) unit tests.
[Fact]
public void Fixture_CircularReferenceHandling_Omit_ButRestOfChildGenerated()
{
var fixture = new Fixture(DefaultFixtureConfiguration.OmitCircularReferences);
var parent = fixture.Generate<Order>();
Assert.NotNull(parent);
Assert.NotNull(parent.Administration);
Assert.Null(parent.Administration.Order);
Assert.NotEqual<Guid>(parent.Administration.Id, default(Guid));
}
[Fact]
public void Fixture_CircularReferenceHandling_AutoWire_IdentityMaintained()
{
var fixture = new Fixture(DefaultFixtureConfiguration.AutoWire);
var parent = fixture.Generate<Order>();
Assert.NotNull(parent);
Assert.NotNull(parent.Administration);
Assert.NotNull(parent.Administration.Order);
Assert.Equal<Guid>(parent.Administration.Id, parent.AdministrationId);
}
I definitely agree Fixture_CircularReferenceHandling_Omit_ButRestOfChildGenerated is incorrect it should have populated the Id on the Administration object
For the second unit test can be solved a couple different ways but I definitely think the framework will need a little more help. Out of the box you could do something like this
var fixture = new Fixture(DefaultFixtureConfiguration.AutoWire);
fixture.Customize<Order>().Apply(o => o.AdministrationId = o.Administration.Id);
var order = fixture.Generate<Order>();
My concern with adding something more specific is that doing a mapping like what automapper does implies that there is an order in property creation (Administration object is created before assigning the AdministrationId property on Order). At this point there isn't any order and adding one makes things less simple.
Do you want me to release the fix for Fixture_CircularReferenceHandling_Omit_ButRestOfChildGenerated and open a different issue to track the discussion for the second topic. I'm not sure if there should be any code change and if what's the best solution.
Personally I will use the AutoWire mode, so for me there is no need to rush a release for the Omit fix. I don't have time today, but will check your suggestions tomorrow.
Thanks! :+1:
Sorry for the delay, too busy at work..
I've tested your fix in v1.5.1 and it's looking good! :+1:
Also the even more extended scenario with OrderLines
(see code below) with a fictive reference back to Administration
is working as expected.
To summarize, I think its time to close this issue. Thanks for your active support and good solution.
public partial class Order
{
public int Id { get; set; }
public System.Guid AdministrationId { get; set; }
public virtual Administration Administration { get; set; }
public virtual ICollection<OrderLine> OrderLines { get; set; }
}
public partial class Administration
{
public System.Guid Id { get; set; }
public virtual ICollection<Order> Order { get; set; }
}
public partial class OrderLine
{
public int Id { get; set; }
public virtual Order Order { get; set; }
public virtual Administration Administration { get; set; }
}
When generating a entity framework object with a mutual reference I receive a runtime exception (see below). Is there a way to disable this behavior?
I tried the set the Administration to null in the constrains but that doen't help.
fixture.Generate<Order>(constraints: new { Administration = (Administration)null })
Result StackTrace:
at SimpleFixture.Impl.CircularReferenceHandler.HandleCircularReference(DataRequest request) at SimpleFixture.Conventions.ComplexConvention.GenerateData(DataRequest request) at SimpleFixture.Impl.ConventionList.TryGetValue(DataRequest dataRequest, Object& value) at SimpleFixture.Fixture.Generate(DataRequest request) at SimpleFixture.Impl.TypePopulator.Populate(Object instance, DataRequest request, ComplexModel model) at SimpleFixture.Conventions.ComplexConvention.GenerateData(DataRequest request) at SimpleFixture.Impl.ConventionList.TryGetValue(DataRequest dataRequest, Object& value) at SimpleFixture.Fixture.Generate(DataRequest request) at SimpleFixture.Conventions.ListConvention.GetList[TValue](DataRequest request) --- End of inner exception stack trace --- at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor) at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments) at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters) at SimpleFixture.Conventions.ListConvention.GenerateData(DataRequest request) at SimpleFixture.Impl.ConventionList.TryGetValue(DataRequest dataRequest, Object& value) at SimpleFixture.Impl.TypedConventions.GenerateData(DataRequest request) at SimpleFixture.Impl.ConventionList.TryGetValue(DataRequest dataRequest, Object& value) at SimpleFixture.Fixture.Generate(DataRequest request) at SimpleFixture.Impl.TypePopulator.Populate(Object instance, DataRequest request, ComplexModel model) at SimpleFixture.Conventions.ComplexConvention.GenerateData(DataRequest request) at SimpleFixture.Impl.ConventionList.TryGetValue(DataRequest dataRequest, Object& value) at SimpleFixture.Fixture.Generate(DataRequest request) at SimpleFixture.Impl.TypePopulator.Populate(Object instance, DataRequest request, ComplexModel model) at SimpleFixture.Conventions.ComplexConvention.GenerateData(DataRequest request) at SimpleFixture.Impl.ConventionList.TryGetValue(DataRequest dataRequest, Object& value) at SimpleFixture.Fixture.Generate(DataRequest request) at SimpleFixture.Conventions.ListConvention.GetList[TValue](DataRequest request) --- End of inner exception stack trace ---
Objects: