Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to configure transactions #30

Closed
gajus opened this issue Feb 15, 2019 · 11 comments
Closed

Allow to configure transactions #30

gajus opened this issue Feb 15, 2019 · 11 comments

Comments

@gajus
Copy link
Owner

gajus commented Feb 15, 2019

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

@nodefish
Copy link

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
Copy link
Owner Author

gajus commented Nov 4, 2019

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

@gajus gajus closed this as completed Nov 4, 2019
@karlhorky
Copy link

karlhorky commented Nov 5, 2019

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
Copy link
Owner Author

gajus commented Nov 5, 2019

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
Copy link

hcharley commented Feb 17, 2020

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
Copy link
Contributor

mmkal commented Feb 17, 2020

@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
Copy link

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

@AndrewJo
Copy link

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
Copy link
Owner Author

gajus commented Oct 18, 2021

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
Copy link

@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
Copy link
Owner Author

gajus commented Mar 28, 2024

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');
  }
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants