diff --git a/src/NHibernate.Test/NHSpecificTest/GH1875/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1875/Fixture.cs new file mode 100644 index 00000000000..cc5d8d282e5 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1875/Fixture.cs @@ -0,0 +1,63 @@ +using System; +using NHibernate.Cfg.MappingSchema; +using NHibernate.Mapping.ByCode; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH1875 +{ + public class BadlyMappedEntity + { + public virtual Guid Id { get; set; } + public virtual long FirstValue { get; set; } + public virtual long SecondValue { get; set; } + } + + [TestFixture] + public class Fixture + { + protected HbmMapping GetMappings() + { + var mapper = new ModelMapper(); + mapper.Class( + ca => + { + ca.Abstract(true); + ca.Id( + x => x.Id, + map => + { + map.Column("BadlyMappedEntityId"); + map.Generator(Generators.GuidComb); + }); + ca.Property(x => x.FirstValue, map => map.Column("SameColumn")); + // SecondValue is mapped with same name as another column, this gives the AbstractEntityMapper + // more entries in the fields array than there are in the includeColumns array; this causes the + // index to fall out of bounds. + ca.Property(x => x.SecondValue, map => map.Column("SameColumn")); + }); + + return mapper.CompileMappingFor(new[] { typeof(BadlyMappedEntity) }); + } + + [Test] + public void ShouldThrowSoundErrorForBadlyMappedEntity() + { + var mappings = GetMappings(); + var cfg = TestConfigurationHelper.GetDefaultConfiguration(); + cfg.AddMapping(mappings); + + ISessionFactory factory = null; + try + { + Assert.That( + () => factory = cfg.BuildSessionFactory(), + Throws.TypeOf().And.Message.Contains("BadlyMappedEntity").And.InnerException + .Message.Contains("SameColumn")); + } + finally + { + factory?.Dispose(); + } + } + } +} diff --git a/src/NHibernate/Async/Persister/Entity/AbstractEntityPersister.cs b/src/NHibernate/Async/Persister/Entity/AbstractEntityPersister.cs index 7376ec72a14..b9ce69ae8d7 100644 --- a/src/NHibernate/Async/Persister/Entity/AbstractEntityPersister.cs +++ b/src/NHibernate/Async/Persister/Entity/AbstractEntityPersister.cs @@ -14,7 +14,6 @@ using System.Data; using System.Data.Common; using System.Text; - using NHibernate.AdoNet; using NHibernate.Cache; using NHibernate.Cache.Entry; @@ -30,7 +29,6 @@ using NHibernate.Metadata; using NHibernate.Properties; using NHibernate.SqlCommand; -using NHibernate.Tuple; using NHibernate.Tuple.Entity; using NHibernate.Type; using NHibernate.Util; diff --git a/src/NHibernate/Persister/Entity/AbstractEntityPersister.cs b/src/NHibernate/Persister/Entity/AbstractEntityPersister.cs index 0d41c7484d2..410381389c3 100644 --- a/src/NHibernate/Persister/Entity/AbstractEntityPersister.cs +++ b/src/NHibernate/Persister/Entity/AbstractEntityPersister.cs @@ -4,7 +4,6 @@ using System.Data; using System.Data.Common; using System.Text; - using NHibernate.AdoNet; using NHibernate.Cache; using NHibernate.Cache.Entry; @@ -20,7 +19,6 @@ using NHibernate.Metadata; using NHibernate.Properties; using NHibernate.SqlCommand; -using NHibernate.Tuple; using NHibernate.Tuple.Entity; using NHibernate.Type; using NHibernate.Util; @@ -2279,7 +2277,17 @@ protected internal SqlCommandInfo GenerateUpdateString(bool[] includeProperty, i if (includeProperty[i] && IsPropertyOfTable(i, j)) { // this is a property of the table, which we are updating - updateBuilder.AddColumns(GetPropertyColumnNames(i), propertyColumnUpdateable[i], PropertyTypes[i]); + try + { + updateBuilder.AddColumns(GetPropertyColumnNames(i), propertyColumnUpdateable[i], PropertyTypes[i]); + } + catch (ArgumentException arex) + { + throw new MappingException( + $"Unable to build the update statement for class {entityMetamodel.Name}: " + + $"a failure occured when adding the property {PropertyNames[i]}", + arex); + } hasColumns = hasColumns || GetPropertyColumnSpan(i) > 0; } } @@ -2368,24 +2376,54 @@ protected virtual SqlCommandInfo GenerateInsertString(bool identityInsert, bool[ if (includeProperty[i] && IsPropertyOfTable(i, j)) { // this property belongs on the table and is to be inserted - builder.AddColumns(GetPropertyColumnNames(i), propertyColumnInsertable[i], PropertyTypes[i]); + try + { + builder.AddColumns(GetPropertyColumnNames(i), propertyColumnInsertable[i], PropertyTypes[i]); + } + catch (ArgumentException arex) + { + throw new MappingException( + $"Unable to build the insert statement for class {entityMetamodel.Name}: " + + $"a failure occured when adding the property {PropertyNames[i]}", + arex); + } } } // add the discriminator if (j == 0) { - AddDiscriminatorToInsert(builder); + try + { + AddDiscriminatorToInsert(builder); + } + catch (ArgumentException arex) + { + throw new MappingException( + $"Unable to build the insert statement for class {entityMetamodel.Name}: " + + "a failure occured when adding the discriminator", + arex); + } } // add the primary key - if (j == 0 && identityInsert) + try { - builder.AddIdentityColumn(GetKeyColumns(0)[0]); + if (j == 0 && identityInsert) + { + builder.AddIdentityColumn(GetKeyColumns(0)[0]); + } + else + { + builder.AddColumns(GetKeyColumns(j), null, GetIdentifierType(j)); + } } - else + catch (ArgumentException arex) { - builder.AddColumns(GetKeyColumns(j), null, GetIdentifierType(j)); + throw new MappingException( + $"Unable to build the insert statement for class {entityMetamodel.Name}: " + + "a failure occured when adding the Id of the class", + arex); } if (Factory.Settings.IsCommentsEnabled) diff --git a/src/NHibernate/SqlCommand/SqlInsertBuilder.cs b/src/NHibernate/SqlCommand/SqlInsertBuilder.cs index 8fdd229b6cc..e2d1a8a3f4b 100644 --- a/src/NHibernate/SqlCommand/SqlInsertBuilder.cs +++ b/src/NHibernate/SqlCommand/SqlInsertBuilder.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using NHibernate.Engine; @@ -55,7 +56,7 @@ public virtual SqlInsertBuilder AddColumn(string columnName, IType propertyType) SqlType[] sqlTypes = propertyType.SqlTypes(factory); if (sqlTypes.Length > 1) throw new AssertionFailure("Adding one column for a composed IType."); - columns[columnName] = sqlTypes[0]; + AddColumnWithValueOrType(columnName, sqlTypes[0]); return this; } @@ -80,7 +81,7 @@ public SqlInsertBuilder AddColumn(string columnName, object val, ILiteralType li /// The SqlInsertBuilder. public SqlInsertBuilder AddColumn(string columnName, string val) { - columns[columnName] = val; + AddColumnWithValueOrType(columnName, val); return this; } @@ -94,13 +95,22 @@ public SqlInsertBuilder AddColumns(string[] columnNames, bool[] insertable, ITyp { if (i >= sqlTypes.Length) throw new AssertionFailure("Different columns and it's IType."); - columns[columnNames[i]] = sqlTypes[i]; + AddColumnWithValueOrType(columnNames[i], sqlTypes[i]); } } return this; } + private void AddColumnWithValueOrType(string columnName, object valueOrType) + { + if (columns.ContainsKey(columnName)) + throw new ArgumentException( + $"The column '{columnName}' has already been added in this SQL builder", + nameof(columnName)); + columns.Add(columnName, valueOrType); + } + public virtual SqlInsertBuilder AddIdentityColumn(string columnName) { string value = Dialect.IdentityInsertString; diff --git a/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs b/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs index 109262b0bde..def92f8fe50 100644 --- a/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs +++ b/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs @@ -61,7 +61,7 @@ public SqlUpdateBuilder AddColumn(string columnName, object val, ILiteralType li /// The SqlUpdateBuilder. public SqlUpdateBuilder AddColumn(string columnName, string val) { - columns[columnName] = val; + AddColumnWithValueOrType(columnName, val); return this; } @@ -74,7 +74,7 @@ public SqlUpdateBuilder AddColumn(string columnName, string val) public SqlUpdateBuilder AddColumns(string[] columnsName, string val) { foreach (string columnName in columnsName) - columns[columnName] = val; + AddColumnWithValueOrType(columnName, val); return this; } @@ -84,7 +84,7 @@ public virtual SqlUpdateBuilder AddColumn(string columnName, IType propertyType) SqlType[] sqlTypes = propertyType.SqlTypes(Mapping); if (sqlTypes.Length > 1) throw new AssertionFailure("Adding one column for a composed IType."); - columns[columnName] = sqlTypes[0]; + AddColumnWithValueOrType(columnName, sqlTypes[0]); return this; } @@ -115,13 +115,22 @@ public SqlUpdateBuilder AddColumns(string[] columnNames, bool[] updateable, ITyp { if (i >= sqlTypes.Length) throw new AssertionFailure("Different columns and it's IType."); - columns[columnNames[i]] = sqlTypes[i]; + AddColumnWithValueOrType(columnNames[i], sqlTypes[i]); } } return this; } + private void AddColumnWithValueOrType(string columnName, object valueOrType) + { + if (columns.ContainsKey(columnName)) + throw new ArgumentException( + $"The column '{columnName}' has already been added in this SQL builder", + nameof(columnName)); + columns.Add(columnName, valueOrType); + } + public SqlUpdateBuilder AppendAssignmentFragment(SqlString fragment) { // SqlString is immutable