Allow to configure transactions

[gajus]

Provide feature equivalent to https://github.com/vitaly-t/pg-promise#configurable-transactions.

[nodefish]

Just in case anyone needs to do this now, you should be able to do it with SET TRANSACTION i.e.:

const res = connection.transaction(async (transactionConnection) => {
    await transactionConnection.query(sql`SET TRANSACTION ISOLATION LEVEL SERIALIZABLE`);
    ...
}

[gajus]

Redundant. This could be done using query just like @nodefish demonstrated.

[karlhorky]

cc @mmkal:

Since this will not be added to Slonik itself, maybe it would be nice to allow a workaround for migrations in @slonik/migrator too - for example, even with transaction isolation level SQL queries, there is no way to currently create a migration with a database creation...

[gajus]

It can be added if there is a benefit to it.

I just don't see what benefit does abstracting it into an API gives.

User's already know (or should know before using) the SQL for transaction isolation levels.

[hcharley]

For what it's worth, as a test isolation tool, I override all the methods of Slonik with a transaction connection:

const withTestDatabaseSlonik = async <T>(url: string, fn: ClientCallback<T>) => {
  const orm = connect(url); // { pool: DatabasePoolType; sql: any; }
  await orm.pool.connect(async (conn) => {
    await conn.query(orm.sql`BEGIN ISOLATION LEVEL SERIALIZABLE;`);

    orm.pool = Object.entries(conn).reduce((acc, [key, val]) => {
      if (key in acc) {
        acc[key] = val;
      }
      return acc;
    }, orm.pool);

    try {
      await fn(orm);
    } catch (e) {
      // Error logging can be helpful:
      if (typeof e.code === 'string' && e.code.match(/^[0-9A-Z]{5}$/)) {
        log.error([e.message, e.code, e.detail, e.hint, e.where].join('\n'));
      }
      throw e;
    } finally {
      await conn.query(orm.sql`ROLLBACK;`);
      await conn.query(orm.sql`RESET ALL;`); // Shouldn't be necessary, but just in case...
    }
  });
};

[mmkal]

@karlhorky I'd be open to adding something to @slonik/migrator - there's no "smartness" re transactions in there now. Since the migrations are plain sql files, it's left to the user. Might make sense to move the discussion to that repo, if you'd like to make an issue (or even better, a PR). I expect it wouldn't be hard to implement, but the API would need to be figured out, and a way to test it.

[karlhorky]

Ok great, thanks @mmkal! I've opened mmkal/pgkit#141 for this.

[AndrewJo]

For those that stumbled upon this issue with the need to use transactions in tests, I wrote a library called mocha-slonik for Mocha test framework to automatically wrap all Slonik's query APIs in transactions that automatically rolls back after each tests. Hope it's useful to others!

[gajus]

You shouldn't need ts-mock-imports if you used a factory pattern to initiate your app, i.e. instead of:

// server.ts
import express, { json } from "express";
import { createPool, sql } from "slonik";

const pool = createPool(process.env.DATABASE_URL);
export const app = express();

app.use(json);
app.post("/articles", async (req, res, next) => {
  const { title, body } = req.body;
  const newArticle = await pool.query(sql`
    INSERT INTO articles (title, body) VALUES (${title}, ${body}) RETURNING *;
  `);

  res.status(201).json(newArticle);
});

app.get("/articles/:articleId", async (req, res, next) => {
  const article = await pool.one(sql`
    SELECT * FROM articles WHERE id = ${req.params.articleId} LIMIT 1;
  `);

  res.json(article);
});

app.listen(8080);

Have:

// createServer.ts
export const createServer = ({app, pool}) => {
  app.use(json);
  app.post("/articles", async (req, res, next) => {
    const { title, body } = req.body;
    const newArticle = await pool.query(sql`
      INSERT INTO articles (title, body) VALUES (${title}, ${body}) RETURNING *;
    `);

    res.status(201).json(newArticle);
  });

  app.get("/articles/:articleId", async (req, res, next) => {
    const article = await pool.one(sql`
      SELECT * FROM articles WHERE id = ${req.params.articleId} LIMIT 1;
    `);

    res.json(article);
  });

  app.listen(8080);
};

// server.ts
import express, { json } from "express";
import { createPool, sql } from "slonik";
import { createServer } from "./createServer";

const pool = createPool(process.env.DATABASE_URL);
export const app = express();

createServer({pool, app});

[AndrewJo]

@gajus I absolutely agree with you on using the factory pattern. I wrote the library to handle generic usage patterns but maybe it's worthwhile to document best practices so that users don't need to resort to hacky solutions like import mocks.

[gajus]

Useful snippet:

type IsolationLevel =
  | 'READ UNCOMMITTED'
  | 'READ COMMITTED'
  | 'REPEATABLE READ'
  | 'SERIALIZABLE';

const setIsolationLevel = async (
  transaction: DatabaseTransactionConnection,
  isolationLevel: IsolationLevel,
) => {
  if (isolationLevel === 'READ UNCOMMITTED') {
    await transaction.query(sql.unsafe`
      SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
    `);
  } else if (isolationLevel === 'READ COMMITTED') {
    await transaction.query(sql.unsafe`
      SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
    `);
  } else if (isolationLevel === 'REPEATABLE READ') {
    await transaction.query(sql.unsafe`
      SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
    `);
  } else if (isolationLevel === 'SERIALIZABLE') {
    await transaction.query(sql.unsafe`
      SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
    `);
  } else {
    throw new Error('Invalid isolation level');
  }
};