Wrong proxy built for base class with interfaced sub-classes

[fredericDelaporte]

When a proxy for a base class is built (without mapping the proxy as an interface), a wrong proxy is built if the base class has sub-classes mapping their proxy as an interface.

The built proxy for the base class does not inherits from the base class. It should.

This causes a type cast exception when attempting a Load<BaseClass>(id).

(This is extracted out of #2052, which suspect this bug as being the cause of the trouble in #2052 (interface access causing a proxy load since v5.1). But the built proxy for the base class was also wrong in 4.x/5.0.x, it was also not inheriting from the base class. So I consider we need to handle separately the proxy building issue from the proxy loading one.)

[hazzik]

I'm struggling to decipher the bug description :-( Can you add a model examples to the description? Ok, I've figured this out.

[hazzik]

The subclasses are not necessary to reproduce the issue. It could be just simplified to a class with an interface as a proxy.

[hazzik]

Actually, this is just a bug in the documentation which has been introduced when we blindly replaced Load method with a strongly typed version.

[hazzik]

But the built proxy for the base class was also wrong in 4.x/5.0.x, it was also not inheriting from the base class.

As far as I can tell this was always implemented like this (or at least since 3.2)

[hazzik]

I've made a pull request. But I think it can be wrong. And instead of "fixing" the code it would be better to fix the documentation.

The reasons why there is a thing as "proxy interface" is to provide an ability to use proxies for two types of classes:

• sealed classes with interface
• classes with non-virtual properties.

While the fix in the PR will work for sealed classes (as it would not allow to use generic version of the Load method with the sealed class and throw InvalidCastException), it will lead for incorrect behavior for classes with non-virtual properties:

consider following:

interface Cat {
   int Id { get; set; }
   string Name { get; set; }
}

/// <class name="Cat" proxy="ICat" >
class Cat {
   public int Id { get; set; } // Intentionally non virtual !!!
   public string Name { get; set; } // Intentionally non virtual !!!
}

// Correct
ICat cat = session.Load<ICat>(id);

// Was throwing exception but now will load without exception but will lead to incorrect behavior. 
// Specifically, all properties of the loaded object would always be null unless cast to an interface
Cat cat = session.Load<Cat>(id); // Id and Name are null.

// Correct
ICat cat = (ICat)session.Load(typeof(Cat), id);

[bahusoid]

classes with non-virtual properties.

IMHO this is not a valid case. Our proxy validator will throw for such class. Non virtual valid case is explicit interface implementation.

Update Got it. You've meant this should work if Proxy is specified explicitly as interface. But original issue is different - proxy class is not specified for base class

[bahusoid]

The subclasses are not necessary to reproduce the issue. It could be just simplified to a class with an interface as a proxy.

I think it's two different cases. If user explicitly specifies that he wants interface as a proxy - it's OK that load throws on cast to object. As we've done what was requested - proxy is based on interface and not class.

But in #2052 interface is not specified as proxy - so proxy should be based on class. See this case here

[hazzik]

IMHO this is not a valid case. Our proxy validator will throw for such class. Non virtual valid case is explicit interface implementation.

Perfectly valid case. Validator does not check classes themself if they have proxy="..." set in the mapping.

I think it's two different cases.

Yes, they are semantically different. I realized the difference later, but they are really subtle.

[bahusoid]

Perfectly valid case.
I realized the difference later, but they are really subtle.

Yeah I updated my comments - I realized you were talking specifically about explicit mapping proxy as interface. But described problem is different - proxy was not specified for base class mapping. See this case here

[hazzik]

So basically the fix would be to correctly set IsClassProxy based on the mapping instead of vague logic of number of collected interfaces for the ProxyFactoryFactory.

[bahusoid]

I think in your example here (and proxy is mapped as interface) cast exception still needs to be thrown. But your test case in PR is valid one - as proxy for base class is not specified proxy should be based on Cat class.

But maybe I don't understand something. Could you please clarify expected proxy hierarchy.
For instance consider the following hierarchy in format EntityClass [ProvidedProxyClass]:
Base [Base] -> Derived1 [IDerived1] -> Derived2 [IDerived2]

I expect proxy for Base as:
class BaseProxy : Base <- cast to Base is allowed

for Derived1
class Derived1Proxy: Base, IDerived1 <- cast to Derived1 is not allowed

for Derived2
class Derived2Proxy : Base, IDerived1, IDerived2 <- cast to Derived2 is not allowed