mikro-orm: Persisting collection after wrap(...).assign fails

Describe the bug Ids assigned to a collection via wrap(...).assign are not persisted.

Stack trace

...

To Reproduce Steps to reproduce the behavior:

@Entity()
export class Book2 {
  @ManyToMany({ entity: () => BookTag2, cascade: [], fixedOrderColumn: 'order' })
  tags = new Collection<BookTag2>(this);
}
...
const id1 = 1; //id of existing tag
const book = new Book2();
wrap(book).assign({
  tags: [id1]
})
orm.em.persistAndFlus(book);

Expected behavior I expect that assigning ids to collections works just like assigning an id to a reference with wrap(...).assign and would be persisted normally.

Additional context Problem also exists when assigning entities, see PR.

Versions

Dependency Version
node 10.18
typescript ?
mikro-orm 3.6.4
your-driver test is mysql but I observed it with postgres

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 28 (24 by maintainers)

Commits related to this issue

Most upvoted comments

currently even passing object with primaryKey id is trying to insert it instead of update it

Yes, this ORM always worked like that - presence of a PK does not mean anything. Only way the ORM will issue update query (from flush operation) is when you have the entity already loaded and you mutate its state. Calling assign can never do that, it is sync method that mutates what you already have loaded.

assign helper will never work like this, because you want to issue update queries, while this ORM is change set based - it first needs to understand the state to be able to update it. Moreover this would never work for client side generated PKs (like UUID). For this reason I also don’t see a way to implement the recursive flag at all - the method would need to be async or we would need to allow doing updates on references (not loaded entities) via UoW - which is something I was thinking about just recently, so just an early idea that will need a bit of experiments…

If you want such helper, write it yourself, it’s not that hard :] See the EntityAssigner class, it is less than 150 loc (whitespace included).

Nope, we can’t change ORM internals because of how assign helper works. Definitely not such integral part like EntityFactory. The change needs to be in the EntityAssigner. Also in general using Object.assign won’t work, as it will never handle things like references and identity look ups - instead the assign call should be propagated like if you would call it manually on the child property.

If you want to experiment with something you’d like to propose, it’s best to clone the ORM and try what your changed do in the test suite. I can assure you this change would make a lot of tests fail.

mergeObjects has nothing to do with entities, that is for “scalar object props”, e.g. json columns, not for relations. This issue is about persisting the collection (1:m, which was previously not supported unless all the items were loaded), not about updating its items (which sounds like what you are up to).

The assign helper does not work this way, it only handles the root entity and its properties. So it will handle updating what items are in a collection, but not how they should be updated. The code is quite simple, take a look yourself. I would not be opposed by allowing what you want (maybe under some flag, but probably enabled by default, like recursive: true).

https://github.com/mikro-orm/mikro-orm/blob/master/packages/core/src/entity/EntityAssigner.ts

Hi there, I found this issue while trying to understand why updates within a collection where not being persisted. I think I’ve read the comments here correctly (and I think I understand them right) in which case my use case should work.

I have an entity that has a collection and when I update that entity with one of the collection members also containing updates, those updates should propagate down and all be persisted. This isn’t working for me and it could totally be user error (I’m not an expert here).

I thought I’d create a super simple repo to reproduce the problem. Would appreciate any guidance but understand this is open source and your likely swamped so no pressure. Big thanks for the library.

Example repo - https://github.com/chasevida/mikro-orm-wrap

I will need to think about this more, maybe we could fire the necessary update query if we see dirty collection on inverse side and we see that the owners are not loaded (I don’t think we would have to load the owners this way). Currently the inverse side collection are never marked as dirty.

Just for you to understand, the problem is that the changeset that would trigger this is on the owning side (where the m:1 property is), so as long as it is not initialized there is no changeset for that entity, therefore no queries made.

Thanks, will do that.