Cascade delete-orphan on no-proxy null association fails

[fubar-coder]

It seems that there is a problem with one-to-one relations or my mapping.

Exception:

Exception occurred getter of TestNhBug.FileEntry.Id
NHibernate.PropertyAccessException: Exception occurred getter of TestNhBug.FileEntry.Id ---> System.Reflection.TargetException: Non-static method requires a target.
   at System.Reflection.RuntimeMethodInfo.CheckConsistency(Object target)
   at System.Reflection.RuntimeMethodInfo.InvokeArgumentsCheck(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 System.Reflection.RuntimePropertyInfo.GetValue(Object obj, Object[] index)
   at NHibernate.Properties.BasicPropertyAccessor.BasicGetter.Get(Object target)
   --- End of inner exception stack trace ---
   at NHibernate.Properties.BasicPropertyAccessor.BasicGetter.Get(Object target)
   at NHibernate.Tuple.Entity.AbstractEntityTuplizer.GetIdentifier(Object entity)
   at NHibernate.Persister.Entity.AbstractEntityPersister.IsTransient(Object entity, ISessionImplementor session)
   at NHibernate.Engine.ForeignKeys.IsTransientFast(String entityName, Object entity, ISessionImplementor session)
   at NHibernate.Engine.ForeignKeys.IsTransientSlow(String entityName, Object entity, ISessionImplementor session)
   at NHibernate.Event.Default.DefaultDeleteEventListener.OnDelete(DeleteEvent event, ISet`1 transientEntities)
   at NHibernate.Impl.SessionImpl.FireDelete(DeleteEvent event, ISet`1 transientEntities)
   at NHibernate.Impl.SessionImpl.Delete(String entityName, Object child, Boolean isCascadeDeleteEnabled, ISet`1 transientEntities)
   at NHibernate.Engine.Cascade.CascadeOn(IEntityPersister persister, Object parent, Object anything)
   at NHibernate.Event.Default.AbstractFlushingEventListener.CascadeOnFlush(IEventSource session, IEntityPersister persister, Object key, Object anything)
   at NHibernate.Event.Default.AbstractFlushingEventListener.PrepareEntityFlushes(IEventSource session)
   at NHibernate.Event.Default.AbstractFlushingEventListener.FlushEverythingToExecutions(FlushEvent event)
   at NHibernate.Event.Default.DefaultAutoFlushEventListener.OnAutoFlush(AutoFlushEvent event)
   at NHibernate.Impl.SessionImpl.AutoFlushIfRequired(ISet`1 querySpaces)
   at NHibernate.Impl.SessionImpl.List(IQueryExpression queryExpression, QueryParameters queryParameters, IList results, Object filterConnection)
   at NHibernate.Impl.SessionImpl.List(IQueryExpression queryExpression, QueryParameters queryParameters, IList results)
   at NHibernate.Impl.AbstractSessionImpl.List(IQueryExpression queryExpression, QueryParameters parameters)
   at NHibernate.Impl.AbstractQueryImpl2.List()
   at NHibernate.Linq.DefaultQueryProvider.ExecuteQuery(NhLinqExpression nhLinqExpression, IQuery query, NhLinqExpression nhQuery)
   at NHibernate.Linq.DefaultQueryProvider.Execute(Expression expression)
   at NHibernate.Linq.DefaultQueryProvider.Execute[TResult](Expression expression)
   at System.Linq.Queryable.Single[TSource](IQueryable`1 source, Expression`1 predicate)
   at TestNhBug.Program.GetOrCreateEntry(ISession session, FileEntry parent, String name, Byte[] data) in c:\users\markjunker\source\repos\TestNhBug\TestNhBug\Program.cs:line 99
   at TestNhBug.Program.Main(String[] args) in c:\users\markjunker\source\repos\TestNhBug\TestNhBug\Program.cs:line 36

This are my mappings:

<?xml version="1.0" encoding="utf-8"?>
<hibernate-mapping
        assembly="TestNhBug"
        namespace="TestNhBug"
        xmlns="urn:nhibernate-mapping-2.2">
  <class name="FileEntry" table="filesystementries">
    <id name="Id" column="id" generator="assigned" />
    <property name="ParentId" column="parent_id" />
    <property name="Name" column="name" length="50" not-null="true" />
    <one-to-one name="Data" lazy="no-proxy" cascade="all-delete-orphan" />
  </class>
  <class name="FileData" table="filesystementrydata">
    <id name="Id" column="id">
      <generator class="foreign">
        <param name="property">Entry</param>
      </generator>
    </id>
    <property name="Data" column="data" not-null="true" type="BinaryBlob" lazy="true" />
    <one-to-one name="Entry" lazy="no-proxy" constrained="true" foreign-key="fk_data_entry" />
  </class>
</hibernate-mapping>

This is my test application:

using System;
using System.Linq;
using System.Text;

using NHibernate;

namespace TestNhBug
{
    class Program
    {
        private static readonly Guid RootGuid = Guid.Parse("00000000-0000-0000-0000-000000000001");

        static void Main(string[] args)
        {
            var connectionString = @"Data Source=(LocalDb)\MSSQLLocalDB;Integrated Security=SSPI";
            var cfg = new NHibernate.Cfg.Configuration();
            cfg.SetProperty(NHibernate.Cfg.Environment.Dialect, "NHibernate.Dialect.MsSql2012Dialect");
            cfg.SetProperty(NHibernate.Cfg.Environment.ConnectionString, connectionString);
            cfg.AddAssembly(typeof(FileData).Assembly);

            var updater = new NHibernate.Tool.hbm2ddl.SchemaUpdate(cfg);
            updater.Execute(true, true);

            try
            {
                using (var sf = cfg.BuildSessionFactory())
                {
                    using (var session = sf.OpenSession())
                    {
                        using (var trans = session.BeginTransaction())
                        {
                            var rootEntry = GetOrCreateRootEntry(session);
                            GetOrCreateEntry(session, rootEntry, "text1.txt",
                                Encoding.UTF8.GetBytes("text1"));
                            var subEntry = GetOrCreateEntry(session, rootEntry, "test1");
                            GetOrCreateEntry(session, subEntry, "text1.txt",
                                Encoding.UTF8.GetBytes("text1 below test1"));

                            trans.Commit();
                        }
                    }
                }

                using (var sf = cfg.BuildSessionFactory())
                {
                    using (var session = sf.OpenSession())
                    {
                        using (var trans = session.BeginTransaction())
                        {
                            var entry = GetForPath(session, "test1", "text1.txt");

                            session.Delete(entry);
                            trans.Commit();
                        }
                    }
                }
            }
            catch (Exception ex)
            {
                Console.Error.WriteLine(ex);
            }
        }

        private static FileEntry GetForPath(ISession session, params string[] path)
        {
            var found = session.Load<FileEntry>(RootGuid);
            foreach (var pathPart in path)
            {
                var next = session.Query<FileEntry>()
                    .Single(x => x.ParentId == found.Id && x.Name == pathPart);
                found = next;
            }

            return found;
        }

        private static FileEntry GetOrCreateRootEntry(ISession session)
        {
            var entry = session.Get<FileEntry>(RootGuid);

            if (entry != null)
                return entry;

            entry = new FileEntry()
            {
                Id = RootGuid,
                Name = string.Empty,
            };

            session.Save(entry);

            session.Flush();

            return entry;
        }

        private static FileEntry GetOrCreateEntry(ISession session, FileEntry parent, string name, byte[] data = null)
        {
            var entry = session.Query<FileEntry>()
                .SingleOrDefault(x => x.Name == name && x.ParentId == parent.Id);
            if (entry != null)
                return entry;

            entry = new FileEntry()
            {
                Id = Guid.NewGuid(),
                ParentId = parent.Id,
                Name = name,
            };

            FileData fileData;
            if (data != null)
            {
                fileData = new FileData
                {
                    Entry = entry,
                    Data = data,
                };

                entry.Data = fileData;
            }
            else
            {
                fileData = null;
            }

            session.Save(entry);

            if (fileData != null)
            {
                session.Save(fileData);
            }

            session.Flush();

            return entry;
        }
    }

    public class FileData
    {
        public virtual Guid Id { get; set; }
        public virtual byte[] Data { get; set; }
        public virtual FileEntry Entry { get; set; }
    }

    public class FileEntry
    {
        public virtual Guid Id { get; set; }
        public virtual Guid? ParentId { get; set; }
        public virtual string Name { get; set; }
        public virtual FileData Data { get; set; }
    }
}

[fredericDelaporte]

Does the issue still occurs if removing the lazy="no-proxy" from FileData.Entry mapping?

no-proxy is less useful on the constrained side. The constrained side mandates the other side to exist, so the default lazy mode can handle it by always yielding an un-initialized lazy-entity proxy without having to check the other side exists. (While on the non-constrained side, only no-proxy allows to lazy-load an one-to-one, by proxifying the owning entity into a lazy-property proxy, allowing to handle the case where the other side is missing. The no-proxy avoids proxifying the associated entity (into a lazy-entity proxy, interface INHibernateProxy), at the cost of proxifying the owning entity (into a lazy-property proxy, interface IFieldInterceptorAccessor).)

Having mapped both side with no-proxy causes them to be both instantiated as lazy-property proxies, which maybe triggers the issue.

There is still a possible NHibernate issue too: it should not fail that way even if the mapping is maybe not adequate for your actual needs. (Unless I have missed a trouble in your code.)

[fubar-coder]

I created PR #1720 for this issue, but I'll also test your suggestion tomorrow.

[fubar-coder]

@fredericDelaporte I updated the PR but the problem still exists. Using "proxy" instead of "no-proxy" on both ends fixes this problem which indicates a problem with "no-proxy".

[hazzik]

There is a problem dereferencing not found lazy="no-proxy" field.

[hazzik]

This is either HHH-9663 (hibernate/hibernate-orm@63e6793) or HHH-11965 (hibernate/hibernate-orm@00492a3).

[hazzik]

Also, we would need to port this CS: hibernate/hibernate-orm@2fc6001?w=1

[fredericDelaporte]

I was also looking into it, and for me, the bug is here:

nhibernate-core/src/NHibernate/Cfg/MappingSchema/HbmOneToOne.cs

Lines 83 to 86 in 418d35a

	public bool IsLazyProperty
	{
	get { return Lazy == HbmLaziness.Proxy; }
	}

The property is not a lazy property when the entity is proxified. It is a lazy property when the entity is not proxified while having laziness still enabled thanks to no-proxy.

Switching this to NoProxy fix the test case. I have yet to check if this may cause any regression.

For me the test case trouble lies in the property being not flagged as lazy, causing the following code to be defeated:

nhibernate-core/src/NHibernate/Persister/Entity/AbstractEntityPersister.cs

Lines 2551 to 2570 in ee91396

	else if (allProperties || !laziness[i])
	{
	//decide which ResultSet to get the property value from:
	bool propertyIsDeferred = hasDeferred
	&& rootPersister.IsSubclassPropertyDeferred(propNames[i], propSubclassNames[i]);
	if (propertyIsDeferred && sequentialSelectEmpty)
	{
	values[i] = null;
	}
	else
	{
	var propertyResultSet = propertyIsDeferred ? sequentialResultSet : rs;
	string[] cols = propertyIsDeferred ? propertyColumnAliases[i] : suffixedPropertyColumns[i];
	values[i] = types[i].Hydrate(propertyResultSet, cols, session, obj);
	}
	}
	else
	{
	values[i] = LazyPropertyInitializer.UnfetchedProperty;
	}

This results in the loaded state having a proxy for the non-existent one-to-one end because OneToOneType.Hydrate always assume the other end exists, yielding the key of the owner as the key of the other end.

The Hibernate issues look to me as other one-to-one & cascade troubles.

Update: It also seems likely to me we have another kind of bug with many-to-one, but with the same root cause. I have to add more tests for ascertaining this.

[hazzik]

I have yet to check if this may cause any regression.

It will be extremely hard to notice. Most likely all tests will pass, but the issues will be in the number of SQL queries generated (For example, you'll get unexpected SELECT N+1 and similar weird stuff).

[hazzik]

It also seems likely to me we have another kind of bug with many-to-one, but with the same root cause. I have to add more tests for ascertaining this.

Do you mean that?

nhibernate-core/src/NHibernate/Cfg/MappingSchema/HbmManyToOne.cs

Lines 22 to 25 in fb544b0

	public bool IsLazyProperty
	{
	get { return false; }
	}

[fredericDelaporte]

Yes.

[hazzik]

Yeah, it shall be NoProxy in both cases:

https://github.com/hibernate/hibernate-orm/blob/292a4e4337b336dd7c3a54a90c0582734eff07d4/hibernate-core/src/main/java/org/hibernate/mapping/Property.java#L237-L260

[fredericDelaporte]

But there is an issue with this change in AbstractFieldInterceptor. It seems to be implemented on the assumption that properties flagged as lazy are never an association, which is no more the case with my proposed changed. I am still analyzing this. (Maybe just switching some clauses will do, but I am still checking the code paths for now.)
(The issue is shown by LazyOneToOneTest, which fails in a weird way, because it first fail an assert (because the lazy one-to-one has returned, instead of null, a proxy for an un-existant entity;) then fails formatting the assert message. (The proxy .ToString fails due to the lazy initialization failing itself.)

Most likely all tests will pass, but the issues will be in the number of SQL queries generated (For example, you'll get unexpected SELECT N+1 and similar weird stuff).

I do not see why this should happen more. Lazy properties and no-proxy are anyway always triggering n+1 loads situation, since their lazy fetch can not be batched (lazy fetch batching is implemented only for entity and collection laziness, not for property laziness). Flagging an association as no-proxy (or a property as lazy) is anyway acknowledging that n+1 loads will occurs if looping on a list of parent entities with an access to the association (or the property).