From 4d708d230613d0bfb19ce949bd9d1b5e62b35bd1 Mon Sep 17 00:00:00 2001 From: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com> Date: Fri, 26 Feb 2021 07:58:59 +0100 Subject: [PATCH] hotfix(medusa): Optimize product retrieval (#185) --- .../routes/admin/products/list-products.js | 14 ++++- packages/medusa/src/repositories/product.ts | 52 ++++++++++++++++++- .../medusa/src/services/__tests__/product.js | 21 ++++---- packages/medusa/src/services/product.js | 52 ++++++++++++++----- 4 files changed, 111 insertions(+), 28 deletions(-) diff --git a/packages/medusa/src/api/routes/admin/products/list-products.js b/packages/medusa/src/api/routes/admin/products/list-products.js index e5190416fd..a1aa7b9982 100644 --- a/packages/medusa/src/api/routes/admin/products/list-products.js +++ b/packages/medusa/src/api/routes/admin/products/list-products.js @@ -14,13 +14,23 @@ export default async (req, res) => { selector.q = req.query.q } + let includeFields = [] + if ("fields" in req.query) { + includeFields = req.query.fields.split(",") + } + + let expandFields = [] + if ("expand" in req.query) { + expandFields = req.query.expand.split(",") + } + if ("is_giftcard" in req.query) { selector.is_giftcard = req.query.is_giftcard === "true" } const listConfig = { - select: defaultFields, - relations: defaultRelations, + select: includeFields.length ? includeFields : defaultFields, + relations: expandFields.length ? expandFields : defaultRelations, skip: offset, take: limit, } diff --git a/packages/medusa/src/repositories/product.ts b/packages/medusa/src/repositories/product.ts index 63659e200f..46e01c29bb 100644 --- a/packages/medusa/src/repositories/product.ts +++ b/packages/medusa/src/repositories/product.ts @@ -1,5 +1,53 @@ -import { EntityRepository, Repository } from "typeorm" +import { flatten, groupBy, map, merge } from "lodash" +import { EntityRepository, FindManyOptions, Repository } from "typeorm" import { Product } from "../models/product" @EntityRepository(Product) -export class ProductRepository extends Repository {} +export class ProductRepository 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__/product.js b/packages/medusa/src/services/__tests__/product.js index de1eb7fe78..422bdb42e5 100644 --- a/packages/medusa/src/services/__tests__/product.js +++ b/packages/medusa/src/services/__tests__/product.js @@ -11,7 +11,8 @@ const eventBusService = { describe("ProductService", () => { describe("retrieve", () => { const productRepo = MockRepository({ - findOne: () => Promise.resolve({ id: IdMap.getId("ironman") }), + findOneWithRelations: () => + Promise.resolve({ id: IdMap.getId("ironman") }), }) const productService = new ProductService({ manager: MockManager, @@ -25,8 +26,8 @@ describe("ProductService", () => { it("successfully retrieves a product", async () => { const result = await productService.retrieve(IdMap.getId("ironman")) - expect(productRepo.findOne).toHaveBeenCalledTimes(1) - expect(productRepo.findOne).toHaveBeenCalledWith({ + expect(productRepo.findOneWithRelations).toHaveBeenCalledTimes(1) + expect(productRepo.findOneWithRelations).toHaveBeenCalledWith(undefined, { where: { id: IdMap.getId("ironman") }, }) @@ -42,7 +43,7 @@ describe("ProductService", () => { options: [], collection: { id: IdMap.getId("cat"), title: "Suits" }, }), - findOne: () => ({ + findOneWithRelations: () => ({ id: IdMap.getId("ironman"), title: "Suit", options: [], @@ -137,7 +138,7 @@ describe("ProductService", () => { describe("update", () => { const productRepository = MockRepository({ - findOne: query => { + findOneWithRelations: (rels, query) => { if (query.where.id === IdMap.getId("ironman&co")) { return Promise.resolve({ id: IdMap.getId("ironman&co"), @@ -322,7 +323,7 @@ describe("ProductService", () => { describe("addOption", () => { const productRepository = MockRepository({ - findOne: query => + findOneWithRelations: query => Promise.resolve({ id: IdMap.getId("ironman"), options: [{ title: "Color" }], @@ -395,7 +396,7 @@ describe("ProductService", () => { describe("reorderVariants", () => { const productRepository = MockRepository({ - findOne: query => + findOneWithRelations: query => Promise.resolve({ id: IdMap.getId("ironman"), variants: [{ id: IdMap.getId("green") }, { id: IdMap.getId("blue") }], @@ -453,7 +454,7 @@ describe("ProductService", () => { describe("reorderOptions", () => { const productRepository = MockRepository({ - findOne: query => + findOneWithRelations: query => Promise.resolve({ id: IdMap.getId("ironman"), options: [ @@ -519,7 +520,7 @@ describe("ProductService", () => { describe("updateOption", () => { const productRepository = MockRepository({ - findOne: query => + findOneWithRelations: query => Promise.resolve({ id: IdMap.getId("ironman"), options: [ @@ -594,7 +595,7 @@ describe("ProductService", () => { describe("deleteOption", () => { const productRepository = MockRepository({ - findOne: query => + findOneWithRelations: query => Promise.resolve({ id: IdMap.getId("ironman"), variants: [ diff --git a/packages/medusa/src/services/product.js b/packages/medusa/src/services/product.js index 7e38385f2a..38b3fb4696 100644 --- a/packages/medusa/src/services/product.js +++ b/packages/medusa/src/services/product.js @@ -93,6 +93,17 @@ class ProductService extends BaseService { const query = this.buildQuery_(selector, config) + if (config.relations && config.relations.length > 0) { + query.relations = config.relations + } + + if (config.select && config.select.length > 0) { + query.select = config.select + } + + const rels = query.relations + delete query.relations + if (q) { const where = query.where @@ -122,7 +133,7 @@ class ProductService extends BaseService { } } - return productRepo.find(query) + return productRepo.findWithRelations(rels, query) } /** @@ -143,20 +154,33 @@ class ProductService extends BaseService { * @return {Promise} the result of the find one operation. */ async retrieve(productId, config = {}) { - return this.atomicPhase_(async manager => { - const productRepo = manager.getCustomRepository(this.productRepository_) - const validatedId = this.validateId_(productId) - const query = this.buildQuery_({ id: validatedId }, config) - const product = await productRepo.findOne(query) - if (!product) { - throw new MedusaError( - MedusaError.Types.NOT_FOUND, - `Product with id: ${productId} was not found` - ) - } + const productRepo = this.manager_.getCustomRepository( + this.productRepository_ + ) + const validatedId = this.validateId_(productId) - return product - }) + const query = { where: { id: validatedId } } + + if (config.relations && config.relations.length > 0) { + query.relations = config.relations + } + + if (config.select && config.select.length > 0) { + query.select = config.select + } + + const rels = query.relations + delete query.relations + const product = await productRepo.findOneWithRelations(rels, query) + + if (!product) { + throw new MedusaError( + MedusaError.Types.NOT_FOUND, + `Product with id: ${productId} was not found` + ) + } + + return product } /**