Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embedded entity without "object" option "breaks" UOW / IdentityMap #4904

Closed
alekbarszczewski opened this issue Nov 8, 2023 · 3 comments
Closed
Labels
bug Something isn't working

Comments

@alekbarszczewski
Copy link

Describe the bug
If entity has embedded property without object: true option then it breaks UnitOfWork / IdentityMap - multiple calls to repository.findOne(PK) always trigger SQL queries despite entity should be "cached" in IdentityMap.

To Reproduce
Entity classes:

// TestEntity1 class
export class TestEntity1 {
  public readonly id: string;
  public readonly customProp: CustomProp;

  constructor(id: string, customProp: CustomProp) {
    this.id = id;
    this.customProp = customProp;
  }
}

// TestEntity2 class (identical to TestEntity1)
export class TestEntity2 {
  public readonly id: string;
  public readonly customProp: CustomProp;

  constructor(id: string, customProp: CustomProp) {
    this.id = id;
    this.customProp = customProp;
  }
}

// CustomProp class
export class CustomProp {
  public readonly someValue: string;

  constructor(someValue: string) {
    this.someValue = someValue;
  }
}

Entity schemas:

// TestEntity1 schema
export const TestEntity1Schema = new EntitySchema({
  class: TestEntity1,
  properties: {
    id: {
      type: 'text',
      primary: true,
    },
    customProp: {
      type: 'CustomProp',
      nullable: false,
      reference: ReferenceType.EMBEDDED,
      object: true, // <-------------------------------- true
    },
  },
});

// TestEntity2 schema
export const TestEntity2Schema = new EntitySchema({
  class: TestEntity2,
  properties: {
    id: {
      type: 'text',
      primary: true,
    },
    customProp: {
      type: 'CustomProp',
      nullable: false,
      reference: ReferenceType.EMBEDDED,
      object: false, // <-------------------------------- false (this is the only difference between those schemas)
    },
  },
});

// CustomProp schema
export const CustomPropSchema = new EntitySchema({
  class: CustomProp,
  embeddable: true,
  properties: {
    someValue: {
      type: 'text',
      nullable: false,
    },
  },
});
// Insert entities (native inserts skip IdentityMap)
await this.testEntity1Repository.nativeInsert(
  new TestEntity1('abc', new CustomProp('yyy')),
);
await this.testEntity2Repository.nativeInsert(
  new TestEntity2('def', new CustomProp('xxx')),
);

// Find TestEntity1 twice by PK
await this.testEntity1Repository.findOne('abc');
await this.testEntity1Repository.findOne('abc'); // should not trigger SQL query

// Find TestEntity2 twice by PK
await this.testEntity2Repository.findOne('def');
await this.testEntity2Repository.findOne('def'); // should not trigger SQL query

// Remove entities (just for cleanup)
this.testEntity1Repository.remove(e1!);
this.testEntity2Repository.remove(e2!);

Current behavior

[query] insert into "test_entity1" ("id", "custom_prop") values ('abc', '{"someValue":"yyy"}') returning "id" [took 4 ms]
[query] insert into "test_entity2" ("id", "custom_prop_some_value") values ('def', 'xxx') returning "id" [took 4 ms]
# TestEntity1 is retrieved from DB once
[query] select "t0".* from "test_entity1" as "t0" where "t0"."id" = 'abc' limit 1 [took 2 ms, 1 result]
# TestEntity2 is retrieved from DB twice
[query] select "t0".* from "test_entity2" as "t0" where "t0"."id" = 'def' limit 1 [took 3 ms, 1 result]
[query] select "t0".* from "test_entity2" as "t0" where "t0"."id" = 'def' limit 1 [took 2 ms, 1 result]
[query] begin
[query] delete from "test_entity1" where "id" in ('abc') [took 2 ms]
[query] delete from "test_entity2" where "id" in ('def') [took 3 ms]
[query] commit

Expected behavior
In example above both entities should be retrieved from DB only once and cached in IdentityMap.
For some reason if entity has embedded property with object: false option then it is not cached in IdentityMap (or can't be found in IdentityMap not sure). Changing entity schema to use object: true fixes the issue, however then embedded property is of JSON type without properties of embedded entity mapped to separate DB table columns.

Versions

Dependency Version
node 18.16.0
typescript 5.2.2
mikro-orm 5.9.3
mikro-orm/postgresql 5.9.3
@B4nan
Copy link
Member

B4nan commented Nov 8, 2023

The problem here is that the ORM thinks your entity is partially loaded, this method returns true:

https://github.com/mikro-orm/mikro-orm/blob/master/packages/core/src/EntityManager.ts#L1920

@alekbarszczewski
Copy link
Author

Got it, but do you think it can/should be fixed? Or maybe there is workaround to mark is as not partially loaded? I tried to add { populate: true } option to findOne() but it does not change anything.

@B4nan
Copy link
Member

B4nan commented Nov 8, 2023

It needs to be fixed :]

@B4nan B4nan added the bug Something isn't working label Nov 8, 2023
@B4nan B4nan closed this as completed in 3df1d17 Nov 8, 2023
B4nan added a commit that referenced this issue Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants