mirror of
https://github.com/google/nomulus.git
synced 2025-04-30 03:57:51 +02:00
Also deletes the autorenew poll message history revision id field in Domain, which is only needed to recreate the ofy key for the poll message. The column already contains null values in it, making it impossible to depend on it. The column itself will be deleted from the schema after this PR is deployed. The logic to update autorenew recurrence end time is changed accordingly: When a poll message already exists, we simply update the endtime, but when it no longer exists, i. e. when it's deleted speculatively after a transfer request, we recreate one using the history entry id that resulted in its creation (e. g. cancelled or rejected transfer). This should fix b/240984498. Though the exact reason for that bug is still unclear to me. Namely, it throws an NPE at this line during an explicit domain transfer approval: https://cs.opensource.google/nomulus/nomulus/+/master:core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java;l=603;bpv=1;bpt=0;drc=ede919d7dcdb7f209b074563b3d449ebee19118a The domain in question has a null autorenewPollMessageHistoryId, but that in itself should not have caused an NPE because we are not operating on the null pointer. On that line the only possible way to throw an NPE is for the domain itself to be null, but if that were the case, the NPE would have been thrown at line 599 where we called a method on the domain object. Regardless of the cause, with this PR we are using an explicitly provided history id and checking for its nullness before using it. If a similar issue arises again, we should have a better idea why. Lastly, the way poll message id is constructed is largely simplified in PollMessageExternalKeyConverter as a result of the removal ofy parent keys in PollMessage. This does present a possibility of failure when immediately before deployment, a registrar requests a poll message and received the old id, but by the time the registrar acks the id, the new version is deployed and therefore does not recognize the old key. The likelihood of this happening should be slim, and we could have prevented it by letting the converter recognize both the old and the new key. However, we would like to eventually phase out the old key, and in theory a registrar could ack a poll message at any time after it was requested. So, there is not a safe time by which all the old ids are acked, lest we develop some elaborate scheme to keep track of which messages were sent with an old id when requested and which of these old ids are acked. Only then can we be truly safe to phase out the old id. The benefit does not seem to warrant the effort. If a registrar does encounter a situation like this, they could open a support bug to have us manually ack the poll message for them. |
||
---|---|---|
.. | ||
src | ||
build.gradle | ||
buildscript-gradle.lockfile | ||
Dockerfile | ||
gradle.lockfile |