TortugaResearch / DotNet-ORM-Cookbook

This repository is meant to show how to perform common tasks using C# with variety of ORMs.
https://tortugaresearch.github.io/DotNet-ORM-Cookbook/
The Unlicense
346 stars 29 forks source link

Use Case: Connection Sharing #38

Open mikependon opened 4 years ago

mikependon commented 4 years ago

This scenario is to let an ORM reused a connection object for multiple calls. Each call should open and close the connection.

In some cases, an exception is thrown if the ORM does not handle it.

Grauenwolf commented 4 years ago

Dapper: only works on connections Chain: this is called an "OpenDataSource". EF/EF Core: I don't know if this is a supported scenario Nhibernate: I don't know if this is a supported scenario

Might want to mention how this is important when mixing multiple ORMs in the same transaction.

mikependon commented 4 years ago

Please validate this, I could be bias here. I am dragging what RepoDb is capable of that others might not.

But honestly, this must be supported by most (if not all) ORMs. There should be a way that the user can persists a connection or do a per-call basis. That depends on the situation of the library consumers (ie: Company Network, Design Topology, Architecture, etc etc).

Grauenwolf commented 4 years ago

There's actually two use cases here:

For example, you can get the connection/ transaction from an NHibernate ISession, then use it to perform an operation with Dapper, Chain, or RepoDB.

FransBouma commented 4 years ago

The scenario is really odd, and IMHO doesn't illustrate the necessity of connection sharing: why would I use EF Core at all here or my own ORM, if all it is used for is providing a connection instance? The test simply passes around a connection, but isn't a more real life scenario that you share a transaction with two repositories?

So you update Customer and Order, both are in their own repository and you want to do that in 1 transaction, how to do that in the ORM at hand?

FransBouma commented 4 years ago

To illustrate why this is pretty bad is this code in the EF scenario:

public ConnectionResult<SqlConnection, OrmCookbookContext> OpenConnection()
{
    var context = CreateDbContext();
    context.Database.OpenConnection(); //Force the connection open since we haven't used it yet.
    var connection = (SqlConnection)context.Database.GetDbConnection();
    return (connection, context);
}

the context owns the connection, however the context goes out of scope and the connection is ending up who knows where... It's like in C++ where you instantiate a local instance and return a pointer to it. In C++ at least you'll get a crash immediately as the object you returned a pointer to is gone, but here in .NET code we semi-allow it, but actually it should be doing the same thing: the connection object shouldn't live on here.

I have implemented it in my ORM but it feels dirty all over. For a book which should illustrate best practices in various scenarios, this, in my opinion should absolutely not be included.

FransBouma commented 4 years ago

It would be great if the scenarios are a bit more focused on what a full ORM can do for you instead of these convoluted nonsense scenarios which are only needed when you are using a micro and have to deal with problems you don't have with a full orm. I mean, I can execute all kinds of operations on a single adapter scenario which governs connections, transactions etc. for me, be it an eager load graph or persistence. Even system.transactions management.

We can do better than this.

FransBouma commented 4 years ago

This test can be rewritten so profilers work too:

        [TestMethod]
        public void ShareConnection()
        {
            var repository = GetScenario();
            var factory = CreateFactory();

            (var connection, var state) = repository.OpenConnection();
            Assert.IsNotNull(connection);
            Assert.AreEqual(ConnectionState.Open, connection.State);

            using (var cmd = factory.CreateCommand())
            {
                cmd.Connection = connection;
                cmd.CommandText = "SELECT 1";
                var result = (int)cmd.ExecuteScalar();
                Assert.AreEqual(1, result);
            }

            repository.CloseConnection(state);

            Assert.AreEqual(ConnectionState.Closed, connection.State);
        }

the cmd can be created from the connection instead, so it's already set in the cmd. I'll make this change

(edit) all tests in this scenario need this change. Will change them (edit) sigh... the transaction won't work here...

Nice exercise in frustration, this scenario... (and then I put it mildly. )

FransBouma commented 4 years ago

So I have to add extra code which introduces a massive lame piece of code to satisfy the test. This is what I have to write:

    using(var adapter = new DataAccessAdapter())
    {
        adapter.SetConnectionTransaction(connection, transaction);
        adapter.KeepConnectionOpen=true;
        return adapter.FetchNewEntity<EmployeeClassificationEntity>(
                            new RelationPredicateBucket(EmployeeClassificationFields.EmployeeClassificationKey
                                        .Equal(employeeClassificationKey)));
    }

which fails the test 'UseSharedConnection' as it has to dispose the adapter, which closes the connection (obviously, why would you not close and dispose the connection) and transaction. I now have to add code which skips all that and keeps them alive. If I don't dispose the adapter, I get a warning which results in a compile error so the dispose is required.

No offense but who on their right mind would use this kind of setup in production?

Grauenwolf commented 4 years ago

If I don't dispose the adapter, I get a warning which results in a compile error so the dispose is required.

There's nothing wrong with suppressing errant compiler warnings. But I'll try to think of a way to make the demonstration less clumsy.

No offense but who on their right mind would use this kind of setup in production?

In the real world, mostly I see it when someone wants to mix two ORMs (or an ORM and raw ADO) in the same transaction. I too thought they were crazy when I was first asked to do it because it would be so much easier to use each ORM the normal way and share a connection string.

But since then I've seen in a few more times in my professional work.