mirror of
https://github.com/google/nomulus.git
synced 2025-05-13 07:57:13 +02:00
Fix a null check bug in HostUpdateFlow
This bug is about a bad use of Optional. We were checking == null instead of .isPresent(), so the check always passed, and we always set a lastSubordinateTime when updating hosts, even if the host was external and should have had a null value in that field. There is almost certainly bad data in prod in the sense that any external host that was ever updated will have a value for this field instead of null. However, this is not consequential as the field is entirely meaningless for external hosts, and will be properly reset if the host is ever moved to be internal. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=146363178
This commit is contained in:
parent
a3baa58cad
commit
223e8c2316
2 changed files with 3 additions and 1 deletions
|
@ -155,7 +155,7 @@ public final class HostUpdateFlow implements TransactionalFlow {
|
||||||
// the lookupSuperordinateDomain(...) call above, so that it will never be stale.
|
// the lookupSuperordinateDomain(...) call above, so that it will never be stale.
|
||||||
.setSuperordinateDomain(
|
.setSuperordinateDomain(
|
||||||
superordinateDomain.isPresent() ? Key.create(superordinateDomain.get()) : null)
|
superordinateDomain.isPresent() ? Key.create(superordinateDomain.get()) : null)
|
||||||
.setLastSuperordinateChange(superordinateDomain == null ? null : now)
|
.setLastSuperordinateChange(superordinateDomain.isPresent() ? now : null)
|
||||||
.build()
|
.build()
|
||||||
// Rely on the host's cloneProjectedAtTime() method to handle setting of transfer data.
|
// Rely on the host's cloneProjectedAtTime() method to handle setting of transfer data.
|
||||||
.cloneProjectedAtTime(now);
|
.cloneProjectedAtTime(now);
|
||||||
|
|
|
@ -562,6 +562,8 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
|
||||||
// The last transfer time should be what was on the superordinate domain at the time of the host
|
// The last transfer time should be what was on the superordinate domain at the time of the host
|
||||||
// update, not what it is later changed to.
|
// update, not what it is later changed to.
|
||||||
assertThat(renamedHost.getLastTransferTime()).isEqualTo(lastTransferTime);
|
assertThat(renamedHost.getLastTransferTime()).isEqualTo(lastTransferTime);
|
||||||
|
// External hosts should always have null lastSuperordinateChange.
|
||||||
|
assertThat(renamedHost.getLastSuperordinateChange()).isNull();
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue