Change default cascade strategy for new applications for Bidirectional one-to-many with belongsTo

[jameskleeh]

Could be a setting? The default setting should be all-delete-orphan

[graemerocher]

Could you write up the argument for this, just so that if anyone else stumbles on this issue they know

[jameskleeh]

Sure!

Given the following domain classes:

class User {
    static hasMany = [posts: Post]
}

class Post { 
    static belongsTo = [user: User]
}

The following code will result in all posts for the given user getting orphaned, not deleted. I haven't heard a valid use case for why this would be desirable behavior.

User user = User.get(1)
user.posts = null
user.save()

[hakanernam]

This may seem wrong for this particular example, but I can think of some scenarios where orphans may be adopted by other entities.

Vehicles belonging to firms. firm may go away, but a vehicle may be acquired by some other firm. Records such as entry/exit records for said vehicle should not be lost. (Yes, this is from an actual production application more or less in similar form)

I would not claim this is good or bad programming, but would claim that changing default behavior in a potentially data destroying way would definitely not desirable.

Especially if a new grails version with the new default ends up removing customer data due to old code relying on old behavior and programmer not being aware of the change. Even knowing the change may not prevent data destruction due to oversight.

Sorry if I misunderstood the point, but really shivered at the thought. =-O

[jameskleeh]

@hakanernam As the title suggests, this change would only apply to new applications. Existing applications would not be affected. I didn't intend to suggest that a use case for the current way it works doesn't exist, just that I hadn't heard of one. From all of the people that I've talked to about this issue over the last couple years, I haven't spoken to one that found the default behavior to best fit their use case. That is why I am suggesting to change the default for new applications only

[jameskleeh]

@graemerocher If it would make the change more agreeable, just making this configurable (application wide) would be a great step in the right direction.

[hakanernam]

@Schlogen Thanks for the clarification. My take on this is anything that can destroy data should be explicit rather than implicit(magical?) If the programmer wants to orphan entities deleted, he/she can accomplish this explicitly by mapping configuration.

I apologize if I seem anxious, but it is due to https://github.com/grails/grails-core/issues/9410 almost getting me (so I thought for 30 seconds) during an production upgrade last week. Yes, there were backups but never, ever would I want to tell the customer a 5 minute downtime now has to extend to restoring backups, “cause I destroyed your data, but no worries”. Imagine my relief when I realized the reason I could not login was a logging exception, not recreated tables by ddl due to me removing “update” from production dbCreate by mistake.

So I submit to you that the worst thing a system, framework, program can ever do is destroy customer data unintentionally or even implicitly.

That said, as a learning exercise about grails internals, I read the doc on the (Section 6.5.2.9 Custom Cascade Behaviour) subject, and hibernate documentation on transitive persistence it refers to. There, I realize keeping orphan entities is also hibernate default behavior:

Now consider the same scenario with parent and child objects being entities, not value-types (e.g. categories and items, or parent and child cats). Entities have their own life cycle and support shared references. Removing an entity from the collection does not mean it can be deleted), and there is by default no cascading of state from one entity to any other associated entities. Hibernate does not implement persistence by reachability by default.

Also it recommends:

If the child object's lifespan is bounded by the lifespan of the parent object, make it a life cycle object by specifying

cascade="all,delete-orphan"(@OneToMany(cascade=CascadeType.ALL, orphanRemoval=true)).

Therefore I would ask some questions:

Q. How will such a change implemented?

• Will it be a scaffolding issue where “all-delete-orphan” is automativally added to mapping block for newly created domain classes?
• Will it be change to hibernate interface to add “delete-orphan” (Or Annotation equivalent) to operations implicitly?
• Or something else?

Seems if you go the second way, now my old application is affected by this change after a grails upgrade, potentially me/customer not knowing it until I actually do delete some parent.

Q. How would I tell Grails I want the current behavior in a new application(or even my current one based on how it is implemented)? Would we have something new like “keep-orphans” cascade config? Making configurable solves this somewhat but still...

I apologize for the long post.

[jameskleeh]

@hakanernam I think the best way is to add it to the configuration yml/groovy. Upgraded applications wont have the configuration option and the default (no option set) will be what it is now, all. New applications will have the setting by default and can change it if they wish.

[hakanernam]

@Schlogen Sounds alright. Thanks for the info.

[graemerocher]

I don't think we should change the default behaviour here. What we can do is provide an option to change the default cascade in GORM default mapping for those who want to