From 5d9aea053ce6e04f242f86fb9053c13dec515d5b Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Wed, 27 Mar 2024 15:31:26 +0100 Subject: [PATCH] fix(utils): Mikro orm prop filtering should check existence (#6842) **What** - Should return non decorated but visible properties. - Should only prevent circular on non scalar fields --- .changeset/light-socks-collect.md | 5 ++ .../modules/__tests__/inventory/index.spec.ts | 6 ++- .../promotion/admin/list-campaigns.spec.ts | 4 -- .../src/dal/mikro-orm/__fixtures__/utils.ts | 53 +++++++++++++++++++ .../__tests__/mikro-orm-serializer.spec.ts | 39 +++++++++++--- .../src/dal/mikro-orm/mikro-orm-serializer.ts | 24 +++++---- 6 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 .changeset/light-socks-collect.md diff --git a/.changeset/light-socks-collect.md b/.changeset/light-socks-collect.md new file mode 100644 index 0000000000..88da18de95 --- /dev/null +++ b/.changeset/light-socks-collect.md @@ -0,0 +1,5 @@ +--- +"@medusajs/utils": patch +--- + +fix(utils): Mikro orm prop filtering should check existence diff --git a/integration-tests/modules/__tests__/inventory/index.spec.ts b/integration-tests/modules/__tests__/inventory/index.spec.ts index 50815c9887..0f1e150a2e 100644 --- a/integration-tests/modules/__tests__/inventory/index.spec.ts +++ b/integration-tests/modules/__tests__/inventory/index.spec.ts @@ -1,9 +1,11 @@ import { IInventoryServiceNext, IStockLocationService } from "@medusajs/types" -import { ContainerRegistrationKeys } from "@medusajs/utils" +import { + ContainerRegistrationKeys, + remoteQueryObjectFromString, +} from "@medusajs/utils" import { ModuleRegistrationName } from "@medusajs/modules-sdk" import { createAdminUser } from "../../../helpers/create-admin-user" -import { remoteQueryObjectFromString } from "@medusajs/utils" const { medusaIntegrationTestRunner } = require("medusa-test-utils") diff --git a/integration-tests/modules/__tests__/promotion/admin/list-campaigns.spec.ts b/integration-tests/modules/__tests__/promotion/admin/list-campaigns.spec.ts index 331a66bb9e..2f30c08c80 100644 --- a/integration-tests/modules/__tests__/promotion/admin/list-campaigns.spec.ts +++ b/integration-tests/modules/__tests__/promotion/admin/list-campaigns.spec.ts @@ -78,7 +78,6 @@ medusaIntegrationTestRunner({ ends_at: expect.any(String), budget: { id: expect.any(String), - campaign: expect.any(Object), type: "spend", limit: 1000, used: 0, @@ -108,7 +107,6 @@ medusaIntegrationTestRunner({ ends_at: expect.any(String), budget: { id: expect.any(String), - campaign: expect.any(Object), type: "usage", limit: 1000, used: 0, @@ -148,7 +146,6 @@ medusaIntegrationTestRunner({ created_at: expect.any(String), budget: { id: expect.any(String), - campaign: expect.any(Object), }, }, { @@ -157,7 +154,6 @@ medusaIntegrationTestRunner({ created_at: expect.any(String), budget: { id: expect.any(String), - campaign: expect.any(Object), }, }, ]) diff --git a/packages/utils/src/dal/mikro-orm/__fixtures__/utils.ts b/packages/utils/src/dal/mikro-orm/__fixtures__/utils.ts index d256dde8ca..8123ea59a4 100644 --- a/packages/utils/src/dal/mikro-orm/__fixtures__/utils.ts +++ b/packages/utils/src/dal/mikro-orm/__fixtures__/utils.ts @@ -229,6 +229,57 @@ class InternalCircularDependencyEntity1 { parent: InternalCircularDependencyEntity1 } +// With un decorated prop + +@Entity() +class Entity1WithUnDecoratedProp { + constructor(props: { id: string; deleted_at: Date | null }) { + this.id = props.id + this.deleted_at = props.deleted_at + } + + unknownProp: string + + @PrimaryKey() + id: string + + @Property() + deleted_at: Date | null + + @OneToMany(() => Entity2WithUnDecoratedProp, (entity2) => entity2.entity1, { + cascade: ["soft-remove"] as any, + }) + entity2 = new Collection(this) +} + +@Entity() +class Entity2WithUnDecoratedProp { + constructor(props: { + id: string + deleted_at: Date | null + entity1: Entity1WithUnDecoratedProp + }) { + this.id = props.id + this.deleted_at = props.deleted_at + this.entity1 = props.entity1 + this.entity1_id = props.entity1.id + } + + unknownProp: string + + @PrimaryKey() + id: string + + @Property() + deleted_at: Date | null + + @ManyToOne(() => Entity1WithUnDecoratedProp, { mapToPk: true }) + entity1_id: string + + @ManyToOne(() => Entity1WithUnDecoratedProp, { persist: false }) + entity1: Entity1WithUnDecoratedProp +} + export { RecursiveEntity1, RecursiveEntity2, @@ -239,4 +290,6 @@ export { DeepRecursiveEntity3, DeepRecursiveEntity4, InternalCircularDependencyEntity1, + Entity1WithUnDecoratedProp, + Entity2WithUnDecoratedProp, } diff --git a/packages/utils/src/dal/mikro-orm/__tests__/mikro-orm-serializer.spec.ts b/packages/utils/src/dal/mikro-orm/__tests__/mikro-orm-serializer.spec.ts index 462ca803bf..9e2f5b5376 100644 --- a/packages/utils/src/dal/mikro-orm/__tests__/mikro-orm-serializer.spec.ts +++ b/packages/utils/src/dal/mikro-orm/__tests__/mikro-orm-serializer.spec.ts @@ -1,19 +1,27 @@ import { MikroORM } from "@mikro-orm/core" -import { Entity1, Entity2 } from "../__fixtures__/utils" +import { + Entity1WithUnDecoratedProp, + Entity2WithUnDecoratedProp, +} from "../__fixtures__/utils" import { mikroOrmSerializer } from "../mikro-orm-serializer" describe("mikroOrmSerializer", () => { beforeEach(async () => { await MikroORM.init({ - entities: [Entity1, Entity2], + entities: [Entity1WithUnDecoratedProp, Entity2WithUnDecoratedProp], dbName: "test", type: "postgresql", }) }) it("should serialize an entity", async () => { - const entity1 = new Entity1({ id: "1", deleted_at: null }) - const entity2 = new Entity2({ + const entity1 = new Entity1WithUnDecoratedProp({ + id: "1", + deleted_at: null, + }) + entity1.unknownProp = "calculated" + + const entity2 = new Entity2WithUnDecoratedProp({ id: "2", deleted_at: null, entity1: entity1, @@ -27,6 +35,7 @@ describe("mikroOrmSerializer", () => { expect(serialized).toEqual({ id: "1", deleted_at: null, + unknownProp: "calculated", entity2: [ { id: "2", @@ -34,6 +43,7 @@ describe("mikroOrmSerializer", () => { entity1: { id: "1", deleted_at: null, + unknownProp: "calculated", }, entity1_id: "1", }, @@ -42,8 +52,13 @@ describe("mikroOrmSerializer", () => { }) it("should serialize an array of entities", async () => { - const entity1 = new Entity1({ id: "1", deleted_at: null }) - const entity2 = new Entity2({ + const entity1 = new Entity1WithUnDecoratedProp({ + id: "1", + deleted_at: null, + }) + entity1.unknownProp = "calculated" + + const entity2 = new Entity2WithUnDecoratedProp({ id: "2", deleted_at: null, entity1: entity1, @@ -57,6 +72,7 @@ describe("mikroOrmSerializer", () => { const expectation = { id: "1", deleted_at: null, + unknownProp: "calculated", entity2: [ { id: "2", @@ -64,6 +80,7 @@ describe("mikroOrmSerializer", () => { entity1: { id: "1", deleted_at: null, + unknownProp: "calculated", }, entity1_id: "1", }, @@ -74,8 +91,13 @@ describe("mikroOrmSerializer", () => { }) it("should serialize an entity preventing circular relation reference", async () => { - const entity1 = new Entity1({ id: "1", deleted_at: null }) - const entity2 = new Entity2({ + const entity1 = new Entity1WithUnDecoratedProp({ + id: "1", + deleted_at: null, + }) + entity1.unknownProp = "calculated" + + const entity2 = new Entity2WithUnDecoratedProp({ id: "2", deleted_at: null, entity1: entity1, @@ -87,6 +109,7 @@ describe("mikroOrmSerializer", () => { expect(serialized).toEqual({ id: "1", deleted_at: null, + unknownProp: "calculated", entity2: [ { id: "2", diff --git a/packages/utils/src/dal/mikro-orm/mikro-orm-serializer.ts b/packages/utils/src/dal/mikro-orm/mikro-orm-serializer.ts index 75ea666d11..44a71527b3 100644 --- a/packages/utils/src/dal/mikro-orm/mikro-orm-serializer.ts +++ b/packages/utils/src/dal/mikro-orm/mikro-orm-serializer.ts @@ -65,26 +65,32 @@ function isPopulated( /** * Customer property filtering for the serialization which takes into account the parent entity to filter out circular references if configured for. - * @param prop + * @param propName * @param meta * @param options * @param parent */ function filterEntityPropToSerialize( - prop: string, + propName: string, meta: EntityMetadata, options: SerializeOptions & { preventCircularRef?: boolean } = {}, parent?: object ): boolean { - const isVisibleRes = isVisible(meta, prop, options) - if (options.preventCircularRef && isVisibleRes && parent) { - return ( - // mapToPk would represent a foreign key and we want to keep them - meta.properties[prop].mapToPk || - parent.constructor.name !== meta.properties[prop].type - ) + const isVisibleRes = isVisible(meta, propName, options) + const prop = meta.properties[propName] + + // Only prevent circular references if prop is a relation + if ( + prop && + options.preventCircularRef && + isVisibleRes && + parent && + prop.reference !== ReferenceType.SCALAR + ) { + // mapToPk would represent a foreign key and we want to keep them + return !!prop.mapToPk || parent.constructor.name !== prop.type } return isVisibleRes }