SQLiteDialect: Correct GUID to string conversion when BinaryGuid=False (regression) (fixes #2110)

doc/reference/modules/configuration.xml

@@ -1081,6 +1081,20 @@ in the parameter binding.</programlisting>
                             </para>
                         </entry>
                     </row>
+                    <row>
+                        <entry>
+                            <literal>sqlite.binaryguid</literal>
+                        </entry>
+                        <entry>
+                            SQLite can store GUIDs in binary or text form, controlled by the BinaryGuid
+                            connection string parameter (default is 'true'). The BinaryGuid setting will affect
+                            how to cast GUID to string in SQL. NHibernate will attempt to detect this
+                            setting automatically from the connection string, but if the connection
+                            or connection string is being handled by the application instead of by NHibernate,
+                            you can use the <literal>sqlite.binaryguid</literal> NHibernate setting to override the behavior.
+                            The value can be <literal>true</literal> or <literal>false</literal>.
+                        </entry>
+                    </row>
                     <row>
                         <entry>
                             <literal>nhibernate-logger</literal>

src/NHibernate.Test/NHSpecificTest/NH3426/Fixture.cs

@@ -1,14 +1,57 @@
 using System;
 using System.Linq;
+using NHibernate.Cfg;
 using NHibernate.Cfg.MappingSchema;
+using NHibernate.Dialect;
 using NHibernate.Mapping.ByCode;
 using NUnit.Framework;
+using Environment = NHibernate.Cfg.Environment;
 
 namespace NHibernate.Test.NHSpecificTest.NH3426
 {
-	[TestFixture]
+	/// <summary>
+	/// Verify that we can convert a GUID column to a string in the standard GUID format inside
+	/// the database engine.
+	/// </summary>
+	[TestFixture(true)]
+	[TestFixture(false)]
 	public class Fixture : TestCaseMappingByCode
 	{
+		private readonly bool _useBinaryGuid;
+
+		public Fixture(bool useBinaryGuid)
+		{
+			_useBinaryGuid = useBinaryGuid;
+		}
+
+		protected override bool AppliesTo(Dialect.Dialect dialect)
+		{
+			// For SQLite, we run the tests for both storage modes (SQLite specific setting).
+			if (dialect is SQLiteDialect)
+				return true;
+
+			// For all other dialects, run the tests only once since the storage mode
+			// is not relevant. (We use the case of _useBinaryGuid==true since this is probably
+			// what most engines do internally.)
+			return _useBinaryGuid;
+		}
+
+		protected override void Configure(Configuration configuration)
+		{
+			base.Configure(configuration);
+
+			if (Dialect is SQLiteDialect)
+			{
+				var connStr = configuration.Properties[Environment.ConnectionString];
+
+				if (_useBinaryGuid)
+					connStr += "BinaryGuid=True;";
+				else
+					connStr += "BinaryGuid=False;";
+
+				configuration.Properties[Environment.ConnectionString] = connStr;
+			}
+		}
 
 		protected override HbmMapping GetMappings()
 		{
@@ -56,7 +99,7 @@ public void SelectGuidToString()
 					.Select(x => new { Id = x.Id.ToString() })
 					.ToList();
 
-				Assert.AreEqual(id.ToUpper(), list[0].Id.ToUpper());
+				Assert.That(list[0].Id.ToUpper(), Is.EqualTo(id.ToUpper()));
 			}
 		}
 
@@ -98,5 +141,53 @@ public void CompareStringColumnWithNullableGuidToString()
 				Assert.That(list, Has.Count.EqualTo(1));
 			}
 		}
+
+		[Test]
+		public void SelectGuidToStringImplicit()
+		{
+			if (Dialect is SQLiteDialect && _useBinaryGuid)
+				Assert.Ignore("Fails with BinaryGuid=True due to GH-2109. (2019-04-09).");
+
+			if (Dialect is FirebirdDialect || Dialect is MySQLDialect || Dialect is Oracle8iDialect)
+				Assert.Ignore("Since strguid() is not applied, it fails on Firebird, MySQL and Oracle " +
+							"because a simple cast cannot be used for GUID to string conversion on " +
+							"these dialects. See GH-2109.");
+
+			using (var session = OpenSession())
+			{
+				// Verify in-db GUID to string conversion when ToString() is applied to the entity that has
+				// a GUID id column (that is, we deliberately avoid mentioning the Id property). This
+				// exposes bug GH-2109.
+				var list = session.Query<Entity>()
+								.Select(x => new { Id = x.ToString() })
+								.ToList();
+
+				Assert.That(list[0].Id.ToUpper(), Is.EqualTo(id.ToUpper()));
+			}
+		}
+
+		[Test]
+		public void WhereGuidToStringImplicit()
+		{
+			if (Dialect is SQLiteDialect && _useBinaryGuid)
+				Assert.Ignore("Fails with BinaryGuid=True due to GH-2109. (2019-04-09).");
+
+			if (Dialect is FirebirdDialect || Dialect is MySQLDialect || Dialect is Oracle8iDialect)
+				Assert.Ignore("Since strguid() is not applied, it fails on Firebird, MySQL and Oracle " +
+							"because a simple cast cannot be used for GUID to string conversion on " +
+							"these dialects. See GH-2109.");
+
+			using (var session = OpenSession())
+			{
+				// Verify in-db GUID to string conversion when ToString() is applied to the entity that has
+				// a GUID id column (that is, we deliberately avoid mentioning the Id property). This
+				// exposes bug GH-2109.
+				var list = session.Query<Entity>()
+								.Where(x => x.ToString().ToUpper() == id)
+								.ToList();
+
+				Assert.That(list, Has.Count.EqualTo(1));
+			}
+		}
 	}
 }

src/NHibernate/Cfg/Environment.cs

@@ -280,6 +280,18 @@ public static string Version
 		/// </summary>
 		public const string FirebirdDisableParameterCasting = "firebird.disable_parameter_casting";
 
+		/// <summary>
+		/// <para>
+		/// SQLite can store GUIDs in binary or text form, controlled by the BinaryGuid
+		/// connection string parameter (default is 'true'). The BinaryGuid setting will affect
+		/// how to cast GUID to string in SQL. NHibernate will attempt to detect this
+		/// setting automatically from the connection string, but if the connection
+		/// or connection string is being handled by the application instead of by NHibernate,
+		/// you can use the 'sqlite.binaryguid' NHibernate setting to override the behavior.
+		/// </para>
+		/// </summary>
+		public const string SqliteBinaryGuid = "sqlite.binaryguid";
+
 		/// <summary>
 		/// <para>Set whether tracking the session id or not. When <see langword="true"/>, each session 
 		/// will have an unique <see cref="Guid"/> that can be retrieved by <see cref="ISessionImplementor.SessionId"/>,
@@ -540,5 +552,40 @@ private static IObjectsFactory CreateCustomObjectsFactory(string assemblyQualifi
 			}
 		}
 
+
+		/// <summary>
+		/// Get a named connection string, if configured.
+		/// </summary>
+		/// <exception cref="HibernateException">
+		/// Thrown when a <see cref="ConnectionStringName"/> was found 
+		/// in the <c>settings</c> parameter but could not be found in the app.config.
+		/// </exception>
+		internal static string GetNamedConnectionString(IDictionary<string, string> settings)
+		{
+			string connStringName;
+			if (!settings.TryGetValue(ConnectionStringName, out connStringName))
+				return null;
+
+			ConnectionStringSettings connectionStringSettings = ConfigurationManager.ConnectionStrings[connStringName];
+			if (connectionStringSettings == null)
+				throw new HibernateException($"Could not find named connection string '{connStringName}'.");
+
+			return connectionStringSettings.ConnectionString;
+		}
+
+
+		/// <summary>
+		/// Get the configured connection string, from <see cref="ConnectionString"/> if that
+		/// is set, otherwise from <see cref="ConnectionStringName"/>, or null if that isn't
+		/// set either.
+		/// </summary>
+		internal static string GetConfiguredConnectionString(IDictionary<string, string> settings)
+		{ 
+			// Connection string in the configuration overrides named connection string.
+			if (!settings.TryGetValue(ConnectionString, out string connString))
+				connString = GetNamedConnectionString(settings);
+
+			return connString;
+		}
 	}
 }

src/NHibernate/Connection/ConnectionProvider.cs

@@ -65,22 +65,15 @@ public virtual void Configure(IDictionary<string, string> settings)
 		}
 
 		/// <summary>
-		/// Get the .NET 2.0 named connection string 
+		///  Get a named connection string, if configured.
 		/// </summary>
 		/// <exception cref="HibernateException">
 		/// Thrown when a <see cref="Environment.ConnectionStringName"/> was found 
-		/// in the <c>settings</c> parameter but could not be found in the app.config
+		/// in the <c>settings</c> parameter but could not be found in the app.config.
 		/// </exception>
 		protected virtual string GetNamedConnectionString(IDictionary<string, string> settings)
 		{
-			string connStringName;
-			if(!settings.TryGetValue(Environment.ConnectionStringName, out connStringName))
-				return null;
-
-			ConnectionStringSettings connectionStringSettings = ConfigurationManager.ConnectionStrings[connStringName];
-			if (connectionStringSettings == null)
-				throw new HibernateException(string.Format("Could not find named connection string {0}", connStringName));
-			return connectionStringSettings.ConnectionString;
+			return Environment.GetNamedConnectionString(settings);
 		}
 
 		/// <summary>

src/NHibernate/Dialect/SQLiteDialect.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Data;
 using System.Data.Common;
 using System.Text;
@@ -18,6 +19,13 @@ namespace NHibernate.Dialect
 	/// </remarks>
 	public class SQLiteDialect : Dialect
 	{
+		/// <summary>
+		/// The effective value of the BinaryGuid connection string parameter.
+		/// The default value in SQLite is true.
+		/// </summary>
+		private bool _binaryGuid = true;
+
+
 		/// <summary>
 		/// 
 		/// </summary>
@@ -94,8 +102,50 @@ protected virtual void RegisterFunctions()
 
 			// NH-3787: SQLite requires the cast in SQL too for not defaulting to string.
 			RegisterFunction("transparentcast", new CastFunction());
-
-			RegisterFunction("strguid", new SQLFunctionTemplate(NHibernateUtil.String, "substr(hex(?1), 7, 2) || substr(hex(?1), 5, 2) || substr(hex(?1), 3, 2) || substr(hex(?1), 1, 2) || '-' || substr(hex(?1), 11, 2) || substr(hex(?1), 9, 2) || '-' || substr(hex(?1), 15, 2) || substr(hex(?1), 13, 2) || '-' || substr(hex(?1), 17, 4) || '-' || substr(hex(?1), 21) "));
+
+			if (_binaryGuid)
+				RegisterFunction("strguid", new SQLFunctionTemplate(NHibernateUtil.String, "substr(hex(?1), 7, 2) || substr(hex(?1), 5, 2) || substr(hex(?1), 3, 2) || substr(hex(?1), 1, 2) || '-' || substr(hex(?1), 11, 2) || substr(hex(?1), 9, 2) || '-' || substr(hex(?1), 15, 2) || substr(hex(?1), 13, 2) || '-' || substr(hex(?1), 17, 4) || '-' || substr(hex(?1), 21) "));
+			else
+				RegisterFunction("strguid", new SQLFunctionTemplate(NHibernateUtil.String, "cast(?1 as char)"));
+		}
+
+
+		public override void Configure(IDictionary<string, string> settings)
+		{
+			base.Configure(settings);
+
+			ConfigureBinaryGuid(settings);
+
+			// Re-register functions depending on settings.
+			RegisterFunctions();
+		}
+
+		private void ConfigureBinaryGuid(IDictionary<string, string> settings)
+		{
+			// We can use a SQLite specific setting to force it, but in common cases it
+			// should be detected automatically from the connection string below.
+			settings.TryGetValue(Cfg.Environment.SqliteBinaryGuid, out var strBinaryGuid);
+
+			if (string.IsNullOrWhiteSpace(strBinaryGuid))
+			{
+				string connectionString = Cfg.Environment.GetConfiguredConnectionString(settings);
+				if (!string.IsNullOrWhiteSpace(connectionString))
+				{
+					var builder = new DbConnectionStringBuilder {ConnectionString = connectionString};
+
+					strBinaryGuid = GetConnectionStringProperty(builder, "BinaryGuid");
+				}
+			}
+
+			// Note that "BinaryGuid=false" is supported by System.Data.SQLite but not Microsoft.Data.Sqlite.
+
+			_binaryGuid = string.IsNullOrWhiteSpace(strBinaryGuid) || bool.Parse(strBinaryGuid);
+		}
+
+		private string GetConnectionStringProperty(DbConnectionStringBuilder builder, string propertyName)
+		{
+			builder.TryGetValue(propertyName, out object propertyValue);
+			return (string) propertyValue;
 		}
 
 		#region private static readonly string[] DialectKeywords = { ... }