ServiceStack / Issues

Issue Tracker for the commercial versions of ServiceStack
11 stars 8 forks source link

Null reference exception when using connection filter with diagnostic observers #796

Closed jbarrau closed 1 year ago

jbarrau commented 1 year ago

Describe the issue

When we include a ConnectionFilter which includes a SQL query performed through ServiceStack.OrmLite, the ConnectionFilter function is applied to the inner DB connection which is an SqlConnection instead of an OrmLiteConnection (here). This does not seem to be properly taken into account by the code from OrmLiteDiagnostics here: dbCmd is an OrmLiteCommand, however its inner DB connection is an SqlConnection, therefore the method GetConnectionId() which calls the command's dbConn.ConnectionId will throw a null reference exception.

Reproduction

The following piece of code can be executed as an independent unit test to reproduce the null reference exception (it only needs references to Microsoft.Data.SqlClient and ServiceStack packages and it uses MSSQLLocalDB to have a local database):

using System;
using System.Collections.Generic;
using System.Data;
using System.Diagnostics;
using NUnit.Framework;
using ServiceStack;
using ServiceStack.OrmLite;
using ServiceStack.OrmLite.SqlServer;
using DataException = ServiceStack.Data.DataException;

namespace ReproduceOrmLiteIssue.Tests
{
    [TestFixture]
    public class Tests
    {
        private const string DbName = "ReproduceOrmLiteIssue_UnitTest";
        private static readonly string LocalDbConnString = $"Data Source=(localdb)\\MSSQLLocalDB;Initial Catalog={DbName};Integrated Security=True;";
        private const string MasterDbConnString = "Data Source=(localdb)\\MSSQLLocalDB;Initial Catalog=master;Integrated Security=True;";
        private static readonly IOrmLiteDialectProvider DialectProvider = new SqlServerOrmLiteDialectProvider();

        [Test]
        public void ReproduceNullReferenceException()
        {
            DropAndRecreateDatabase(DbName);

            // Perform a first DB query to make sure that the OrmLite diagnostic listener gets registered
            var dbConnectionFactory = new OrmLiteConnectionFactory(LocalDbConnString, DialectProvider) { ConnectionFilter = ConnectionFilterDefaultSchemaSanityCheck };
            using var connection = dbConnectionFactory.OpenDbConnection();
            connection.Scalar<int>("SELECT 1");

            // Add an observer to trigger the code from the diagnostics
            DiagnosticListener.AllListeners.Subscribe(new TestDiagnosticObserver());

            // Run another DB query
            using var newConnection = dbConnectionFactory.OpenDbConnection();
            Assert.AreEqual(1, newConnection.Scalar<int>("SELECT 1")); // <-- this line triggers a null reference exception
        }

        /// <summary>
        /// Check that the user's default schema is "dbo". Some code may fail if it's anything else.
        /// </summary>
        private static IDbConnection ConnectionFilterDefaultSchemaSanityCheck(IDbConnection connection)
        {
            var defaultSchema = connection.Scalar<string>("SELECT SCHEMA_NAME()");
            if (defaultSchema != "dbo")
            {
                throw new DataException($"The default schema should be \"dbo\", not \"{defaultSchema}\"");
            }

            return connection;
        }

        private static void DropAndRecreateDatabase(string dbName)
        {
            using var connection = new OrmLiteConnectionFactory(MasterDbConnString, DialectProvider).OpenDbConnection();

            connection.ExecuteNonQuery($"IF db_id('{dbName}') IS NOT NULL ALTER DATABASE [{dbName}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE");
            connection.ExecuteNonQuery($"IF db_id('{dbName}') IS NOT NULL DROP DATABASE [{dbName}]");
            connection.ExecuteNonQuery($"CREATE DATABASE [{dbName}] COLLATE Latin1_General_CS_AS");
        }
    }

    public sealed class TestDiagnosticObserver : 
        IObserver<DiagnosticListener>, 
        IObserver<KeyValuePair<string, object>>
    {
        private readonly List<IDisposable> subscriptions = new();

        void IObserver<DiagnosticListener>.OnNext(DiagnosticListener diagnosticListener)
        {
            Console.WriteLine($@"diagnosticListener: {diagnosticListener.Name}");
            if (diagnosticListener.Name == Diagnostics.Listeners.OrmLite)
            {
                var subscription = diagnosticListener.Subscribe(this);
                subscriptions.Add(subscription);
            }
        }

        public void OnCompleted()
        {
        }

        public void OnError(Exception error)
        {
        }

        public void OnNext(KeyValuePair<string, object> kvp)
        {
        }

        void IObserver<DiagnosticListener>.OnError(Exception error)
        {
        }

        void IObserver<DiagnosticListener>.OnCompleted()
        {
            subscriptions.ForEach(x => x.Dispose());
            subscriptions.Clear();
        }
    }
}

The exception which is produced has the following error message and stack trace:

System.NullReferenceException : Object reference not set to an instance of an object.
   at ServiceStack.OrmLite.OrmLiteCommand.get_ConnectionId() in /home/runner/work/ServiceStack/ServiceStack/ServiceStack.OrmLite/src/ServiceStack.OrmLite/OrmLiteCommand.cs:line 21
   at ServiceStack.OrmLite.OrmLiteConnectionFactoryExtensions.GetConnectionId(IDbCommand dbCmd) in /home/runner/work/ServiceStack/ServiceStack/ServiceStack.OrmLite/src/ServiceStack.OrmLite/OrmLiteConnectionFactory.cs:line 345
   at ServiceStack.OrmLite.OrmLiteExecFilter.Exec[T](IDbConnection dbConn, Func`2 filter) in /home/runner/work/ServiceStack/ServiceStack/ServiceStack.OrmLite/src/ServiceStack.OrmLite/OrmLiteExecFilter.cs:line 62
   at ServiceStack.OrmLite.OrmLiteReadExpressionsApi.Exec[T](IDbConnection dbConn, Func`2 filter) in /home/runner/work/ServiceStack/ServiceStack/ServiceStack.OrmLite/src/ServiceStack.OrmLite/OrmLiteReadExpressionsApi.cs:line 18
   at ServiceStack.OrmLite.OrmLiteReadApi.Scalar[T](IDbConnection dbConn, String sql, Object anonType) in /home/runner/work/ServiceStack/ServiceStack/ServiceStack.OrmLite/src/ServiceStack.OrmLite/OrmLiteReadApi.cs:line 229
   at ReproduceOrmLiteIssue.Tests.Tests.ConnectionFilterDefaultSchemaSanityCheck(IDbConnection connection) in C:\shift\Source\Shift\Tests\ReproduceOrmLiteIssue.Tests\Tests.cs:line 44
   at ServiceStack.OrmLite.OrmLiteConnection.Open() in /home/runner/work/ServiceStack/ServiceStack/ServiceStack.OrmLite/src/ServiceStack.OrmLite/OrmLiteConnection.cs:line 112
   at ServiceStack.OrmLite.OrmLiteConnectionFactory.OpenDbConnection() in /home/runner/work/ServiceStack/ServiceStack/ServiceStack.OrmLite/src/ServiceStack.OrmLite/OrmLiteConnectionFactory.cs:line 95
   at ReproduceOrmLiteIssue.Tests.Tests.ReproduceNullReferenceException() in C:\shift\Source\Shift\Tests\ReproduceOrmLiteIssue.Tests\Tests.cs:line 35

Expected behavior

We would expect no exception to happen

System Info

Windows, ServiceStack version 6.10, .NET 6.0, JetBrains Rider 2023.2

Additional context

No response

Validations

mythz commented 1 year ago

Thanks for the repro, this should now be resolved from this commit that's available from v6.10.1+ that's now available in our Pre Release Packages.

jbarrau commented 1 year ago

Thanks!