From c1e04605cfda76fa10d53278124f0fa1242f4359 Mon Sep 17 00:00:00 2001 From: Alexandre Rousseau Date: Fri, 28 May 2021 17:21:40 +0200 Subject: [PATCH] FIX Insert cache only on master connection close #5919 --- src/cache/DbQueryResultCache.ts | 4 +- src/driver/mongodb/MongoQueryRunner.ts | 5 ++ src/query-runner/BaseQueryRunner.ts | 4 + src/query-runner/QueryRunner.ts | 24 +++--- test/github-issues/5919/entities.ts | 10 +++ test/github-issues/5919/issue-6399.ts | 107 +++++++++++++++++++++++++ 6 files changed, 144 insertions(+), 10 deletions(-) create mode 100644 test/github-issues/5919/entities.ts create mode 100644 test/github-issues/5919/issue-6399.ts diff --git a/src/cache/DbQueryResultCache.ts b/src/cache/DbQueryResultCache.ts index be243ce6acb..92974cee1f8 100644 --- a/src/cache/DbQueryResultCache.ts +++ b/src/cache/DbQueryResultCache.ts @@ -151,7 +151,9 @@ export class DbQueryResultCache implements QueryResultCache { * Stores given query result in the cache. */ async storeInCache(options: QueryResultCacheOptions, savedCache: QueryResultCacheOptions|undefined, queryRunner?: QueryRunner): Promise { - queryRunner = this.getQueryRunner(queryRunner); + if (queryRunner === undefined || queryRunner?.getReplicationMode() !== "master") { + queryRunner = this.connection.createQueryRunner(); + } let insertedValues: ObjectLiteral = options; if (this.connection.driver instanceof SqlServerDriver) { // todo: bad abstraction, re-implement this part, probably better if we create an entity metadata for cache table diff --git a/src/driver/mongodb/MongoQueryRunner.ts b/src/driver/mongodb/MongoQueryRunner.ts index efc784d24ed..281708c19fb 100644 --- a/src/driver/mongodb/MongoQueryRunner.ts +++ b/src/driver/mongodb/MongoQueryRunner.ts @@ -46,6 +46,7 @@ import {TableUnique} from "../../schema-builder/table/TableUnique"; import {Broadcaster} from "../../subscriber/Broadcaster"; import {TableCheck} from "../../schema-builder/table/TableCheck"; import {TableExclusion} from "../../schema-builder/table/TableExclusion"; +import {ReplicationMode} from "../types/ReplicationMode"; /** * Runs queries on a single MongoDB connection. @@ -505,6 +506,10 @@ export class MongoQueryRunner implements QueryRunner { async getViews(collectionNames: string[]): Promise { throw new Error(`Schema update queries are not supported by MongoDB driver.`); } + + getReplicationMode(): ReplicationMode { + return 'master'; + } /** * Checks if database with the given name exist. diff --git a/src/query-runner/BaseQueryRunner.ts b/src/query-runner/BaseQueryRunner.ts index 9afc083896c..ebaa4c928d7 100644 --- a/src/query-runner/BaseQueryRunner.ts +++ b/src/query-runner/BaseQueryRunner.ts @@ -190,6 +190,10 @@ export abstract class BaseQueryRunner { } } + getReplicationMode(): ReplicationMode { + return this.mode; + } + // ------------------------------------------------------------------------- // Protected Methods // ------------------------------------------------------------------------- diff --git a/src/query-runner/QueryRunner.ts b/src/query-runner/QueryRunner.ts index 6eaf0726818..b7ae2ea08e0 100644 --- a/src/query-runner/QueryRunner.ts +++ b/src/query-runner/QueryRunner.ts @@ -1,18 +1,19 @@ -import {TableColumn} from "../schema-builder/table/TableColumn"; +import {ObjectLiteral} from "../common/ObjectLiteral"; +import {Connection} from "../connection/Connection"; +import {SqlInMemory} from "../driver/SqlInMemory"; +import {IsolationLevel} from "../driver/types/IsolationLevel"; +import {ReplicationMode} from "../driver/types/ReplicationMode"; +import {EntityManager} from "../entity-manager/EntityManager"; +import {ReadStream} from "../platform/PlatformTools"; import {Table} from "../schema-builder/table/Table"; +import {TableCheck} from "../schema-builder/table/TableCheck"; +import {TableColumn} from "../schema-builder/table/TableColumn"; +import {TableExclusion} from "../schema-builder/table/TableExclusion"; import {TableForeignKey} from "../schema-builder/table/TableForeignKey"; import {TableIndex} from "../schema-builder/table/TableIndex"; -import {Connection} from "../connection/Connection"; -import {ReadStream} from "../platform/PlatformTools"; -import {EntityManager} from "../entity-manager/EntityManager"; -import {ObjectLiteral} from "../common/ObjectLiteral"; -import {SqlInMemory} from "../driver/SqlInMemory"; import {TableUnique} from "../schema-builder/table/TableUnique"; import {View} from "../schema-builder/view/View"; import {Broadcaster} from "../subscriber/Broadcaster"; -import {TableCheck} from "../schema-builder/table/TableCheck"; -import {IsolationLevel} from "../driver/types/IsolationLevel"; -import {TableExclusion} from "../schema-builder/table/TableExclusion"; /** * Runs queries on a single database connection. @@ -141,6 +142,11 @@ export interface QueryRunner { */ getViews(viewPaths: string[]): Promise; + /** + * Returns replication mode (ex: `master` or `slave`). + */ + getReplicationMode(): ReplicationMode; + /** * Checks if a database with the given name exist. */ diff --git a/test/github-issues/5919/entities.ts b/test/github-issues/5919/entities.ts new file mode 100644 index 00000000000..9c02b767e01 --- /dev/null +++ b/test/github-issues/5919/entities.ts @@ -0,0 +1,10 @@ +import { Column, Entity, PrimaryGeneratedColumn } from "../../../src"; + +@Entity() +export class Comment { + @PrimaryGeneratedColumn() + id: number; + + @Column() + text: string; +} diff --git a/test/github-issues/5919/issue-6399.ts b/test/github-issues/5919/issue-6399.ts new file mode 100644 index 00000000000..96ca8517333 --- /dev/null +++ b/test/github-issues/5919/issue-6399.ts @@ -0,0 +1,107 @@ +import { expect } from "chai"; +import Sinon from "sinon"; +import { Connection } from "../../../src"; +import { + closeTestingConnections, + createTestingConnections, + reloadTestingDatabases, +} from "../../utils/test-utils"; +import { Comment } from "./entities"; + +describe("github issues > #5919 Caching won't work with replication enabled", () => { + let connections: Connection[]; + + beforeEach(async () => { + connections = await createTestingConnections({ + entities: [Comment], + schemaCreate: true, + dropSchema: true, + cache: true, + enabledDrivers: ["postgres"], + }); + await reloadTestingDatabases(connections); + }); + afterEach(() => closeTestingConnections(connections)); + + it("should not another queryRunner for cache with a given masterQueryRunner", () => + Promise.all( + connections.map(async (connection) => { + const comment1 = new Comment(); + comment1.text = "tata"; + await connection.manager.save(comment1); + + const masterQueryRunner = connection.createQueryRunner( + "master" + ); + const createQueryRunnerSpy = Sinon.spy( + connection, + "createQueryRunner" + ); + + const results1 = await connection + .createQueryBuilder() + .from(Comment, "c") + .cache(true) + .setQueryRunner(masterQueryRunner) + .getRawMany(); + + expect(results1.length).eq(1); + + expect(createQueryRunnerSpy.notCalled); + + // add another one and ensure cache works + const comment2 = new Comment(); + comment2.text = "tata"; + await connection.manager.save(comment2); + + const results2 = await connection + .createQueryBuilder() + .from(Comment, "c") + .cache(true) + .setQueryRunner(masterQueryRunner) + .getRawMany(); + + expect(results2.length).eq(1); + }) + )); + + it("should create another queryRunner for cache with a given slaveQueryRunner", () => + Promise.all( + connections.map(async (connection) => { + const comment1 = new Comment(); + comment1.text = "tata"; + await connection.manager.save(comment1); + + const slaveQueryRunner = connection.createQueryRunner("slave"); + const createQueryRunnerSpy = Sinon.spy( + connection, + "createQueryRunner" + ); + + const results1 = await connection + .createQueryBuilder() + .from(Comment, "c") + .cache(true) + .setQueryRunner(slaveQueryRunner) + .getRawMany(); + + expect(results1.length).eq(1); + + expect(createQueryRunnerSpy.calledOnce); + + // add another one and ensure cache works + const comment2 = new Comment(); + comment2.text = "tata"; + await connection.manager.save(comment2); + + const results2 = await connection + .createQueryBuilder() + .from(Comment, "c") + .cache(true) + .setQueryRunner(slaveQueryRunner) + .getRawMany(); + + expect(results2.length).eq(1); + }) + )); +});