From 9491f57aca2264e293889dc960e49ddfb20350f8 Mon Sep 17 00:00:00 2001 From: Amro El-Fakharany Date: Mon, 15 Sep 2014 09:50:32 +0200 Subject: [PATCH 1/3] NH-3693 - test cases and fix for alias to bean failures with Firebird --- .../Criteria/Lambda/LambdaFixtureBase.cs | 4 +- .../AliasToBeanResultTransformerFixture.cs | 131 +++++++++++++++++- src/NHibernate/NHibernate.csproj | 1 + .../Properties/BasicPropertyAccessor.cs | 6 + .../Properties/ChainedPropertyAccessor.cs | 2 + .../Transform/AliasToBeanResultTransformer.cs | 38 +---- .../QueryAliasToObjectPropertySetter.cs | 48 +++++++ 7 files changed, 197 insertions(+), 33 deletions(-) create mode 100644 src/NHibernate/Transform/QueryAliasToObjectPropertySetter.cs diff --git a/src/NHibernate.Test/Criteria/Lambda/LambdaFixtureBase.cs b/src/NHibernate.Test/Criteria/Lambda/LambdaFixtureBase.cs index 616e8ec4534..8394d11fb66 100644 --- a/src/NHibernate.Test/Criteria/Lambda/LambdaFixtureBase.cs +++ b/src/NHibernate.Test/Criteria/Lambda/LambdaFixtureBase.cs @@ -128,7 +128,9 @@ private void AssertObjectsAreEqual(object expected, object actual, string name) if ((expectedType.IsValueType) || (expected is System.Type) - || (expected is string)) + || (expected is string) + || (expected is FieldInfo) + || (expected is PropertyInfo)) { Assert.AreEqual(expected, actual, fieldPath); _fieldPath.Pop(); diff --git a/src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs b/src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs index c3619818413..6b4ae6eab7c 100644 --- a/src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs +++ b/src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs @@ -1,4 +1,3 @@ -using System; using System.Collections; using System.Collections.Generic; using NHibernate.Transform; @@ -41,6 +40,43 @@ public struct TestStruct public string Something { get; set; } } + public class PublicPropertiesSimpleDTO + { + public object Id { get; set; } + public string Name { get; set; } + } + + public class PrivateFieldsSimpleDTO + { + private object id; + private string name; + + public object Id { get { return id; } } + public string Name { get { return name; } } + } + + public class BasePublicPropsSimpleDTO + { + public object Id { get; set; } + } + + public class PublicInheritedPropertiesSimpleDTO : BasePublicPropsSimpleDTO + { + public string Name { get; set; } + } + + public class BasePrivateFieldSimpleDTO + { + private object id; + public object Id { get { return id; } } + } + + public class PrivateInheritedFieldsSimpleDTO : BasePrivateFieldSimpleDTO + { + private string name; + public string Name { get { return name; } } + } + #region Overrides of TestCase protected override IList Mappings @@ -77,6 +113,99 @@ public void WorkWithOutPublicParameterLessCtor() } } + [Test] + public void ToPublicProperties_WithoutAnyProjections() + { + try + { + Setup(); + + using (ISession s = OpenSession()) + { + var transformer = Transformers.AliasToBean(); + IList l = s.CreateSQLQuery("select * from Simple") + .SetResultTransformer(transformer) + .List(); + Assert.That(l.Count, Is.EqualTo(2)); + Assert.That(l, Has.All.Not.Null); + } + } + finally + { + Cleanup(); + } + } + + [Test] + public void ToPrivateFields_WithoutAnyProjections() + { + try + { + Setup(); + + using (ISession s = OpenSession()) + { + var transformer = Transformers.AliasToBean(); + IList l = s.CreateSQLQuery("select * from Simple") + .SetResultTransformer(transformer) + .List(); + Assert.That(l.Count, Is.EqualTo(2)); + Assert.That(l, Has.All.Not.Null); + } + } + finally + { + Cleanup(); + } + } + + [Test] + public void ToInheritedPublicProperties_WithoutProjections() + { + try + { + Setup(); + + using (ISession s = OpenSession()) + { + var transformer = Transformers.AliasToBean(); + IList l = s.CreateSQLQuery("select * from Simple") + .SetResultTransformer(transformer) + .List(); + Assert.That(l.Count, Is.EqualTo(2)); + Assert.That(l, Has.All.Not.Null); + } + } + finally + { + Cleanup(); + } + } + + [Test] + public void ToInheritedPrivateFields_WithoutProjections() + { + try + { + Setup(); + + using (ISession s = OpenSession()) + { + var transformer = Transformers.AliasToBean(); + IList l = s.CreateSQLQuery("select * from Simple") + .SetResultTransformer(transformer) + .List(); + Assert.That(l.Count, Is.EqualTo(2)); + Assert.That(l, Has.All.Not.Null); + Assert.That(l, Has.All.Property("Id").Not.Null); + } + } + finally + { + Cleanup(); + } + } + [Test] public void WorkWithPublicParameterLessCtor() { diff --git a/src/NHibernate/NHibernate.csproj b/src/NHibernate/NHibernate.csproj index 19b538de969..61e9bfc7533 100644 --- a/src/NHibernate/NHibernate.csproj +++ b/src/NHibernate/NHibernate.csproj @@ -677,6 +677,7 @@ + diff --git a/src/NHibernate/Properties/BasicPropertyAccessor.cs b/src/NHibernate/Properties/BasicPropertyAccessor.cs index 77ab011f58b..a750ca27ffe 100644 --- a/src/NHibernate/Properties/BasicPropertyAccessor.cs +++ b/src/NHibernate/Properties/BasicPropertyAccessor.cs @@ -130,6 +130,12 @@ internal static BasicSetter GetSetterOrNull(System.Type type, string propertyNam return null; } + // According to http://msdn.microsoft.com/EN-US/library/kz0a8sxy%28v=VS.110,d=hv.2%29.aspx + // the assumption articulated in the comment following "if(type.IsValueType)" is wrong at least since .NET 2.0! + // This part of the code has beed changed twice to fix the following Issues in order: NH-1728 then NH-1904 + // As it stands now the implementation prevents AliasToBeanTransformer from finding the correct property + // on a class if the dialect returns column names in a different case than expected. + BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly; if (type.IsValueType) diff --git a/src/NHibernate/Properties/ChainedPropertyAccessor.cs b/src/NHibernate/Properties/ChainedPropertyAccessor.cs index 5228ffdbb73..21850d6202a 100644 --- a/src/NHibernate/Properties/ChainedPropertyAccessor.cs +++ b/src/NHibernate/Properties/ChainedPropertyAccessor.cs @@ -2,7 +2,9 @@ namespace NHibernate.Properties { + // To be removed in v6.0 [Serializable] + [Obsolete("This class has no more usages in NHibernate and will be removed in a future version.")] public class ChainedPropertyAccessor : IPropertyAccessor { private readonly IPropertyAccessor[] chain; diff --git a/src/NHibernate/Transform/AliasToBeanResultTransformer.cs b/src/NHibernate/Transform/AliasToBeanResultTransformer.cs index 8190b59147a..c78fad77cd4 100644 --- a/src/NHibernate/Transform/AliasToBeanResultTransformer.cs +++ b/src/NHibernate/Transform/AliasToBeanResultTransformer.cs @@ -1,7 +1,6 @@ using System; using System.Collections; using System.Reflection; -using NHibernate.Properties; namespace NHibernate.Transform { @@ -27,10 +26,9 @@ namespace NHibernate.Transform [Serializable] public class AliasToBeanResultTransformer : AliasedTupleSubsetResultTransformer { + private readonly QueryAliasToObjectPropertySetter _propertySetter; private const BindingFlags flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; private readonly System.Type resultClass; - private ISetter[] setters; - private readonly IPropertyAccessor propertyAccessor; private readonly ConstructorInfo constructor; public AliasToBeanResultTransformer(System.Type resultClass) @@ -48,22 +46,17 @@ public AliasToBeanResultTransformer(System.Type resultClass) if (constructor == null && resultClass.IsClass) { throw new ArgumentException("The target class of a AliasToBeanResultTransformer need a parameter-less constructor", - "resultClass"); + "resultClass"); } - propertyAccessor = - new ChainedPropertyAccessor(new[] - { - PropertyAccessorFactory.GetPropertyAccessor(null), - PropertyAccessorFactory.GetPropertyAccessor("field") - }); + _propertySetter = QueryAliasToObjectPropertySetter.MakeFor(resultClass); } public override bool IsTransformedValueATupleElement(String[] aliases, int tupleLength) { return false; - } + } public override object TransformTuple(object[] tuple, String[] aliases) @@ -76,30 +69,13 @@ public override object TransformTuple(object[] tuple, String[] aliases) try { - if (setters == null) - { - setters = new ISetter[aliases.Length]; - for (int i = 0; i < aliases.Length; i++) - { - string alias = aliases[i]; - if (alias != null) - { - setters[i] = propertyAccessor.GetSetter(resultClass, alias); - } - } - } - - // if resultClass is not a class but a value type, we need to use Activator.CreateInstance result = resultClass.IsClass - ? constructor.Invoke(null) - : Cfg.Environment.BytecodeProvider.ObjectsFactory.CreateInstance(resultClass, true); + ? constructor.Invoke(null) + : Cfg.Environment.BytecodeProvider.ObjectsFactory.CreateInstance(resultClass, true); for (int i = 0; i < aliases.Length; i++) { - if (setters[i] != null) - { - setters[i].Set(result, tuple[i]); - } + _propertySetter.SetProperty(aliases[i], tuple[i], result); } } catch (InstantiationException e) diff --git a/src/NHibernate/Transform/QueryAliasToObjectPropertySetter.cs b/src/NHibernate/Transform/QueryAliasToObjectPropertySetter.cs new file mode 100644 index 00000000000..67561a0fac6 --- /dev/null +++ b/src/NHibernate/Transform/QueryAliasToObjectPropertySetter.cs @@ -0,0 +1,48 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; + +namespace NHibernate.Transform +{ + [Serializable] + public class QueryAliasToObjectPropertySetter + { + private readonly IEnumerable _fields; + private readonly IEnumerable _properties; + + private QueryAliasToObjectPropertySetter(FieldInfo[] fields, PropertyInfo[] properties) + { + _fields = fields; + _properties = properties; + } + + public static QueryAliasToObjectPropertySetter MakeFor(System.Type objType) + { + var bindingFlags = BindingFlags.Instance | + BindingFlags.Public | + BindingFlags.NonPublic | + BindingFlags.IgnoreCase; + var fields = objType.GetFields(bindingFlags); + var properties = objType.GetProperties(bindingFlags); + + return new QueryAliasToObjectPropertySetter(fields, properties); + } + + public void SetProperty(string alias, object value, object resultObj) + { + var property = _properties.SingleOrDefault(prop => string.Equals(prop.Name, alias, StringComparison.OrdinalIgnoreCase)); + var field = _fields.SingleOrDefault(prop => string.Equals(prop.Name, alias, StringComparison.OrdinalIgnoreCase)); + if (field == null && property == null) + throw new PropertyNotFoundException(resultObj.GetType(), alias, "setter"); + + if (field != null) + { + field.SetValue(resultObj, value); + return; + } + if (property != null && property.CanWrite) + property.SetValue(resultObj, value, new object[0]); + } + } +} From cc9ff7d061377ee05478ef3841bbbde416c3b2db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Wed, 12 Apr 2017 12:27:13 +0200 Subject: [PATCH 2/3] NH-1904 - public properties cannot have the same name with different case, test case and fix for struct components --- .../NHSpecificTest/NH1904/Model.cs | 15 ++++++ .../NHSpecificTest/NH1904/StructFixture.cs | 47 +++++++++++++++++++ .../NH1904/StructMappings.hbm.xml | 22 +++++++++ src/NHibernate.Test/NHibernate.Test.csproj | 2 + .../Properties/BasicPropertyAccessor.cs | 14 ------ 5 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 src/NHibernate.Test/NHSpecificTest/NH1904/StructFixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/NH1904/StructMappings.hbm.xml diff --git a/src/NHibernate.Test/NHSpecificTest/NH1904/Model.cs b/src/NHibernate.Test/NHSpecificTest/NH1904/Model.cs index fa86d6d2cac..fd7f3cae73c 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH1904/Model.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH1904/Model.cs @@ -9,4 +9,19 @@ public class Invoice protected virtual DateTime issued { get; set; } } + + public class InvoiceWithAddress : Invoice + { + public virtual Address BillingAddress { get; set; } + } + + public struct Address + { + public string Line { get; set; } + public string line { get; set; } + public string Line2 { get; set; } + public string City { get; set; } + public string ZipCode { get; set; } + public string Country { get; set; } + } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH1904/StructFixture.cs b/src/NHibernate.Test/NHSpecificTest/NH1904/StructFixture.cs new file mode 100644 index 00000000000..b616a8c99ac --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH1904/StructFixture.cs @@ -0,0 +1,47 @@ +using System; +using System.Collections; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH1904 +{ + [TestFixture] + public class StructFixture : BugTestCase + { + protected override IList Mappings => + new string[] + { + "NHSpecificTest." + BugNumber + ".StructMappings.hbm.xml" + }; + + [Test] + public void ExecuteQuery() + { + using (ISession session = OpenSession()) + using (ITransaction transaction = session.BeginTransaction()) + { + var invoice = new InvoiceWithAddress + { + Issued = DateTime.Now, + BillingAddress = new Address { Line = "84 rue du 22 septembre", City = "Courbevoie", ZipCode = "92400", Country = "France" } + }; + session.Save(invoice); + transaction.Commit(); + } + + using (ISession session = OpenSession()) + { + var invoices = session.CreateCriteria().List(); + } + } + + protected override void OnTearDown() + { + base.OnTearDown(); + using (ISession session = OpenSession()) + { + session.CreateQuery("delete from InvoiceWithAddress").ExecuteUpdate(); + session.Flush(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH1904/StructMappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/NH1904/StructMappings.hbm.xml new file mode 100644 index 00000000000..92cd91115e5 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH1904/StructMappings.hbm.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/src/NHibernate.Test/NHibernate.Test.csproj b/src/NHibernate.Test/NHibernate.Test.csproj index f2c73983919..152ab1994a6 100644 --- a/src/NHibernate.Test/NHibernate.Test.csproj +++ b/src/NHibernate.Test/NHibernate.Test.csproj @@ -742,6 +742,7 @@ + @@ -3251,6 +3252,7 @@ + diff --git a/src/NHibernate/Properties/BasicPropertyAccessor.cs b/src/NHibernate/Properties/BasicPropertyAccessor.cs index a750ca27ffe..8086675c81b 100644 --- a/src/NHibernate/Properties/BasicPropertyAccessor.cs +++ b/src/NHibernate/Properties/BasicPropertyAccessor.cs @@ -130,22 +130,8 @@ internal static BasicSetter GetSetterOrNull(System.Type type, string propertyNam return null; } - // According to http://msdn.microsoft.com/EN-US/library/kz0a8sxy%28v=VS.110,d=hv.2%29.aspx - // the assumption articulated in the comment following "if(type.IsValueType)" is wrong at least since .NET 2.0! - // This part of the code has beed changed twice to fix the following Issues in order: NH-1728 then NH-1904 - // As it stands now the implementation prevents AliasToBeanTransformer from finding the correct property - // on a class if the dialect returns column names in a different case than expected. - BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly; - if (type.IsValueType) - { - // the BindingFlags.IgnoreCase is important here because if type is a struct, the GetProperty method does - // not ignore case by default. If type is a class, it _does_ ignore case... we're better off explicitly - // stating that casing should be ignored so we get the same behavior for both structs and classes - bindingFlags = bindingFlags | BindingFlags.IgnoreCase; - } - PropertyInfo property = type.GetProperty(propertyName, bindingFlags); if (property != null && property.CanWrite) From 792186f880252269af5c27f74fae3adb6e10df82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Wed, 12 Apr 2017 16:22:23 +0200 Subject: [PATCH 3/3] NH-3693 - case sensitive resolver with fallback to case insensitive. --- .../AliasToBeanResultTransformerFixture.cs | 289 +++++++++--------- src/NHibernate/NHibernate.csproj | 1 - .../Transform/AliasToBeanResultTransformer.cs | 241 +++++++++++++-- .../QueryAliasToObjectPropertySetter.cs | 48 --- src/NHibernate/Transform/Transformers.cs | 20 +- 5 files changed, 379 insertions(+), 220 deletions(-) delete mode 100644 src/NHibernate/Transform/QueryAliasToObjectPropertySetter.cs diff --git a/src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs b/src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs index 6b4ae6eab7c..c53a2818d13 100644 --- a/src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs +++ b/src/NHibernate.Test/TransformTests/AliasToBeanResultTransformerFixture.cs @@ -1,6 +1,7 @@ using System.Collections; -using System.Collections.Generic; +using System.Reflection; using NHibernate.Transform; +using NHibernate.Util; using NUnit.Framework; namespace NHibernate.Test.TransformTests @@ -11,7 +12,7 @@ public class AliasToBeanResultTransformerFixture : TestCase public class WithOutPublicParameterLessCtor { private string something; - protected WithOutPublicParameterLessCtor() {} + protected WithOutPublicParameterLessCtor() { } public WithOutPublicParameterLessCtor(string something) { @@ -48,8 +49,10 @@ public class PublicPropertiesSimpleDTO public class PrivateFieldsSimpleDTO { +#pragma warning disable CS0649 private object id; private string name; +#pragma warning restore CS0649 public object Id { get { return id; } } public string Name { get { return name; } } @@ -67,21 +70,56 @@ public class PublicInheritedPropertiesSimpleDTO : BasePublicPropsSimpleDTO public class BasePrivateFieldSimpleDTO { +#pragma warning disable CS0649 private object id; +#pragma warning restore CS0649 public object Id { get { return id; } } } public class PrivateInheritedFieldsSimpleDTO : BasePrivateFieldSimpleDTO { +#pragma warning disable CS0649 private string name; +#pragma warning restore CS0649 public string Name { get { return name; } } } + public class PropertiesInsensitivelyDuplicated + { + public object Id { get; set; } + public string NaMe { get; set; } + public string NamE { get; set; } + } + + public class BasePrivateFieldSimpleDTO2 + { +#pragma warning disable CS0649 + private object _id; +#pragma warning restore CS0649 + public object iD => _id; + } + + public class PrivateInheritedFieldsSimpleDTO2 : BasePrivateFieldSimpleDTO2 + { +#pragma warning disable CS0649 + private string namE; +#pragma warning restore CS0649 + public string Name { get { return namE; } } + } + + public class NewPropertiesSimpleDTO : PrivateInheritedFieldsSimpleDTO2 + { + public new string Name { get; set; } + internal string NaMe { get; set; } + + public object Id { get; set; } + } + #region Overrides of TestCase protected override IList Mappings { - get { return new[] {"TransformTests.Simple.hbm.xml"}; } + get { return new[] { "TransformTests.Simple.hbm.xml" }; } } protected override string MappingsAssembly @@ -89,199 +127,170 @@ protected override string MappingsAssembly get { return "NHibernate.Test"; } } - #endregion - - [Test] - public void WorkWithOutPublicParameterLessCtor() + protected override void OnSetUp() { - try + using (var s = OpenSession()) { - Setup(); - - using (ISession s = OpenSession()) + using (s.BeginTransaction()) { - IList l = - s.CreateSQLQuery("select s.Name as something from Simple s").SetResultTransformer( - Transformers.AliasToBean()).List(); - Assert.That(l.Count, Is.EqualTo(2)); - Assert.That(l, Has.All.Not.Null); + s.Save(new Simple { Name = "Name1" }); + s.Save(new Simple { Name = "Name2" }); + s.Transaction.Commit(); } } - finally - { - Cleanup(); - } } - [Test] - public void ToPublicProperties_WithoutAnyProjections() + protected override void OnTearDown() { - try + using (var s = OpenSession()) { - Setup(); - - using (ISession s = OpenSession()) + using (s.BeginTransaction()) { - var transformer = Transformers.AliasToBean(); - IList l = s.CreateSQLQuery("select * from Simple") - .SetResultTransformer(transformer) - .List(); - Assert.That(l.Count, Is.EqualTo(2)); - Assert.That(l, Has.All.Not.Null); + s.Delete("from Simple"); + s.Transaction.Commit(); } } - finally - { - Cleanup(); - } } + #endregion + [Test] - public void ToPrivateFields_WithoutAnyProjections() + public void WorkWithOutPublicParameterLessCtor() { - try - { - Setup(); + AssertCardinalityAndSomething(); + } - using (ISession s = OpenSession()) - { - var transformer = Transformers.AliasToBean(); - IList l = s.CreateSQLQuery("select * from Simple") - .SetResultTransformer(transformer) - .List(); - Assert.That(l.Count, Is.EqualTo(2)); - Assert.That(l, Has.All.Not.Null); - } - } - finally - { - Cleanup(); - } + [Test] + public void ToPublicProperties_WithoutAnyProjections() + { + AssertCardinalityNameAndId(); } [Test] - public void ToInheritedPublicProperties_WithoutProjections() + public void ToPrivateFields_WithoutAnyProjections() { - try - { - Setup(); + AssertCardinalityNameAndId(); + } - using (ISession s = OpenSession()) - { - var transformer = Transformers.AliasToBean(); - IList l = s.CreateSQLQuery("select * from Simple") - .SetResultTransformer(transformer) - .List(); - Assert.That(l.Count, Is.EqualTo(2)); - Assert.That(l, Has.All.Not.Null); - } - } - finally - { - Cleanup(); - } + [Test] + public void ToInheritedPublicProperties_WithoutProjections() + { + AssertCardinalityNameAndId(); } [Test] public void ToInheritedPrivateFields_WithoutProjections() { - try - { - Setup(); - - using (ISession s = OpenSession()) - { - var transformer = Transformers.AliasToBean(); - IList l = s.CreateSQLQuery("select * from Simple") - .SetResultTransformer(transformer) - .List(); - Assert.That(l.Count, Is.EqualTo(2)); - Assert.That(l, Has.All.Not.Null); - Assert.That(l, Has.All.Property("Id").Not.Null); - } - } - finally - { - Cleanup(); - } + AssertCardinalityNameAndId(); } [Test] - public void WorkWithPublicParameterLessCtor() + public void WorkWithPublicParameterLessCtor_Fields() { - try - { - Setup(); - - var queryString = "select s.Name as something from Simple s"; - AssertAreWorking(queryString); // working for field access + AssertCardinalityAndSomething(); + } - queryString = "select s.Name as Something from Simple s"; - AssertAreWorking(queryString); // working for property access - } - finally - { - Cleanup(); - } + [Test] + public void WorkWithPublicParameterLessCtor_Properties() + { + AssertCardinalityAndSomething("select s.Name as Something from Simple s"); } [Test] public void WorksWithStruct() { - try - { - Setup(); + AssertCardinalityAndSomething(); + } - IList result; - using (ISession s = OpenSession()) - { - result = s.CreateSQLQuery("select s.Name as something from Simple s") - .SetResultTransformer(Transformers.AliasToBean()) - .List(); - } - Assert.AreEqual(2, result.Count); - } - finally - { - Cleanup(); - } + [Test] + public void WorksWithNewProperty() + { + AssertCardinalityNameAndId(); } - private void AssertAreWorking(string queryString) + [Test] + public void WorksWithManyCandidates() { - using (ISession s = OpenSession()) + using (var s = OpenSession()) { - IList l = - s.CreateSQLQuery(queryString).SetResultTransformer( - Transformers.AliasToBean()).List(); + var transformer = Transformers.AliasToBean(); + var l = s.CreateSQLQuery("select id as ID, Name as NamE from Simple") + .SetResultTransformer(transformer) + .List(); Assert.That(l.Count, Is.EqualTo(2)); Assert.That(l, Has.All.Not.Null); + Assert.That(l, Has.Some.Property("Name").EqualTo("Name1")); + Assert.That(l, Has.Some.Property("Name").EqualTo("Name2")); + Assert.That(l, Has.All.Property("Id").Not.Null); + Assert.That(l, Has.All.Property("iD").Null); + Assert.That(l, Has.All.Property("NaMe").Null); } } - private void Cleanup() + [Test] + public void ToPropertiesInsensitivelyDuplicated_WithoutAnyProjections() { - using (ISession s = OpenSession()) + using (var s = OpenSession()) { - using (s.BeginTransaction()) + var transformer = Transformers.AliasToBean(); + Assert.Throws(() => { - s.Delete("from Simple"); - s.Transaction.Commit(); - } + s.CreateSQLQuery("select * from Simple") + .SetResultTransformer(transformer) + .List(); + }); } } - private void Setup() + [Test] + public void Serialization() { - using (ISession s = OpenSession()) + AssertSerialization(); + AssertSerialization(); + AssertSerialization(); + AssertSerialization(); + AssertSerialization(); + } + + private void AssertCardinalityNameAndId(IResultTransformer transformer = null) + { + using (var s = OpenSession()) { - using (s.BeginTransaction()) - { - s.Save(new Simple {Name = "Name1"}); - s.Save(new Simple {Name = "Name2"}); - s.Transaction.Commit(); - } + transformer = transformer ?? Transformers.AliasToBean(); + var l = s.CreateSQLQuery("select * from Simple") + .SetResultTransformer(transformer) + .List(); + var testClass = typeof(T).Name; + Assert.That(l.Count, Is.EqualTo(2), testClass); + Assert.That(l, Has.All.Not.Null, testClass); + Assert.That(l, Has.Some.Property("Name").EqualTo("Name1"), testClass); + Assert.That(l, Has.Some.Property("Name").EqualTo("Name2"), testClass); + Assert.That(l, Has.All.Property("Id").Not.Null, testClass); } } + + private void AssertCardinalityAndSomething(string queryString = "select s.Name as something from Simple s") + { + using (var s = OpenSession()) + { + var transformer = Transformers.AliasToBean(); + var l = s.CreateSQLQuery(queryString) + .SetResultTransformer(transformer) + .List(); + var testClass = typeof(T).Name; + Assert.That(l.Count, Is.EqualTo(2), testClass); + Assert.That(l, Has.All.Not.Null, testClass); + Assert.That(l, Has.Some.Property("Something").EqualTo("Name1"), testClass); + Assert.That(l, Has.Some.Property("Something").EqualTo("Name2"), testClass); + } + } + + private void AssertSerialization() + { + var transformer = Transformers.AliasToBean(); + var bytes = SerializationHelper.Serialize(transformer); + transformer = (IResultTransformer)SerializationHelper.Deserialize(bytes); + AssertCardinalityNameAndId(transformer: transformer); + } } } \ No newline at end of file diff --git a/src/NHibernate/NHibernate.csproj b/src/NHibernate/NHibernate.csproj index 61e9bfc7533..19b538de969 100644 --- a/src/NHibernate/NHibernate.csproj +++ b/src/NHibernate/NHibernate.csproj @@ -677,7 +677,6 @@ - diff --git a/src/NHibernate/Transform/AliasToBeanResultTransformer.cs b/src/NHibernate/Transform/AliasToBeanResultTransformer.cs index c78fad77cd4..0caabc5696c 100644 --- a/src/NHibernate/Transform/AliasToBeanResultTransformer.cs +++ b/src/NHibernate/Transform/AliasToBeanResultTransformer.cs @@ -1,13 +1,15 @@ using System; using System.Collections; +using System.Collections.Generic; +using System.Linq; using System.Reflection; namespace NHibernate.Transform { /// - /// Result transformer that allows to transform a result to - /// a user specified class which will be populated via setter - /// methods or fields matching the alias names. + /// Result transformer that allows to transform a result to + /// a user specified class which will be populated via setter + /// methods or fields matching the alias names. /// /// /// @@ -15,50 +17,60 @@ namespace NHibernate.Transform /// .CreateAlias("Student", "st") /// .CreateAlias("Course", "co") /// .SetProjection( Projections.ProjectionList() - /// .Add( Projections.Property("co.Description"), "CourseDescription" ) + /// .Add( Projections.Property("co.Description"), "CourseDescription") /// ) - /// .SetResultTransformer( new AliasToBeanResultTransformer(typeof(StudentDTO)) ) + /// .SetResultTransformer( new AliasToBeanResultTransformer(typeof(StudentDTO))) /// .List(); /// /// StudentDTO dto = (StudentDTO)resultWithAliasedBean[0]; /// /// + /// + /// Resolves setter for an alias with a heuristic: search among properties then fields for matching name and case, then, + /// if no matching property or field was found, retry with a case insensitive match. For members having the same name, it + /// sorts them by inheritance depth then by visibility from public to private, and takes those ranking first. + /// [Serializable] - public class AliasToBeanResultTransformer : AliasedTupleSubsetResultTransformer + public class AliasToBeanResultTransformer : AliasedTupleSubsetResultTransformer, IEquatable { - private readonly QueryAliasToObjectPropertySetter _propertySetter; - private const BindingFlags flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; - private readonly System.Type resultClass; - private readonly ConstructorInfo constructor; + private readonly System.Type _resultClass; + private readonly ConstructorInfo _beanConstructor; + private readonly Dictionary> _fieldsByNameCaseSensitive; + private readonly Dictionary> _fieldsByNameCaseInsensitive; + private readonly Dictionary> _propertiesByNameCaseSensitive; + private readonly Dictionary> _propertiesByNameCaseInsensitive; public AliasToBeanResultTransformer(System.Type resultClass) { - if (resultClass == null) - { - throw new ArgumentNullException("resultClass"); - } - this.resultClass = resultClass; + _resultClass = resultClass ?? throw new ArgumentNullException("resultClass"); - constructor = resultClass.GetConstructor(flags, null, System.Type.EmptyTypes, null); + const BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; + _beanConstructor = resultClass.GetConstructor(bindingFlags, null, System.Type.EmptyTypes, null); // if resultClass is a ValueType (struct), GetConstructor will return null... // in that case, we'll use Activator.CreateInstance instead of the ConstructorInfo to create instances - if (constructor == null && resultClass.IsClass) + if (_beanConstructor == null && resultClass.IsClass) { - throw new ArgumentException("The target class of a AliasToBeanResultTransformer need a parameter-less constructor", - "resultClass"); + throw new ArgumentException( + "The target class of a AliasToBeanResultTransformer need a parameter-less constructor", + nameof(resultClass)); } - _propertySetter = QueryAliasToObjectPropertySetter.MakeFor(resultClass); - } + var fields = new List>(); + var properties = new List>(); + FetchFieldsAndProperties(fields, properties); + _fieldsByNameCaseSensitive = GetMapByName(fields, StringComparer.Ordinal); + _fieldsByNameCaseInsensitive = GetMapByName(fields, StringComparer.OrdinalIgnoreCase); + _propertiesByNameCaseSensitive = GetMapByName(properties, StringComparer.Ordinal); + _propertiesByNameCaseInsensitive = GetMapByName(properties, StringComparer.OrdinalIgnoreCase); + } public override bool IsTransformedValueATupleElement(String[] aliases, int tupleLength) { return false; } - public override object TransformTuple(object[] tuple, String[] aliases) { if (aliases == null) @@ -69,22 +81,22 @@ public override object TransformTuple(object[] tuple, String[] aliases) try { - result = resultClass.IsClass - ? constructor.Invoke(null) - : Cfg.Environment.BytecodeProvider.ObjectsFactory.CreateInstance(resultClass, true); + result = _resultClass.IsClass + ? _beanConstructor.Invoke(null) + : Cfg.Environment.BytecodeProvider.ObjectsFactory.CreateInstance(_resultClass, true); for (int i = 0; i < aliases.Length; i++) { - _propertySetter.SetProperty(aliases[i], tuple[i], result); + SetProperty(aliases[i], tuple[i], result); } } catch (InstantiationException e) { - throw new HibernateException("Could not instantiate result class: " + resultClass.FullName, e); + throw new HibernateException("Could not instantiate result class: " + _resultClass.FullName, e); } catch (MethodAccessException e) { - throw new HibernateException("Could not instantiate result class: " + resultClass.FullName, e); + throw new HibernateException("Could not instantiate result class: " + _resultClass.FullName, e); } return result; @@ -95,6 +107,173 @@ public override IList TransformList(IList collection) return collection; } + #region Setter resolution + + /// + /// Set the value of a property or field matching an alias. + /// + /// The alias for which resolving the property or field. + /// The value to which the property or field should be set. + /// The object on which to set the property or field. It must be of the type for which + /// this instance has been built. + /// Thrown if no matching property or field can be found. + /// Thrown if many matching properties or fields are found, having the + /// same visibility and inheritance depth. + private void SetProperty(string alias, object value, object resultObj) + { + if (TrySet(alias, value, resultObj, _propertiesByNameCaseSensitive)) + return; + if (TrySet(alias, value, resultObj, _fieldsByNameCaseSensitive)) + return; + if (TrySet(alias, value, resultObj, _propertiesByNameCaseInsensitive)) + return; + if (TrySet(alias, value, resultObj, _fieldsByNameCaseInsensitive)) + return; + + throw new PropertyNotFoundException(resultObj.GetType(), alias, "setter"); + } + + private bool TrySet(string alias, object value, object resultObj, Dictionary> fieldsMap) + { + if (fieldsMap.TryGetValue(alias, out var field)) + { + CheckMember(field, alias); + field.Member.SetValue(resultObj, value); + return true; + } + return false; + } + + private bool TrySet(string alias, object value, object resultObj, Dictionary> propertiesMap) + { + if (propertiesMap.TryGetValue(alias, out var property)) + { + CheckMember(property, alias); + property.Member.SetValue(resultObj, value); + return true; + } + return false; + } + + private void CheckMember(NamedMember member, string alias) where T : MemberInfo + { + if (member.Member != null) + return; + + if (member.AmbiguousMembers == null || member.AmbiguousMembers.Length < 2) + { + // Should never happen, check NamedMember instanciations. + throw new InvalidOperationException( + $"{nameof(NamedMember.Member)} missing and {nameof(NamedMember.AmbiguousMembers)} invalid."); + } + + throw new AmbiguousMatchException( + $"Unable to find adequate property or field to set on '{member.AmbiguousMembers[0].DeclaringType.Name}' for alias '{alias}', " + + $"many {(member.AmbiguousMembers[0] is PropertyInfo ? "properties" : "fields")} matches: " + + $"{string.Join(", ", member.AmbiguousMembers.Select(m => m.Name))}"); + } + + private void FetchFieldsAndProperties(List> fields, List> properties) + { + const BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly; + var currentType = _resultClass; + var rank = 1; + // For grasping private members, we need to manually walk the hierarchy. + while (currentType != null && currentType != typeof(object)) + { + fields.AddRange( + currentType + .GetFields(bindingFlags) + .Select(f => new RankedMember { Member = f, VisibilityRank = GetFieldVisibilityRank(f), HierarchyRank = rank })); + properties.AddRange( + currentType + .GetProperties(bindingFlags) + .Where(p => p.CanWrite && p.GetIndexParameters().Length == 0) + .Select(p => new RankedMember { Member = p, VisibilityRank = GetPropertyVisibilityRank(p), HierarchyRank = rank })); + currentType = currentType.BaseType; + rank++; + } + } + + private int GetFieldVisibilityRank(FieldInfo field) + { + if (field.IsPublic) + return 1; + if (field.IsFamilyOrAssembly) + return 2; + if (field.IsFamily) + return 3; + if (field.IsPrivate) + return 4; + return 5; + } + + private int GetPropertyVisibilityRank(PropertyInfo property) + { + var setter = property.SetMethod; + if (setter.IsPublic) + return 1; + if (setter.IsFamilyOrAssembly) + return 2; + if (setter.IsFamily) + return 3; + if (setter.IsPrivate) + return 4; + return 5; + } + + private Dictionary> GetMapByName(IEnumerable> members, StringComparer comparer) where T : MemberInfo + { + return members + .GroupBy(m => m.Member.Name, + (k, g) => + new NamedMember(k, + g + .GroupBy(m => new { m.HierarchyRank, m.VisibilityRank }) + .OrderBy(subg => subg.Key.HierarchyRank).ThenBy(subg => subg.Key.VisibilityRank) + .First() + .Select(m => m.Member) + .ToArray()), + comparer) + .ToDictionary(f => f.Name, comparer); + } + + private struct RankedMember where T : MemberInfo + { + public T Member; + public int HierarchyRank; + public int VisibilityRank; + } + + [Serializable] + private struct NamedMember where T : MemberInfo + { + public NamedMember(string name, T[] members) + { + if (members == null) + throw new ArgumentNullException(nameof(members)); + Name = name; + if (members.Length == 1) + { + Member = members[0]; + AmbiguousMembers = null; + } + else + { + Member = null; + AmbiguousMembers = members; + } + } + + public string Name; + public T Member; + public T[] AmbiguousMembers; + } + + #endregion + + #region Equality & hash-code + public override bool Equals(object obj) { return Equals(obj as AliasToBeanResultTransformer); @@ -110,12 +289,14 @@ public bool Equals(AliasToBeanResultTransformer other) { return true; } - return Equals(other.resultClass, resultClass); + return Equals(other._resultClass, _resultClass); } public override int GetHashCode() { - return resultClass.GetHashCode(); + return _resultClass.GetHashCode(); } + + #endregion } } \ No newline at end of file diff --git a/src/NHibernate/Transform/QueryAliasToObjectPropertySetter.cs b/src/NHibernate/Transform/QueryAliasToObjectPropertySetter.cs deleted file mode 100644 index 67561a0fac6..00000000000 --- a/src/NHibernate/Transform/QueryAliasToObjectPropertySetter.cs +++ /dev/null @@ -1,48 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Reflection; - -namespace NHibernate.Transform -{ - [Serializable] - public class QueryAliasToObjectPropertySetter - { - private readonly IEnumerable _fields; - private readonly IEnumerable _properties; - - private QueryAliasToObjectPropertySetter(FieldInfo[] fields, PropertyInfo[] properties) - { - _fields = fields; - _properties = properties; - } - - public static QueryAliasToObjectPropertySetter MakeFor(System.Type objType) - { - var bindingFlags = BindingFlags.Instance | - BindingFlags.Public | - BindingFlags.NonPublic | - BindingFlags.IgnoreCase; - var fields = objType.GetFields(bindingFlags); - var properties = objType.GetProperties(bindingFlags); - - return new QueryAliasToObjectPropertySetter(fields, properties); - } - - public void SetProperty(string alias, object value, object resultObj) - { - var property = _properties.SingleOrDefault(prop => string.Equals(prop.Name, alias, StringComparison.OrdinalIgnoreCase)); - var field = _fields.SingleOrDefault(prop => string.Equals(prop.Name, alias, StringComparison.OrdinalIgnoreCase)); - if (field == null && property == null) - throw new PropertyNotFoundException(resultObj.GetType(), alias, "setter"); - - if (field != null) - { - field.SetValue(resultObj, value); - return; - } - if (property != null && property.CanWrite) - property.SetValue(resultObj, value, new object[0]); - } - } -} diff --git a/src/NHibernate/Transform/Transformers.cs b/src/NHibernate/Transform/Transformers.cs index f6d2a6172cf..92755f94db2 100644 --- a/src/NHibernate/Transform/Transformers.cs +++ b/src/NHibernate/Transform/Transformers.cs @@ -13,14 +13,32 @@ public static class Transformers public static readonly ToListResultTransformer ToList = new ToListResultTransformer(); /// - /// Creates a resulttransformer that will inject aliased values into instances + /// Creates a result transformer that will inject aliased values into instances /// of via property methods or fields. /// + /// The type of the instances to build. + /// A result transformer for supplied type. + /// + /// Resolves setter for an alias with a heuristic: search among properties then fields for matching name and case, then, + /// if no matching property or field was found, retry with a case insensitive match. For members having the same name, it + /// sorts them by inheritance depth then by visibility from public to private, and takes those ranking first. + /// public static IResultTransformer AliasToBean(System.Type target) { return new AliasToBeanResultTransformer(target); } + /// + /// Creates a result transformer that will inject aliased values into instances + /// of via property methods or fields. + /// + /// The type of the instances to build. + /// A result transformer for supplied type. + /// + /// Resolves setter for an alias with a heuristic: search among properties then fields for matching name and case, then, + /// if no matching property or field was found, retry with a case insensitive match. For members having the same name, it + /// sorts them by inheritance depth then by visibility from public to private, and takes those ranking first. + /// public static IResultTransformer AliasToBean() { return AliasToBean(typeof(T));