WIP: Experiment/inject transaction connection

[briandamaged]

Experimental branch that extracts the connection acquisition logic out of Transaction. Instead, Transactions are provided with a withConnection(..) function that handles the details.

The end-result: Transactions no longer need to figure out if the connection should be released.

[briandamaged]

FYI: I don't have any plans to merge this branch. I just created the PR to get initial feedback on the high-level approach. Afterwards, we can go back and implement this more cleanly.

[briandamaged]

Oops -- I thought I deleted this part already. I'll fix this in a little while. (I currently have the tests running on loop so that we can detect unhandled promise rejections)

[briandamaged]

FYI: This function is currently duplicated in a few places. We can consolidate it as part of the re-implementation.

[briandamaged]

I'm not sure if I'll keep the pipe([...]) function in the re-implementation. It's a functional programming concept that many developers might find mystifying.

[briandamaged]

This PR builds upon #3707 , which makes the diffs a bit challenging to read. So, here's a high-level overview of the concepts.

Originally, all of the Transaction#acquireConnection(..) implementations included some form of the following pattern:

async acquireConnection(config, cb) {
  // Figure out how the connection should be obtained.  Specifically: did the caller
  // provide a connection, or does the Transaction need to procure one itself?
  const configConnection = config && config.connection;
  const connection = configConnection || (await this.client.acquireConnection());

  try {
    // Do other post-processing on the connection, some of which might be
    // implementation-specific
    // ....

    return await cb(connection);
  } finally {
    // Do other post-processing on the connection, some of which might be
    // implementation-specific
    // ....

    if(configConnection) {
      // Do nothing.  The caller passed in a connection explicitly, so it is their
      // responsibility to release it.
    } else {
      // This Transaction instance acquired the connection itself; therefore, it
      // is the Transaction's responsibility to release it.
      await this.client.releaseConnection(connection);
    }
  }
}

As you can see, the Transaction is doing quite a bit of work to figure out the caller's intentions!

With the new code, this logic has been reduced to:

async acquireConnection(connection, cb) {
  try {
    // Do other post-processing on the connection, some of which might be
    // implementation-specific
    // ....

    return await cb(connection);
  } finally {
    // Do other post-processing on the connection, some of which might be
    // implementation-specific
    // ....
  }
}

As you can see, the Transaction no longer needs to figure out if it's responsible for acquiring/releasing the connection. Instead, it can focus on the Dialect-specific details.

So, where does the connection come from, then? Short answer: the Transaction delegates control to a withConnection(..) strategy function, which encapsulates the details. For example:

// The `withConnection` function will be provided by the caller
const { withConnection } = config;

return withConnection(async (connection) {
  return this.acquireConnection(connection, async ()=> {
    // ...
  });
});

So, let's suppose that we're creating a top-level Transaction from a knex instance. In this case, it's the knex instance's responsibility to acquire and release the connection. So, its withConnection(..) implementation will look like this:

async function withConnection(cb) {
  const connection = await this.client.acquireConnection();
  try {
    return await cb(connection);
  } finally {
    await this.client.releaseConnection(connection);
  }
}

In contrast, let's suppose we're creating a nested Transaction. In this case, we always want to reuse the existing connection. So, the withConnection(..) implementation will look like this:

async function withConnection(cb) {
  return cb(outerTx.connection);
}

Notice that it doesn't release the connection. That's because it is the caller's responsibility to release the connection.

Anyway, that's the gist of this refactoring. There's still more work to be done, but it at least extracts this responsibility out of the Transaction class.