From ed11834d6ea200d142831c0d173befb048b6d1ab Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Fri, 29 Nov 2024 12:46:26 +0100 Subject: [PATCH] Fix(dml): DML default/nullable management (#10363) RESOLVES FRMW-2813 **What** Extract what we have done in the order module DML migration. This includes all the changes we discussed around default value and nullable management From now on, when the default is being set through the DML on a property, no default initializer will be created, this means that when a mikro orm class is being instantiated, no default will be applied at instantiation time. This fixes the issue when retrieving data and not selecting fields that have a default. The default initializer was taking the relay making you think the data was present except that it was the value of the default initializer. From now on, the default value is applied as follows - db column default - before flush - on persist The same applies for nullable properties/columns Co-authored-by: Harminder Virk <1706381+thetutlage@users.noreply.github.com> --- .../src/dml/__tests__/entity-builder.spec.ts | 21 ++- .../helpers/entity-builder/define-property.ts | 21 ++- .../__tests__/entity.spec.ts | 147 ++++++++++++++++++ 3 files changed, 176 insertions(+), 13 deletions(-) create mode 100644 packages/core/utils/src/dml/integration-tests/__tests__/entity.spec.ts diff --git a/packages/core/utils/src/dml/__tests__/entity-builder.spec.ts b/packages/core/utils/src/dml/__tests__/entity-builder.spec.ts index 31fb8ee1ad..c3b44bde1d 100644 --- a/packages/core/utils/src/dml/__tests__/entity-builder.spec.ts +++ b/packages/core/utils/src/dml/__tests__/entity-builder.spec.ts @@ -755,10 +755,10 @@ describe("Entity builder", () => { const userInstance = new User() - expect(userInstance.username).toEqual(null) + expect(userInstance.username).toEqual(undefined) expect(userInstance.spend_limit).toEqual(undefined) - expect(userInstance.raw_spend_limit).toEqual(null) + expect(userInstance.raw_spend_limit).toEqual(undefined) userInstance.username = "john" expect(userInstance.username).toEqual("john") @@ -1121,7 +1121,7 @@ describe("Entity builder", () => { const metaData = MetadataStorage.getMetadataFromDecorator(User) const userInstance = new User() - expect(userInstance.role).toEqual(null) + expect(userInstance.role).toEqual(undefined) userInstance.role = "admin" expect(userInstance.role).toEqual("admin") @@ -1578,7 +1578,10 @@ describe("Entity builder", () => { expect(metaData.path).toEqual("User") expect(metaData.hooks).toEqual({ - beforeCreate: ["generateId"], + beforeCreate: [ + "generateId", + "deleted_at_setDefaultValueOnBeforeCreate", + ], onInit: ["generateId"], }) @@ -1685,7 +1688,10 @@ describe("Entity builder", () => { expect(metaData.path).toEqual("User") expect(metaData.hooks).toEqual({ - beforeCreate: ["generateId"], + beforeCreate: [ + "generateId", + "deleted_at_setDefaultValueOnBeforeCreate", + ], onInit: ["generateId"], }) @@ -1793,7 +1799,10 @@ describe("Entity builder", () => { expect(metaData.path).toEqual("User") expect(metaData.hooks).toEqual({ - beforeCreate: ["generateId"], + beforeCreate: [ + "generateId", + "deleted_at_setDefaultValueOnBeforeCreate", + ], onInit: ["generateId"], }) diff --git a/packages/core/utils/src/dml/helpers/entity-builder/define-property.ts b/packages/core/utils/src/dml/helpers/entity-builder/define-property.ts index 15a5962732..a52387832f 100644 --- a/packages/core/utils/src/dml/helpers/entity-builder/define-property.ts +++ b/packages/core/utils/src/dml/helpers/entity-builder/define-property.ts @@ -119,13 +119,20 @@ export function defineProperty( /** * Here we initialize nullable properties with a null value */ - if (field.nullable) { - Object.defineProperty(MikroORMEntity.prototype, field.fieldName, { - value: null, - configurable: true, - enumerable: true, - writable: true, - }) + if (isDefined(field.defaultValue) || field.nullable) { + const defaultValueSetterHookName = `${field.fieldName}_setDefaultValueOnBeforeCreate` + MikroORMEntity.prototype[defaultValueSetterHookName] = function () { + if (isDefined(field.defaultValue) && this[propertyName] === undefined) { + this[propertyName] = field.defaultValue + return + } + + if (field.nullable && this[propertyName] === undefined) { + this[propertyName] = null + return + } + } + BeforeCreate()(MikroORMEntity.prototype, defaultValueSetterHookName) } if (SPECIAL_PROPERTIES[field.fieldName]) { diff --git a/packages/core/utils/src/dml/integration-tests/__tests__/entity.spec.ts b/packages/core/utils/src/dml/integration-tests/__tests__/entity.spec.ts new file mode 100644 index 0000000000..4d7c475068 --- /dev/null +++ b/packages/core/utils/src/dml/integration-tests/__tests__/entity.spec.ts @@ -0,0 +1,147 @@ +import { MetadataStorage, MikroORM } from "@mikro-orm/core" +import { model } from "../../entity-builder" +import { + mikroORMEntityBuilder, + toMikroOrmEntities, +} from "../../helpers/create-mikro-orm-entity" +import { createDatabase, dropDatabase } from "pg-god" +import { CustomTsMigrationGenerator, mikroOrmSerializer } from "../../../dal" +import { EntityConstructor } from "@medusajs/types" +import { pgGodCredentials } from "../utils" +import { FileSystem } from "../../../common" +import { join } from "path" + +export const fileSystem = new FileSystem( + join(__dirname, "../../integration-tests-migrations-enum") +) + +describe("EntityBuilder", () => { + const dbName = "EntityBuilder-default" + + let orm!: MikroORM + let User: EntityConstructor + + afterAll(async () => { + await fileSystem.cleanup() + }) + + beforeEach(async () => { + MetadataStorage.clear() + mikroORMEntityBuilder.clear() + + const user = model.define("user", { + id: model.id().primaryKey(), + username: model.text(), + points: model.number().default(0).nullable(), + }) + + ;[User] = toMikroOrmEntities([user]) + + await createDatabase({ databaseName: dbName }, pgGodCredentials) + + orm = await MikroORM.init({ + entities: [User], + tsNode: true, + dbName, + password: pgGodCredentials.password, + host: pgGodCredentials.host, + user: pgGodCredentials.user, + type: "postgresql", + migrations: { + generator: CustomTsMigrationGenerator, + path: fileSystem.basePath, + }, + }) + + const migrator = orm.getMigrator() + await migrator.createMigration() + await migrator.up() + }) + + afterEach(async () => { + await orm.close() + + await dropDatabase( + { databaseName: dbName, errorIfNonExist: false }, + pgGodCredentials + ) + }) + + it("set the points to default value before creating the record", async () => { + let manager = orm.em.fork() + + const user1 = manager.create(User, { + username: "User 1", + }) + expect(user1.points).toBe(undefined) + + await manager.persistAndFlush([user1]) + manager = orm.em.fork() + + const user = await manager.findOne(User, { + id: user1.id, + }) + + expect(await mikroOrmSerializer(user)).toEqual({ + id: user1.id, + username: "User 1", + points: 0, + created_at: expect.any(Date), + updated_at: expect.any(Date), + deleted_at: null, + }) + }) + + it("set the points to null when explicitly set to null", async () => { + let manager = orm.em.fork() + + const user1 = manager.create(User, { + username: "User 1", + points: null, + }) + expect(user1.points).toBe(null) + + await manager.persistAndFlush([user1]) + manager = orm.em.fork() + + const user = await manager.findOne(User, { + id: user1.id, + }) + + expect(await mikroOrmSerializer(user)).toEqual({ + id: user1.id, + username: "User 1", + points: null, + created_at: expect.any(Date), + updated_at: expect.any(Date), + deleted_at: null, + }) + }) + + it("set the points to null during updated", async () => { + let manager = orm.em.fork() + + const user1 = manager.create(User, { + username: "User 1", + }) + expect(user1.points).toBe(undefined) + + await manager.persistAndFlush([user1]) + manager = orm.em.fork() + + user1.points = null + await manager.persistAndFlush([user1]) + + const user = await manager.findOne(User, { + id: user1.id, + }) + expect(await mikroOrmSerializer(user)).toEqual({ + id: user1.id, + username: "User 1", + points: null, + created_at: expect.any(Date), + updated_at: expect.any(Date), + deleted_at: null, + }) + }) +})