Skip to content

Commit

Permalink
FIX Insert cache only on master connection
Browse files Browse the repository at this point in the history
  • Loading branch information
madeindjs committed May 31, 2021
1 parent 283413d commit fe7c2cc
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 10 deletions.
4 changes: 3 additions & 1 deletion src/cache/DbQueryResultCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
queryRunner = this.getQueryRunner(queryRunner);
if (queryRunner === undefined || queryRunner?.getReplicationMode() === "slave") {
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
Expand Down
5 changes: 5 additions & 0 deletions src/driver/mongodb/MongoQueryRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -505,6 +506,10 @@ export class MongoQueryRunner implements QueryRunner {
async getViews(collectionNames: string[]): Promise<View[]> {
throw new Error(`Schema update queries are not supported by MongoDB driver.`);
}

getReplicationMode(): ReplicationMode {
return 'master';
}

/**
* Checks if database with the given name exist.
Expand Down
4 changes: 4 additions & 0 deletions src/query-runner/BaseQueryRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ export abstract class BaseQueryRunner {
}
}

getReplicationMode(): ReplicationMode {
return this.mode;
}

// -------------------------------------------------------------------------
// Protected Methods
// -------------------------------------------------------------------------
Expand Down
24 changes: 15 additions & 9 deletions src/query-runner/QueryRunner.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -141,6 +142,11 @@ export interface QueryRunner {
*/
getViews(viewPaths: string[]): Promise<View[]>;

/**
* Returns replication mode (ex: `master` or `slave`).
*/
getReplicationMode(): ReplicationMode;

/**
* Checks if a database with the given name exist.
*/
Expand Down
10 changes: 10 additions & 0 deletions test/github-issues/5919/entities.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { Column, Entity, PrimaryGeneratedColumn } from "../../../src";

@Entity()
export class Comment {
@PrimaryGeneratedColumn()
id: number;

@Column()
text: string;
}
107 changes: 107 additions & 0 deletions test/github-issues/5919/issue-6399.ts
Original file line number Diff line number Diff line change
@@ -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);
})
));
});

0 comments on commit fe7c2cc

Please sign in to comment.