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

V6. When using STI, and two different children of an STI have properties referencing different children of another STI, an error can occur on update #4440

Closed
YanDjin opened this issue Jun 11, 2023 · 6 comments

Comments

@YanDjin
Copy link

YanDjin commented Jun 11, 2023

Describe the bug
The scenario:

  • STI 1 -> User has children Manager and Employee
  • STI 2 -> Collaborator has children ManagerCollaborator and EmployeeCollaborator
    • Manager has a OneToMany relation to ManagerCollaborator
    • Employee has a OneToMany relation to EmployeeCollaborator

When fetching a ManagerCollaborator, the __originalEntityData will contain two entries with the same data, __originalEntityData .manager and __originalEntityData .employee because the are referencing the same column, even though the property employee does not exist in Manager
if you set this fetched managerCollaborator as a relation of another entity, and flush, the diff between the original data and the current data will get discovered and the EntityManager will try to update the entry.

To Reproduce
Steps to reproduce the behavior:

  • Have two different STIs base entities
  • Two of the children of STI 1 reference in OneToMany relationships different children of STI 2
  • when finding a entity of STI 2, you will have two properties in the __originalEntityData containing the data, the property of the used entity, and the property in the other child

https://github.com/YanDjin/mikro-orm/blob/bug/sti-update/tests/issues/GH4440.test.ts
this test replicates the issue.

Expected behavior
The em should not make the diff using the columns of another child of the STI

Versions

Dependency Version
node 18.16.0
typescript 5.0.4
mikro-orm 6.0.0-dev.86
your-driver postgres/sqlite
@B4nan
Copy link
Member

B4nan commented Jun 14, 2023

That repro is wrong, you can't use a detached entity this way, managerCollaborator is no longer managed after you call em.clear(). You should be doing this instead:

const manager = new Manager();
manager.email = 'bbb@aaa.com';
manager.managerCollaborators.set([orm.em.getReference(ManagerCollaborator, 1)]);
await orm.em.flush();

Or in a single step via em.create:

const manager = orm.em.create(Manager, {
  email: 'bbb@aaa.com',
  managerCollaborators: [1],
});
await orm.em.flush();

@B4nan B4nan closed this as not planned Won't fix, can't repro, duplicate, stale Jun 14, 2023
@B4nan
Copy link
Member

B4nan commented Jun 14, 2023

Maybe we could treat entities managed by a different fork as a reference automatically, so your code would work automatically.

@YanDjin
Copy link
Author

YanDjin commented Jun 14, 2023

The bug occurs even without the em.clear(). I added that when I was trying to figure out from where the bug came from.
I updated the test, sorry for the confusion.

@YanDjin
Copy link
Author

YanDjin commented Jun 17, 2023

Can you please re-open the issue @B4nan ?

@B4nan
Copy link
Member

B4nan commented Jun 18, 2023

The repro is still kinda wrong, you can't redefine a property in the child class, as all the child classes are merged to the base entity type, and that is used internally. This is a known problem, there are other open issues about that, e.g. #2388. I want to look into that in v6 too.

@B4nan
Copy link
Member

B4nan commented Sep 30, 2023

FYI with #4769 your new test is also passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants