dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.73k stars 3.18k forks source link

Execute SQL on connection open #15906

Closed jamiewinder closed 4 years ago

jamiewinder commented 5 years ago

Hi. I'm using EF Core with a database that implements row-level security. In order for this to work, I need to ensure that certain session variables are set whenever the connection is opened and before any other commands are allowed to run.

In other words, I need to run an SQL command immediately every time the connection opens.

Judging by the response to #14769, the StateChange event on the connection seems to be a good place to do this, so I'm trying to do the following:

        public void ConnectAsTenant(
            Guid userId)
        {
            var connection = this.Database.GetDbConnection();
            connection.StateChange += (_, evt) =>
            {
                if (evt.CurrentState == System.Data.ConnectionState.Open)
                {
                    try
                    {
                        var cmd = $"SET SESSION tenant.user_id TO '{userId}';";
                        this.Database.ExecuteSqlCommand(cmd);
                    }
                    catch
                    {
                        this.Database.CloseConnection();
                        throw;
                    }
                }
            };
        }

However, it appears that executing SQL within this event handler is not allowed. I get an exception of:

A second operation started on this context before a previous operation completed. This is usually caused by different threads using the same instance of DbContext, however instance members are not guaranteed to be thread safe. This could also be caused by a nested query being evaluated on the client, if this is the case rewrite the query avoiding nested invocations.'

That originates from the ExecuteSqlCommand line.

Is this to be expected? How else can I achieve this? The only workaround I can think of is to force the connection open before the context is accessed and run the above straight away as I believe this causes the connection to stay open rather than be subject to opening / closing at EF's discretion.

ajcvickers commented 5 years ago

@jamiewinder This is something we will discuss, but I don't have high expectations that we will make it work. The intention of the workaround on #14796 is not that EF code would be called when EF is opening the connection, but instead that the connection was used directly by creating a DbCommand from the connection and executing SQL on that.

jamiewinder commented 5 years ago

Thanks. I actually got it to work in end, though it isn't pretty. I'll post it here in case anyone else finds it useful / can find fault with it:

        public void AsTenant(Guid userId)
        {
            var connection = this.Database.GetDbConnection();
            connection.StateChange += (sender, evt) =>
            {
                if (evt.CurrentState == System.Data.ConnectionState.Open)
                {
                    var senderConnection = (DbConnection)sender;
                    try
                    {
                        var commandText = $"SET SESSION tenant.user_id TO '{userId}';";
                        using (var command = senderConnection.CreateCommand())
                        {
                            command.CommandText = commandText;
                            command.ExecuteNonQuery();
                        }
                    }
                    catch
                    {
                        senderConnection.Close();
                        throw;
                    }
                }
            };
        }

The key part being using the connection object that emitted the event to execute the command rather than trying to do it through the context (since that'll presumably still be in a non-ready state during the handler).

EDIT - Added using block around command in response to @ajcvickers feedback below.

ajcvickers commented 5 years ago

@jamiewinder Looks good. However, the command should be disposed.

ajcvickers commented 5 years ago

Notes from triage: we believe that this should work with 3.0 once the guard is fixed to be a true multi-threading guard and is placed only around the parts that really need to be protected--which probably should not include the connection open call.

ajcvickers commented 4 years ago

Verified fixed in current master.