From 790d412bb3aec1864ea4a61555e21020b12d56c7 Mon Sep 17 00:00:00 2001 From: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com> Date: Sun, 28 Feb 2021 10:01:48 +0100 Subject: [PATCH 1/3] hotfix(medusa): Optimize cart retrieval (#189) --- packages/medusa/src/repositories/cart.ts | 52 ++++++++++++++++++- .../medusa/src/services/__tests__/cart.js | 50 ++++++++++-------- packages/medusa/src/services/cart.js | 4 +- 3 files changed, 81 insertions(+), 25 deletions(-) diff --git a/packages/medusa/src/repositories/cart.ts b/packages/medusa/src/repositories/cart.ts index 6955ad4895..89adbf87cf 100644 --- a/packages/medusa/src/repositories/cart.ts +++ b/packages/medusa/src/repositories/cart.ts @@ -1,5 +1,53 @@ -import { EntityRepository, Repository } from "typeorm" +import { EntityRepository, FindManyOptions, Repository } from "typeorm" +import { flatten, groupBy, map, merge } from "lodash" import { Cart } from "../models/cart" @EntityRepository(Cart) -export class CartRepository extends Repository {} +export class CartRepository extends Repository { + public async findWithRelations( + relations: Array = [], + optionsWithoutRelations: Omit, "relations"> = {} + ): Promise { + const entities = await this.find(optionsWithoutRelations) + const entitiesIds = entities.map(({ id }) => id) + + const groupedRelations = {} + for (const rel of relations) { + const [topLevel] = rel.split(".") + if (groupedRelations[topLevel]) { + groupedRelations[topLevel].push(rel) + } else { + groupedRelations[topLevel] = [rel] + } + } + + const entitiesIdsWithRelations = await Promise.all( + Object.entries(groupedRelations).map(([_, rels]) => { + return this.findByIds(entitiesIds, { + select: ["id"], + relations: rels as string[], + }) + }) + ).then(flatten) + const entitiesAndRelations = entitiesIdsWithRelations.concat(entities) + + const entitiesAndRelationsById = groupBy(entitiesAndRelations, "id") + return map(entitiesAndRelationsById, entityAndRelations => + merge({}, ...entityAndRelations) + ) + } + + public async findOneWithRelations( + relations: Array = [], + optionsWithoutRelations: Omit, "relations"> = {} + ): Promise { + // Limit 1 + optionsWithoutRelations.take = 1 + + const result = await this.findWithRelations( + relations, + optionsWithoutRelations + ) + return result[0] + } +} diff --git a/packages/medusa/src/services/__tests__/cart.js b/packages/medusa/src/services/__tests__/cart.js index b268ff4740..ded354f5ee 100644 --- a/packages/medusa/src/services/__tests__/cart.js +++ b/packages/medusa/src/services/__tests__/cart.js @@ -34,7 +34,8 @@ describe("CartService", () => { describe("retrieve", () => { let result const cartRepository = MockRepository({ - findOne: () => Promise.resolve({ id: IdMap.getId("emptyCart") }), + findOneWithRelations: () => + Promise.resolve({ id: IdMap.getId("emptyCart") }), }) beforeAll(async () => { jest.clearAllMocks() @@ -47,10 +48,13 @@ describe("CartService", () => { }) it("calls cart model functions", () => { - expect(cartRepository.findOne).toHaveBeenCalledTimes(1) - expect(cartRepository.findOne).toHaveBeenCalledWith({ - where: { id: IdMap.getId("emptyCart") }, - }) + expect(cartRepository.findOneWithRelations).toHaveBeenCalledTimes(1) + expect(cartRepository.findOneWithRelations).toHaveBeenCalledWith( + undefined, + { + where: { id: IdMap.getId("emptyCart") }, + } + ) }) }) @@ -314,7 +318,7 @@ describe("CartService", () => { } const cartRepository = MockRepository({ - findOne: q => { + findOneWithRelations: (rels, q) => { if (q.where.id === IdMap.getId("cartWithLine")) { return Promise.resolve({ items: [ @@ -472,7 +476,7 @@ describe("CartService", () => { }, } const cartRepository = MockRepository({ - findOne: q => { + findOneWithRelations: (rels, q) => { if (q.where.id === IdMap.getId("withShipping")) { return Promise.resolve({ shipping_methods: [ @@ -574,7 +578,7 @@ describe("CartService", () => { describe("update", () => { const cartRepository = MockRepository({ - findOne: q => { + findOneWithRelations: (rels, q) => { if (q.where.id === "withpays") { return Promise.resolve({ payment_sessions: [ @@ -602,8 +606,8 @@ describe("CartService", () => { cartService.setPaymentSessions = jest.fn() await cartService.update("withpays", {}) - expect(cartRepository.findOne).toHaveBeenCalledWith({ - relations: [ + expect(cartRepository.findOneWithRelations).toHaveBeenCalledWith( + [ "items", "shipping_methods", "shipping_address", @@ -617,8 +621,10 @@ describe("CartService", () => { "discounts.rule", "discounts.regions", ], - where: { id: "withpays" }, - }) + { + where: { id: "withpays" }, + } + ) }) }) @@ -636,7 +642,7 @@ describe("CartService", () => { } const cartRepository = MockRepository({ - findOne: q => { + findOneWithRelations: (rels, q) => { if (q.where.id === IdMap.getId("cannot")) { return Promise.resolve({ items: [ @@ -719,7 +725,7 @@ describe("CartService", () => { }, } const cartRepository = MockRepository({ - findOne: () => Promise.resolve({}), + findOneWithRelations: () => Promise.resolve({}), }) const cartService = new CartService({ manager: MockManager, @@ -792,7 +798,7 @@ describe("CartService", () => { describe("updateBillingAddress", () => { const cartRepository = MockRepository({ - findOne: () => + findOneWithRelations: () => Promise.resolve({ region: { countries: [{ iso_2: "us" }] }, }), @@ -854,7 +860,7 @@ describe("CartService", () => { describe("updateShippingAddress", () => { const cartRepository = MockRepository({ - findOne: () => + findOneWithRelations: () => Promise.resolve({ region: { countries: [{ iso_2: "us" }] }, }), @@ -957,7 +963,7 @@ describe("CartService", () => { ), } const cartRepository = MockRepository({ - findOne: () => + findOneWithRelations: () => Promise.resolve({ items: [ { @@ -1056,7 +1062,7 @@ describe("CartService", () => { describe("setPaymentSession", () => { const cartRepository = MockRepository({ - findOne: () => { + findOneWithRelations: () => { return Promise.resolve({ region: { payment_providers: [ @@ -1165,7 +1171,7 @@ describe("CartService", () => { } const cartRepository = MockRepository({ - findOne: q => { + findOneWithRelations: (rels, q) => { if (q.where.id === IdMap.getId("cart-to-filter")) { return Promise.resolve(cart3) } @@ -1287,7 +1293,7 @@ describe("CartService", () => { }) const cartRepository = MockRepository({ - findOne: q => { + findOneWithRelations: (rels, q) => { switch (q.where.id) { case IdMap.getId("lines"): return Promise.resolve(cart3) @@ -1420,7 +1426,7 @@ describe("CartService", () => { describe("applyDiscount", () => { const cartRepository = MockRepository({ - findOne: q => { + findOneWithRelations: (rels, q) => { if (q.where.id === IdMap.getId("with-d")) { return Promise.resolve({ id: IdMap.getId("cart"), @@ -1599,7 +1605,7 @@ describe("CartService", () => { describe("removeDiscount", () => { const cartRepository = MockRepository({ - findOne: q => { + findOneWithRelations: (rels, q) => { return Promise.resolve({ id: IdMap.getId("cart"), discounts: [ diff --git a/packages/medusa/src/services/cart.js b/packages/medusa/src/services/cart.js index cd9bf09ced..6d1a2f1b1a 100644 --- a/packages/medusa/src/services/cart.js +++ b/packages/medusa/src/services/cart.js @@ -269,7 +269,9 @@ class CartService extends BaseService { query.select = select } - const raw = await cartRepo.findOne(query) + const rels = query.relations + delete query.relations + const raw = await cartRepo.findOneWithRelations(rels, query) if (!raw) { throw new MedusaError( From 6335b04bc4f24210183c8a11a9804b49e4ee0df9 Mon Sep 17 00:00:00 2001 From: olivermrbl Date: Mon, 1 Mar 2021 10:43:51 +0100 Subject: [PATCH 2/3] hotfix(medusa): Efficient product querying --- packages/medusa/src/repositories/product.ts | 12 ++++++-- packages/medusa/src/services/product.js | 33 +++++++++++---------- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/packages/medusa/src/repositories/product.ts b/packages/medusa/src/repositories/product.ts index 46e01c29bb..dd1dc83c8f 100644 --- a/packages/medusa/src/repositories/product.ts +++ b/packages/medusa/src/repositories/product.ts @@ -6,9 +6,17 @@ import { Product } from "../models/product" export class ProductRepository extends Repository { public async findWithRelations( relations: Array = [], - optionsWithoutRelations: Omit, "relations"> = {} + idsOrOptionsWithoutRelations: Omit< + FindManyOptions, + "relations" + > = {} ): Promise { - const entities = await this.find(optionsWithoutRelations) + let entities + if (Array.isArray(idsOrOptionsWithoutRelations)) { + entities = await this.findByIds(idsOrOptionsWithoutRelations) + } else { + entities = await this.find(idsOrOptionsWithoutRelations) + } const entitiesIds = entities.map(({ id }) => id) const groupedRelations = {} diff --git a/packages/medusa/src/services/product.js b/packages/medusa/src/services/product.js index 38b3fb4696..3481160f1f 100644 --- a/packages/medusa/src/services/product.js +++ b/packages/medusa/src/services/product.js @@ -80,7 +80,7 @@ class ProductService extends BaseService { * @param {Object} listOptions - the query object for find * @return {Promise} the result of the find operation */ - list(selector = {}, config = { relations: [], skip: 0, take: 20 }) { + async list(selector = {}, config = { relations: [], skip: 0, take: 20 }) { const productRepo = this.manager_.getCustomRepository( this.productRepository_ ) @@ -101,7 +101,7 @@ class ProductService extends BaseService { query.select = config.select } - const rels = query.relations + let rels = query.relations delete query.relations if (q) { @@ -110,27 +110,28 @@ class ProductService extends BaseService { delete where.description delete where.title - query.join = { - alias: "product", - leftJoinAndSelect: { - variant: "product.variants", - collection: "product.collection", - }, - } - - query.where = qb => { - qb.where(where) - - qb.andWhere( + const raw = await productRepo + .createQueryBuilder("product") + .leftJoinAndSelect("product.variants", "variant") + .leftJoinAndSelect("product.collection", "collection") + .select(["product.id"]) + .where(where) + .andWhere( new Brackets(qb => { - qb.where(`product.title ILIKE :q`, { q: `%${q}%` }) + qb.where(`product.description ILIKE :q`, { q: `%${q}%` }) + .orWhere(`product.title ILIKE :q`, { q: `%${q}%` }) .orWhere(`product.description ILIKE :q`, { q: `%${q}%` }) .orWhere(`variant.title ILIKE :q`, { q: `%${q}%` }) .orWhere(`variant.sku ILIKE :q`, { q: `%${q}%` }) .orWhere(`collection.title ILIKE :q`, { q: `%${q}%` }) }) ) - } + .getMany() + + return productRepo.findWithRelations( + rels, + raw.map(i => i.id) + ) } return productRepo.findWithRelations(rels, query) From 8a621415687cd0e996f038d53d1374f5b8bab7c3 Mon Sep 17 00:00:00 2001 From: olivermrbl Date: Mon, 1 Mar 2021 11:46:32 +0100 Subject: [PATCH 3/3] --amend --- packages/medusa/src/services/product.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/medusa/src/services/product.js b/packages/medusa/src/services/product.js index 3481160f1f..efc7fabde1 100644 --- a/packages/medusa/src/services/product.js +++ b/packages/medusa/src/services/product.js @@ -120,7 +120,6 @@ class ProductService extends BaseService { new Brackets(qb => { qb.where(`product.description ILIKE :q`, { q: `%${q}%` }) .orWhere(`product.title ILIKE :q`, { q: `%${q}%` }) - .orWhere(`product.description ILIKE :q`, { q: `%${q}%` }) .orWhere(`variant.title ILIKE :q`, { q: `%${q}%` }) .orWhere(`variant.sku ILIKE :q`, { q: `%${q}%` }) .orWhere(`collection.title ILIKE :q`, { q: `%${q}%` })