Skip to content

Child object temporary added, then, removed, but gets inserted anyway #2028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rverberne opened this issue Feb 28, 2019 · 5 comments
Open

Comments

@rverberne
Copy link

In a simple parent -> child relationship when adding a child to a existing parent, then doing a query, then removing the child, the child gets inserted in the database as a orphan.

It can cause SQL errors later on, other transactions/constraints dont expect the orphans.

Is this expected behaviour or a bug?

using (var transaction = session.BeginTransaction())
{
    Parent parent = session.Get<Parent>("1");

    var child = new Child();
    child.Id = "99";
    child.ChildName = "Child 99";

    parent.Childs.Add(child);

    // Auto flush? Inserts the child record?
    Settings settings = session
        .QueryOver<Settings>()
        .Take(1)
        .SingleOrDefault();

    // Remove child from parent, child is no longer referenced
    parent.Childs.Remove(child);

    // Child record is not deleted here,
    // parent_id column will be NULL.
    session.Update(parent);
    transaction.Commit();
}
@fredericDelaporte
Copy link
Member

The default session flush mode is FlushMode.Auto, which causes flushes when querying for ensuring correct query results (in regard to the session state).
Either map your collection as cascade="all-delete-orphan" for actually deleting orphaned children, or lower the flush mode to Commit.

By the way, you have reached the issue tracker, meant for bug reports or feature requests.

Please post on the nhusers group for support requests (or see Groups).

@rverberne
Copy link
Author

Either map your collection as cascade="all-delete-orphan"

The Childs collection IS mapped with Cascade.All | Cascade.DeleteOrphans.

@maca88
Copy link
Contributor

maca88 commented Feb 28, 2019

I was able to reproduce the issue by adding the following test into Extralazy.ExtraLazyFixture class:

public void SetAddChildWithoutParentAndRemove()
{
  User gavin;
  Document hia;

  using (var s = OpenSession())
  using (var t = s.BeginTransaction())
  {
    gavin = new User("gavin", "secret");
    s.Persist(gavin);
    t.Commit();
  }

  using (var s = OpenSession())
  using (var t = s.BeginTransaction())
  {
    gavin = s.Get<User>("gavin");
    hia = new Document("HiA", $"content{0}", null);
    gavin.Documents.Add(hia);
    s.Flush();
    gavin.Documents.Remove(hia);

    t.Commit();
  }

  using (var s = OpenSession())
  using (var t = s.BeginTransaction())
  {
    hia = s.Get<Document>(hia.Title);
    Assert.That(hia, Is.Null);

    t.Commit();
  }
}

and altering the Document class mapping in order to allow null for the parent. The collection here is a set that is mapped with inverse="true" and lazy="extra". It works fine if the collection is mapped as inverse="false" or if the lazy attribute is set to true.

The issue here is that the existance check uses the element key and also the collection key:
SELECT 1 FROM documents WHERE owner='gavin' AND Title='HiA'
As the collection key in this case is null, the existance check when removing the element will return false and the element won't be removed. A possbile solution would be to set the element owner when the element is added to the collection.

@MrGroovy Can you provide how your Childs collection is mapped (inverse, lazy, set/bag/list)?

@rverberne
Copy link
Author

rverberne commented Feb 28, 2019

Just the Childs mapping:

Set(
    o => o.Childs,
    m =>
    {
        m.Cascade(Cascade.All | Cascade.DeleteOrphans);
        m.Key(mm =>
        {
            mm.Column("parent_id");
        });
    },
    m => m.OneToMany());

Entire program:

Click to open

using NHibernate;
using NHibernate.Dialect;
using NHibernate.Driver;
using NHibernate.Mapping.ByCode;
using NHibernate.Mapping.ByCode.Conformist;
using System.Collections.Generic;

namespace NhOrphans
{
    public class Program
    {
        public static void Main(string[] args)
        {
            var config = new NHibernate.Cfg.Configuration();
            config.SetProperty(NHibernate.Cfg.Environment.ConnectionDriver, typeof(SQLite20Driver).AssemblyQualifiedName);
            config.SetProperty(NHibernate.Cfg.Environment.Dialect, typeof(SQLiteDialect).AssemblyQualifiedName);
            config.SetProperty(NHibernate.Cfg.Environment.ReleaseConnections, "on_close");
            //config.SetProperty(NHibernate.Cfg.Environment.ConnectionString, "data source=:memory:;DateTimeKind=Utc;");
            config.SetProperty(NHibernate.Cfg.Environment.ConnectionString, "data source=Test.db3;DateTimeKind=Utc;");
            config.SetProperty(NHibernate.Cfg.Environment.FormatSql, true.ToString());
            config.SetProperty(NHibernate.Cfg.Environment.ShowSql, false.ToString());

            ModelMapper mapper = new ModelMapper();
            mapper.AddMapping(new ParentMapping());
            mapper.AddMapping(new ChildMapping());
            mapper.AddMapping(new SettingsMapping());
            config.AddMapping(mapper.CompileMappingForAllExplicitlyAddedEntities());
            ISessionFactory sessionFactory = config.BuildSessionFactory();
            ISession session = sessionFactory.OpenSession();
            session.FlushMode = FlushMode.Auto;
            //var schemaExport = new SchemaExport(config);
            //schemaExport.Execute(false, true, false, session.Connection, null);

            using (var transaction = session.BeginTransaction())
            {
                Parent parent = session.Get<Parent>("1");

                var child = new Child();
                child.Id = "99";
                child.ChildName = "Child 99";

                parent.Childs.Add(child);

                // Auto flush? Inserts the child record?
                Settings settings = session
                    .QueryOver<Settings>()
                    .Take(1)
                    .SingleOrDefault();

                // Remove child from parent, child is no longer referenced
                parent.Childs.Remove(child);

                // Child record is not deleted here,
                // parent_id column will be NULL.
                session.Update(parent);
                transaction.Commit();
            }
        }
    }

    public class Parent
    {
        public virtual string Id { get; set; }
        public virtual string ParentName { get; set; }
        public virtual ISet<Child> Childs { get; set; }

        public Parent()
        {
            Childs = new HashSet<Child>();
        }
    }

    public class Child
    {
        public virtual string Id { get; set; }
        public virtual string ChildName { get; set; }
    }

    public class Settings
    {
        public virtual string Id { get; set; }
        public virtual int SomeSetting { get; set; }
    }

    public class ParentMapping : ClassMapping<Parent>
    {
        public ParentMapping()
        {
            Table("parent");
            Id(o => o.Id, m =>
            {
                m.Column("id");
                m.Generator(Generators.Assigned);
            });
            Property(o => o.ParentName, m => m.Column("name"));
            Set(
                o => o.Childs,
                m =>
                {
                    m.Cascade(Cascade.All | Cascade.DeleteOrphans);
                    m.Key(mm =>
                    {
                        mm.Column("parent_id");
                    });
                },
                m => m.OneToMany());
        }
    }

    public class ChildMapping : ClassMapping<Child>
    {
        public ChildMapping()
        {
            Table("child");
            Id(o => o.Id, m =>
            {
                m.Column("id");
                m.Generator(Generators.Assigned);
            });
            Property(o => o.ChildName, m => m.Column("name"));
        }
    }

    public class SettingsMapping : ClassMapping<Settings>
    {
        public SettingsMapping()
        {
            Table("settings");
            Id(o => o.Id, m =>
            {
                m.Column("id");
                m.Generator(Generators.Assigned);
            });
            Property(o => o.SomeSetting, m => m.Column("some_setting"));
        }
    }
}

@maca88
Copy link
Contributor

maca88 commented Feb 28, 2019

I see, your example hits another issue that has to do with the auto flushing which adds the insert of the child into the action queue which is executed later when updating the parent. Here possible solutions are:

  • Remove the insert action from the action queue when the child is removed from the collection
  • Do a flush when auto flush occurs for this scenario

None of those two are easy to implement as far as I looked.

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

No branches or pull requests

3 participants