Incomplete Entities after using FindOption 'fields'

[montycb]

After loading an entity with the 'fields' FindOption Subsequent load operations will return incomplete entities.
See included testcase

import 'reflect-metadata';
import { Collection, Entity, ManyToMany, ManyToOne, MikroORM, PrimaryKey, Property, wrap, Embeddable, Embedded, EntityCaseNamingStrategy, OneToMany } from '@mikro-orm/core';
import { PostgreSqlDriver } from '@mikro-orm/postgresql';

@Entity()
export class A {
  @PrimaryKey()
    id!: number

  @Property()
    test1!: string

  @ManyToMany({ entity: () => B, mappedBy: 'a' })
    b = new Collection<B>(this)
}

@Entity()
export class B {
  @PrimaryKey()
    id!: number

  @Property()
    test2!: string

  @Property()
    test3!: string

  @ManyToMany({ entity: () => A, inversedBy: 'b' })
    a = new Collection<A>(this)
}


describe('loadItems with fields', () => {
    let orm: MikroORM<SqliteDriver>;

    beforeAll(async () => {
        orm = await MikroORM.init({
            entities: [A, B],
            dbName: ':memory:',
            type: 'sqlite',
        })
    await orm.schema.refreshDatabase()
    })

    test('loadItems with fields', async () => {
      const b = orm.em.create(B, {test2: 'qwer', test3: 'asdf'})
      const a = orm.em.create(A, {test1: 'yxcv', b: [b]})

      await orm.em.persistAndFlush([a, b])
      orm.em.clear()

      await orm.em.findOne(B, 1, {fields: ['test2']}) // fail
      // await orm.em.findOne(B, 1) // ok

      const a2 = await orm.em.findOne(A, 1)
      const test3 = (await a2.b.loadItems())[0].test3
      
      // no 'find' or 'populate' specified, but still undefined if 'fields' was previously used
      expect(test3).toMatch('asdf') 
    })
})

[B4nan]

The problem you are describing was previously expected behavior (generally, e.g. for em.findOne as well), and refresh: true was required to update the already loaded entity instance. That partially changed with v5 and auto-refreshing (#2263), which is apparently not working for Collection.init properly (which is used from the loadItems method). You could use em.populate to get around this, I guess I should really make the refactor to use that internally inside the Collection.init in v6 to resolve this.

await orm.em.populate(a2, ['b']);
const test3 = (await a2.b.loadItems())[0].test3; // the `loadItems` call here is a no-op, as we previously loaded the collection via `em.populate()`

Just to be sure, do you understand that the first findOne(B) call will give you the very same B entity instance? Basically doing em.findOne(B, 1) multiple times will give you the cached result from the identity map, only the very first call will query the database.

[montycb]

Yes, I understand that this is the same entity, that is exactly the problem I'm having. The cached entity is missing some fields, but is not refreshed even though it is necessary in this case. If the entity was added with 'fields' to the cache, I'm always getting the same entity, even without 'fields'. The cache should probably keep track of this.

[B4nan]

The cache should probably keep track of this.

Not really, the fix for this won't change that - you always get only one instance of the entity - the fix will be about loading the missing fields, so the previous instance will no longer be partially loaded. That's why I am asking if you understand this, as that is the core concept here, not a bug. The bug is only in the entity not being reloaded automatically with the loadItems call - but it would be with em.findOne or with em.populate.

If you want to have multiple entity instances, you need to use a separate EM fork (which is what the FindOptions.disableIdentityMap option is doing behind the scenes).

[montycb]

Yes, I understand that. I don't want multiple instances of the same entity. I'm not too familiar with the mikro-orm code, so my assumption with the cache is probably wrong. I simply want, that the missing fields are automatically loaded into the existing entity.

[B4nan]

I will keep this refactoring for v6, as it might be semi-breaking, the ordering of the collection items might change a bit (although from what I can see in the tests, it was rather wrong in v5 too, as the PK is used for ordering, even if it's a UUID, so probably fine, but let's stay on the safe side). Also there is one serialization problem that this refactoring introduces and is no longer present in v6 due to other refactorings already done in there.

[B4nan]

So this ended up as a pretty painful refactoring, but it's finally working as expected.

Closing as implemented in v6 via #4571.