Allow passing a connection from publish to executeSql

[ibash]

Adds the option to pass a connection from publish to executeSql.
This allows creating transactions that encompass inserting jobs.

The test seems a bit messy, open to suggestions.

Fixes #199

[coveralls]

Coverage remained the same at 100.0% when pulling 4ffdcf1 on ibash:master into 6fcf398 on timgit:master.

[gavacho]

Maybe this ? can be removed. It looks like options will always be defined based on how its called.

[ibash]

There's a few places where executeSql is called without any values too: https://github.com/timgit/pg-boss/blob/master/src/contractor.js#L26

I'm not super familiar with typescript, is that valid?

[gavacho]

My bad! I assumed executeSql was only called there

[gavacho]

@timgit What do you think about this patch?

[timgit]

A couple things I'd like to see in this PR.

1. Standardize on a db contract so it matches the interface used in the constructor. It's assumed that query() exists, but when we pass a custom db in the constructor, it's executeSql().
2. The only way to use a custom db connection during publish() requires a custom db in the constructor. Some users may not know this and skip the constructor step. If this is how it should behave, this mismatch should throw an error. Another alternative would be to not require the custom db in the constructor and let the publish simply call that passed db object entirely.

[ibash]

Playing catchup on a few things and might not have time to get at this for a few days.

Looking at it with fresh eyes, 100% agree the interface is kind of weird.

Another alternative would be to not require the custom db in the constructor and let the publish simply call that passed db object entirely.

Are you saying instead of this:

await boss.publish({ name: 'passConnection', options: { connection } })

We instead pass in a db object? like this?

const wrapper = {
  async executeSql(text, values) {
    // up to user to do connection.query here ...
  }
}
await boss.publish({ name: 'passConnection', options: { db: wrapper } })

It's a bit more work, but would make it obvious that executeSql is the only thing that matters.

Or, could even do something like this:

const executeSql = async (text, values) => {
  // up to user to do connection.query here ...
}
await boss.publish({ name: 'passConnection', options: { executeSql } })

Personally I prefer the last form, as it gets straight to the point.

[timgit]

Yes, this form aligns with the constructor option, which makes it a bit more clear of what the intention is.

await boss.publish({ name: 'passConnection', options: { db: wrapper } })

I agree it's a bit more setup for the caller, but this should make the internals a bit more ignorant of what's going on. The goal would be to simply swap out the normal db service with an override if it's passed in. This should make publish() ignorant of why a custom db is being passed in. I agree that using transactions would be the most common use case, but this doesn't have to be a "transaction" feature of pg-boss in order to pull it off.

[alexopenkowski-the-nu-company]

Sad that this has never made it into some release. Inserting directly into the job table to use transactions feels a bit error-prone to me, especially when things change under the hood.