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

em.persist(entity) under specific conditions empties removeStack and causes em.flush() to error. #1003

Closed
Jcoke opened this issue Oct 28, 2020 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@Jcoke
Copy link

Jcoke commented Oct 28, 2020

Describe the bug
Deleting a child entity with a relation to parent X, then persisting a new child entity with a relation to the same parent causes the removeStack to be emptied, and em.flush() to error.

I load a parent (OneToMany) and its children (ManyToOne) from the database:

const storedParent = em.findOneOrFail(Parent, "parentId", {populate: ["children"]})

I delete one of the children using em.remove():

em.remove(storedParent.children[0])
console.log(em.getUnitOfWork().getRemoveStack().size) // 1

When I persist a new Child entity with a relation to the Parent entity, the removeStack is emptied:

const newChild = new Child().assign(
  { id: "newChildId", parent: storedParent },
  { em }
);
em.persist(newChild)
console.log(em.getUnitOfWork().getRemoveStack().size) // 0

After this, em.flush() errors on duplicate primary key as it tries to insert the removed child:

em.flush() // Error, duplicate PrimaryKey on storedParent.children[0].id

To Reproduce

Entity definition:

@Entity()
export class Parent{
  @PrimaryKey()
  id: string;

  @OneToMany({
    entity: () => Child,
    mappedBy: (c) => c.parent,
  })
  children = new Collection<Child>(this);
}

@Entity()
export class Child{
  @PrimaryKey()
  id: string;

  @ManyToOne({
    entity: () => Parent,
    wrappedReference: true,
    index: true,
    onDelete: "cascade",
  })
  parent: IdentifiedReference<Parent>;

  @Field()
  @Property({ persist: false })
  get parentId(): string {
    return this.parent.id;
  }
}

Create intial dataset:

const parent = new Parent().assign(
  { id: "parentId" },
  { em }
);
em.persist(parent);

const child1 = new Child().assign(
  { id: "childId1", parent },
  { em }
);
em.persist(child1);

const child2 = new Child().assign(
  { id: "childId2", parent },
  { em }
);
em.persist(child2);

await em.flush();

Query created data, delete a child, create and persist a new child

const storedParent = await em.findOneOrFail(Parent, "parentId", {
  populate: ["children"],
});
const removeChild = storedParent.children[0]

// Remove child
em.remove(removeChild)

// Remove stack gets + 1 element
console.log(em.getUnitOfWork().getRemoveStack().size);

// Add unrelated child to same parent
const newChild = new Child().assign(
  {
    id: "newChildId",
    parent: storedParent,
  }
  { em }
);

// Remove stack size is still 1
console.log(em.getUnitOfWork().getRemoveStack().size);

// Persist 
em.persist(newChild);

// Remove stack is empty after persist
console.log(em.getUnitOfWork().getRemoveStack().size);

// Flushing errors on trying to create a child with the id of the removed child
em.flush()

Current fix: explicitly removing the child from the parent as well, but this feels unintuitive.

em.remove(removeChild)
storedParent.children.remove(removeChild);

Expected behavior
I would expect that a child can be added (persisted) to a parent, after another child has been deleted from the same parent using em.remove(child)

Versions

Dependency Version
node 12.16.2
typescript 3.9.7
mikro-orm 4.2.1
postgres 12
@B4nan B4nan added the bug Something isn't working label Oct 28, 2020
@B4nan
Copy link
Member

B4nan commented Oct 28, 2020

Might be issue with the assign helper, it is meant for updating existing entities, not really for inserting new entities. Better to use em.create() (it will call the ctor for you, with the right parameters).

But the issue will be probably cascade persist, as persisting the child is then cascaded to the parent property, and persisting an entity marked for removal will clear the entity from remove stack automatically.

edit: I can confirm the behaviour you described (as well as that em.create() won't help here). Thanks for the repro with detailed description! Only if all the bug reports looked like this 👍

@B4nan B4nan closed this as completed in a9a1bee Oct 28, 2020
@B4nan
Copy link
Member

B4nan commented Oct 28, 2020

You can try the fix in 4.2.4-dev.73 before 4.3 is out.

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