-
Notifications
You must be signed in to change notification settings - Fork 934
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
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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:
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... |
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) |
This nhibernate-core/src/NHibernate/Proxy/AbstractProxyFactory.cs Lines 23 to 26 in 8d13f24
Could you please clarify what's it about? Custom proxy |
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 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. |
|
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. It also fail on 5.1.x in the same way. |
Looking into the underlying reasons, this has to be re-evaluated as just a forgotten "possible breaking change". When 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 |
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) |
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: 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:
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. |
No, the trouble is the base class (
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. |
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
The other problem, I mentioned lastly, is then not connected with this issue. I will create another issue for that one. |
@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.. |
@micmerchant, you can still switch back to the previous proxy factory, the time things are sorted out.
It does also generate a wrong proxy, but it does not hydrate the proxy when accessing |
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:
It has many 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... |
Closing this. Fixing #2067 has restored the behavior relied upon by the test case here. |
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
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:
as well the following table with a lookup:
with the following mapping: The base class proxy definition is the class itself. The subclasses proxy definitions point to interfaces.
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:
Test code:
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:
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)?
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)
I've attached a test fixture as well the mappings.
Thanks in advance!
ProxyTypeEvaluation.zip
The text was updated successfully, but these errors were encountered: