From 68d5e3ce61cae27c6c8856293813207278b696e9 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 31 Aug 2020 02:08:36 +0300 Subject: [PATCH 1/7] Throw for LINQ queries on unmapped entities --- .../Async/Linq/LinqQuerySamples.cs | 31 ++++++++++++++++++- src/NHibernate.Test/Linq/LinqQuerySamples.cs | 30 ++++++++++++++++++ .../Hql/Ast/ANTLR/AstPolymorphicProcessor.cs | 9 ++++-- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs b/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs index 9b8d04f2c6c..abc2930eee2 100644 --- a/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs +++ b/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs @@ -13,9 +13,10 @@ using System.Collections.Generic; using System.Linq; using NHibernate.DomainModel.Northwind.Entities; +using NHibernate.Hql.Ast.ANTLR; +using NHibernate.Linq; using NSubstitute; using NUnit.Framework; -using NHibernate.Linq; namespace NHibernate.Test.Linq { @@ -23,6 +24,34 @@ namespace NHibernate.Test.Linq [TestFixture] public class LinqQuerySamplesAsync : LinqTestCase { + class NotMappedEntity + { + public virtual int Id { get; set; } + public virtual string Name { get; set; } + } + + [Test] + public void ShouldThrowForQueryOnNotMappedEntityAsync() + { + var querySyntaxException = Assert.ThrowsAsync(() => session.Query().Select(x => x.Id).ToListAsync()); + Assert.That(querySyntaxException.Message, Does.Contain(nameof(NotMappedEntity))); + } + + [Test] + public void ShouldThrowForQueryOnNotMappedEntityNameAsync() + { + var entityName = "NotMappedEntityName"; + var querySyntaxException = Assert.ThrowsAsync(() => session.Query(entityName).ToListAsync()); + Assert.That(querySyntaxException.Message, Does.Contain(entityName)); + } + + [Test] + public void ShouldThrowForDmlQueryOnNotMappedEntityAsync() + { + var querySyntaxException = Assert.ThrowsAsync(() => session.Query().DeleteAsync()); + Assert.That(querySyntaxException.Message, Does.Contain(nameof(NotMappedEntity))); + } + [Test] public async Task GroupTwoQueriesAndSumAsync() { diff --git a/src/NHibernate.Test/Linq/LinqQuerySamples.cs b/src/NHibernate.Test/Linq/LinqQuerySamples.cs index 393d55ccf0a..7f251fb3331 100755 --- a/src/NHibernate.Test/Linq/LinqQuerySamples.cs +++ b/src/NHibernate.Test/Linq/LinqQuerySamples.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Linq; using NHibernate.DomainModel.Northwind.Entities; +using NHibernate.Hql.Ast.ANTLR; +using NHibernate.Linq; using NSubstitute; using NUnit.Framework; @@ -11,6 +13,34 @@ namespace NHibernate.Test.Linq [TestFixture] public class LinqQuerySamples : LinqTestCase { + class NotMappedEntity + { + public virtual int Id { get; set; } + public virtual string Name { get; set; } + } + + [Test] + public void ShouldThrowForQueryOnNotMappedEntity() + { + var querySyntaxException = Assert.Throws(() => session.Query().Select(x => x.Id).ToList()); + Assert.That(querySyntaxException.Message, Does.Contain(nameof(NotMappedEntity))); + } + + [Test] + public void ShouldThrowForQueryOnNotMappedEntityName() + { + var entityName = "NotMappedEntityName"; + var querySyntaxException = Assert.Throws(() => session.Query(entityName).ToList()); + Assert.That(querySyntaxException.Message, Does.Contain(entityName)); + } + + [Test] + public void ShouldThrowForDmlQueryOnNotMappedEntity() + { + var querySyntaxException = Assert.Throws(() => session.Query().Delete()); + Assert.That(querySyntaxException.Message, Does.Contain(nameof(NotMappedEntity))); + } + [Test] public void GroupTwoQueriesAndSum() { diff --git a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs index 9b92315de48..a28acded575 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs @@ -9,7 +9,7 @@ public class AstPolymorphicProcessor { private readonly IASTNode _ast; private readonly ISessionFactoryImplementor _factory; - private IEnumerable> _nodeMapping; + private Dictionary _nodeMapping; private AstPolymorphicProcessor(IASTNode ast, ISessionFactoryImplementor factory) { @@ -29,8 +29,11 @@ private IASTNode[] Process() // Find all the polymorphic query sources _nodeMapping = new PolymorphicQuerySourceDetector(_factory).Process(_ast); - if (_nodeMapping.Count() > 0) + if (_nodeMapping.Count > 0) { + foreach (var kv in _nodeMapping.Where(x => x.Value.Length == 0)) + throw new QuerySyntaxException(kv.Key + " is not mapped"); + return DuplicateTree().ToArray(); } else @@ -72,4 +75,4 @@ private static IASTNode DuplicateTree(IASTNode ast, IDictionary Date: Mon, 31 Aug 2020 12:07:25 +0300 Subject: [PATCH 2/7] Check all nodes --- .../Hql/Ast/ANTLR/AstPolymorphicProcessor.cs | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs index a28acded575..8fa9e00dff6 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs @@ -2,6 +2,7 @@ using System.Linq; using NHibernate.Engine; using NHibernate.Hql.Ast.ANTLR.Tree; +using NHibernate.Util; namespace NHibernate.Hql.Ast.ANTLR { @@ -30,11 +31,14 @@ private IASTNode[] Process() _nodeMapping = new PolymorphicQuerySourceDetector(_factory).Process(_ast); if (_nodeMapping.Count > 0) - { - foreach (var kv in _nodeMapping.Where(x => x.Value.Length == 0)) - throw new QuerySyntaxException(kv.Key + " is not mapped"); - - return DuplicateTree().ToArray(); + { + if (_nodeMapping.All(x => x.Value.Length == 0)) + throw new QuerySyntaxException( + _nodeMapping.Keys.Count == 1 + ? _nodeMapping.First().Key + " is not mapped" + : string.Join(", ", _nodeMapping.Keys) + " are not mapped"); + + return DuplicateTree(); } else { @@ -42,18 +46,10 @@ private IASTNode[] Process() } } - private IEnumerable DuplicateTree() + private IASTNode[] DuplicateTree() { var replacements = CrossJoinDictionaryArrays.PerformCrossJoin(_nodeMapping); - - var dups = new IASTNode[replacements.Count()]; - - for (var i = 0; i < replacements.Count(); i++) - { - dups[i] = DuplicateTree(_ast, replacements[i]); - } - - return dups; + return replacements.ToArray(x => DuplicateTree(_ast, x)); } private static IASTNode DuplicateTree(IASTNode ast, IDictionary nodeMapping) From 4f1ed539eefd24fce930526c2125aed71d35d050 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 31 Aug 2020 12:09:40 +0300 Subject: [PATCH 3/7] Skip delete query if no polymorphic entities --- src/NHibernate.Test/TestCase.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/NHibernate.Test/TestCase.cs b/src/NHibernate.Test/TestCase.cs index f1d04a96640..e3f4124bd0a 100644 --- a/src/NHibernate.Test/TestCase.cs +++ b/src/NHibernate.Test/TestCase.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Data; using System.Data.Common; +using System.Linq; using System.Reflection; using log4net; using NHibernate.Cfg; @@ -230,13 +231,20 @@ private string GetCombinedFailureMessage(TestContext.ResultAdapter result, strin protected virtual bool CheckDatabaseWasCleaned() { - if (Sfi.GetAllClassMetadata().Count == 0) + var allClassMetadata = Sfi.GetAllClassMetadata(); + if (allClassMetadata.Count == 0) { // Return early in the case of no mappings, also avoiding // a warning when executing the HQL below. return true; } + var explicitPolymorphismEntities = allClassMetadata.Values.Where(x => x is NHibernate.Persister.Entity.IQueryable queryable && queryable.IsExplicitPolymorphism).ToArray(); + + //TODO: Maybe add explicit load query checks + if (explicitPolymorphismEntities.Length == allClassMetadata.Count) + return true; + bool empty; using (ISession s = OpenSession()) { From 252818d20acd2daf13e4b661767da0397078b2af Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 31 Aug 2020 12:57:41 +0300 Subject: [PATCH 4/7] Skip invalid tests --- .../Async/Linq/FunctionTests.cs | 40 ------------------- src/NHibernate.Test/Linq/FunctionTests.cs | 4 ++ 2 files changed, 4 insertions(+), 40 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/FunctionTests.cs b/src/NHibernate.Test/Async/Linq/FunctionTests.cs index f65b9dc9854..e47e3d22c37 100644 --- a/src/NHibernate.Test/Async/Linq/FunctionTests.cs +++ b/src/NHibernate.Test/Async/Linq/FunctionTests.cs @@ -367,16 +367,6 @@ where item.Id.Equals(-1) await (ObjectDumper.WriteAsync(query)); } - [Test] - public async Task WhereShortEqualAsync() - { - var query = from item in session.Query() - where item.Short.Equals(-1) - select item; - - await (ObjectDumper.WriteAsync(query)); - } - [Test] public async Task WhereBoolConstantEqualAsync() { @@ -458,36 +448,6 @@ where item.BodyWeight.Equals(-1) await (ObjectDumper.WriteAsync(query)); } - - [Test] - public async Task WhereFloatEqualAsync() - { - var query = from item in session.Query() - where item.Float.Equals(-1) - select item; - - await (ObjectDumper.WriteAsync(query)); - } - - [Test] - public async Task WhereCharEqualAsync() - { - var query = from item in session.Query() - where item.Char.Equals('A') - select item; - - await (ObjectDumper.WriteAsync(query)); - } - - [Test] - public async Task WhereByteEqualAsync() - { - var query = from item in session.Query() - where item.Byte.Equals(1) - select item; - - await (ObjectDumper.WriteAsync(query)); - } [Test] public async Task WhereDecimalEqualAsync() diff --git a/src/NHibernate.Test/Linq/FunctionTests.cs b/src/NHibernate.Test/Linq/FunctionTests.cs index 127429d11b8..b92e7b3ece9 100644 --- a/src/NHibernate.Test/Linq/FunctionTests.cs +++ b/src/NHibernate.Test/Linq/FunctionTests.cs @@ -356,6 +356,7 @@ where item.Id.Equals(-1) } [Test] + [Ignore("Not mapped entity")] public void WhereShortEqual() { var query = from item in session.Query() @@ -448,6 +449,7 @@ where item.BodyWeight.Equals(-1) } [Test] + [Ignore("Not mapped entity")] public void WhereFloatEqual() { var query = from item in session.Query() @@ -458,6 +460,7 @@ where item.Float.Equals(-1) } [Test] + [Ignore("Not mapped entity")] public void WhereCharEqual() { var query = from item in session.Query() @@ -468,6 +471,7 @@ where item.Char.Equals('A') } [Test] + [Ignore("Not mapped entity")] public void WhereByteEqual() { var query = from item in session.Query() From 3bbb542dc1d744965ce59e5d2a72417f66c8aa42 Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 31 Aug 2020 13:25:31 +0300 Subject: [PATCH 5/7] small refact --- .../Hql/Ast/ANTLR/AstPolymorphicProcessor.cs | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs index 8fa9e00dff6..a68ed016146 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs @@ -30,20 +30,18 @@ private IASTNode[] Process() // Find all the polymorphic query sources _nodeMapping = new PolymorphicQuerySourceDetector(_factory).Process(_ast); - if (_nodeMapping.Count > 0) - { - if (_nodeMapping.All(x => x.Value.Length == 0)) - throw new QuerySyntaxException( - _nodeMapping.Keys.Count == 1 - ? _nodeMapping.First().Key + " is not mapped" - : string.Join(", ", _nodeMapping.Keys) + " are not mapped"); + if (_nodeMapping.Count == 0) + return new[] {_ast}; - return DuplicateTree(); - } - else - { - return new[] { _ast }; - } + var parsers = DuplicateTree(); + + if (parsers.Length == 0) + throw new QuerySyntaxException( + _nodeMapping.Keys.Count == 1 + ? _nodeMapping.First().Key + " is not mapped" + : string.Join(", ", _nodeMapping.Keys) + " are not mapped"); + + return parsers; } private IASTNode[] DuplicateTree() From 589b6dd9e3dd99b31b029ac4e0a39cb66e07bf1d Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 31 Aug 2020 14:02:30 +0300 Subject: [PATCH 6/7] Proper entity name retrieval --- .../Async/Linq/LinqQuerySamples.cs | 15 ++++++++++++--- src/NHibernate.Test/Linq/LinqQuerySamples.cs | 14 +++++++++++--- .../Hql/Ast/ANTLR/AstPolymorphicProcessor.cs | 15 +++++++++------ .../Ast/ANTLR/PolymorphicQuerySourceDetector.cs | 2 +- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs b/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs index abc2930eee2..24091d1fc3d 100644 --- a/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs +++ b/src/NHibernate.Test/Async/Linq/LinqQuerySamples.cs @@ -40,7 +40,7 @@ public void ShouldThrowForQueryOnNotMappedEntityAsync() [Test] public void ShouldThrowForQueryOnNotMappedEntityNameAsync() { - var entityName = "NotMappedEntityName"; + var entityName = "SomeNamespace.NotMappedEntityName"; var querySyntaxException = Assert.ThrowsAsync(() => session.Query(entityName).ToListAsync()); Assert.That(querySyntaxException.Message, Does.Contain(entityName)); } @@ -48,8 +48,17 @@ public void ShouldThrowForQueryOnNotMappedEntityNameAsync() [Test] public void ShouldThrowForDmlQueryOnNotMappedEntityAsync() { - var querySyntaxException = Assert.ThrowsAsync(() => session.Query().DeleteAsync()); - Assert.That(querySyntaxException.Message, Does.Contain(nameof(NotMappedEntity))); + Assert.Multiple( + () => + { + var querySyntaxException = Assert.ThrowsAsync(() => session.Query().DeleteAsync()); + Assert.That(querySyntaxException.Message, Does.Contain(nameof(NotMappedEntity))); + + var entityName = "SomeNamespace.NotMappedEntityName"; + querySyntaxException = Assert.ThrowsAsync(() => session.DeleteAsync($"from {entityName}")); + Assert.That(querySyntaxException.Message, Does.Contain(entityName)); + return Task.CompletedTask; + }); } [Test] diff --git a/src/NHibernate.Test/Linq/LinqQuerySamples.cs b/src/NHibernate.Test/Linq/LinqQuerySamples.cs index 7f251fb3331..f29fa18dd92 100755 --- a/src/NHibernate.Test/Linq/LinqQuerySamples.cs +++ b/src/NHibernate.Test/Linq/LinqQuerySamples.cs @@ -29,7 +29,7 @@ public void ShouldThrowForQueryOnNotMappedEntity() [Test] public void ShouldThrowForQueryOnNotMappedEntityName() { - var entityName = "NotMappedEntityName"; + var entityName = "SomeNamespace.NotMappedEntityName"; var querySyntaxException = Assert.Throws(() => session.Query(entityName).ToList()); Assert.That(querySyntaxException.Message, Does.Contain(entityName)); } @@ -37,8 +37,16 @@ public void ShouldThrowForQueryOnNotMappedEntityName() [Test] public void ShouldThrowForDmlQueryOnNotMappedEntity() { - var querySyntaxException = Assert.Throws(() => session.Query().Delete()); - Assert.That(querySyntaxException.Message, Does.Contain(nameof(NotMappedEntity))); + Assert.Multiple( + () => + { + var querySyntaxException = Assert.Throws(() => session.Query().Delete()); + Assert.That(querySyntaxException.Message, Does.Contain(nameof(NotMappedEntity))); + + var entityName = "SomeNamespace.NotMappedEntityName"; + querySyntaxException = Assert.Throws(() => session.Delete($"from {entityName}")); + Assert.That(querySyntaxException.Message, Does.Contain(entityName)); + }); } [Test] diff --git a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs index a68ed016146..ceb2a725519 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs @@ -35,13 +35,16 @@ private IASTNode[] Process() var parsers = DuplicateTree(); - if (parsers.Length == 0) - throw new QuerySyntaxException( - _nodeMapping.Keys.Count == 1 - ? _nodeMapping.First().Key + " is not mapped" - : string.Join(", ", _nodeMapping.Keys) + " are not mapped"); + if (parsers.Length == 0) + { + var entityNames = _nodeMapping.Keys.ToArray(x => PolymorphicQuerySourceDetector.GetClassName(x)); + throw new QuerySyntaxException( + entityNames.Length == 1 + ? entityNames[0] + " is not mapped" + : string.Join(", ", entityNames) + " are not mapped"); + } - return parsers; + return parsers; } private IASTNode[] DuplicateTree() diff --git a/src/NHibernate/Hql/Ast/ANTLR/PolymorphicQuerySourceDetector.cs b/src/NHibernate/Hql/Ast/ANTLR/PolymorphicQuerySourceDetector.cs index 4b1cc080e17..262f6062387 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/PolymorphicQuerySourceDetector.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/PolymorphicQuerySourceDetector.cs @@ -45,7 +45,7 @@ private void AddImplementorsToMap(IASTNode querySource, string className, string implementors.ToArray(implementor => MakeIdent(querySource, implementor))); } - private static string GetClassName(IASTNode querySource) + internal static string GetClassName(IASTNode querySource) { switch (querySource.Type) { From 9828f236c919643ade2a4f6731ae7da67ff1b7ca Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Mon, 7 Sep 2020 13:15:20 +0300 Subject: [PATCH 7/7] whitespaces --- .../Hql/Ast/ANTLR/AstPolymorphicProcessor.cs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs index ceb2a725519..259e6d203d4 100644 --- a/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs +++ b/src/NHibernate/Hql/Ast/ANTLR/AstPolymorphicProcessor.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Linq; using NHibernate.Engine; using NHibernate.Hql.Ast.ANTLR.Tree; using NHibernate.Util; @@ -35,16 +34,16 @@ private IASTNode[] Process() var parsers = DuplicateTree(); - if (parsers.Length == 0) - { - var entityNames = _nodeMapping.Keys.ToArray(x => PolymorphicQuerySourceDetector.GetClassName(x)); - throw new QuerySyntaxException( - entityNames.Length == 1 - ? entityNames[0] + " is not mapped" - : string.Join(", ", entityNames) + " are not mapped"); - } + if (parsers.Length == 0) + { + var entityNames = _nodeMapping.Keys.ToArray(x => PolymorphicQuerySourceDetector.GetClassName(x)); + throw new QuerySyntaxException( + entityNames.Length == 1 + ? entityNames[0] + " is not mapped" + : string.Join(", ", entityNames) + " are not mapped"); + } - return parsers; + return parsers; } private IASTNode[] DuplicateTree()