Skip to content
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

Possible wrong proxy determination which could lead to a sql exception #2052

Closed
micmerchant opened this issue Mar 12, 2019 · 18 comments
Closed

Comments

@micmerchant
Copy link
Contributor

micmerchant commented Mar 12, 2019

Hello,

this is a follow up to issue #2043.

The nhiberante documentation states in 21.1.3. Single-ended association proxies that the id of proxies can be accessed without initialization

Certain operations do not require proxy initialization

  • Equals(), if the persistent class does not override Equals()
  • GetHashCode(), if the persistent class does not override GetHashCode()
  • The identifier getter method

Let me explain one of our core concepts of our schema. We are heavily using union-subclassing inheritance with abstract base classes. Some of our models have a proxy definition some not. We are also mixing hbm mappings with fluent automapping.

Please don't blame me for our database schema (it's not mine), or how my (former) co-workers were/are misusing it :)

So let's say there are the following tables:

public class EntityClassProxy : IEntity
{
	public virtual Guid Id { get; set; }
	public virtual string Name { get; set; }
}

class SubEntityInterfaceProxy : EntityClassProxy, ISubEntityProxy
{
	public virtual string AnotherName { get; set; }
}

class AnotherSubEntityInterfaceProxy : EntityClassProxy, IAnotherSubEntityProxy
{
	public virtual string AnotherName { get; set; }
}

as well the following table with a lookup:

public class EntityWithSuperClassInterfaceLookup
{
	public virtual Guid Id { get; set; }
	public virtual string Name { get; set; }
	public virtual IEntity EntityLookup { get; set; }
}

with the following mapping: The base class proxy definition is the class itself. The subclasses proxy definitions point to interfaces.

protected override HbmMapping GetMappings()
{
	var mapper = new ModelMapper();

	mapper.Class<EntityClassProxy>(rc =>
	{
		// calling the id getter of the proxy triggers a database query
		rc.Proxy(typeof(EntityClassProxy));

		// calling the id getter of the proxy doesn't trigger a database query
		//rc.Proxy(typeof(IEntity)); 

		rc.Id(x => x.Id);
		rc.Property(x => x.Name);
	});

	mapper.UnionSubclass<SubEntityInterfaceProxy>(rc =>
	{
		rc.Proxy(typeof(ISubEntityProxy));

		rc.Property(x => x.AnotherName);
	});

	mapper.UnionSubclass<AnotherSubEntityInterfaceProxy>(rc =>
	{
		rc.Proxy(typeof(IAnotherSubEntityProxy));

		rc.Property(x => x.AnotherName);
	});

	mapper.Class<EntityWithSuperClassInterfaceLookup>(rc =>
	{
		rc.Id(x => x.Id);
		rc.Property(x => x.Name);
		rc.ManyToOne(x => x.EntityLookup, x  => x.Class(typeof(EntityClassProxy)));
	});

	return mapper.CompileMappingForAllExplicitlyAddedEntities();
}

Due to the union-subclass mapping, there is no foreign key for the entity lookup. So you can basically misuse the lookup and reference a proxy object which actually must not exist (e.g. record in a different table) and this happens in our application as well in some of our unit tests.

Data creation:

var entitySCIL = new EntityWithSuperClassInterfaceLookup
{
	Id = Guid.NewGuid(),
	Name = "Name 1",
	//EntityLookup = (IAbstractEntity)session.Load(typeof(SubEntityInterfaceProxy), subEntityIP.Id)

	// by using a wrong id, the id query throws of course a database exception
	EntityLookup = (IEntity)session.Load(typeof(SubEntityInterfaceProxy), Guid.NewGuid()) 
};
session.Save(entitySCIL);

Test code:

[Test]
public void LoadEntityWithSuperClassLookup()
{
	using (var session = OpenSession())
	{				
		var entity = session.Get<EntityWithSuperClassInterfaceLookup>(_entityWithSuperClassLookupId);
		Guid id = entity.EntityLookup.Id;
	}
}

Case 1: Base class uses the class itself as proxy definition
When you query the object and access the id of the lookup proxy a query is triggerd which leads to the following exception:

Test Name:	LoadEntityWithSuperClassLookup
Test FullName:	NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.Fixture.LoadEntityWithSuperClassLookup
Test Source:	C:\git\NHibernate\src\NHibernate.Test\NHSpecificTest\IlogsProxyTest\ProxyTypeEvaluation\Fixture.cs : line 112
Test Outcome:	Failed
Test Duration:	0:01:41,957

Result StackTrace:	
bei NHibernate.Impl.SessionFactoryImpl.DefaultEntityNotFoundDelegate.HandleEntityNotFound(String entityName, Object id) in C:\git\NHibernate\src\NHibernate\Impl\SessionFactoryImpl.cs:Zeile 86.
   bei NHibernate.Proxy.AbstractLazyInitializer.CheckTargetState() in C:\git\NHibernate\src\NHibernate\Proxy\AbstractLazyInitializer.cs:Zeile 245.
   bei NHibernate.Proxy.AbstractLazyInitializer.Initialize() in C:\git\NHibernate\src\NHibernate\Proxy\AbstractLazyInitializer.cs:Zeile 111.
   bei NHibernate.Proxy.AbstractLazyInitializer.GetImplementation() in C:\git\NHibernate\src\NHibernate\Proxy\AbstractLazyInitializer.cs:Zeile 175.
   bei ISubEntityProxyProxy.get_Id()
   bei NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.Fixture.LoadEntityWithSuperClassLookup() in C:\git\NHibernate\src\NHibernate.Test\NHSpecificTest\IlogsProxyTest\ProxyTypeEvaluation\Fixture.cs:Zeile 117.
Result Message:	NHibernate.ObjectNotFoundException : No row with the given identifier exists[NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.EntityClassProxy#0f626ebf-fc05-4800-92c3-bfd7136cde50]

The proxy interfaces of the lookup look suspicious to me. Why is the lookup of type ISubEntityProxyProxy? Shouldn't it derive from EntityClassProxy? And why are all sub interfaces returned (e.g. IAnotherSubEntityProxy)?

entity.EntityLookup.GetType().ToString()
"ISubEntityProxyProxy"
entity.EntityLookup.GetType().GetInterfaces()
{System.Type[5]}
    [0]: {Name = "ISerializable" FullName = "System.Runtime.Serialization.ISerializable"}
    [1]: {Name = "INHibernateProxy" FullName = "NHibernate.Proxy.INHibernateProxy"}
    [2]: {Name = "ISubEntityProxy" FullName = "NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.ISubEntityProxy"}
    [3]: {Name = "IEntity" FullName = "NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.IEntity"}
    [4]: {Name = "IAnotherSubEntityProxy" FullName = "NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.IAnotherSubEntityProxy"}

Case 2: Base class uses an interface as proxy definition
It works as stated in the documentation. You can access the id of the proxy object, without triggering a query.

The proxy looks basically good to me, except the interfaces (similar to above)

entity.EntityLookup.GetType().ToString()
"IEntityProxy"
entity.EntityLookup.GetType().GetInterfaces()
{System.Type[5]}
    [0]: {Name = "ISerializable" FullName = "System.Runtime.Serialization.ISerializable"}
    [1]: {Name = "INHibernateProxy" FullName = "NHibernate.Proxy.INHibernateProxy"}
    [2]: {Name = "IEntity" FullName = "NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.IEntity"}
    [3]: {Name = "ISubEntityProxy" FullName = "NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.ISubEntityProxy"}
    [4]: {Name = "IAnotherSubEntityProxy" FullName = "NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.IAnotherSubEntityProxy"}

I've attached a test fixture as well the mappings.

Thanks in advance!

ProxyTypeEvaluation.zip

@micmerchant micmerchant changed the title Possible wrong proxy determination which could lead to an sql exception Possible wrong proxy determination which could lead to a sql exception Mar 12, 2019
@bahusoid

This comment has been minimized.

@micmerchant

This comment has been minimized.

@micmerchant

This comment has been minimized.

@bahusoid
Copy link
Member

bahusoid commented Mar 12, 2019

The issue reproduction can be simplified to the following:

var entity = (IEntity) session.Load(typeof(EntityClassProxy), Guid.NewGuid());
//Tries to initialize entity and throw:
var id = entity.Id;

It seems @hazzik is the best person to look at it. But this issue is gone if the following line:

_cacheEntry = new ProxyCacheEntry(IsClassProxy ? PersistentClass : typeof(object), Interfaces);

Is changed to:

_cacheEntry = new ProxyCacheEntry( PersistentClass, Interfaces);

I'm pretty sure it's not the right fix but only a place to look at...

@fredericDelaporte
Copy link
Member

There is some old legacy around how proxy factories handle proxies on an interface, especially in the way the information is conveyed down to the proxy builder. I have also involved myself in the latest changes and especially in putting in place the static proxy factory. I will have a look if Alexander does not do it before me.

@micmerchant, you may also configure NHibernate for using the old (and obsoleted) DefaultProxyFactory (by using NHibernate.Bytecode.DefaultProxyFactoryFactory. If this one still works for your case, it may pinpoint us a thing we have missed when putting in place the static one.

@bahusoid
Copy link
Member

This IsClassProxy property looks suspicious:

protected bool IsClassProxy
{
get { return Interfaces.Length == 1; }
}

Could you please clarify what's it about? Custom proxy EntityClassProxy implements one interface so is treated as proxy. So if some other bogus interface is added everything works as expected.

@fredericDelaporte
Copy link
Member

fredericDelaporte commented Mar 13, 2019

We are here right in the old legacies... At some places, the fact that the proxy has to derive from a concrete entity class rather than just object is conveyed by having only one explicit interface to implement. (The INhibernateProxy one for entity proxies.) I write "explicit" because the base type inherited interfaces are later added to the list of interfaces to implement.

For lazy property proxies, I do not remember right now how it correctly handles that case, and maybe that is the trouble, but I have not yet actually looked into the issue reported here. There is something about it here.

I have considered changing all this, but I guess I have not done it at that time because it would have been breaking.

Related PR are #1451 and #1709.

@bahusoid
Copy link
Member

bahusoid commented Mar 13, 2019

To me it looks like this check should be replaced with PersistentClass.IsInterface

fredericDelaporte added a commit to fredericDelaporte/nhibernate-core that referenced this issue Mar 17, 2019
@fredericDelaporte
Copy link
Member

fredericDelaporte commented Mar 17, 2019

With:

configuration.SetProperty(
	Environment.ProxyFactoryFactoryClass,
	typeof(DefaultProxyFactoryFactory).FullName);

The provided test case does no more fail. So using the old proxy factory can be a workaround.
(But calling Load<EntityClassProxy> still fails, the proxy being not derived from EntityClassProxy. So the old proxy factory has also some troubles with this case.)

It also fail on 5.1.x in the same way.
On 5.0.x, only the later test (Load<EntityClassProxy>) fails. So the reported case here is a regression of 5.1.x (which has introduced the static proxy factory).

fredericDelaporte added a commit to fredericDelaporte/nhibernate-core that referenced this issue Mar 17, 2019
fredericDelaporte added a commit to fredericDelaporte/nhibernate-core that referenced this issue Mar 17, 2019
@fredericDelaporte
Copy link
Member

fredericDelaporte commented Mar 17, 2019

Looking into the underlying reasons, this has to be re-evaluated as just a forgotten "possible breaking change".
The static proxy factory does implement all interfaces of the base class, in order to guarantee their proper interception. The "default" (old) proxy factory was not doing this, so calls on them could reach the inherited state instead of the proxy delegated one.
Since the static proxy intercepts calls on all interfaces methods or properties, the IEntity.Id call is now intercepted. As it is not the actual id getter (which is EntityClassProxy.Id in that case), it does trigger a state loading. Since the state does not exists, it fails.
For having IEntity.Id considered as the id getter for the proxy, IEntity must be declared as being the proxy base interface. (Proxy(typeof(IEntity)))
The fact that previously it was not triggering a data access when IEntity was not the proxy base interface was a bug. We will not revert its fix.

When IEntity is not declared as being the proxy interface, NHibernate should not consider that one of its property could also be actually the id. After all, it can be explicitly implemented by the concrete class in order to yield something else than another identically named and typed property of the concrete class. That is why all calls to members of an interface not declared as being the proxy interface must load the state, in order to delegate to it its implementation.
When the mapping explicitly declares that an interface is to be considered as the contract for the entity proxy, then NHibernate will search in it which property is the id.

That said, there is another bug blurring the case here, already there with the old proxy factory, which causes the proxy to be quite weird for this case. The generated proxy should implement EntityClassProxy, but it does not. This bug should be fixed, but in 5.3 I think. This other bug boils down to: the proxy factory does not support generating a proxy for a concrete class when the class have sub-classes mapped on interfaced proxies.
When there are sub-classes with their proxy mapped as an interface, this interface is added to the interfaces to be implemented by the generated proxy for the entity. I guess this is done to allow casting the proxy to the subclass when it is later determined that the proxy entity is actually of the subclass.
But due to the loose semantic we currently have for distinguishing proxy of concrete classes from proxy of interfaces, this causes the concrete class to be ignored.
(This other old bug needs its dedicated issue/PR.)

@bahusoid
Copy link
Member

I would say that's not quite obvious expected behavior. I think if possible we should log warning about such mapping - as it can be left unnoticed and lead to strange performance issues (due to unexpected lazy initialization)

@micmerchant
Copy link
Contributor Author

micmerchant commented Mar 18, 2019

So now, I'm lost in the proxy world :).

According to you, moving back to the old DefaultProxyFactory wouldn't work in all cases.

Solution 1:
If I understand you right, you are suggesting to use the base class interface, which defines the id property as proxy contract for subclasses?

mapper.UnionSubclass<SubEntityInterfaceProxy>(rc =>
{
	//rc.Proxy(typeof(ISubEntityProxy));
	rc.Proxy(typeof(IEntity));

	rc.Property(x => x.AnotherName);
});

If that's the case, that would be very weird, at least to me, and you need to revise the proper parts of the documentation.

Solution 2:
Removing all proxy contracts worked at least in our production unit tests. But, and this is the bad thing now, here is a mapping regarding the above models and a unit test which fails with:

Test Name:	LoadEntityWithSubClassLookup
Test FullName:	NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.Fixture.LoadEntityWithSubClassLookup
Test Source:	C:\git\NHibernate\src\NHibernate.Test\NHSpecificTest\IlogsProxyTest\ProxyTypeEvaluation\Fixture.cs : line 144
Test Outcome:	Failed
Test Duration:	0:00:25,197

Result StackTrace:	
bei NHibernate.Proxy.StaticProxyFactory.GetProxy(Object id, ISessionImplementor session) in C:\git\NHibernate\src\NHibernate\Proxy\StaticProxyFactory.cs:Zeile 36.
   bei NHibernate.Tuple.Entity.AbstractEntityTuplizer.CreateProxy(Object id, ISessionImplementor session) in C:\git\NHibernate\src\NHibernate\Tuple\Entity\AbstractEntityTuplizer.cs:Zeile 250.
   bei NHibernate.Persister.Entity.AbstractEntityPersister.CreateProxy(Object id, ISessionImplementor session) in C:\git\NHibernate\src\NHibernate\Persister\Entity\AbstractEntityPersister.cs:Zeile 4316.
   bei NHibernate.Event.Default.DefaultLoadEventListener.CreateProxyIfNecessary(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options, IPersistenceContext persistenceContext) in C:\git\NHibernate\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:Zeile 233.
   bei NHibernate.Event.Default.DefaultLoadEventListener.ProxyOrLoad(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in C:\git\NHibernate\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:Zeile 164.
   bei NHibernate.Event.Default.DefaultLoadEventListener.OnLoad(LoadEvent event, LoadType loadType) in C:\git\NHibernate\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:Zeile 96.
   bei NHibernate.Impl.SessionImpl.FireLoad(LoadEvent event, LoadType loadType) in C:\git\NHibernate\src\NHibernate\Impl\SessionImpl.cs:Zeile 2155.
   bei NHibernate.Impl.SessionImpl.InternalLoad(String entityName, Object id, Boolean eager, Boolean isNullable) in C:\git\NHibernate\src\NHibernate\Impl\SessionImpl.cs:Zeile 1281.
   bei NHibernate.Type.EntityType.ResolveIdentifier(Object id, ISessionImplementor session) in C:\git\NHibernate\src\NHibernate\Type\EntityType.cs:Zeile 365.
   bei NHibernate.Type.EntityType.ResolveIdentifier(Object value, ISessionImplementor session, Object owner) in C:\git\NHibernate\src\NHibernate\Type\EntityType.cs:Zeile 398.
   bei NHibernate.Engine.TwoPhaseLoad.InitializeEntity(Object entity, Boolean readOnly, ISessionImplementor session, PreLoadEvent preLoadEvent, PostLoadEvent postLoadEvent, Action`2 cacheBatchingHandler) in C:\git\NHibernate\src\NHibernate\Engine\TwoPhaseLoad.cs:Zeile 122.
   bei NHibernate.Loader.Loader.InitializeEntitiesAndCollections(IList hydratedObjects, DbDataReader reader, ISessionImplementor session, Boolean readOnly, CacheBatcher cacheBatcher) in C:\git\NHibernate\src\NHibernate\Loader\Loader.cs:Zeile 679.
   bei NHibernate.Loader.Loader.DoQuery(ISessionImplementor session, QueryParameters queryParameters, Boolean returnProxies, IResultTransformer forcedResultTransformer, QueryCacheResultBuilder queryCacheResultBuilder) in C:\git\NHibernate\src\NHibernate\Loader\Loader.cs:Zeile 547.
   bei NHibernate.Loader.Loader.DoQueryAndInitializeNonLazyCollections(ISessionImplementor session, QueryParameters queryParameters, Boolean returnProxies, IResultTransformer forcedResultTransformer, QueryCacheResultBuilder queryCacheResultBuilder) in C:\git\NHibernate\src\NHibernate\Loader\Loader.cs:Zeile 285.
   bei NHibernate.Loader.Loader.DoQueryAndInitializeNonLazyCollections(ISessionImplementor session, QueryParameters queryParameters, Boolean returnProxies) in C:\git\NHibernate\src\NHibernate\Loader\Loader.cs:Zeile 263.
   bei NHibernate.Loader.Loader.LoadEntity(ISessionImplementor session, Object id, IType identifierType, Object optionalObject, String optionalEntityName, Object optionalIdentifier, IEntityPersister persister) in C:\git\NHibernate\src\NHibernate\Loader\Loader.cs:Zeile 1606.
   bei NHibernate.Loader.Entity.AbstractEntityLoader.Load(ISessionImplementor session, Object id, Object optionalObject, Object optionalId) in C:\git\NHibernate\src\NHibernate\Loader\Entity\AbstractEntityLoader.cs:Zeile 44.
   bei NHibernate.Loader.Entity.AbstractEntityLoader.Load(Object id, Object optionalObject, ISessionImplementor session) in C:\git\NHibernate\src\NHibernate\Loader\Entity\AbstractEntityLoader.cs:Zeile 39.
   bei NHibernate.Persister.Entity.AbstractEntityPersister.Load(Object id, Object optionalObject, LockMode lockMode, ISessionImplementor session) in C:\git\NHibernate\src\NHibernate\Persister\Entity\AbstractEntityPersister.cs:Zeile 4012.
   bei NHibernate.Event.Default.DefaultLoadEventListener.LoadFromDatasource(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in C:\git\NHibernate\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:Zeile 354.
   bei NHibernate.Event.Default.DefaultLoadEventListener.DoLoad(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in C:\git\NHibernate\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:Zeile 332.
   bei NHibernate.Event.Default.DefaultLoadEventListener.Load(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in C:\git\NHibernate\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:Zeile 113.
   bei NHibernate.Event.Default.DefaultLoadEventListener.ProxyOrLoad(LoadEvent event, IEntityPersister persister, EntityKey keyToLoad, LoadType options) in C:\git\NHibernate\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:Zeile 169.
   bei NHibernate.Event.Default.DefaultLoadEventListener.OnLoad(LoadEvent event, LoadType loadType) in C:\git\NHibernate\src\NHibernate\Event\Default\DefaultLoadEventListener.cs:Zeile 96.
   bei NHibernate.Impl.SessionImpl.FireLoad(LoadEvent event, LoadType loadType) in C:\git\NHibernate\src\NHibernate\Impl\SessionImpl.cs:Zeile 2155.
   bei NHibernate.Impl.SessionImpl.Get(String entityName, Object id) in C:\git\NHibernate\src\NHibernate\Impl\SessionImpl.cs:Zeile 1235.
   bei NHibernate.Impl.SessionImpl.Get(Type entityClass, Object id) in C:\git\NHibernate\src\NHibernate\Impl\SessionImpl.cs:Zeile 1175.
   bei NHibernate.Impl.SessionImpl.Get[T](Object id) in C:\git\NHibernate\src\NHibernate\Impl\SessionImpl.cs:Zeile 1161.
   bei NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.Fixture.LoadEntityWithSubClassLookup() in C:\git\NHibernate\src\NHibernate.Test\NHSpecificTest\IlogsProxyTest\ProxyTypeEvaluation\Fixture.cs:Zeile 148.
--TypeLoadException
   bei System.Reflection.Emit.TypeBuilder.TermCreateClass(RuntimeModule module, Int32 tk, ObjectHandleOnStack type)
   bei System.Reflection.Emit.TypeBuilder.CreateTypeNoLock()
   bei System.Reflection.Emit.TypeBuilder.CreateTypeInfo()
   bei NHibernate.Proxy.NHibernateProxyBuilder.CreateProxyType(Type baseType, IReadOnlyCollection`1 baseInterfaces) in C:\git\NHibernate\src\NHibernate\Proxy\NHibernateProxyBuilder.cs:Zeile 107.
   bei NHibernate.Proxy.StaticProxyFactory.CreateProxyActivator(ProxyCacheEntry pke) in C:\git\NHibernate\src\NHibernate\Proxy\StaticProxyFactory.cs:Zeile 63.
   bei NHibernate.Proxy.StaticProxyFactory.<GetProxy>b__5_0(ProxyCacheEntry pke) in C:\git\NHibernate\src\NHibernate\Proxy\StaticProxyFactory.cs:Zeile 28.
   bei System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   bei NHibernate.Proxy.StaticProxyFactory.GetProxy(Object id, ISessionImplementor session) in C:\git\NHibernate\src\NHibernate\Proxy\StaticProxyFactory.cs:Zeile 28.
Result Message:	
NHibernate.HibernateException : Creating a proxy instance failed
  ----> System.TypeLoadException : Zugriff verweigert: "NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.SubEntityInterfaceProxy".
Result StandardOutput:	
10:44:46,933 ERROR StaticProxyFactory:0 - Creating a proxy instance failed
System.TypeLoadException: Zugriff verweigert: "NHibernate.Test.NHSpecificTest.IlogsProxyTest.ProxyTypeEvaluation.SubEntityInterfaceProxy".
   bei System.Reflection.Emit.TypeBuilder.TermCreateClass(RuntimeModule module, Int32 tk, ObjectHandleOnStack type)
   bei System.Reflection.Emit.TypeBuilder.CreateTypeNoLock()
   bei System.Reflection.Emit.TypeBuilder.CreateTypeInfo()
   bei NHibernate.Proxy.NHibernateProxyBuilder.CreateProxyType(Type baseType, IReadOnlyCollection`1 baseInterfaces) in C:\git\NHibernate\src\NHibernate\Proxy\NHibernateProxyBuilder.cs:Zeile 107.
   bei NHibernate.Proxy.StaticProxyFactory.CreateProxyActivator(ProxyCacheEntry pke) in C:\git\NHibernate\src\NHibernate\Proxy\StaticProxyFactory.cs:Zeile 63.
   bei NHibernate.Proxy.StaticProxyFactory.<GetProxy>b__5_0(ProxyCacheEntry pke) in C:\git\NHibernate\src\NHibernate\Proxy\StaticProxyFactory.cs:Zeile 28.
   bei System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   bei NHibernate.Proxy.StaticProxyFactory.GetProxy(Object id, ISessionImplementor session) in C:\git\NHibernate\src\NHibernate\Proxy\StaticProxyFactory.cs:Zeile 28.

Mapping:

mapper.UnionSubclass<SubEntityInterfaceProxy>(rc =>
{
	// ISubEntityProxy as proxy contract seems to be the only way getting it working
	// not setting the proxy doesn't work
	//rc.Proxy(typeof(ISubEntityProxy));

	// IEntity as proxy contract doesn't work
	rc.Proxy(typeof(IEntity));

	rc.Property(x => x.AnotherName);
});

Data generation:

var entitySubCIL = new EntityWithSubClassInterfaceLookup
				{
					Id = Guid.NewGuid(),
					Name = "Name 1",

					// by using a wrong id, the id query throws of course a database exception
					SubEntityLookup = (ISubEntityProxy)session.Load(typeof(SubEntityInterfaceProxy), _subEntityWithInterfaceProxyId) 
				};
_entityWithSubClassLookupId = entitySubCIL.Id;
session.Save(entitySubCIL);

Test:

[Test]
public void LoadEntityWithSubClassLookup()
{
	using (var session = OpenSession())
	{
		// triggers the exception 
		var entity = session.Get<EntityWithSubClassInterfaceLookup>(_entityWithSubClassLookupId);
		Guid id = entity.SubEntityLookup.Id;
		string name = entity.SubEntityLookup.AnotherName;
	}
}

Unless I'm doing something wrong (and I honestly hope, I'm doing something wrong), ISubEntityProxy seems to be the only valid proxy contract for this mapping/test, but that would be inconsistent with your suggestion to use IEntity as proxy.

@fredericDelaporte
Copy link
Member

If I understand you right, you are suggesting to use the base class interface, which defines the id property as proxy contract for subclasses?

No, the trouble is the base class (EntityClassProxy) mapping only. It should be mapped with .Proxy(typeof(IEntity)) if you want to be able to access IEntity.Id without triggering a data load.

Removing all proxy contracts...

I do not understand that other case. The mapping you write below that still declares a proxy contract. Anyway, it looks like another issue, if that is actually one, so better discuss it in a new issue if you think there is some other bug here to investigate.

@micmerchant
Copy link
Contributor Author

Oh, I understand. Well, this is the only thing, I can't change that easily. We are using a Model-Generator which generates most of the hbm Mapping-Files. I'd like to get rid of the hbms/Generator and do everything via fluent automapping, but this is currently not possible.

By

Removing all proxy contracts...
I meant the following.
You suggested to add an interface proxy contract to the base class. I instead did it somehow inverse and removed all interface proxy contracts in sub classes and so on. That worked for our production unit tests.

The other problem, I mentioned lastly, is then not connected with this issue. I will create another issue for that one.

@bahusoid
Copy link
Member

bahusoid commented Mar 18, 2019

When IEntity is not declared as being the proxy interface, NHibernate should not consider that one of its property could also be actually the id. After all, it can be explicitly implemented by the concrete class in order to yield something else than another identically named and typed property of the concrete class. That is why all calls to members of an interface not declared as being the proxy interface must load the state, in order to delegate to it its implementation.

@fredericDelaporte As far as I can tell this is not how it's working for simple entities (without subclass hierarchy). Check my tests that I've added - no exception is thrown (means no proxy initialization) both for implicit/explicit interfaces..

@fredericDelaporte
Copy link
Member

@micmerchant, you can still switch back to the previous proxy factory, the time things are sorted out.

configuration.SetProperty(
	Environment.ProxyFactoryFactoryClass,
	typeof(DefaultProxyFactoryFactory).FullName);

It does also generate a wrong proxy, but it does not hydrate the proxy when accessing IEntity.Id.

@fredericDelaporte
Copy link
Member

@fredericDelaporte As far as I can tell this is not how it's working for simple entities (without subclass hierarchy). Check my tests that I've added - no exception is thrown (means no proxy initialization) both for implicit/explicit interfaces.

Confirmed by re-checking 1c347bd tests rebased on #2067. But that is a bug. The generated proxy have duplicated methods instead of explicitly implementing the interfaces ones. And "by chance", the runtime does not choke at that and takes the first one, which, for the id case, are always the actual identifier methods. Even PeVerify does not report anything. But the generated proxy is:

[Serializable]
[StructLayout(LayoutKind.Auto, CharSet = CharSet.Auto)]
public class EntityClassProxyProxy : EntityClassProxy, ISerializable, INHibernateProxy, ISubEntityProxy, IAnotherSubEntityProxy, IEntity
{
	private ILazyInitializer __lazyInitializer;

	private NHibernateProxyFactoryInfo __proxyInfo;

	public EntityClassProxyProxy(ILazyInitializer _lazyInitializer, NHibernateProxyFactoryInfo _proxyInfo)
	{
		this.__lazyInitializer = _lazyInitializer;
		this.__proxyInfo = _proxyInfo;
	}

	public override Guid get_Id()
	{
		if (this.__lazyInitializer == null)
		{
			return base.get_Id();
		}
		return (Guid)this.__lazyInitializer.get_Identifier();
	}

	public override void set_Id(Guid guid)
	{
		if (this.__lazyInitializer == null)
		{
			base.set_Id(guid);
			return;
		}
		this.__lazyInitializer.Initialize();
		this.__lazyInitializer.set_Identifier(guid);
		((EntityClassProxy)this.__lazyInitializer.GetImplementation()).set_Id(guid);
	}

	public override string get_Name()
	{
		if (this.__lazyInitializer == null)
		{
			return base.get_Name();
		}
		return ((EntityClassProxy)this.__lazyInitializer.GetImplementation()).get_Name();
	}

	public override void set_Name(string name)
	{
		if (this.__lazyInitializer == null)
		{
			base.set_Name(name);
			return;
		}
		((EntityClassProxy)this.__lazyInitializer.GetImplementation()).set_Name(name);
	}

	public override string ToString()
	{
		if (this.__lazyInitializer == null)
		{
			return base.ToString();
		}
		return ((object)this.__lazyInitializer.GetImplementation()).ToString();
	}

	ILazyInitializer INHibernateProxy.get_HibernateLazyInitializer()
	{
		return this.__lazyInitializer;
	}

	public override string get_AnotherName()
	{
		if (this.__lazyInitializer == null)
		{
			return null;
		}
		return ((ISubEntityProxy)this.__lazyInitializer.GetImplementation()).get_AnotherName();
	}

	public override void set_AnotherName(string anotherName)
	{
		if (this.__lazyInitializer == null)
		{
			return;
		}
		((ISubEntityProxy)this.__lazyInitializer.GetImplementation()).set_AnotherName(anotherName);
	}

	public override string get_AnotherName()
	{
		if (this.__lazyInitializer == null)
		{
			return null;
		}
		return ((IAnotherSubEntityProxy)this.__lazyInitializer.GetImplementation()).get_AnotherName();
	}

	public override void set_AnotherName(string anotherName)
	{
		if (this.__lazyInitializer == null)
		{
			return;
		}
		((IAnotherSubEntityProxy)this.__lazyInitializer.GetImplementation()).set_AnotherName(anotherName);
	}

	public override Guid get_Id()
	{
		if (this.__lazyInitializer == null)
		{
			return base.get_Id();
		}
		return ((IEntity)this.__lazyInitializer.GetImplementation()).get_Id();
	}

	public override void set_Id(Guid id)
	{
		if (this.__lazyInitializer == null)
		{
			base.set_Id(id);
			return;
		}
		((IEntity)this.__lazyInitializer.GetImplementation()).set_Id(id);
	}

	public override string get_Name()
	{
		if (this.__lazyInitializer == null)
		{
			return base.get_Name();
		}
		return ((IEntity)this.__lazyInitializer.GetImplementation()).get_Name();
	}

	public override void set_Name(string name)
	{
		if (this.__lazyInitializer == null)
		{
			base.set_Name(name);
			return;
		}
		((IEntity)this.__lazyInitializer.GetImplementation()).set_Name(name);
	}

	public EntityClassProxyProxy(SerializationInfo serializationInfo, StreamingContext streamingContext)
	{
	}

	[SecurityCritical]
	public override void GetObjectData(SerializationInfo serializationInfo, StreamingContext context)
	{
		serializationInfo.SetType(typeof(NHibernateProxyObjectReference));
		new NHibernateProxyObjectReference(this.__proxyInfo, this.__lazyInitializer.get_Identifier(), this.__lazyInitializer.get_IsUninitialized() ? null : this.__lazyInitializer.GetImplementation()).GetObjectData(serializationInfo, context);
	}
}

It has many get_Id, many set_Id, many get_Name, ... All public.

So, with #2067, the "not able to access Id without triggering a load" issue is gone, but just until we fix the way interface methods are currently implemented...

@fredericDelaporte
Copy link
Member

Closing this. Fixing #2067 has restored the behavior relied upon by the test case here.
(But it does still relies on another bug, #2085. The fix of this another bug may cause @micmerchant's use case to no more work again.)

hazzik pushed a commit that referenced this issue May 2, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
See #2052 and #2271
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