Make loadByForeignKeyCached() return an Optional type

Next up (and a much larger commit) will be giving loadByForeignKey() the same
treatment.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=225182377
This commit is contained in:
mcilwain 2018-12-12 07:36:57 -08:00 committed by jianglai
parent 400994237c
commit 015c854a92
4 changed files with 18 additions and 21 deletions

View file

@ -44,6 +44,7 @@ import google.registry.model.transfer.TransferData;
import google.registry.model.transfer.TransferStatus; import google.registry.model.transfer.TransferStatus;
import java.util.List; import java.util.List;
import java.util.Map.Entry; import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set; import java.util.Set;
import java.util.function.Function; import java.util.function.Function;
import javax.annotation.Nullable; import javax.annotation.Nullable;
@ -94,7 +95,7 @@ public final class EppResourceUtils {
@Nullable @Nullable
public static <T extends EppResource> T loadByForeignKey( public static <T extends EppResource> T loadByForeignKey(
Class<T> clazz, String foreignKey, DateTime now) { Class<T> 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 foreignKey id to match
* @param now the current logical time to project resources at * @param now the current logical time to project resources at
*/ */
@Nullable public static <T extends EppResource> Optional<T> loadByForeignKeyCached(
public static <T extends EppResource> T loadByForeignKeyCached(
Class<T> clazz, String foreignKey, DateTime now) { Class<T> clazz, String foreignKey, DateTime now) {
return loadByForeignKeyHelper( return loadByForeignKeyHelper(
clazz, foreignKey, now, RegistryConfig.isEppResourceCachingEnabled()); clazz, foreignKey, now, RegistryConfig.isEppResourceCachingEnabled());
} }
@Nullable private static <T extends EppResource> Optional<T> loadByForeignKeyHelper(
private static <T extends EppResource> T loadByForeignKeyHelper(
Class<T> clazz, String foreignKey, DateTime now, boolean useCache) { Class<T> clazz, String foreignKey, DateTime now, boolean useCache) {
checkArgument( checkArgument(
ForeignKeyedEppResource.class.isAssignableFrom(clazz), ForeignKeyedEppResource.class.isAssignableFrom(clazz),
@ -140,14 +139,14 @@ public final class EppResourceUtils {
: ofy().load().type(ForeignKeyIndex.mapToFkiClass(clazz)).id(foreignKey).now(); : ofy().load().type(ForeignKeyIndex.mapToFkiClass(clazz)).id(foreignKey).now();
// The value of fki.getResourceKey() might be null for hard-deleted prober data. // The value of fki.getResourceKey() might be null for hard-deleted prober data.
if (fki == null || isAtOrAfter(now, fki.getDeletionTime()) || fki.getResourceKey() == null) { if (fki == null || isAtOrAfter(now, fki.getDeletionTime()) || fki.getResourceKey() == null) {
return null; return Optional.empty();
} }
T resource = T resource =
useCache useCache
? EppResource.loadCached(fki.getResourceKey()) ? EppResource.loadCached(fki.getResourceKey())
: ofy().load().key(fki.getResourceKey()).now(); : ofy().load().key(fki.getResourceKey()).now();
if (resource == null || isAtOrAfter(now, resource.getDeletionTime())) { 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 // 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 // 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 // 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 // fail when it tries to save anything via Ofy, since "now" is needed to be > the last update
// time for writes. // time for writes.
return cloneProjectedAtTime( return Optional.of(
resource, latestOf(now, resource.getUpdateAutoTimestamp().getTimestamp())); cloneProjectedAtTime(
resource, latestOf(now, resource.getUpdateAutoTimestamp().getTimestamp())));
} }
/** /**

View file

@ -33,9 +33,7 @@ public class DomainLookupCommand extends DomainOrHostLookupCommand {
@Override @Override
protected Optional<WhoisResponse> getResponse(InternetDomainName domainName, DateTime now) { protected Optional<WhoisResponse> getResponse(InternetDomainName domainName, DateTime now) {
final DomainResource domainResource = return loadByForeignKeyCached(DomainResource.class, domainName.toString(), now)
loadByForeignKeyCached(DomainResource.class, domainName.toString(), now); .map(domain -> new DomainWhoisResponse(domain, fullOutput, now));
return Optional.ofNullable(
domainResource == null ? null : new DomainWhoisResponse(domainResource, fullOutput, now));
} }
} }

View file

@ -30,9 +30,7 @@ public class NameserverLookupByHostCommand extends DomainOrHostLookupCommand {
@Override @Override
protected Optional<WhoisResponse> getResponse(InternetDomainName hostName, DateTime now) { protected Optional<WhoisResponse> getResponse(InternetDomainName hostName, DateTime now) {
final HostResource hostResource = return loadByForeignKeyCached(HostResource.class, hostName.toString(), now)
loadByForeignKeyCached(HostResource.class, hostName.toString(), now); .map(host -> new NameserverWhoisResponse(host, now));
return Optional.ofNullable(
hostResource == null ? null : new NameserverWhoisResponse(hostResource, now));
} }
} }

View file

@ -15,6 +15,7 @@
package google.registry.whois; package google.registry.whois;
import static com.google.common.truth.Truth.assertThat; 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.EppResourceUtils.loadByForeignKeyCached;
import static google.registry.model.registrar.Registrar.State.ACTIVE; import static google.registry.model.registrar.Registrar.State.ACTIVE;
import static google.registry.model.registrar.Registrar.Type.PDT; import static google.registry.model.registrar.Registrar.Type.PDT;
@ -146,9 +147,9 @@ public class WhoisActionTest {
persistSimpleResources(makeRegistrarContacts(registrar)); persistSimpleResources(makeRegistrarContacts(registrar));
// Populate the cache for both the domain and contact. // Populate the cache for both the domain and contact.
DomainResource domain = DomainResource domain =
loadByForeignKeyCached(DomainResource.class, "cat.lol", clock.nowUtc()); loadByForeignKeyCached(DomainResource.class, "cat.lol", clock.nowUtc()).get();
ContactResource contact = 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. // 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(domain.asBuilder().setDeletionTime(clock.nowUtc().minusDays(1)).build());
persistResource( persistResource(
@ -237,7 +238,7 @@ public class WhoisActionTest {
@Test @Test
public void testRun_domainNotFound_usesCache() { public void testRun_domainNotFound_usesCache() {
// Populate the cache with the nonexistence of this domain. // 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. // Add a new valid cat.lol domain that won't be found because the cache will be hit instead.
persistActiveDomain("cat.lol"); persistActiveDomain("cat.lol");
newWhoisAction("domain cat.lol\r\n").run(); newWhoisAction("domain cat.lol\r\n").run();
@ -361,7 +362,7 @@ public class WhoisActionTest {
persistResource(makeHostResource("ns1.cat.xn--q9jyb4c", "1.2.3.4")); persistResource(makeHostResource("ns1.cat.xn--q9jyb4c", "1.2.3.4"));
// Populate the cache. // Populate the cache.
HostResource host = 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. // Make a change to the persisted host that won't be seen because the cache will be hit.
persistResource( persistResource(
host.asBuilder() host.asBuilder()