diff --git a/java/google/registry/model/EppResourceUtils.java b/java/google/registry/model/EppResourceUtils.java index b013917cc..8a85ac251 100644 --- a/java/google/registry/model/EppResourceUtils.java +++ b/java/google/registry/model/EppResourceUtils.java @@ -44,6 +44,7 @@ import google.registry.model.transfer.TransferData; import google.registry.model.transfer.TransferStatus; import java.util.List; import java.util.Map.Entry; +import java.util.Optional; import java.util.Set; import java.util.function.Function; import javax.annotation.Nullable; @@ -94,7 +95,7 @@ public final class EppResourceUtils { @Nullable public static T loadByForeignKey( Class clazz, String foreignKey, DateTime now) { - return loadByForeignKeyHelper(clazz, foreignKey, now, false); + return loadByForeignKeyHelper(clazz, foreignKey, now, false).orElse(null); } /** @@ -120,15 +121,13 @@ public final class EppResourceUtils { * @param foreignKey id to match * @param now the current logical time to project resources at */ - @Nullable - public static T loadByForeignKeyCached( + public static Optional loadByForeignKeyCached( Class clazz, String foreignKey, DateTime now) { return loadByForeignKeyHelper( clazz, foreignKey, now, RegistryConfig.isEppResourceCachingEnabled()); } - @Nullable - private static T loadByForeignKeyHelper( + private static Optional loadByForeignKeyHelper( Class clazz, String foreignKey, DateTime now, boolean useCache) { checkArgument( ForeignKeyedEppResource.class.isAssignableFrom(clazz), @@ -140,14 +139,14 @@ public final class EppResourceUtils { : ofy().load().type(ForeignKeyIndex.mapToFkiClass(clazz)).id(foreignKey).now(); // The value of fki.getResourceKey() might be null for hard-deleted prober data. if (fki == null || isAtOrAfter(now, fki.getDeletionTime()) || fki.getResourceKey() == null) { - return null; + return Optional.empty(); } T resource = useCache ? EppResource.loadCached(fki.getResourceKey()) : ofy().load().key(fki.getResourceKey()).now(); if (resource == null || isAtOrAfter(now, resource.getDeletionTime())) { - return null; + return Optional.empty(); } // When setting status values based on a time, choose the greater of "now" and the resource's // UpdateAutoTimestamp. For non-mutating uses (info, whois, etc.), this is equivalent to rolling @@ -155,8 +154,9 @@ public final class EppResourceUtils { // doesn't appear stale. For mutating flows, if we had to roll now forward then the flow will // fail when it tries to save anything via Ofy, since "now" is needed to be > the last update // time for writes. - return cloneProjectedAtTime( - resource, latestOf(now, resource.getUpdateAutoTimestamp().getTimestamp())); + return Optional.of( + cloneProjectedAtTime( + resource, latestOf(now, resource.getUpdateAutoTimestamp().getTimestamp()))); } /** diff --git a/java/google/registry/whois/DomainLookupCommand.java b/java/google/registry/whois/DomainLookupCommand.java index 7932d8e0d..a7b769dab 100644 --- a/java/google/registry/whois/DomainLookupCommand.java +++ b/java/google/registry/whois/DomainLookupCommand.java @@ -33,9 +33,7 @@ public class DomainLookupCommand extends DomainOrHostLookupCommand { @Override protected Optional getResponse(InternetDomainName domainName, DateTime now) { - final DomainResource domainResource = - loadByForeignKeyCached(DomainResource.class, domainName.toString(), now); - return Optional.ofNullable( - domainResource == null ? null : new DomainWhoisResponse(domainResource, fullOutput, now)); + return loadByForeignKeyCached(DomainResource.class, domainName.toString(), now) + .map(domain -> new DomainWhoisResponse(domain, fullOutput, now)); } } diff --git a/java/google/registry/whois/NameserverLookupByHostCommand.java b/java/google/registry/whois/NameserverLookupByHostCommand.java index a70532904..cad8a3d6b 100644 --- a/java/google/registry/whois/NameserverLookupByHostCommand.java +++ b/java/google/registry/whois/NameserverLookupByHostCommand.java @@ -30,9 +30,7 @@ public class NameserverLookupByHostCommand extends DomainOrHostLookupCommand { @Override protected Optional getResponse(InternetDomainName hostName, DateTime now) { - final HostResource hostResource = - loadByForeignKeyCached(HostResource.class, hostName.toString(), now); - return Optional.ofNullable( - hostResource == null ? null : new NameserverWhoisResponse(hostResource, now)); + return loadByForeignKeyCached(HostResource.class, hostName.toString(), now) + .map(host -> new NameserverWhoisResponse(host, now)); } } diff --git a/javatests/google/registry/whois/WhoisActionTest.java b/javatests/google/registry/whois/WhoisActionTest.java index fa816b8b8..6b061befd 100644 --- a/javatests/google/registry/whois/WhoisActionTest.java +++ b/javatests/google/registry/whois/WhoisActionTest.java @@ -15,6 +15,7 @@ package google.registry.whois; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.EppResourceUtils.loadByForeignKeyCached; import static google.registry.model.registrar.Registrar.State.ACTIVE; import static google.registry.model.registrar.Registrar.Type.PDT; @@ -146,9 +147,9 @@ public class WhoisActionTest { persistSimpleResources(makeRegistrarContacts(registrar)); // Populate the cache for both the domain and contact. DomainResource domain = - loadByForeignKeyCached(DomainResource.class, "cat.lol", clock.nowUtc()); + loadByForeignKeyCached(DomainResource.class, "cat.lol", clock.nowUtc()).get(); ContactResource contact = - loadByForeignKeyCached(ContactResource.class, "5372808-ERL", clock.nowUtc()); + loadByForeignKeyCached(ContactResource.class, "5372808-ERL", clock.nowUtc()).get(); // Make a change to the domain and contact that won't be seen because the cache will be hit. persistResource(domain.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build()); persistResource( @@ -237,7 +238,7 @@ public class WhoisActionTest { @Test public void testRun_domainNotFound_usesCache() { // Populate the cache with the nonexistence of this domain. - assertThat(loadByForeignKeyCached(DomainResource.class, "cat.lol", clock.nowUtc())).isNull(); + assertThat(loadByForeignKeyCached(DomainResource.class, "cat.lol", clock.nowUtc())).isEmpty(); // Add a new valid cat.lol domain that won't be found because the cache will be hit instead. persistActiveDomain("cat.lol"); newWhoisAction("domain cat.lol\r\n").run(); @@ -361,7 +362,7 @@ public class WhoisActionTest { persistResource(makeHostResource("ns1.cat.xn--q9jyb4c", "1.2.3.4")); // Populate the cache. HostResource host = - loadByForeignKeyCached(HostResource.class, "ns1.cat.xn--q9jyb4c", clock.nowUtc()); + loadByForeignKeyCached(HostResource.class, "ns1.cat.xn--q9jyb4c", clock.nowUtc()).get(); // Make a change to the persisted host that won't be seen because the cache will be hit. persistResource( host.asBuilder()