Rollback with delay_touching on relationship table may cause "update where NULL" query

[oehlschl]

Sorry for the delay here @mtuckergd; thanks for merging a fix and cutting a new release.

After testing with the new version, there's no infinite loop (👍 ), but for our case at least there is still an erroneous update query that fires in the case of rollbacks to "has_many through" relationships. I was able to replicate this on old versions, so thankfully it's not new behavior, and it's an even narrower edge case than what triggered the infinite loop.

I've done some testing in various scenarios and narrowed it to rolling back after writing both a new record and a new relationship record, as may happens with custom "has_many through" associations. NOTE: I was NOT able to replicate with "has_and_belongs_to_many" relationships.

In these cases, there's no infinite loop, but there is an attempt to touch the removed record from the class that joins it:

D, [2018-08-26T18:16:17.761495 #73358] DEBUG -- :    (0.0ms)  begin transaction
D, [2018-08-26T18:16:17.762345 #73358] DEBUG -- :   SQL (0.1ms)  INSERT INTO "posts" ("created_at", "updated_at") VALUES (?, ?)  [["created_at", "2018-08-27 01:16:17.761768"], ["updated_at", "2018-08-27 01:16:17.761768"]]
D, [2018-08-26T18:16:17.769743 #73358] DEBUG -- :   SQL (0.1ms)  INSERT INTO "tags" ("created_at", "updated_at") VALUES (?, ?)  [["created_at", "2018-08-27 01:16:17.769232"], ["updated_at", "2018-08-27 01:16:17.769232"]]
D, [2018-08-26T18:16:17.772731 #73358] DEBUG -- :   SQL (0.1ms)  INSERT INTO "tag_relationships" ("post_id", "tag_id") VALUES (?, ?)  [["post_id", 2], ["tag_id", 1]]
D, [2018-08-26T18:16:17.773108 #73358] DEBUG -- :    (0.0ms)  rollback transaction
D, [2018-08-26T18:16:17.773287 #73358] DEBUG -- :    (0.0ms)  begin transaction
D, [2018-08-26T18:16:17.773849 #73358] DEBUG -- :   SQL (0.1ms)  UPDATE "tags" SET "updated_at" = '2018-08-27 01:16:17.773340' WHERE "tags"."id" IS NULL
D, [2018-08-26T18:16:17.773994 #73358] DEBUG -- :    (0.0ms)  commit transaction

I have some hunches, but please take a look at the PR with the spec and let me know what you think.

Also, I'm sorry for the late response; if I recall, this was some of the thinking that went into the proposed infinite loop fix.

[oehlschl]

Spec to replicate in #20

FYI also @krainboltgreene

[oehlschl]

From a quick glance, any record that's skipped over here:

activerecord-delay_touching/lib/activerecord/delay_touching.rb

Line 90 in e71ade4

next unless record.persisted?

Should not updated in the query here:

activerecord-delay_touching/lib/activerecord/delay_touching.rb

Line 98 in e71ade4

klass.unscoped.where(klass.primary_key => records).update_all(changes)