From: <fab...@us...> - 2009-06-01 18:19:17
|
Revision: 4399 http://nhibernate.svn.sourceforge.net/nhibernate/?rev=4399&view=rev Author: fabiomaulo Date: 2009-06-01 18:18:23 +0000 (Mon, 01 Jun 2009) Log Message: ----------- Fix NH-1789 Modified Paths: -------------- trunk/nhibernate/src/NHibernate/Util/ReflectHelper.cs trunk/nhibernate/src/NHibernate.Test/NHibernate.Test.csproj trunk/nhibernate/src/NHibernate.Test/UtilityTest/ReflectHelperFixture.cs Added Paths: ----------- trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/Cat.cs trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/DomainObject.cs trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ICat.cs trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/IDomainObject.cs trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/Mappings.hbm.xml trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ProxyEqualityProblemTest.cs Modified: trunk/nhibernate/src/NHibernate/Util/ReflectHelper.cs =================================================================== --- trunk/nhibernate/src/NHibernate/Util/ReflectHelper.cs 2009-06-01 16:46:16 UTC (rev 4398) +++ trunk/nhibernate/src/NHibernate/Util/ReflectHelper.cs 2009-06-01 18:18:23 UTC (rev 4399) @@ -39,7 +39,9 @@ { try { - MethodInfo method = clazz.GetMethod(methodName, parametersTypes); + MethodInfo method = !clazz.IsInterface + ? clazz.GetMethod(methodName, parametersTypes) + : GetMethodFromInterface(clazz, methodName, parametersTypes); if (method == null) { return false; @@ -59,6 +61,29 @@ } } + private static MethodInfo GetMethodFromInterface(System.Type type, string methodName, System.Type[] parametersTypes) + { + const BindingFlags flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.DeclaredOnly; + if(type == null) + { + return null; + } + MethodInfo method = type.GetMethod(methodName, flags, null, parametersTypes, null); + if (method == null) + { + System.Type[] interfaces = type.GetInterfaces(); + foreach (var @interface in interfaces) + { + method = GetMethodFromInterface(@interface, methodName, parametersTypes); + if(method != null) + { + return method; + } + } + } + return method; + } + /// <summary> /// Determine if the specified <see cref="System.Type"/> overrides the /// implementation of GetHashCode from <see cref="Object"/> @@ -67,7 +92,7 @@ /// <returns><see langword="true" /> if any type in the hierarchy overrides GetHashCode().</returns> public static bool OverridesGetHashCode(System.Type clazz) { - return OverrideMethod(clazz, "Equals", System.Type.EmptyTypes); + return OverrideMethod(clazz, "GetHashCode", System.Type.EmptyTypes); } /// <summary> Added: trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/Cat.cs =================================================================== --- trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/Cat.cs (rev 0) +++ trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/Cat.cs 2009-06-01 18:18:23 UTC (rev 4399) @@ -0,0 +1,34 @@ +namespace NHibernate.Test.NHSpecificTest.NH1789 +{ + public class Cat : DomainObject, ICat + { + public Cat(string name, long id) + { + Name = name; + _id = id; + } + + /// <summary> + /// NHibernate parameterless constructor + /// </summary> + protected Cat() {} + + #region ICat Members + + ///<summary> + /// This is a string which uniquely identifies an instance in the case that the ids + /// are both transient + ///</summary> + public override string BusinessKey + { + get { return Name; } + } + + /// <summary> + /// Name of the cat + /// </summary> + public virtual string Name { get; set; } + + #endregion + } +} \ No newline at end of file Added: trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/DomainObject.cs =================================================================== --- trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/DomainObject.cs (rev 0) +++ trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/DomainObject.cs 2009-06-01 18:18:23 UTC (rev 4399) @@ -0,0 +1,133 @@ +using System; +using NHibernate.Proxy; + +namespace NHibernate.Test.NHSpecificTest.NH1789 +{ + ///<summary> + /// The base for all objects in the model + ///</summary> + [Serializable] + public abstract class DomainObject : IDomainObject + { + protected long _id; + + #region IDomainObject Members + + /// <summary> + /// Database ID. This ID doesn't have any business meaning. + /// </summary> + public virtual long ID + { + get { return _id; } + set { _id = value; } + } + + /// <summary> + /// Transient objects are not associated with an + /// item already in storage. For instance, a + /// Customer is transient if its ID is 0. + /// </summary> + public virtual bool IsTransient() + { + return ID.Equals(0); + } + + ///<summary> + /// This is a string which uniquely identifies an instance in the case that the ids + /// are both transient + ///</summary> + public abstract string BusinessKey { get; } + + /// <summary> + /// Returns the concrete type of the object, not the proxy one. + /// </summary> + /// <returns></returns> + public virtual System.Type GetConcreteType() + { + return NHibernateProxyHelper.GuessClass(this); + } + + #endregion + + ///<summary> + /// The equals method works with the persistence framework too + ///</summary> + ///<param name="obj"></param> + ///<returns></returns> + public override bool Equals(object obj) + { + Console.Out.WriteLine("Calling Equals"); + + var compareTo = obj as IDomainObject; + + if (compareTo == null) + { + return false; + } + + if (compareTo.GetConcreteType() != GetConcreteType()) + { + return false; + } + + if (BothNonDefaultIds(compareTo)) + { + return ID.CompareTo(compareTo.ID).Equals(0); + } + + if ((IsTransient() || compareTo.IsTransient()) && HasSameBusinessSignatureAs(compareTo)) + { + return true; + } + + return false; + } + + private bool HasSameBusinessSignatureAs(IDomainObject compareTo) + { + return BusinessKey.Equals(compareTo.BusinessKey); + } + + private bool BothNonDefaultIds(IDomainObject compareTo) + { + return !ID.Equals(0) && !compareTo.ID.Equals(0); + } + + /// <summary> + /// Must be implemented to compare two objects + /// </summary> + public override int GetHashCode() + { + return ID.GetHashCode(); + } + + /// <summary> + /// Turn a proxy object into a "real" object. If the <paramref name="proxy"/> you give in parameter is not a INHibernateProxy, it will returns the same object without any change. + /// </summary> + /// <typeparam name="T">Type in which the unproxied object should be returned</typeparam> + /// <param name="proxy">Proxy object</param> + /// <returns>Unproxied object</returns> + public static T UnProxy<T>(object proxy) + { + //If the object is not a proxy, just cast it and returns it + if (!(proxy is INHibernateProxy)) + { + return (T) proxy; + } + + //Otherwise, use the NHibernate methods to get the implementation, and cast it + var p = (INHibernateProxy) proxy; + return (T) p.HibernateLazyInitializer.GetImplementation(); + } + + /// <summary> + /// Turn a proxy object into a "real" object. If the <paramref name="proxy"/> you give in parameter is not a INHibernateProxy, it will returns the same object without any change. + /// </summary> + /// <param name="proxy">Proxy object</param> + /// <returns>Unproxied object</returns> + public static object UnProxy(object proxy) + { + return UnProxy<object>(proxy); + } + } +} \ No newline at end of file Added: trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ICat.cs =================================================================== --- trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ICat.cs (rev 0) +++ trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ICat.cs 2009-06-01 18:18:23 UTC (rev 4399) @@ -0,0 +1,13 @@ +namespace NHibernate.Test.NHSpecificTest.NH1789 +{ + /// <summary> + /// An interface + /// </summary> + public interface ICat : IDomainObject + { + /// <summary> + /// Name of the cat + /// </summary> + string Name { get; set; } + } +} \ No newline at end of file Added: trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/IDomainObject.cs =================================================================== --- trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/IDomainObject.cs (rev 0) +++ trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/IDomainObject.cs 2009-06-01 18:18:23 UTC (rev 4399) @@ -0,0 +1,35 @@ +namespace NHibernate.Test.NHSpecificTest.NH1789 +{ + /// <summary> + /// Domain Object + /// </summary> + public interface IDomainObject + { + /// <summary> + /// Database unique ID for this object. This ID shouldn't have any business meaning. + /// </summary> + long ID { get; set; } + + ///<summary> + /// This is a string which uniquely identifies an instance in the case that the ids + /// are both transient + ///</summary> + string BusinessKey { get; } + + /// <summary> + /// Transient objects are not associated with an + /// item already in storage. For instance, a + /// Customer is transient if its ID is 0. + /// </summary> + bool IsTransient(); + + /// <summary> + /// Returns the concrete type of the object, not the proxy one. + /// </summary> + /// <returns></returns> + System.Type GetConcreteType(); + + bool Equals(object that); + int GetHashCode(); + } +} \ No newline at end of file Added: trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/Mappings.hbm.xml =================================================================== --- trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/Mappings.hbm.xml (rev 0) +++ trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/Mappings.hbm.xml 2009-06-01 18:18:23 UTC (rev 4399) @@ -0,0 +1,16 @@ +<?xml version="1.0" encoding="utf-8" ?> +<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test" + namespace="NHibernate.Test.NHSpecificTest.NH1789"> + <class name="ICat"> + <id name="ID"> + <generator class="assigned" /> + </id> + + <discriminator column="Type"></discriminator> + + <property name="Name" /> + + <subclass name="Cat" discriminator-value="cat"></subclass> + + </class> +</hibernate-mapping> \ No newline at end of file Added: trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ProxyEqualityProblemTest.cs =================================================================== --- trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ProxyEqualityProblemTest.cs (rev 0) +++ trunk/nhibernate/src/NHibernate.Test/NHSpecificTest/NH1789/ProxyEqualityProblemTest.cs 2009-06-01 18:18:23 UTC (rev 4399) @@ -0,0 +1,124 @@ +using System.Collections; +using NHibernate.Proxy; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH1789 +{ + [TestFixture] + public class ProxyEqualityProblemTest : BugTestCase + { + protected override void OnSetUp() + { + base.OnSetUp(); + using (ISession session = OpenSession()) + { + ICat cat1 = new Cat("Marcel", 1); + ICat cat2 = new Cat("Maurice", 2); + + session.Save(cat1); + session.Save(cat2); + session.Flush(); + } + } + + protected override void OnTearDown() + { + base.OnTearDown(); + using (ISession session = OpenSession()) + { + string hql = "from System.Object"; + session.Delete(hql); + session.Flush(); + } + } + + /// <summary> + /// This test fails: when compariing a proxy with a non-proxy, I want the proxy to use the Equals() method on DomainObject to check for equality. + /// It doesn't do it, so the equality fails. + /// </summary> + [Test] + public void TestProxyEqualityProblem() + { + using (ISession session = OpenSession()) + { + //We load a proxy version of Maurice + var mauriceProxy = session.Load<ICat>((long) 2); + + Assert.IsTrue(mauriceProxy is INHibernateProxy, "The proxy should be of type INHibernateProxy"); + + //From it's proxy, we get a non-proxied (concrete?) version + var mauriceNonProxy = DomainObject.UnProxy<ICat>(mauriceProxy); + + Assert.IsTrue(!(mauriceNonProxy is INHibernateProxy), "The non-proxy shouldn't be of type INHibernateProxy"); + + //We check if the name and ID matches (as they should be because they are the same entity!) + Assert.AreEqual(mauriceProxy.Name, mauriceNonProxy.Name, "The two objects should have the same name"); + Assert.AreEqual(mauriceProxy.ID, mauriceNonProxy.ID, "The two objects should have the same ID"); + + //Now here's the problem: + //When calling Equals() on the non-proxy, everything works (calling the overriden Equals() method on DomainObject as it should be) + Assert.IsTrue(mauriceNonProxy.Equals(mauriceProxy), "The two instances should be declared equal"); + + //But when calling it on the proxy, it doesn't, and they return a false for the equality, and that's a bug IMHO + bool condition = mauriceProxy.Equals(mauriceNonProxy); + Assert.IsTrue(condition, "The two instances should be declared equal"); + } + } + + /// <summary> + /// Here, instead of querying an ICat, we query a Cat directly: everything works, and DomainObject.Equals() is properly called on the proxy. + /// </summary> + [Test] + public void TestProxyEqualityWhereItDoesWork() + { + using (ISession session = OpenSession()) + { + //We load a proxy version of Maurice + var mauriceProxy = session.Load<Cat>((long) 2); + + Assert.IsTrue(mauriceProxy is INHibernateProxy, "The proxy should be of type INHibernateProxy"); + + //From it's proxy, we get a non-proxied (concrete?) version + var mauriceNonProxy = DomainObject.UnProxy<Cat>(mauriceProxy); + + Assert.IsTrue(!(mauriceNonProxy is INHibernateProxy), "The non-proxy shouldn't be of type INHibernateProxy"); + + //We check if the name and ID matches (as they should be because they are the same entity!) + Assert.AreEqual(mauriceProxy.Name, mauriceNonProxy.Name, "The two objects should have the same name"); + Assert.AreEqual(mauriceProxy.ID, mauriceNonProxy.ID, "The two objects should have the same ID"); + + //Because we queried a concrete class (Cat instead of ICat), here it works both ways: + Assert.IsTrue(mauriceNonProxy.Equals(mauriceProxy), "The two instances should be declared equal"); + Assert.IsTrue(mauriceProxy.Equals(mauriceNonProxy), "The two instances should be declared equal"); + } + } + + /// <summary> + /// That's how I discovered something was wrong: here my object is not found in the collection, even if it's there. + /// </summary> + [Test] + public void TestTheProblemWithCollection() + { + using (ISession session = OpenSession()) + { + //As before, we load a proxy, a non-proxy of the same entity, and checks everything is correct: + var mauriceProxy = session.Load<ICat>((long) 2); + Assert.IsTrue(mauriceProxy is INHibernateProxy, "The proxy should be of type INHibernateProxy"); + var mauriceNonProxy = DomainObject.UnProxy<ICat>(mauriceProxy); + Assert.IsTrue(!(mauriceNonProxy is INHibernateProxy), "The non-proxy shouldn't be of type INHibernateProxy"); + Assert.AreEqual(mauriceProxy.Name, mauriceNonProxy.Name, "The two objects should have the same name"); + Assert.AreEqual(mauriceProxy.ID, mauriceNonProxy.ID, "The two objects should have the same ID"); + + //Ok now we add the proxy version into a collection: + var collection = new ArrayList(); + collection.Add(mauriceProxy); + + //The proxy should be able to find itself: + Assert.IsTrue(collection.Contains(mauriceProxy), "The proxy should be present in the collection"); + + //Now, the non-proxy should also be able to find itself in the collection, using the Equals() on DomainObject... + Assert.IsTrue(collection.Contains(mauriceNonProxy), "The proxy should be present in the collection"); + } + } + } +} \ No newline at end of file Modified: trunk/nhibernate/src/NHibernate.Test/NHibernate.Test.csproj =================================================================== --- trunk/nhibernate/src/NHibernate.Test/NHibernate.Test.csproj 2009-06-01 16:46:16 UTC (rev 4398) +++ trunk/nhibernate/src/NHibernate.Test/NHibernate.Test.csproj 2009-06-01 18:18:23 UTC (rev 4399) @@ -440,6 +440,11 @@ <Compile Include="NHSpecificTest\NH1783\SampleTest.cs" /> <Compile Include="NHSpecificTest\NH1788\Fixture.cs" /> <Compile Include="NHSpecificTest\NH1788\Person.cs" /> + <Compile Include="NHSpecificTest\NH1789\Cat.cs" /> + <Compile Include="NHSpecificTest\NH1789\DomainObject.cs" /> + <Compile Include="NHSpecificTest\NH1789\ICat.cs" /> + <Compile Include="NHSpecificTest\NH1789\IDomainObject.cs" /> + <Compile Include="NHSpecificTest\NH1789\ProxyEqualityProblemTest.cs" /> <Compile Include="NHSpecificTest\NH1792\Fixture.cs" /> <Compile Include="NHSpecificTest\NH1792\Product.cs" /> <Compile Include="NHSpecificTest\NH1794\Fixture.cs" /> @@ -1842,6 +1847,7 @@ <EmbeddedResource Include="Ado\AlmostSimple.hbm.xml" /> <EmbeddedResource Include="CacheTest\EntityWithFilters.xml" /> <Content Include="DynamicEntity\package.html" /> + <EmbeddedResource Include="NHSpecificTest\NH1789\Mappings.hbm.xml" /> <EmbeddedResource Include="NHSpecificTest\NH1801\Mappings.hbm.xml" /> <EmbeddedResource Include="NHSpecificTest\NH1796\Mappings.hbm.xml" /> <EmbeddedResource Include="NHSpecificTest\NH1553\MsSQL\Mappings.hbm.xml" /> Modified: trunk/nhibernate/src/NHibernate.Test/UtilityTest/ReflectHelperFixture.cs =================================================================== --- trunk/nhibernate/src/NHibernate.Test/UtilityTest/ReflectHelperFixture.cs 2009-06-01 16:46:16 UTC (rev 4398) +++ trunk/nhibernate/src/NHibernate.Test/UtilityTest/ReflectHelperFixture.cs 2009-06-01 18:18:23 UTC (rev 4399) @@ -19,6 +19,26 @@ Assert.AreEqual(1, (int) result, "Should have found value of 1"); } + public interface IMyBaseWithEqual + { + bool Equals(object that); + int GetHashCode(); + } + + public interface IMyInheritedWithEqual : IMyBaseWithEqual + { + } + + public interface IEmpty + { + + } + + public interface IComplex: IEmpty, IMyInheritedWithEqual + { + + } + [Test] public void OverridesEquals() { @@ -26,9 +46,35 @@ Assert.IsTrue(ReflectHelper.OverridesEquals(typeof(string)), "String does override equals"); Assert.IsFalse(ReflectHelper.OverridesEquals(typeof(IDisposable)), "IDisposable does not override equals"); Assert.IsTrue(ReflectHelper.OverridesEquals(typeof(BRhf)), "Base class overrides equals"); + Assert.That(!ReflectHelper.OverridesEquals(typeof (object)), "System.Object does not override."); } [Test] + public void InheritedInterfaceOverridesEquals() + { + Assert.That(ReflectHelper.OverridesEquals(typeof(IMyBaseWithEqual)), "does override."); + Assert.That(ReflectHelper.OverridesEquals(typeof(IMyInheritedWithEqual)), "does override."); + Assert.That(ReflectHelper.OverridesEquals(typeof(IComplex)), "does override."); + } + + [Test] + public void OverridesGetHashCode() + { + Assert.IsFalse(ReflectHelper.OverridesGetHashCode(this.GetType()), "ReflectHelperFixture does not override GetHashCode"); + Assert.IsTrue(ReflectHelper.OverridesGetHashCode(typeof(string)), "String does override equals"); + Assert.IsFalse(ReflectHelper.OverridesGetHashCode(typeof(IDisposable)), "IDisposable does not override GetHashCode"); + Assert.IsTrue(ReflectHelper.OverridesGetHashCode(typeof(BRhf)), "Base class overrides GetHashCode"); + Assert.That(!ReflectHelper.OverridesGetHashCode(typeof(object)), "System.Object does not override."); + } + + [Test] + public void InheritedInterfaceOverridesGetHashCode() + { + Assert.That(ReflectHelper.OverridesGetHashCode(typeof(IMyBaseWithEqual)), "does override."); + Assert.That(ReflectHelper.OverridesGetHashCode(typeof(IMyInheritedWithEqual)), "does override."); + Assert.That(ReflectHelper.OverridesGetHashCode(typeof(IComplex)), "does override."); + } + [Test] public void NoTypeFoundReturnsNull() { System.Type noType = ReflectHelper.TypeFromAssembly("noclass", "noassembly", false); This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site. |