CheckList fails on unidirectional one-to-many relationships #212

Open
brantb opened this Issue Mar 14, 2013 · 6 comments

Projects

None yet

3 participants

@brantb
brantb commented Mar 14, 2013

Given the following domain model and mappings:

public class Order {
    public virtual Guid Id { get; set; }
    public virtual ICollection<LineItem> LineItems { get; set; }
}
public class LineItem {
    public virtual Guid Id { get; set; }
}
// OrderMap.cs
Id(x => x.Id).GeneratedBy.GuidComb();
HasMany(x => x.LineItems)
  .Not.Inverse()
  .Not.KeyNullable()
  .Not.KeyUpdate()
  .Cascade.AllDeleteOrphan();
// LineItemMap.cs
Id(x => x.Id).GeneratedBy.GuidComb();

The following code will throw a PropertyValueException when CheckList() is called:

var order = new Order() { LineItems = new List<LineItem>() };
order.LineItems.Add(new LineItem());
new PersistenceSpecification<Order>(session)
  /* NHibernate.PropertyValueException: not-null property references 
   * a null or transient value LineItem._Order.LineItemsBackref */
    .CheckList(o => o.LineItems, order.LineItems)
    .VerifyTheMappings();

This happens because CheckList() tries to immediately save the LineItems in the list, which doesn't work because the relationship is unidirectional.

@chester89
Collaborator

ok, I understand the issue - now the question is what do we do about it?

@chester89 chester89 was assigned Mar 24, 2013
@brantb
brantb commented Mar 28, 2013

Unfortunately, I don't have any ideas.

My tests were failing and I spent some time figuring out why. I reported the issue in hopes of saving other people's time doing the same. I don't necessarily expect a fix, although I suppose the documentation could be updated to mention that this isn't a supported scenario.

@chester89
Collaborator

about documentation - I couldn't agree more.

@dhilgarth

What about giving ReferenceList (and ReferenceProperty) an indicator whether or not it is should be saved with / by the parent? Something like this:

public class ReferenceList<T, TListElement> : List<T, TListElement>
{
    private readonly bool _isInverse;

    public ReferenceList(Accessor property, IEnumerable<TListElement> value)
        : this(property, value, true)
    { }

    public ReferenceList(Accessor property, IEnumerable<TListElement> value, bool isInverse)
        : base(property, value)
    {
        _isInverse = isInverse;
    }

    public override void HasRegistered(PersistenceSpecification<T> specification)
    {
        if(!_isInverse)
            return;

        foreach (TListElement item in Expected)
        {
            specification.TransactionalSave(item);
        }
    }
}

In addition to changing the classes, it would be necessary to update the extension methods to actually allow supplying this parameter.

@dhilgarth

A short test for my specific scenario shows that this would work.

@chester89
Collaborator

That's great news.
I'll try to apply your changes to the codebase - if all goes well, that'll
be in the next release.
Thanks.

2013/6/5 Daniel Hilgarth notifications@github.com

A short test for my specific scenario shows that this would work.

Reply to this email directly or view it on GitHubhttps://github.com/jagregory/fluent-nhibernate/issues/212#issuecomment-18962270
.

ó Õ×ÁÖÅÎÉÅÍ,
þÅÒÍ£ÎÎÏ× çÌÅÂ,
ÔÅÌ. (916) 314-9324

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment