Lazy properties static proxy #1709

fredericDelaporte · 2018-05-27T16:51:18Z

This is a follow up to #1451.

The new static proxy factory was delegating the lazy properties proxy implementation to the old dynamic proxies.

With this PR, it does implement its own field interceptor, with far less reflection.

The PR also obsoletes dynamic proxies.

fredericDelaporte · 2018-05-27T16:52:11Z

src/AsyncGenerator.yml

@@ -196,6 +196,8 @@
      name: LinqReadonlyTestsContext
    - conversion: Ignore
      name: MultiThreadRunner
+    - conversion: Ignore
+      name: PeVerifier


It was generating async tests for asynchronously calling the PEVerify. There is no point in testing that. Included in this commit because I have added tests using PeVerifier.

fredericDelaporte · 2018-05-27T16:55:54Z

src/NHibernate.Test/DynamicEntity/DataProxyHandler.cs

@@ -1,8 +1,10 @@
+using System;
 using System.Collections;
 using NHibernate.Proxy.DynamicProxy;

 namespace NHibernate.Test.DynamicEntity


The dynamic entity tests are using NHibernate dynamic proxies as a general purpose proxy provider. But that is not the purpose of NHibernate to provide such a thing. Those tests will have to be rewritten with an independent proxy provider if they are to be kept after dropping dynamic proxies.

fredericDelaporte · 2018-05-27T17:02:17Z

src/NHibernate/Proxy/FieldInterceptorProxyBuilder.cs

+		public static TypeInfo CreateProxyType(System.Type baseType)
+		{
+			// Avoid having a suffix ending with "Proxy", for disambiguation with INHibernateProxy proxies
+			var typeName = $"{baseType.Name}ProxyForFieldInterceptor";


Previous implementation was causing field interceptor proxies to be build with same type and assembly name than INHibernateProxy. Apparently this was not causing any issue, even when lazily loading an entity as a INHibernateProxy loading its target implementation as a field interceptor proxy due to the entity having lazy properties.
Still it seems to me cleaner to avoid the confusion.

fredericDelaporte · 2018-05-27T17:04:34Z

src/NHibernate/Proxy/FieldInterceptorProxyBuilder.cs

+			// be restored by lazy load. The field interceptor proxy cannot do that: it only knows how to load
+			// lazy properties, and not the other ones. (Moreover, its session is not restored on persistence
+			// context deserialization, contrary to NHibernateProxy. This causes deserialized proxies to only be
+			// usable for accessing data already loaded before serialization.)


Those limitations are the same than those of dynamic proxies serialization of a field interceptor proxy.

fredericDelaporte · 2018-05-27T17:07:08Z

src/NHibernate/Proxy/NHibernateProxyBuilder.cs

@@ -24,7 +23,6 @@ internal class NHibernateProxyBuilder
 		private static readonly PropertyInfo LazyInitializerIdentifierProperty = LazyInitializerType.GetProperty(nameof(ILazyInitializer.Identifier));
 		private static readonly MethodInfo LazyInitializerInitializeMethod = LazyInitializerType.GetMethod(nameof(ILazyInitializer.Initialize));
 		private static readonly MethodInfo LazyInitializerGetImplementationMethod = LazyInitializerType.GetMethod(nameof(ILazyInitializer.GetImplementation), System.Type.EmptyTypes);
-		private static readonly IProxyAssemblyBuilder ProxyAssemblyBuilder = new DefaultProxyAssemblyBuilder();


I have not seen any interest in keeping the NHibernate.Proxy.DynamicProxy.IProxyAssemblyBuilder pattern now that it was used only by non-injectable private members. So I have rewritten its features into ProxyBuilderHelper, and obsoleted it.

fredericDelaporte · 2018-05-27T17:10:09Z

src/NHibernate/Proxy/ProxyBuilderHelper.cs

+		internal static void EnableDynamicAssemblySaving(bool enable, string saveAssemblyPath)
+		{
+			_saveAssembly = enable;
+			_saveAssemblyPath = saveAssemblyPath;


DefaultProxyAssemblyBuilder was enabling that with compilation directives, which was not very practical. Moreover, it was not handling the save path, which was further complicating using the feature with potentially more file system access right troubles.

fredericDelaporte · 2018-05-30T11:16:54Z

src/NHibernate/Proxy/StaticProxyFactory.cs

-			var factory = new ProxyFactory();
-			var interceptor = new DefaultDynamicLazyFieldInterceptor();
-			return factory.CreateProxy(PersistentClass, interceptor, typeof(IFieldInterceptorAccessor));
+			var cacheEntry = new ProxyCacheEntry(IsClassProxy ? PersistentClass : typeof(object), System.Type.EmptyTypes);


IsClassProxy ? PersistentClass : typeof(object) has to be reverted to PersistentClass: the field interception proxy does not handle being an interface proxy. (I will add a test, this is lacking.)

(Optionally, it may be attempted here to handle being an interface proxy. But the INHibernateProxy interface will have to be filtered out of Interfaces. And the support would only be partial, as there will be no way of handling interface methods, but only properties. So I do not think it should be done.)

fredericDelaporte · 2018-05-30T12:52:21Z

src/NHibernate/Proxy/FieldInterceptorProxyBuilder.cs

+			if (baseType.IsInterface)
+			{
+				parentType = typeof(object);
+				interfaces.Add(baseType);


This does indeed not make sense for a field interceptor. The baseType can only be an interface for entities mapped as abstract. The field interceptor proxy should never be built for such case, it is built only on entity load from database. And anyway, it can not work in such case, since it uses its base implementation for storing its state.

fredericDelaporte · 2018-05-30T13:45:51Z

I have addressed the two previous concerns, but I now look into changing even more the lazy property proxy implementation, for having it delegating its state like then lazy entity proxy. It would give two benefits: handling serialization the same way, and supporting the proxy mapping option, which currently is ignored by lazy property proxies.

fredericDelaporte · 2018-05-31T12:41:38Z

I now look into changing even more the lazy property proxy implementation, for having it delegating its state like then lazy entity proxy.

This would require too much changes. Delegating its state causes issues to field setters and getters (from IPropertyAccessor), which ends up setting fields of the proxy instead of fields of the implementation. The entity proxy does not have the issue, because it is unwrapped and not directly supplied to accessors. This is handled in the persistence context, which makes a distinction between lazy entity proxies and entities. But there is no such thing with lazy properties proxies, which are handled as entities by the persistence context. Changing that seems to me a bit too much for the scope of this PR.

So I have squashed the fixups, and I consider this PR as ready for reviewing.

fredericDelaporte · 2018-05-31T14:01:10Z

Checking again the dynamic proxy factory case, its serialization was ensuring the proxy type was rebuilt, which my implementation currently does not. Switching this PR to WIP again.

Testing this is a bit inconvenient as it requires to stop the process for unloading dynamic assemblies before deserializing. I intend to add some Explicit test allowing to check this manually.

Update: my implementation is broken (it fails deserializing), but there was no serialization test of lazy property proxies to demonstrate it.
But the dynamic lazy property proxy is also broken, while it does not fail deserializing. Instead, it yields a proxy object with all of its properties (lazy or not) having their default values. This happens because the proxy stores its data in its base class and implements ISerializable without attempting to serialize the base class data. This is a regression of b26e622 (fix for NH-3058, NH v3.4), which has ceased delegating the state to a persitentClass instance for storing it into the proxy base class. This is also the cause of #1481.

fredericDelaporte · 2018-06-01T01:13:29Z

Commits redone, for having a correct serialization from the first one. Serialization tests added.

Now this PR should really be ready.

fredericDelaporte · 2018-06-01T13:59:15Z

About serialization, there is one "non-user-bug" case still unsupported: if the user supplies a serialization surrogate handling the entity type, it will be ignored by the proxy serialization. We do not have access to surrogates from the GetObjectData method. The only way to overcome this would be to copy the state of the proxy (excepted uninitialized properties) into a raw entity class instance and then to add it into info. Afterward copy it back to the proxy on deserialization (excepted uninitialized properties again). I do not consider it is worth the trouble.

fredericDelaporte · 2018-06-01T18:00:38Z

Before merging I intend to check #1245 test case provided in its Jira (NH-2873), in case the failure is in the way proxies are built. (But indeed I expect it to be already fixed, by NH-3058.)

(Done: no failure.)

fredericDelaporte added t: Improvement p: Minor c: ByteCode providers labels May 27, 2018

fredericDelaporte added this to the 5.2 milestone May 27, 2018

fredericDelaporte commented May 27, 2018

View reviewed changes

fredericDelaporte mentioned this pull request May 28, 2018

Fix static proxy serialization #1711

Merged

fredericDelaporte commented May 30, 2018

View reviewed changes

fredericDelaporte changed the title ~~Lazy properties static proxy~~ WIP: Lazy properties static proxy May 30, 2018

fredericDelaporte commented May 30, 2018

View reviewed changes

fredericDelaporte force-pushed the LazyPropertiesStaticProxy branch from 26f0af4 to 632d8a7 Compare May 31, 2018 12:22

fredericDelaporte changed the title ~~WIP: Lazy properties static proxy~~ Lazy properties static proxy May 31, 2018

fredericDelaporte force-pushed the LazyPropertiesStaticProxy branch from 632d8a7 to 5695457 Compare May 31, 2018 12:40

fredericDelaporte changed the title ~~Lazy properties static proxy~~ WIP: Lazy properties static proxy May 31, 2018

Implement a static IFieldInterceptorAccessor

fed670d

fredericDelaporte force-pushed the LazyPropertiesStaticProxy branch from 5695457 to 262f316 Compare June 1, 2018 01:03

fredericDelaporte changed the title ~~WIP: Lazy properties static proxy~~ Lazy properties static proxy Jun 1, 2018

Obsolete dynamic proxies

5d77746

fredericDelaporte force-pushed the LazyPropertiesStaticProxy branch from 262f316 to 5d77746 Compare June 1, 2018 01:12

hazzik approved these changes Jun 1, 2018

View reviewed changes

fredericDelaporte mentioned this pull request Jun 1, 2018

NH-2873 - NH3.2.0: public virtual getter & backend field Options #1245

Closed

fredericDelaporte merged commit df7fb3a into nhibernate:master Jun 2, 2018

fredericDelaporte added the r: Fixed label Jun 2, 2018

fredericDelaporte deleted the LazyPropertiesStaticProxy branch June 2, 2018 17:04

fredericDelaporte mentioned this pull request Jun 3, 2018

Entity with initialized lazy property loses the property value after deserialization #1481

Closed

hazzik mentioned this pull request Jul 20, 2018

Remove copy-paste remnants from FieldInterceptorProxyBuilder #1816

Merged

This was referenced Oct 23, 2018

Obsolete a forgotten dynamic proxy class #1883

Merged

GH-1881 - Fix interface method attributes when generating proxies #1882

Closed

fredericDelaporte mentioned this pull request Mar 13, 2019

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

Closed

bahusoid mentioned this pull request Jun 4, 2019

Add test for entity with lazy property nhibernate/nhibernate-envers#15

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Lazy properties static proxy #1709

Lazy properties static proxy #1709

Uh oh!

fredericDelaporte commented May 27, 2018

Uh oh!

fredericDelaporte May 27, 2018 •

edited

Loading

Uh oh!

fredericDelaporte May 27, 2018

Uh oh!

fredericDelaporte May 27, 2018

Uh oh!

fredericDelaporte May 27, 2018

Uh oh!

fredericDelaporte May 27, 2018 •

edited

Loading

Uh oh!

fredericDelaporte May 27, 2018

Uh oh!

fredericDelaporte May 30, 2018

Uh oh!

fredericDelaporte May 30, 2018 •

edited by hazzik

Loading

Uh oh!

fredericDelaporte commented May 30, 2018 •

edited

Loading

Uh oh!

fredericDelaporte commented May 31, 2018 •

edited

Loading

Uh oh!

fredericDelaporte commented May 31, 2018 •

edited

Loading

Uh oh!

fredericDelaporte commented Jun 1, 2018

Uh oh!

fredericDelaporte commented Jun 1, 2018

Uh oh!

fredericDelaporte commented Jun 1, 2018 •

edited

Loading

Uh oh!

Uh oh!

Lazy properties static proxy #1709

Lazy properties static proxy #1709

Uh oh!

Conversation

fredericDelaporte commented May 27, 2018

Uh oh!

fredericDelaporte May 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte May 27, 2018

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte May 27, 2018

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte May 27, 2018

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte May 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte May 27, 2018

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte May 30, 2018

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte May 30, 2018 • edited by hazzik Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte commented May 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredericDelaporte commented May 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredericDelaporte commented May 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredericDelaporte commented Jun 1, 2018

Uh oh!

fredericDelaporte commented Jun 1, 2018

Uh oh!

fredericDelaporte commented Jun 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fredericDelaporte May 27, 2018 •

edited

Loading

fredericDelaporte May 27, 2018 •

edited

Loading

fredericDelaporte May 30, 2018 •

edited by hazzik

Loading

fredericDelaporte commented May 30, 2018 •

edited

Loading

fredericDelaporte commented May 31, 2018 •

edited

Loading

fredericDelaporte commented May 31, 2018 •

edited

Loading

fredericDelaporte commented Jun 1, 2018 •

edited

Loading