Use delete instead of delete-orphan to manage the relationship between Library/ExternalIntegration and ConfigurationSetting.

[leonardr]

This is an attempt to resolve https://jira.nypl.org/browse/SIMPLY-2983, with potential implications for future work.

Coming back to the "delete-orphan" cascade technique (https://docs.sqlalchemy.org/en/14/orm/cascades.html#delete-orphan) after getting more experience with SQLAlchemy, I'm pretty sure it's not suited for a lot of the places we're using it. delete-orphan is designed for:

a related object that is “owned” by its parent, with a NOT NULL foreign key, so that removal of the item from the parent collection results in its deletion.

An example of a place where this is suitable is Patron/Annotation. An Annotation cannot exist without a Patron, so setting annotation.patron = None is the same as deleting it.

An example of a place where I think we should use "delete" instead of "delete-orphan" is Library/CirculationEvent. CirculationEvent.library is a foreign key, but it's not a problem if that value is null. I don't think this comes up a lot because we never actually set CirculationEvent.library = None, but it's incorrect.

And we should definitely be using "delete" instead of "delete-orphan" in Library/ConfigurationSetting and ExternalIntegration/ConfigurationSetting, because there we have two foreign keys, either or both of which might be null. We also have an ongoing problem where ConfigurationSetting objects are randomly deleted in a way that indicates a delete cascade is going haywire.

This branch makes the delete-orphan->delete change for the two relationships where I think this problem is the worst--in fact, where I suspect this problem is causing SIMPLY-2983. Depending on how this goes, I'll do a follow-up branch that changes delete-orphan->delete for the other cases where I think it makes sense.

[EdwinGuzman]

I can probably test this in the UI by creating a Library and ConfigurationSetting and deleting the Library and looking for the ConfigurationSetting in the db?

[leonardr]

I can probably test this in the UI by creating a Library and ConfigurationSetting and deleting the Library and looking for the ConfigurationSetting in the db?

Deleting the Library will trigger the "delete" part of the cascade, so the ConfigurationSettings will still end up deleted. That's worth a try to verify that it still works, but I've never found a good way to trigger the "delete-orphan" part specifically. I believe "delete-orphan" only happens by mistake.

[jonathangreen]

This change looks good to me! Good catch!

I did a little testing in the UI to see how deletes cascade to see if this will leave any configuration settings around that would have been legitimately deleted before and here is what I found:

Test setup

• New CM for testing
• Add a library
• Add a overdrive collection and add that collection to the library

I started fresh and performed that setup before conducting each of these tests:

1. Disassociate the library from the collection
2. Delete the library
3. Delete the collection, then run database_reaper

Look at the settings left in the configurationsettings table after each step. With both the new and the old code, the results were the same, so I don't think these results should hold up this PR.

I was a bit surprised by the results though, so maybe another ticket is in order to make sure that configurationsettings get cleaned up if they are no longer needed.

1. Disassociate the library from the collection

After this the configurationsettings referencing the library and externalintegrations for the collection were still in the table, but their values were set to null.

2. Delete the library

All the configurationsettings referencing the library are removed.

3. Delete the collection, then run database_reaper

All of the configurationsettings and externalintegrations for the collection remain in the database, even though the collection itself is removed.

[leonardr]

Thanks, Jonathan, at least #1 and #3 there look like they need some work. I filed https://jira.nypl.org/browse/SIMPLY-3504 to cover that work.