Handle `AtomicInternalId` edge cases
While investigating #288811 (closed), we saw a few quirks related to AtomicInternalId
.
-
We generate the IID in a
before_validation
hook so the IID could be incremented even if the model does not get saved.For example, doing
Epic.new(group: g, title: '').valid?
would increment thelast_value
in theinternal_ids
table. No transaction rollback happens here because#valid?
is not wrapped in a transaction unlike#save
I think we should consider moving the IID hooks to
before_save?
? This is not really a big problem though and the worst it can do is having skipped iids. -
Also related to validation: When validation fails during a
#save
, the IID increment is rolled back but theiid
attribute on the model is kept. And when the model is then made valid and then saved again, theinternal_ids
would now be out of sync because we skip the internal id hooks when theiid
is already set.We have the
track_#{scope}_#{column}!
that's supposed to handle this, but then it has areturn unless @internal_id_needs_tracking
that prevents it from running. The intent of this is so that it only runs when theiid
was set viamodel.iid = xxx
.Reproduction steps:
e = Epic.new(group: g, author: User.first) e.save # This fails because the title is blank but `e.iid` is now set e.title = "Set a title so that it's now valid" e.save # this results in an epic with a new IID but with the `internal_ids.last_value` not incremented