Jaguar-dart / jaguar_orm

Source-generated ORM with relations (one-to-one, one-to-many, many-to-many), preloading, cascading, polymorphic relations, etc
https://jaguar-dart.github.io
BSD 3-Clause "New" or "Revised" License
217 stars 52 forks source link

Should remove with cascade in ManyToMany relationship delete related object? #101

Closed MartinHlavna closed 5 years ago

MartinHlavna commented 5 years ago

Hello, i ran to the following use case

  1. Consider two entity models Post and Tag with ManyToMany relationship.
  2. In jaguar_orm, this results in 3 objects. Post, Tag and PostTag object representing single relation between Post and Tag
  3. Generated beans using jaguar_orm_gen are _PostBean, _TagBean and _PostTagBean
  4. _PostBean contains method remove with cascade named parameter
  5. If cascade = true, _PostBean calls postTagBean.detachPost(newModel);
  6. postTagBean.detachPost has following code
    Future<int> detachPost(Post model) async {
    final dels = await findByPost(model.id);
    await removeByPost(model.id);
    final exp = Or();
    for (final t in dels) {
      exp.or(tagBean.id.eq(t.tagId));
    }
    return await tagBean.removeWhere(exp);
    }
  7. Because this is ManyToMany relationship following data situation is very likely to happen.
  8. We have Posts A,B, Tags T1,T2,T3
  9. Post A contains Tags T1,T2
  10. Post B contains Tags T2,T3
  11. Removing Post A would delete also Tags T1, T2. With correctly working referential integrity, this would fail as Tag T2 is referenced by Post B. With disabled referential integrity, this would create "dead" relation Post B => Tag T2
  12. Same scenario could happen by deleting Tag

Proposed solution:

  1. Add new boolean named parameter orphanRemoval to ManyToMany annotation
  2. Default value of true would be closest to current behaviour.
  3. If orphanRemoval is true, cascaded remove should check if there are any more relations that references right side of detached relation. Only if there are no more pivot elements with same right side id, then right side element should be deleted
  4. If orhanRemoval is false, cascaded remove should only delete pivot element ,leaving right side element in database for further usage.
tejainece commented 5 years ago

https://github.com/Jaguar-dart/jaguar_orm/pull/100 makes sure that not all records are removed if there are no associations to detach in pivot table.

Another issue remains that the other end of the many-to-many relation is removed even if there are some other relations left for it. Should it be handled?

MartinHlavna commented 5 years ago

If my proposed solution is OK, I can look at this.

tejainece commented 5 years ago

I agree with your proposal. Only change I would like to propose is instead of orphanRemoval on ManyToMany annotation, it would be nice to have it in generated remove method which shall be passed on to generated detach method.

MartinHlavna commented 5 years ago

Opened PR #106. Did some testing, looks like this works. I was hovewer suprised that remove takes optional positional arguments. It may be bee good in future to change this to named parameters, but that would be breaking change right now.

tejainece commented 5 years ago

@MartinHlavna What if we don't remove the right side at all but just remove the entries from the pivot table.

MartinHlavna commented 5 years ago

Hello, sorry for the delay I had some personal issues that I had to solve and didn't have time for this. Removing only right side could be solution for most users. However there may be use cases where it is desired behaviour to remove right side. Most ORMs in other frameworks tends to support both use cases.

That would be also breaking change.

tejainece commented 5 years ago

Hi,

I am planning to absorb your suggested changes.

tejainece commented 5 years ago

Fixed by https://github.com/Jaguar-dart/jaguar_orm/commit/15a233b5cc5ccdb19eaa8cacda56d2071c08027a