Skip to content

Commit e888c42

Browse files
oskarboskarb
committed
Merge fix for NH-3591: IncrementGenerator unnecessarily creates a new connection to the db
2 parents 38d28d8 + 48b4f8b commit e888c42

File tree

4 files changed

+142
-27
lines changed

4 files changed

+142
-27
lines changed

‎src/NHibernate.Test/DriverTest/FirebirdClientDriverFixture.cs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
using System.Data;
2+
using NHibernate.Driver;
3+
using NUnit.Framework;
4+
using SharpTestsEx;
5+
6+
namespace NHibernate.Test.DriverTest
7+
{
8+
[TestFixture]
9+
public class FirebirdClientDriverFixture
10+
{
11+
#region Fields
12+
13+
private string _connectionString;
14+
private FirebirdClientDriver _driver;
15+
16+
#endregion
17+
18+
#region Tests
19+
20+
[Test]
21+
public void ConnectionPooling_OpenThenCloseThenOpenAnotherOne_OnlyOneConnectionIsPooled()
22+
{
23+
MakeDriver();
24+
var connection1 = MakeConnection();
25+
var connection2 = MakeConnection();
26+
27+
//open first connection
28+
connection1.Open();
29+
VerifyCountOfEstablishedConnectionsIs(1);
30+
31+
//return it to the pool
32+
connection1.Close();
33+
VerifyCountOfEstablishedConnectionsIs(1);
34+
35+
//open the second connection
36+
connection2.Open();
37+
VerifyCountOfEstablishedConnectionsIs(1);
38+
39+
//return it to the pool
40+
connection2.Close();
41+
VerifyCountOfEstablishedConnectionsIs(1);
42+
}
43+
44+
[Test]
45+
public void ConnectionPooling_OpenThenCloseTwoAtTheSameTime_TowConnectionsArePooled()
46+
{
47+
MakeDriver();
48+
var connection1 = MakeConnection();
49+
var connection2 = MakeConnection();
50+
51+
//open first connection
52+
connection1.Open();
53+
VerifyCountOfEstablishedConnectionsIs(1);
54+
55+
//open second one
56+
connection2.Open();
57+
VerifyCountOfEstablishedConnectionsIs(2);
58+
59+
//return connection1 to the pool
60+
connection1.Close();
61+
VerifyCountOfEstablishedConnectionsIs(2);
62+
63+
//return connection2 to the pool
64+
connection2.Close();
65+
VerifyCountOfEstablishedConnectionsIs(2);
66+
}
67+
68+
#endregion
69+
70+
#region Private Members
71+
72+
private void MakeDriver()
73+
{
74+
var cfg = TestConfigurationHelper.GetDefaultConfiguration();
75+
var dlct = cfg.GetProperty("dialect");
76+
if (!dlct.Contains("Firebird"))
77+
Assert.Ignore("Applies only to Firebird");
78+
79+
_driver = new FirebirdClientDriver();
80+
_connectionString = cfg.GetProperty("connection.connection_string");
81+
}
82+
83+
private IDbConnection MakeConnection()
84+
{
85+
var result = _driver.CreateConnection();
86+
result.ConnectionString = _connectionString;
87+
return result;
88+
}
89+
90+
private void VerifyCountOfEstablishedConnectionsIs(int expectedCount)
91+
{
92+
var physicalConnections = GetEstablishedConnections();
93+
physicalConnections.Should().Be(expectedCount);
94+
}
95+
96+
private int GetEstablishedConnections()
97+
{
98+
using (var conn = _driver.CreateConnection())
99+
{
100+
conn.ConnectionString = _connectionString;
101+
conn.Open();
102+
using (var cmd = conn.CreateCommand())
103+
{
104+
cmd.CommandText = "select count(*) from mon$attachments where mon$attachment_id <> current_connection";
105+
return (int) cmd.ExecuteScalar();
106+
}
107+
}
108+
}
109+
110+
#endregion
111+
}
112+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ public void IncrementGeneratorShouldIncludeClassLevelSchemaWhenGettingNextId()
3131
// think this would be a huge problem.
3232
// Having said that, if someone sees this and have a better idea to test,
3333
// please feel free to change it.
34-
FieldInfo sqlFieldInfo = generator.GetType().GetField("sql", BindingFlags.NonPublic | BindingFlags.Instance);
35-
string sql = (string)sqlFieldInfo.GetValue(generator);
34+
FieldInfo sqlFieldInfo = generator.GetType().GetField("_sql", BindingFlags.NonPublic | BindingFlags.Instance);
35+
string sql = sqlFieldInfo.GetValue(generator).ToString();
3636

3737
Assert.AreEqual("select max(Id) from test.TestNH1061", sql);
3838
}

‎src/NHibernate.Test/NHibernate.Test.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@
222222
<Compile Include="DialectTest\MsSqlCe40DialectFixture.cs" />
223223
<Compile Include="DialectTest\SchemaTests\ColumnMetaDataFixture.cs" />
224224
<Compile Include="DriverTest\DbProviderFactoryDriveConnectionCommandProviderTest.cs" />
225+
<Compile Include="DriverTest\FirebirdClientDriverFixture.cs" />
225226
<Compile Include="DriverTest\ReflectionBasedDriverTest.cs" />
226227
<Compile Include="DriverTest\Sql2008DateTime2Test.cs" />
227228
<Compile Include="DriverTest\SqlClientDriverFixture.cs" />

‎src/NHibernate/Id/IncrementGenerator.cs

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Data;
4+
using System.Data.Common;
45
using System.Runtime.CompilerServices;
56
using System.Text;
6-
77
using NHibernate.Engine;
88
using NHibernate.Exceptions;
9+
using NHibernate.SqlCommand;
10+
using NHibernate.SqlTypes;
911
using NHibernate.Type;
10-
using NHibernate.Util;
11-
using System.Data.Common;
1212

1313
namespace NHibernate.Id
1414
{
@@ -27,11 +27,11 @@ namespace NHibernate.Id
2727
/// </remarks>
2828
public class IncrementGenerator : IIdentifierGenerator, IConfigurable
2929
{
30-
private static readonly IInternalLogger log = LoggerProvider.LoggerFor(typeof(IncrementGenerator));
30+
private static readonly IInternalLogger Logger = LoggerProvider.LoggerFor(typeof(IncrementGenerator));
3131

32-
private long next;
33-
private string sql;
34-
private System.Type returnClass;
32+
private long _next;
33+
private SqlString _sql;
34+
private System.Type _returnClass;
3535

3636
/// <summary>
3737
///
@@ -49,9 +49,12 @@ public void Configure(IType type, IDictionary<string, string> parms, Dialect.Dia
4949
if (!parms.TryGetValue("tables", out tableList))
5050
parms.TryGetValue(PersistentIdGeneratorParmsNames.Tables, out tableList);
5151
string[] tables = tableList.Split(", ".ToCharArray(), StringSplitOptions.RemoveEmptyEntries);
52+
5253
if (!parms.TryGetValue("column", out column))
5354
parms.TryGetValue(PersistentIdGeneratorParmsNames.PK, out column);
54-
returnClass = type.ReturnedClass;
55+
56+
_returnClass = type.ReturnedClass;
57+
5558
parms.TryGetValue(PersistentIdGeneratorParmsNames.Schema, out schema);
5659
parms.TryGetValue(PersistentIdGeneratorParmsNames.Catalog, out catalog);
5760

@@ -72,7 +75,8 @@ public void Configure(IType type, IDictionary<string, string> parms, Dialect.Dia
7275
column = "ids_." + column;
7376
}
7477

75-
sql = "select max(" + column + ") from " + buf;
78+
var sqlTxt = string.Format("select max({0}) from {1}", column, buf);
79+
_sql = new SqlString(sqlTxt);
7680
}
7781

7882
/// <summary>
@@ -84,52 +88,50 @@ public void Configure(IType type, IDictionary<string, string> parms, Dialect.Dia
8488
[MethodImpl(MethodImplOptions.Synchronized)]
8589
public object Generate(ISessionImplementor session, object obj)
8690
{
87-
if (sql != null)
91+
if (_sql != null)
8892
{
8993
GetNext(session);
9094
}
91-
return IdentifierGeneratorFactory.CreateNumber(next++, returnClass);
95+
return IdentifierGeneratorFactory.CreateNumber(_next++, _returnClass);
9296
}
9397

9498
private void GetNext(ISessionImplementor session)
9599
{
96-
log.Debug("fetching initial value: " + sql);
100+
Logger.Debug("fetching initial value: " + _sql);
97101

98102
try
99103
{
100-
IDbConnection conn = session.Factory.ConnectionProvider.GetConnection();
101-
IDbCommand qps = conn.CreateCommand();
102-
qps.CommandText = sql;
103-
qps.CommandType = CommandType.Text;
104+
var cmd = session.Batcher.PrepareCommand(CommandType.Text, _sql, SqlTypeFactory.NoTypes);
105+
IDataReader reader = null;
104106
try
105107
{
106-
IDataReader rs = qps.ExecuteReader();
108+
reader = session.Batcher.ExecuteReader(cmd);
107109
try
108110
{
109-
if (rs.Read())
111+
if (reader.Read())
110112
{
111-
next = !rs.IsDBNull(0) ? Convert.ToInt64(rs.GetValue(0)) + 1 : 1L;
113+
_next = !reader.IsDBNull(0) ? Convert.ToInt64(reader.GetValue(0)) + 1 : 1L;
112114
}
113115
else
114116
{
115-
next = 1L;
117+
_next = 1L;
116118
}
117-
sql = null;
118-
log.Debug("first free id: " + next);
119+
_sql = null;
120+
Logger.Debug("first free id: " + _next);
119121
}
120122
finally
121123
{
122-
rs.Close();
124+
reader.Close();
123125
}
124126
}
125127
finally
126128
{
127-
session.Factory.ConnectionProvider.CloseConnection(conn);
129+
session.Batcher.CloseCommand(cmd, reader);
128130
}
129131
}
130132
catch (DbException sqle)
131133
{
132-
log.Error("could not get increment value", sqle);
134+
Logger.Error("could not get increment value", sqle);
133135
throw ADOExceptionHelper.Convert(session.Factory.SQLExceptionConverter, sqle,
134136
"could not fetch initial value for increment generator");
135137
}

0 commit comments

Comments
 (0)