Skip to content

Commit 35a582f

Browse files
increddibellyfredericDelaporte
increddibelly
and
fredericDelaporte
committed
Improve exception message in case of duplicated column (#1875)
Fix the SQL builder overriding parametrized columns silently Co-authored-by: Frédéric Delaporte <12201973+fredericdelaporte@users.noreply.github.com>
1 parent 1b0efb7 commit 35a582f

File tree

5 files changed

+136
-18
lines changed

5 files changed

+136
-18
lines changed

‎src/NHibernate.Test/NHSpecificTest/GH1875/Fixture.cs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
using System;
2+
using NHibernate.Cfg.MappingSchema;
3+
using NHibernate.Mapping.ByCode;
4+
using NUnit.Framework;
5+
6+
namespace NHibernate.Test.NHSpecificTest.GH1875
7+
{
8+
public class BadlyMappedEntity
9+
{
10+
public virtual Guid Id { get; set; }
11+
public virtual long FirstValue { get; set; }
12+
public virtual long SecondValue { get; set; }
13+
}
14+
15+
[TestFixture]
16+
public class Fixture
17+
{
18+
protected HbmMapping GetMappings()
19+
{
20+
var mapper = new ModelMapper();
21+
mapper.Class<BadlyMappedEntity>(
22+
ca =>
23+
{
24+
ca.Abstract(true);
25+
ca.Id(
26+
x => x.Id,
27+
map =>
28+
{
29+
map.Column("BadlyMappedEntityId");
30+
map.Generator(Generators.GuidComb);
31+
});
32+
ca.Property(x => x.FirstValue, map => map.Column("SameColumn"));
33+
// SecondValue is mapped with same name as another column, this gives the AbstractEntityMapper
34+
// more entries in the fields array than there are in the includeColumns array; this causes the
35+
// index to fall out of bounds.
36+
ca.Property(x => x.SecondValue, map => map.Column("SameColumn"));
37+
});
38+
39+
return mapper.CompileMappingFor(new[] { typeof(BadlyMappedEntity) });
40+
}
41+
42+
[Test]
43+
public void ShouldThrowSoundErrorForBadlyMappedEntity()
44+
{
45+
var mappings = GetMappings();
46+
var cfg = TestConfigurationHelper.GetDefaultConfiguration();
47+
cfg.AddMapping(mappings);
48+
49+
ISessionFactory factory = null;
50+
try
51+
{
52+
Assert.That(
53+
() => factory = cfg.BuildSessionFactory(),
54+
Throws.TypeOf<MappingException>().And.Message.Contains("BadlyMappedEntity").And.InnerException
55+
.Message.Contains("SameColumn"));
56+
}
57+
finally
58+
{
59+
factory?.Dispose();
60+
}
61+
}
62+
}
63+
}

‎src/NHibernate/Async/Persister/Entity/AbstractEntityPersister.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
using System.Data;
1515
using System.Data.Common;
1616
using System.Text;
17-
1817
using NHibernate.AdoNet;
1918
using NHibernate.Cache;
2019
using NHibernate.Cache.Entry;
@@ -30,7 +29,6 @@
3029
using NHibernate.Metadata;
3130
using NHibernate.Properties;
3231
using NHibernate.SqlCommand;
33-
using NHibernate.Tuple;
3432
using NHibernate.Tuple.Entity;
3533
using NHibernate.Type;
3634
using NHibernate.Util;

‎src/NHibernate/Persister/Entity/AbstractEntityPersister.cs

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Data;
55
using System.Data.Common;
66
using System.Text;
7-
87
using NHibernate.AdoNet;
98
using NHibernate.Cache;
109
using NHibernate.Cache.Entry;
@@ -20,7 +19,6 @@
2019
using NHibernate.Metadata;
2120
using NHibernate.Properties;
2221
using NHibernate.SqlCommand;
23-
using NHibernate.Tuple;
2422
using NHibernate.Tuple.Entity;
2523
using NHibernate.Type;
2624
using NHibernate.Util;
@@ -2279,7 +2277,17 @@ protected internal SqlCommandInfo GenerateUpdateString(bool[] includeProperty, i
22792277
if (includeProperty[i] && IsPropertyOfTable(i, j))
22802278
{
22812279
// this is a property of the table, which we are updating
2282-
updateBuilder.AddColumns(GetPropertyColumnNames(i), propertyColumnUpdateable[i], PropertyTypes[i]);
2280+
try
2281+
{
2282+
updateBuilder.AddColumns(GetPropertyColumnNames(i), propertyColumnUpdateable[i], PropertyTypes[i]);
2283+
}
2284+
catch (ArgumentException arex)
2285+
{
2286+
throw new MappingException(
2287+
$"Unable to build the update statement for class {entityMetamodel.Name}: " +
2288+
$"a failure occured when adding the property {PropertyNames[i]}",
2289+
arex);
2290+
}
22832291
hasColumns = hasColumns || GetPropertyColumnSpan(i) > 0;
22842292
}
22852293
}
@@ -2368,24 +2376,54 @@ protected virtual SqlCommandInfo GenerateInsertString(bool identityInsert, bool[
23682376
if (includeProperty[i] && IsPropertyOfTable(i, j))
23692377
{
23702378
// this property belongs on the table and is to be inserted
2371-
builder.AddColumns(GetPropertyColumnNames(i), propertyColumnInsertable[i], PropertyTypes[i]);
2379+
try
2380+
{
2381+
builder.AddColumns(GetPropertyColumnNames(i), propertyColumnInsertable[i], PropertyTypes[i]);
2382+
}
2383+
catch (ArgumentException arex)
2384+
{
2385+
throw new MappingException(
2386+
$"Unable to build the insert statement for class {entityMetamodel.Name}: " +
2387+
$"a failure occured when adding the property {PropertyNames[i]}",
2388+
arex);
2389+
}
23722390
}
23732391
}
23742392

23752393
// add the discriminator
23762394
if (j == 0)
23772395
{
2378-
AddDiscriminatorToInsert(builder);
2396+
try
2397+
{
2398+
AddDiscriminatorToInsert(builder);
2399+
}
2400+
catch (ArgumentException arex)
2401+
{
2402+
throw new MappingException(
2403+
$"Unable to build the insert statement for class {entityMetamodel.Name}: " +
2404+
"a failure occured when adding the discriminator",
2405+
arex);
2406+
}
23792407
}
23802408

23812409
// add the primary key
2382-
if (j == 0 && identityInsert)
2410+
try
23832411
{
2384-
builder.AddIdentityColumn(GetKeyColumns(0)[0]);
2412+
if (j == 0 && identityInsert)
2413+
{
2414+
builder.AddIdentityColumn(GetKeyColumns(0)[0]);
2415+
}
2416+
else
2417+
{
2418+
builder.AddColumns(GetKeyColumns(j), null, GetIdentifierType(j));
2419+
}
23852420
}
2386-
else
2421+
catch (ArgumentException arex)
23872422
{
2388-
builder.AddColumns(GetKeyColumns(j), null, GetIdentifierType(j));
2423+
throw new MappingException(
2424+
$"Unable to build the insert statement for class {entityMetamodel.Name}: " +
2425+
"a failure occured when adding the Id of the class",
2426+
arex);
23892427
}
23902428

23912429
if (Factory.Settings.IsCommentsEnabled)

‎src/NHibernate/SqlCommand/SqlInsertBuilder.cs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23

34
using NHibernate.Engine;
@@ -55,7 +56,7 @@ public virtual SqlInsertBuilder AddColumn(string columnName, IType propertyType)
5556
SqlType[] sqlTypes = propertyType.SqlTypes(factory);
5657
if (sqlTypes.Length > 1)
5758
throw new AssertionFailure("Adding one column for a composed IType.");
58-
columns[columnName] = sqlTypes[0];
59+
AddColumnWithValueOrType(columnName, sqlTypes[0]);
5960
return this;
6061
}
6162

@@ -80,7 +81,7 @@ public SqlInsertBuilder AddColumn(string columnName, object val, ILiteralType li
8081
/// <returns>The SqlInsertBuilder.</returns>
8182
public SqlInsertBuilder AddColumn(string columnName, string val)
8283
{
83-
columns[columnName] = val;
84+
AddColumnWithValueOrType(columnName, val);
8485
return this;
8586
}
8687

@@ -94,13 +95,22 @@ public SqlInsertBuilder AddColumns(string[] columnNames, bool[] insertable, ITyp
9495
{
9596
if (i >= sqlTypes.Length)
9697
throw new AssertionFailure("Different columns and it's IType.");
97-
columns[columnNames[i]] = sqlTypes[i];
98+
AddColumnWithValueOrType(columnNames[i], sqlTypes[i]);
9899
}
99100
}
100101

101102
return this;
102103
}
103104

105+
private void AddColumnWithValueOrType(string columnName, object valueOrType)
106+
{
107+
if (columns.ContainsKey(columnName))
108+
throw new ArgumentException(
109+
$"The column '{columnName}' has already been added in this SQL builder",
110+
nameof(columnName));
111+
columns.Add(columnName, valueOrType);
112+
}
113+
104114
public virtual SqlInsertBuilder AddIdentityColumn(string columnName)
105115
{
106116
string value = Dialect.IdentityInsertString;

‎src/NHibernate/SqlCommand/SqlUpdateBuilder.cs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public SqlUpdateBuilder AddColumn(string columnName, object val, ILiteralType li
6161
/// <returns>The SqlUpdateBuilder.</returns>
6262
public SqlUpdateBuilder AddColumn(string columnName, string val)
6363
{
64-
columns[columnName] = val;
64+
AddColumnWithValueOrType(columnName, val);
6565
return this;
6666
}
6767

@@ -74,7 +74,7 @@ public SqlUpdateBuilder AddColumn(string columnName, string val)
7474
public SqlUpdateBuilder AddColumns(string[] columnsName, string val)
7575
{
7676
foreach (string columnName in columnsName)
77-
columns[columnName] = val;
77+
AddColumnWithValueOrType(columnName, val);
7878

7979
return this;
8080
}
@@ -84,7 +84,7 @@ public virtual SqlUpdateBuilder AddColumn(string columnName, IType propertyType)
8484
SqlType[] sqlTypes = propertyType.SqlTypes(Mapping);
8585
if (sqlTypes.Length > 1)
8686
throw new AssertionFailure("Adding one column for a composed IType.");
87-
columns[columnName] = sqlTypes[0];
87+
AddColumnWithValueOrType(columnName, sqlTypes[0]);
8888
return this;
8989
}
9090

@@ -115,13 +115,22 @@ public SqlUpdateBuilder AddColumns(string[] columnNames, bool[] updateable, ITyp
115115
{
116116
if (i >= sqlTypes.Length)
117117
throw new AssertionFailure("Different columns and it's IType.");
118-
columns[columnNames[i]] = sqlTypes[i];
118+
AddColumnWithValueOrType(columnNames[i], sqlTypes[i]);
119119
}
120120
}
121121

122122
return this;
123123
}
124124

125+
private void AddColumnWithValueOrType(string columnName, object valueOrType)
126+
{
127+
if (columns.ContainsKey(columnName))
128+
throw new ArgumentException(
129+
$"The column '{columnName}' has already been added in this SQL builder",
130+
nameof(columnName));
131+
columns.Add(columnName, valueOrType);
132+
}
133+
125134
public SqlUpdateBuilder AppendAssignmentFragment(SqlString fragment)
126135
{
127136
// SqlString is immutable

0 commit comments

Comments
 (0)