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

Populating Nested Field returns the wrong number of associated entities #144

Open
thiagomini opened this issue Sep 25, 2023 · 5 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@thiagomini
Copy link
Contributor

thiagomini commented Sep 25, 2023

Describe the bug

  • Given we are using Entity Schema definitions
  • And we have a Company entity class with its schema
  • And a Company has many User's
  • And a User has many Profile's
  • And we have a Company "Acme" that has "User1" and "User2" as users
  • And "User1" has two profiles
  • And "User2" has one profile
  • When we try to fetch the Company from the database, populating both user and profile
  • Then the "User2" has three profiles associated.

The Diagram below illustrates that:

flowchart LR
    A[Company 'A'] -->|Has| B(User 1)
    A[Company 'A'] -->|Has| C(User 2)
    B --> |Has| D(Profile 1)  
    B --> |Has| E(Profile 2)  
    C --> |Has| F(Profile 3)
Loading

Stack trace

AssertionError: expected { Object (0, 1, ...) } to have a length of 1 but got 3

- Expected
+ Received

- 1
+ 3

 ❯ src/mikro-orm/mikro-orm-internal.module.spec.ts:100:45
     98|     // Assert
     99|     const userWithASingleProfile = companyWithProfiles.users.find(u => u.id === user2.id);
    100|     expect(userWithASingleProfile.profiles).toHaveLength(1);
       |                                             ^
    101|   });
    102| 

To Reproduce
Steps to reproduce the behavior:

  1. Clone the minimum reproducible code repository: https://github.com/thiagomini/nest-mikro-orm-example/tree/bug/nested-populated-entities
  2. run yarn
  3. run docker compose up -d
  4. run yarn test

Expected behavior
"User2" should have only ONE associated Profile.

Additional Information

💡 Link to the test case

Versions

Dependency Version
node 18.12.1
typescript 5.2.2
mikro-orm 5.8.3
pg 8.11.2
@thiagomini thiagomini added the bug Something isn't working label Sep 25, 2023
@thiagomini
Copy link
Contributor Author

thiagomini commented Sep 25, 2023

Hey, @B4nan , this is either some lousy mistake on my part or an important issue to fix - It looks like populating nested fields is making the wrong associations, which might cause severe bugs 🤔 . I appreciate it if you could help me out with this one!

💡 Edit: I noticed that if we manually initialize each user's profile collection, it returns the correct data. The issue is probably related to the EntityManager - I'd guess it's not filtering which grand-children entities belong to each child entity - As you can see, it's basically grouping all of the existing profiles (3 in total) and associating them with each user.

@thiagomini
Copy link
Contributor Author

@B4nan Hey again Martin, sorry for bothering you again with this, but that issue has been blocking a bit of our progress in a project here. Could you give us some insight? Maybe just pointing out where in the MikroORM code we could try to fix that issue. Thank you!

@B4nan
Copy link
Member

B4nan commented Sep 28, 2023

Can you create repro without nest js? Just a simple script, ideally everything in a single file.

@thiagomini
Copy link
Contributor Author

Alright, I'll work on it, thanks Martin!

@thiagomini
Copy link
Contributor Author

thiagomini commented Sep 28, 2023

Hey again, @B4nan! Here's the Repro: https://github.com/thiagomini/mikro-orm-repro

And here's the test case: https://github.com/thiagomini/mikro-orm-repro/blob/f48a1d83f8ea9d13da0f0e6f54645a3c7533f2d4/src/mikro-orm.test.ts#L32C2-L32C2

So, apparently, the issue might indeed be with the NestJS package because the very same test here is actually passing 😅 - I'll create another version tomorrow using vitest instead of jest, just to rule out that possibility.

⚠️ Edit: I've added a yarn vitest command that executes it instead of jest, and the test also passes. To it is definitely the Nest package!

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