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

Replacing OneToMany relationship with a new collection doesn't cause delete on old entities #3309

Closed
Ichtil opened this issue Jan 18, 2024 · 0 comments · Fixed by #3310
Closed
Assignees
Labels
Milestone

Comments

@Ichtil
Copy link
Contributor

Ichtil commented Jan 18, 2024

Expected behavior

An entity has following properties

  • @WhenCreated column
  • @Version column
  • @OneToMany relationship with Cascade.ALL and orphanRemoval=true (in this example lets call the bean field goodsCapacities)

On existing entity with the goodsCapacities filled, when we call setGoodsCapacities(new ArrayList<>()) and filling it with new items and then persisting the entity, the oneToMany relationship is saved correctly. However, the version and when created columns are not updated. I am not sure if this is a wanted behaviour, as the bean itself wasn't changed, but the relationship that the bean owns was changed.

Actual behavior

When we want to force the version/whenModified columns to be updated, we have 2 options

  • calling database.markAsDirty
  • manually setting whenModified field

In both of 2 options (together with .setGoodsCapacities(new ArrayList<>()), the old entities from the goodsCapacities are not deleted. The entity instance after saving does not have the old entities loaded, but after refreshing it, they are loaded.

There is a workaround to replace goodsCapacities.set(new ArrayList<>()) with goodsCapacities.clear() (markAsDirty or manually setting whenModified is still needed to update version).

We have already dealt with similiar issue before #2952. There also seems to be a similiar issue currently open #3213.

Steps to reproduce

Please see attached PR with failing unit tests. !3310


Rob notes:

My take is that this bug hasn't really got anything to do with markAsDirty - its more specifically about:

  • OneToMany with orphanRemoval = true
  • Use of setChildren(new ArrayList()) [where we hit the bug] versus using getChildren().clear()
  • This not working when the parent bean is dirty [via using markAsDirty or making any property on the parent bean dirty ]

There is a question here about whether a parent bean should be updated when it has not been modified per se but a orphanRemoval=true collection has been modified. To me this is a separate question, right now the answer is that the existing behaviour is correct and that apps that what the parent bean updated (update the version and whenModified properties) then use markAsDirty is good and expected.

This bug (with orphanRemoval)

Technically there might be 3 fixes for this bug with 1 of those fixes being - lets stop using setChildren(new ArrayList()) with orphanRemoval collections and instead ALWAYS use clear().

On the parent bean we actually have 2 methods for setting the orphanRemoval collection being:

  // tests fail using this setChildren2()

  public void setChildren2(List<OmBeanListChild> children) {
    // not ideal for use with orphanRemoval as this list is now not tracking orphan removals
    this.children = children;
  }

  // tests pass using this setChildren()

  public void setChildren(List<OmBeanListChild> children) {
    // ebeans BeanList list implementation is used (which tracks orphans being removed) 
    this.children.clear();
    this.children.addAll(children);
  }
  • Workaround: Always use clear() with orphanRemoval
  • BugFix via enhancement? Change the enhancement so that a list assignment for an orphanRemoval collection is actually changed to be a clear() addAll()
  • BugFix via internals of SaveManyBeans ... the bug fix we likely go for here (handle the dirty parent + orphanRemoval + 'vanilla' collection case - the SaveManyBeans isChangedProperty() isn't working for this case).
@Ichtil Ichtil changed the title Setting a new collection on a OneToMany relationship doesn't cause delete on old entities Replacing OneToMany relationship with a new collection doesn't cause delete on old entities Jan 18, 2024
@rbygrave rbygrave added this to the 13.26.1 milestone Feb 9, 2024
@rbygrave rbygrave added the bug label Feb 9, 2024
@rbygrave rbygrave self-assigned this Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants