Skip to content

Commit

Permalink
fix: prevent transactions in the Cordova driver (typeorm#7771)
Browse files Browse the repository at this point in the history
the cordova driver silently ignores all attempts to begin
or commit transactions.  this leads to data loss and invalid
operations occurring when developers expect transactions to work
and then they are silently ignored
  • Loading branch information
imnotjames authored Jun 28, 2021
1 parent 5cf368a commit fc4133c
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 7 deletions.
50 changes: 46 additions & 4 deletions src/driver/cordova/CordovaQueryRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import {Broadcaster} from "../../subscriber/Broadcaster";
* Runs queries on a single sqlite database connection.
*/
export class CordovaQueryRunner extends AbstractSqliteQueryRunner {

/**
* Database driver used by connection.
*/
driver: CordovaDriver;

// -------------------------------------------------------------------------
// Constructor
// -------------------------------------------------------------------------
Expand Down Expand Up @@ -54,7 +54,7 @@ export class CordovaQueryRunner extends AbstractSqliteQueryRunner {
for (let i = 0; i < result.rows.length; i++) {
resultSet.push(result.rows.item(i));
}

ok(resultSet);
}
}, (err: any) => {
Expand Down Expand Up @@ -98,6 +98,48 @@ export class CordovaQueryRunner extends AbstractSqliteQueryRunner {
});
}*/

/**
* Would start a transaction but this driver does not support transactions.
*/
async startTransaction(): Promise<void> {
throw new Error('Transactions are not supported by the Cordova driver')
}

/**
* Would start a transaction but this driver does not support transactions.
*/
async commitTransaction(): Promise<void> {
throw new Error('Transactions are not supported by the Cordova driver')
}

/**
* Would start a transaction but this driver does not support transactions.
*/
async rollbackTransaction(): Promise<void> {
throw new Error('Transactions are not supported by the Cordova driver')
}

/**
* Removes all tables from the currently connected database.
* Be careful with using this method and avoid using it in production or migrations
* (because it can clear all your database).
*/
async clearDatabase(): Promise<void> {
await this.query(`PRAGMA foreign_keys = OFF;`);
try {
const selectViewDropsQuery = `SELECT 'DROP VIEW "' || name || '";' as query FROM "sqlite_master" WHERE "type" = 'view'`;
const dropViewQueries: ObjectLiteral[] = await this.query(selectViewDropsQuery);

const selectTableDropsQuery = `SELECT 'DROP TABLE "' || name || '";' as query FROM "sqlite_master" WHERE "type" = 'table' AND "name" != 'sqlite_sequence'`;
const dropTableQueries: ObjectLiteral[] = await this.query(selectTableDropsQuery);

await Promise.all(dropViewQueries.map(q => this.query(q["query"])));
await Promise.all(dropTableQueries.map(q => this.query(q["query"])));
} finally {
await this.query(`PRAGMA foreign_keys = ON;`);
}
}

// -------------------------------------------------------------------------
// Protected Methods
// -------------------------------------------------------------------------
Expand All @@ -108,4 +150,4 @@ export class CordovaQueryRunner extends AbstractSqliteQueryRunner {
protected parametrize(objectLiteral: ObjectLiteral, startIndex: number = 0): string[] {
return Object.keys(objectLiteral).map((key, index) => `"${key}"` + "=?");
}
}
}
15 changes: 12 additions & 3 deletions src/schema-builder/RdbmsSchemaBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,15 @@ export class RdbmsSchemaBuilder implements SchemaBuilder {
this.queryRunner = this.connection.createQueryRunner();
// CockroachDB implements asynchronous schema sync operations which can not been executed in transaction.
// E.g. if you try to DROP column and ADD it again in the same transaction, crdb throws error.
if (!(this.connection.driver instanceof CockroachDriver))
const isUsingTransactions = (
!(this.connection.driver instanceof CockroachDriver) &&
this.connection.options.migrationsTransactionMode !== "none"
);

if (isUsingTransactions) {
await this.queryRunner.startTransaction();
}

try {
const tablePaths = this.entityToSyncMetadatas.map(metadata => metadata.tablePath);
// TODO: typeorm_metadata table needs only for Views for now.
Expand All @@ -83,14 +90,16 @@ export class RdbmsSchemaBuilder implements SchemaBuilder {
if (this.connection.queryResultCache)
await this.connection.queryResultCache.synchronize(this.queryRunner);

if (!(this.connection.driver instanceof CockroachDriver))
if (isUsingTransactions) {
await this.queryRunner.commitTransaction();
}

} catch (error) {

try { // we throw original error even if rollback thrown an error
if (!(this.connection.driver instanceof CockroachDriver))
if (isUsingTransactions) {
await this.queryRunner.rollbackTransaction();
}
} catch (rollbackError) { }
throw error;

Expand Down

0 comments on commit fc4133c

Please sign in to comment.