nestjs: Entity id is not populated

Describe the bug

Given we are using Entity Schema definitions
And we have a User entity class with its schema
And we’re using the autoLoadEntitites: true option.
When we execute three tests in a row (using entity manager)
Then we got an error: Error: You cannot call 'EntityManager.findOne()' with empty 'where' parameter

This is how each test looks like:

  test('creates a new user', async () => {
    // Arrange
    const entityManager = testingModule.get(EntityManager);
    const repository = testingModule.get<EntityRepository<User>>(
      getRepositoryToken(User),
    );

    const user = new User({
      email: 'user@mail.com',
      firstName: 'John',
      lastName: 'Doe',
      createdAt: new Date(),
      updatedAt: new Date(),
    });

    // Act
    await entityManager.persistAndFlush(user);
    console.log(user);

    // Assert
    const newUserInDb = await repository.findOne(user.id);
    expect(newUserInDb).toMatchObject({
      id: expect.any(String),
      firstName: 'John',
      lastName: 'Doe',
      createdAt: expect.any(Date),
    });
  });

Stack trace

 src/mikro-orm/mikro-orm-internal.module.spec.ts > MikroOrmInternalModule > creates a new user
Error: You cannot call 'EntityManager.findOne()' with empty 'where' parameter
 ❯ EntityValidator.validateEmptyWhere node_modules/@mikro-orm/core/entity/EntityValidator.js:90:19
 ❯ SqlEntityManager.findOne node_modules/@mikro-orm/core/EntityManager.js:307:22
 ❯ src/mikro-orm/mikro-orm-internal.module.spec.ts:103:25
    101| 
    102|     // Assert
    103|     const newUserInDb = await repository.findOne(user.id);
       |                         ^
    104|     expect(newUserInDb).toMatchObject({
    105|       id: expect.any(String),

To Reproduce Steps to reproduce the behavior:

Clone the minimum reproducible code repository: https://github.com/thiagomini/nest-mikro-orm-example/tree/bug/entity-id-is-not-populated (Watch out for the specific branch)
run yarn
run docker compose up -d
run yarn test

Expected behavior All tests should have passed

Additional context

The weird thing is that it fails only when we run exactly three tests in a row. If you run only one or two, they pass.
You can test the same error running it with jest by executing yarn run jest
I noticed that in the third test, the EntityManager.unitOfWork.identityMap is empty - even when calling the persistAndFlush(), it looks like the ORM isn’t able to recognize it
⚠️ This is almost certainly related to schema definitions. I’ve created a separate branch that is using decorators instead, and all tests pass.

Versions

Dependency	Version
node	18.12.1
typescript	5.1.3
mikro-orm	next
mikro-orm/postgresql	next
pg	8.11.2

About this issue

Original URL
State: closed
Created a year ago
Comments: 23 (23 by maintainers)

Most upvoted comments

Because the forFeature calls were adding more entries to the entities array on each app init. Havent checked it more deeply, but I still think its about the multiple discovery of the entity schema instances.

B4nan on Aug 17, 2023

One unrelated note about the tests - this will always just give you the user entity you already have, as it is loaded in the context and you query for it by the primary key. There is no query fired, and you get the exact same object reference as user.

const newUserInDb = await repository.findOne(user.id);

B4nan on Aug 15, 2023

Sorry for the confusion, it’s actually about something else, its about having the __originalEntityData property - that is the state of the entity when it was loaded (or last flushed), the state that reflects what is in the database. If we see that, its considered as update.

https://github.com/mikro-orm/mikro-orm/blob/master/packages/core/src/unit-of-work/ChangeSetComputer.ts#L29

B4nan on Aug 15, 2023

Try v5.2.1

B4nan on Aug 16, 2023

Ok, I think I found it.

B4nan on Aug 16, 2023

Ok, this is pretty tricky, but the issue seems to be that you are calling MikroORM.init in the beforeEach hook, and discovering the same entity class for each test, over and over. Haven’t found the exact cause yet, I’d expect it will be something about rediscovering the same EntitySchema instance multiple times, as it is you who create that instance, as opposed to decorators where it will be created internally, so every time you call the init method.

Long time haven’t seen anything so weird :]

Ohh, I see, thanks for the context, Martin! Is there a way to enforce this, or am I just using it wrong?

There are several things you can do, the common theme is that you need a new fork. Either first call em.clear(), or do em.fork().findOne(), or use disableIdentityMap: true in the find options (which creates a temporary fork under the hood, so basically the same), or use refresh: true there, which will ignore the identity map for this request.

B4nan on Aug 15, 2023

Clearing the context means you remove the link from the EM to the entities, but they still hold the state object (WrappedEntity instance), that is what decides whether the entity is considered managed or not. You should never reuse the same entity instance across multiple EM forks, this is a side effect of doing so.

It’s basically about the WrappedEntity.__initialized property, we could probably unset this when calling em.clear, but it feels a bit wrong, that specific entity instance is initialized already. Moreover, you could as well merge an existing (and initialized) entity instance, calling em.clear() would suddenly unset this flag, but that entity was initialized by the time you added it to the context, so why should clearing the context change it anyhow?

Just thinking, I am not entirely sure about this, and I am open to discussion. But doing such changes would be almost certainly breaking and would have to go to v6 if something.

B4nan on Aug 15, 2023

I see you are rolling back the changes in your afterEach hook, maybe its connected to that. I haven’t tried this approach myself, but I recall one issue from someone who made it work and provided some example repo…

Could be as well something with resolving the EM from nest DI - so first thing I would do is to verify whether its an issue with the DI or with the ORM itself. I would bet on the nest DI, as I’d expect to see such issue already if it would be about the ORM. But maybe people don’t use the EntitySchema that often, hard to say.

B4nan on Aug 15, 2023

This is indeed pretty weird. We have tests for the EntitySchema definition, but looking at the entities now, we dont have any for the bigints. Maybe there will be some in the tests/issues, havent check those…

The code that handles mapping the PK to entity is here:

https://github.com/mikro-orm/mikro-orm/blob/master/packages/core/src/unit-of-work/ChangeSetPersister.ts#L109-L111

Its an autoincrement column, so with postgres it should be part of the returning clause, that is the first thing to ensure. Then the mapPrimaryKey should be used for mapping the PK for single entity flush, when the insertId is returner from the driver, and alternatively the mapReturnedValues method handles hydrating all the other columns from the returning clause. The reloadVersionValues method is there to ensure this works with mysql, where we dont have the returning clause support and need to fire a separate query to fetch the actual values. But for that we need the PK (it needs to be mapped via the mapPrimaryKey).

The weird thing is that it fails only when we run exactly three tests in a row. If you run only one or two, they pass.

This sounds like you are messing up with the context.

B4nan on Aug 15, 2023