From 4491b7b909f365e22696eb4576d10c477d64ab59 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Thu, 13 Dec 2018 13:17:30 -0800 Subject: [PATCH] Make loadByForeignKey() and related methods return Optional This is safer and addresses a common source of confusion in the codebase because it's always explicit that the resource returned may not be present, whether because it's soft-deleted when projected to the given time or because it never existed in the first place. In production code, the presence of the returned value is always checked. In test code, its presence is assumed using .get() where that is expected and convenient, as it not being present will throw an NPE that will cause the test to fail anyway. Note that the roughly equivalent reloadResourceByForeignKey(), which is widely used in test code, is not having this same treatment applied to it. That is out of the scope of this CL, and has much smaller returns anyway because it's only used in tests (where the unexpected absence of a given resource would just cause the test to fail). ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=225424002 --- .../google/registry/dns/RefreshDnsAction.java | 15 +-- .../dns/writer/clouddns/CloudDnsWriter.java | 9 +- .../dns/writer/dnsupdate/DnsUpdateWriter.java | 15 ++- .../registry/flows/ResourceFlowUtils.java | 7 +- .../flows/domain/DomainAllocateFlow.java | 7 +- .../domain/DomainApplicationInfoFlow.java | 13 +-- .../registry/flows/host/HostFlowUtils.java | 11 +-- .../registry/model/EppResourceUtils.java | 17 ++-- .../registry/model/domain/DomainBase.java | 22 ++++- java/google/registry/rdap/RdapActionBase.java | 12 +++ .../registry/rdap/RdapDomainAction.java | 7 +- .../registry/rdap/RdapDomainSearchAction.java | 45 +++++---- .../registry/rdap/RdapNameserverAction.java | 7 +- .../rdap/RdapNameserverSearchAction.java | 25 +++-- .../rde/imports/RdeHostLinkAction.java | 15 ++- .../registry/tools/GetApplicationCommand.java | 6 +- .../registry/tools/GetEppResourceCommand.java | 19 ++-- .../google/registry/tools/GetHostCommand.java | 6 +- .../registry/tools/LockDomainCommand.java | 10 +- .../registry/tools/RenewDomainCommand.java | 9 +- .../tools/UniformRapidSuspensionCommand.java | 12 ++- .../registry/tools/UnlockDomainCommand.java | 9 +- .../registry/tools/UnrenewDomainCommand.java | 19 ++-- .../tools/UpdateApplicationStatusCommand.java | 10 +- .../tools/UpdateClaimsNoticeCommand.java | 12 ++- .../registry/tools/UpdateDomainCommand.java | 9 +- .../registry/tools/UpdateSmdCommand.java | 11 ++- .../DeleteContactsAndHostsActionTest.java | 46 ++++----- .../batch/DeleteProberDataActionTest.java | 12 ++- .../writer/dnsupdate/DnsUpdateWriterTest.java | 4 +- .../flows/EppLifecycleDomainTest.java | 8 +- .../registry/flows/EppLifecycleHostTest.java | 12 +-- .../registry/flows/ResourceFlowTestCase.java | 9 +- .../flows/domain/DomainAllocateFlowTest.java | 2 +- .../DomainApplicationDeleteFlowTest.java | 11 ++- .../DomainApplicationUpdateFlowTest.java | 38 ++++---- .../flows/domain/DomainDeleteFlowTest.java | 3 +- .../flows/domain/DomainUpdateFlowTest.java | 94 ++++++++++--------- .../flows/host/HostCreateFlowTest.java | 4 +- .../registry/flows/host/HostInfoFlowTest.java | 2 +- .../flows/host/HostUpdateFlowTest.java | 2 +- .../registry/model/EppResourceTest.java | 13 ++- .../model/contact/ContactResourceTest.java | 3 +- .../model/domain/DomainApplicationTest.java | 4 +- .../model/domain/DomainResourceTest.java | 10 +- .../registry/model/host/HostResourceTest.java | 6 +- .../model/index/ForeignKeyIndexTest.java | 9 +- .../registry/tools/EppLifecycleToolsTest.java | 3 +- .../tools/GenerateDnsReportCommandTest.java | 2 +- .../registry/tools/LockDomainCommandTest.java | 2 +- .../tools/UnlockDomainCommandTest.java | 2 +- .../tools/UnrenewDomainCommandTest.java | 4 +- 52 files changed, 374 insertions(+), 290 deletions(-) diff --git a/java/google/registry/dns/RefreshDnsAction.java b/java/google/registry/dns/RefreshDnsAction.java index 64920eb8b..f9c687a32 100644 --- a/java/google/registry/dns/RefreshDnsAction.java +++ b/java/google/registry/dns/RefreshDnsAction.java @@ -65,13 +65,14 @@ public final class RefreshDnsAction implements Runnable { private T loadAndVerifyExistence(Class clazz, String foreignKey) { - T resource = loadByForeignKey(clazz, foreignKey, clock.nowUtc()); - if (resource == null) { - String typeName = clazz.getAnnotation(ExternalMessagingName.class).value(); - throw new NotFoundException( - String.format("%s %s not found", typeName, domainOrHostName)); - } - return resource; + return loadByForeignKey(clazz, foreignKey, clock.nowUtc()) + .orElseThrow( + () -> + new NotFoundException( + String.format( + "%s %s not found", + clazz.getAnnotation(ExternalMessagingName.class).value(), + domainOrHostName))); } private static void verifyHostIsSubordinate(HostResource host) { diff --git a/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java b/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java index e3f0469cf..152921277 100644 --- a/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java +++ b/java/google/registry/dns/writer/clouddns/CloudDnsWriter.java @@ -120,9 +120,9 @@ public class CloudDnsWriter extends BaseDnsWriter { // Canonicalize name String absoluteDomainName = getAbsoluteHostName(domainName); - // Load the target domain. Note that it can be null if this domain was just deleted. + // Load the target domain. Note that it can be absent if this domain was just deleted. Optional domainResource = - Optional.ofNullable(loadByForeignKey(DomainResource.class, domainName, clock.nowUtc())); + loadByForeignKey(DomainResource.class, domainName, clock.nowUtc()); // Return early if no DNS records should be published. // desiredRecordsBuilder is populated with an empty set to indicate that all existing records @@ -188,11 +188,10 @@ public class CloudDnsWriter extends BaseDnsWriter { // Canonicalize name String absoluteHostName = getAbsoluteHostName(hostName); - // Load the target host. Note that it can be null if this host was just deleted. + // Load the target host. Note that it can be absent if this host was just deleted. // desiredRecords is populated with an empty set to indicate that all existing records // should be deleted. - Optional host = - Optional.ofNullable(loadByForeignKey(HostResource.class, hostName, clock.nowUtc())); + Optional host = loadByForeignKey(HostResource.class, hostName, clock.nowUtc()); // Return early if the host is deleted. if (!host.isPresent()) { diff --git a/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java b/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java index c20813dd6..4baecb065 100644 --- a/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java +++ b/java/google/registry/dns/writer/dnsupdate/DnsUpdateWriter.java @@ -14,6 +14,7 @@ package google.registry.dns.writer.dnsupdate; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Verify.verify; import static com.google.common.collect.Sets.intersection; import static com.google.common.collect.Sets.union; @@ -126,9 +127,12 @@ public class DnsUpdateWriter extends BaseDnsWriter { * this domain refresh request */ private void publishDomain(String domainName, String requestingHostName) { - DomainResource domain = loadByForeignKey(DomainResource.class, domainName, clock.nowUtc()); + Optional domainOptional = + loadByForeignKey(DomainResource.class, domainName, clock.nowUtc()); update.delete(toAbsoluteName(domainName), Type.ANY); - if (domain != null) { + // If the domain is now deleted, then don't update DNS for it. + if (domainOptional.isPresent()) { + DomainResource domain = domainOptional.get(); // As long as the domain exists, orphan glues should be cleaned. deleteSubordinateHostAddressSet(domain, requestingHostName, update); if (domain.shouldPublishToDns()) { @@ -213,9 +217,10 @@ public class DnsUpdateWriter extends BaseDnsWriter { for (String hostName : intersection( domain.loadNameserverFullyQualifiedHostNames(), domain.getSubordinateHosts())) { - HostResource host = loadByForeignKey(HostResource.class, hostName, clock.nowUtc()); - update.add(makeAddressSet(host)); - update.add(makeV6AddressSet(host)); + Optional host = loadByForeignKey(HostResource.class, hostName, clock.nowUtc()); + checkState(host.isPresent(), "Host %s cannot be loaded", hostName); + update.add(makeAddressSet(host.get())); + update.add(makeV6AddressSet(host.get())); } } diff --git a/java/google/registry/flows/ResourceFlowUtils.java b/java/google/registry/flows/ResourceFlowUtils.java index bed58fcc1..f94b30478 100644 --- a/java/google/registry/flows/ResourceFlowUtils.java +++ b/java/google/registry/flows/ResourceFlowUtils.java @@ -292,11 +292,8 @@ public final class ResourceFlowUtils { } public static R verifyExistence( - Class clazz, String targetId, R resource) throws ResourceDoesNotExistException { - if (resource == null) { - throw new ResourceDoesNotExistException(clazz, targetId); - } - return resource; + Class clazz, String targetId, Optional resource) throws ResourceDoesNotExistException { + return resource.orElseThrow(() -> new ResourceDoesNotExistException(clazz, targetId)); } public static void verifyResourceDoesNotExist( diff --git a/java/google/registry/flows/domain/DomainAllocateFlow.java b/java/google/registry/flows/domain/DomainAllocateFlow.java index 501d173f5..bac773ea2 100644 --- a/java/google/registry/flows/domain/DomainAllocateFlow.java +++ b/java/google/registry/flows/domain/DomainAllocateFlow.java @@ -224,10 +224,9 @@ public class DomainAllocateFlow implements TransactionalFlow { private DomainApplication loadAndValidateApplication( String applicationRoid, DateTime now) throws EppException { - DomainApplication application = loadDomainApplication(applicationRoid, now); - if (application == null) { - throw new MissingApplicationException(applicationRoid); - } + DomainApplication application = + loadDomainApplication(applicationRoid, now) + .orElseThrow(() -> new MissingApplicationException(applicationRoid)); if (application.getApplicationStatus().isFinalStatus()) { throw new HasFinalStatusException(); } diff --git a/java/google/registry/flows/domain/DomainApplicationInfoFlow.java b/java/google/registry/flows/domain/DomainApplicationInfoFlow.java index 8224ce739..4a89ecb7a 100644 --- a/java/google/registry/flows/domain/DomainApplicationInfoFlow.java +++ b/java/google/registry/flows/domain/DomainApplicationInfoFlow.java @@ -23,10 +23,10 @@ import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership; import static google.registry.flows.domain.DomainFlowUtils.addSecDnsExtensionIfPresent; import static google.registry.flows.domain.DomainFlowUtils.loadForeignKeyedDesignatedContacts; import static google.registry.flows.domain.DomainFlowUtils.verifyApplicationDomainMatchesTargetId; +import static google.registry.model.EppResourceUtils.loadDomainApplication; import static google.registry.model.ofy.ObjectifyService.ofy; import com.google.common.collect.ImmutableList; -import com.googlecode.objectify.Key; import google.registry.flows.EppException; import google.registry.flows.EppException.ParameterValuePolicyErrorException; import google.registry.flows.EppException.RequiredParameterMissingException; @@ -89,13 +89,10 @@ public final class DomainApplicationInfoFlow implements Flow { throw new MissingApplicationIdException(); } DomainApplication application = - ofy().load().key(Key.create(DomainApplication.class, applicationId)).now(); - verifyExistence( - DomainApplication.class, - applicationId, - application != null && clock.nowUtc().isBefore(application.getDeletionTime()) - ? application - : null); + verifyExistence( + DomainApplication.class, + applicationId, + loadDomainApplication(applicationId, clock.nowUtc())); verifyApplicationDomainMatchesTargetId(application, targetId); verifyOptionalAuthInfo(authInfo, application); LaunchInfoExtension launchInfo = eppInput.getSingleExtension(LaunchInfoExtension.class).get(); diff --git a/java/google/registry/flows/host/HostFlowUtils.java b/java/google/registry/flows/host/HostFlowUtils.java index 41da5bea9..704eaa397 100644 --- a/java/google/registry/flows/host/HostFlowUtils.java +++ b/java/google/registry/flows/host/HostFlowUtils.java @@ -87,16 +87,15 @@ public class HostFlowUtils { } // This is a subordinate host String domainName = - hostName - .parts() - .stream() + hostName.parts().stream() .skip(hostName.parts().size() - (tld.get().parts().size() + 1)) .collect(joining(".")); - DomainResource superordinateDomain = loadByForeignKey(DomainResource.class, domainName, now); - if (superordinateDomain == null || !isActive(superordinateDomain, now)) { + Optional superordinateDomain = + loadByForeignKey(DomainResource.class, domainName, now); + if (!superordinateDomain.isPresent() || !isActive(superordinateDomain.get(), now)) { throw new SuperordinateDomainDoesNotExistException(domainName); } - return Optional.of(superordinateDomain); + return superordinateDomain; } /** Superordinate domain for this hostname does not exist. */ diff --git a/java/google/registry/model/EppResourceUtils.java b/java/google/registry/model/EppResourceUtils.java index 8a85ac251..3de360c28 100644 --- a/java/google/registry/model/EppResourceUtils.java +++ b/java/google/registry/model/EppResourceUtils.java @@ -76,7 +76,7 @@ public final class EppResourceUtils { /** * Loads the last created version of an {@link EppResource} from Datastore by foreign key. * - *

Returns null if no resource with this foreign key was ever created, or if the most recently + *

Returns empty if no resource with this foreign key was ever created, or if the most recently * created resource was deleted before time "now". * *

Loading an {@link EppResource} by itself is not sufficient to know its current state since @@ -92,10 +92,9 @@ public final class EppResourceUtils { * @param foreignKey id to match * @param now the current logical time to project resources at */ - @Nullable - public static T loadByForeignKey( + public static Optional loadByForeignKey( Class clazz, String foreignKey, DateTime now) { - return loadByForeignKeyHelper(clazz, foreignKey, now, false).orElse(null); + return loadByForeignKeyHelper(clazz, foreignKey, now, false); } /** @@ -160,19 +159,19 @@ public final class EppResourceUtils { } /** - * Returns the domain application with the given application id if it exists, or null if it does + * Returns the domain application with the given application id if it exists, or absent if it does * not or is soft-deleted as of the given time. */ - @Nullable - public static DomainApplication loadDomainApplication(String applicationId, DateTime now) { + public static Optional loadDomainApplication( + String applicationId, DateTime now) { DomainApplication application = ofy().load().key(Key.create(DomainApplication.class, applicationId)).now(); if (application == null || isAtOrAfter(now, application.getDeletionTime())) { - return null; + return Optional.empty(); } // Applications don't have any speculative changes that become effective later, so no need to // clone forward in time. - return application; + return Optional.of(application); } /** diff --git a/java/google/registry/model/domain/DomainBase.java b/java/google/registry/model/domain/DomainBase.java index 022a42cd5..110cc2a92 100644 --- a/java/google/registry/model/domain/DomainBase.java +++ b/java/google/registry/model/domain/DomainBase.java @@ -21,6 +21,7 @@ import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet; import static com.google.common.collect.Sets.difference; import static com.google.common.collect.Sets.union; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.util.CollectionUtils.forceEmptyToNull; import static google.registry.util.CollectionUtils.nullToEmpty; import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import static google.registry.util.CollectionUtils.nullToEmptyImmutableSortedCopy; @@ -231,21 +232,38 @@ public abstract class DomainBase extends EppResource { return thisCastToDerived(); } - public B setNameservers(ImmutableSet> nameservers) { - getInstance().nsHosts = nameservers; + public B setNameservers(Key nameserver) { + getInstance().nsHosts = ImmutableSet.of(nameserver); return thisCastToDerived(); } + public B setNameservers(ImmutableSet> nameservers) { + getInstance().nsHosts = forceEmptyToNull(nameservers); + return thisCastToDerived(); + } + + public B addNameserver(Key nameserver) { + return addNameservers(ImmutableSet.of(nameserver)); + } + public B addNameservers(ImmutableSet> nameservers) { return setNameservers( ImmutableSet.copyOf(union(getInstance().getNameservers(), nameservers))); } + public B removeNameserver(Key nameserver) { + return removeNameservers(ImmutableSet.of(nameserver)); + } + public B removeNameservers(ImmutableSet> nameservers) { return setNameservers( ImmutableSet.copyOf(difference(getInstance().getNameservers(), nameservers))); } + public B setContacts(DesignatedContact contact) { + return setContacts(ImmutableSet.of(contact)); + } + public B setContacts(ImmutableSet contacts) { checkArgument(contacts.stream().noneMatch(IS_REGISTRANT), "Registrant cannot be a contact"); // Replace the non-registrant contacts inside allContacts. diff --git a/java/google/registry/rdap/RdapActionBase.java b/java/google/registry/rdap/RdapActionBase.java index 270b797d3..d0a313d90 100644 --- a/java/google/registry/rdap/RdapActionBase.java +++ b/java/google/registry/rdap/RdapActionBase.java @@ -272,6 +272,18 @@ public abstract class RdapActionBase implements Runnable { || registrarParam.get().equals(eppResource.getPersistedCurrentSponsorClientId())); } + /** + * Returns true if the EPP resource should be visible. + * + *

This is true iff: + * 1. The passed in resource exists and is not deleted (deleted ones will have been projected + * forward in time to empty), + * 2. The request did not specify a registrar to filter on, or the registrar matches. + */ + boolean shouldBeVisible(Optional eppResource, DateTime now) { + return eppResource.isPresent() && shouldBeVisible(eppResource.get(), now); + } + /** * Returns true if the registrar should be visible. * diff --git a/java/google/registry/rdap/RdapDomainAction.java b/java/google/registry/rdap/RdapDomainAction.java index 2563a3987..2d975f112 100644 --- a/java/google/registry/rdap/RdapDomainAction.java +++ b/java/google/registry/rdap/RdapDomainAction.java @@ -29,6 +29,7 @@ import google.registry.request.Action; import google.registry.request.HttpException.BadRequestException; import google.registry.request.HttpException.NotFoundException; import google.registry.request.auth.Auth; +import java.util.Optional; import javax.inject.Inject; import org.joda.time.DateTime; @@ -74,14 +75,14 @@ public class RdapDomainAction extends RdapActionBase { pathSearchString, getHumanReadableObjectTypeName(), e.getMessage())); } // The query string is not used; the RDAP syntax is /rdap/domain/mydomain.com. - DomainResource domainResource = + Optional domainResource = loadByForeignKey( DomainResource.class, pathSearchString, shouldIncludeDeleted() ? START_OF_TIME : now); - if ((domainResource == null) || !shouldBeVisible(domainResource, now)) { + if (!shouldBeVisible(domainResource, now)) { throw new NotFoundException(pathSearchString + " not found"); } return rdapJsonFormatter.makeRdapJsonForDomain( - domainResource, + domainResource.get(), true, fullServletPath, rdapWhoisServer, diff --git a/java/google/registry/rdap/RdapDomainSearchAction.java b/java/google/registry/rdap/RdapDomainSearchAction.java index 59c5ffac4..3999f6660 100644 --- a/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/java/google/registry/rdap/RdapDomainSearchAction.java @@ -216,13 +216,13 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { */ private RdapSearchResults searchByDomainNameWithoutWildcard( final RdapSearchPattern partialStringQuery, final DateTime now) { - DomainResource domainResource = + Optional domainResource = loadByForeignKey(DomainResource.class, partialStringQuery.getInitialString(), now); - ImmutableList results = - ((domainResource == null) || !shouldBeVisible(domainResource, now)) - ? ImmutableList.of() - : ImmutableList.of(domainResource); - return makeSearchResults(results, now); + return makeSearchResults( + shouldBeVisible(domainResource, now) + ? ImmutableList.of(domainResource.get()) + : ImmutableList.of(), + now); } /** Searches for domains by domain name with an initial string, wildcard and possible suffix. */ @@ -343,15 +343,15 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { // the key. Optional desiredRegistrar = getDesiredRegistrar(); if (desiredRegistrar.isPresent()) { - HostResource host = + Optional host = loadByForeignKey( HostResource.class, partialStringQuery.getInitialString(), shouldIncludeDeleted() ? START_OF_TIME : now); - return ((host == null) - || !desiredRegistrar.get().equals(host.getPersistedCurrentSponsorClientId())) + return (!host.isPresent() + || !desiredRegistrar.get().equals(host.get().getPersistedCurrentSponsorClientId())) ? ImmutableList.of() - : ImmutableList.of(Key.create(host)); + : ImmutableList.of(Key.create(host.get())); } else { Key hostKey = loadAndGetKey( @@ -370,15 +370,14 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { // with no initial string. DomainResource domainResource = loadByForeignKey( - DomainResource.class, - partialStringQuery.getSuffix(), - shouldIncludeDeleted() ? START_OF_TIME : now); - if (domainResource == null) { - // Don't allow wildcards with suffixes which are not domains we manage. That would risk a - // table scan in some easily foreseeable cases. - throw new UnprocessableEntityException( - "A suffix in a lookup by nameserver name must be a domain defined in the system"); - } + DomainResource.class, + partialStringQuery.getSuffix(), + shouldIncludeDeleted() ? START_OF_TIME : now) + .orElseThrow( + () -> + new UnprocessableEntityException( + "A suffix in a lookup by nameserver name " + + "must be a domain defined in the system")); Optional desiredRegistrar = getDesiredRegistrar(); ImmutableList.Builder> builder = new ImmutableList.Builder<>(); for (String fqhn : ImmutableSortedSet.copyOf(domainResource.getSubordinateHosts())) { @@ -386,12 +385,12 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { // then the query ns.exam*.example.com would match against nameserver ns.example.com. if (partialStringQuery.matches(fqhn)) { if (desiredRegistrar.isPresent()) { - HostResource host = + Optional host = loadByForeignKey( HostResource.class, fqhn, shouldIncludeDeleted() ? START_OF_TIME : now); - if ((host != null) - && desiredRegistrar.get().equals(host.getPersistedCurrentSponsorClientId())) { - builder.add(Key.create(host)); + if (host.isPresent() + && desiredRegistrar.get().equals(host.get().getPersistedCurrentSponsorClientId())) { + builder.add(Key.create(host.get())); } } else { Key hostKey = diff --git a/java/google/registry/rdap/RdapNameserverAction.java b/java/google/registry/rdap/RdapNameserverAction.java index 27e7e44f1..4c898169d 100644 --- a/java/google/registry/rdap/RdapNameserverAction.java +++ b/java/google/registry/rdap/RdapNameserverAction.java @@ -29,6 +29,7 @@ import google.registry.request.Action; import google.registry.request.HttpException.BadRequestException; import google.registry.request.HttpException.NotFoundException; import google.registry.request.auth.Auth; +import java.util.Optional; import javax.inject.Inject; import org.joda.time.DateTime; @@ -76,13 +77,13 @@ public class RdapNameserverAction extends RdapActionBase { } // If there are no undeleted nameservers with the given name, the foreign key should point to // the most recently deleted one. - HostResource hostResource = + Optional hostResource = loadByForeignKey( HostResource.class, pathSearchString, shouldIncludeDeleted() ? START_OF_TIME : now); - if ((hostResource == null) || !shouldBeVisible(hostResource, now)) { + if (!shouldBeVisible(hostResource, now)) { throw new NotFoundException(pathSearchString + " not found"); } return rdapJsonFormatter.makeRdapJsonForHost( - hostResource, true, fullServletPath, rdapWhoisServer, now, OutputDataType.FULL); + hostResource.get(), true, fullServletPath, rdapWhoisServer, now, OutputDataType.FULL); } } diff --git a/java/google/registry/rdap/RdapNameserverSearchAction.java b/java/google/registry/rdap/RdapNameserverSearchAction.java index b36eddda5..b77909d05 100644 --- a/java/google/registry/rdap/RdapNameserverSearchAction.java +++ b/java/google/registry/rdap/RdapNameserverSearchAction.java @@ -183,9 +183,9 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase { */ private RdapSearchResults searchByNameUsingForeignKey( final RdapSearchPattern partialStringQuery, final DateTime now) { - HostResource hostResource = + Optional hostResource = loadByForeignKey(HostResource.class, partialStringQuery.getInitialString(), now); - if ((hostResource == null) || !shouldBeVisible(hostResource, now)) { + if (!shouldBeVisible(hostResource, now)) { metricInformationBuilder.setNumHostsRetrieved(0); throw new NotFoundException("No nameservers found"); } @@ -193,15 +193,20 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase { return RdapSearchResults.create( ImmutableList.of( rdapJsonFormatter.makeRdapJsonForHost( - hostResource, false, fullServletPath, rdapWhoisServer, now, OutputDataType.FULL))); + hostResource.get(), + false, + fullServletPath, + rdapWhoisServer, + now, + OutputDataType.FULL))); } /** Searches for nameservers by name using the superordinate domain as a suffix. */ private RdapSearchResults searchByNameUsingSuperordinateDomain( final RdapSearchPattern partialStringQuery, final DateTime now) { - DomainResource domainResource = + Optional domainResource = loadByForeignKey(DomainResource.class, partialStringQuery.getSuffix(), now); - if (domainResource == null) { + if (!domainResource.isPresent()) { // Don't allow wildcards with suffixes which are not domains we manage. That would risk a // table scan in many easily foreseeable cases. The user might ask for ns*.zombo.com, // forcing us to query for all hosts beginning with ns, then filter for those ending in @@ -211,16 +216,16 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase { "A suffix after a wildcard in a nameserver lookup must be an in-bailiwick domain"); } List hostList = new ArrayList<>(); - for (String fqhn : ImmutableSortedSet.copyOf(domainResource.getSubordinateHosts())) { + for (String fqhn : ImmutableSortedSet.copyOf(domainResource.get().getSubordinateHosts())) { if (cursorString.isPresent() && (fqhn.compareTo(cursorString.get()) <= 0)) { continue; } // We can't just check that the host name starts with the initial query string, because // then the query ns.exam*.example.com would match against nameserver ns.example.com. if (partialStringQuery.matches(fqhn)) { - HostResource hostResource = loadByForeignKey(HostResource.class, fqhn, now); - if ((hostResource != null) && shouldBeVisible(hostResource, now)) { - hostList.add(hostResource); + Optional hostResource = loadByForeignKey(HostResource.class, fqhn, now); + if (shouldBeVisible(hostResource, now)) { + hostList.add(hostResource.get()); if (hostList.size() > rdapResultSetMaxSize) { break; } @@ -230,7 +235,7 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase { return makeSearchResults( hostList, IncompletenessWarningType.COMPLETE, - domainResource.getSubordinateHosts().size(), + domainResource.get().getSubordinateHosts().size(), CursorType.NAME, now); } diff --git a/java/google/registry/rde/imports/RdeHostLinkAction.java b/java/google/registry/rde/imports/RdeHostLinkAction.java index e8f8ee147..8b81f826f 100644 --- a/java/google/registry/rde/imports/RdeHostLinkAction.java +++ b/java/google/registry/rde/imports/RdeHostLinkAction.java @@ -195,11 +195,14 @@ public class RdeHostLinkAction implements Runnable { .stream() .skip(hostName.parts().size() - (tld.get().parts().size() + 1)) .collect(joining(".")); - DomainResource superordinateDomain = loadByForeignKey(DomainResource.class, domainName, now); + Optional superordinateDomain = + loadByForeignKey(DomainResource.class, domainName, now); // Hosts can't be linked if domains import hasn't been run checkState( - superordinateDomain != null, "Superordinate domain does not exist: %s", domainName); - return Optional.of(superordinateDomain); + superordinateDomain.isPresent(), + "Superordinate domain does not exist or is deleted: %s", + domainName); + return superordinateDomain; } } @@ -209,10 +212,4 @@ public class RdeHostLinkAction implements Runnable { SUPERORDINATE_DOMAIN_IN_PENDING_DELETE, HOST_LINKED; } - - private static class HostLinkException extends RuntimeException { - HostLinkException(String hostname, String xml, Throwable cause) { - super(String.format("Error linking host %s; xml=%s", hostname, xml), cause); - } - } } diff --git a/java/google/registry/tools/GetApplicationCommand.java b/java/google/registry/tools/GetApplicationCommand.java index ffa7bb6bf..e639d2d66 100644 --- a/java/google/registry/tools/GetApplicationCommand.java +++ b/java/google/registry/tools/GetApplicationCommand.java @@ -31,9 +31,7 @@ final class GetApplicationCommand extends GetEppResourceCommand { @Override public void runAndPrint() { - for (String applicationId : mainParameters) { - printResource( - "Application", applicationId, loadDomainApplication(applicationId, readTimestamp)); - } + mainParameters.forEach( + appId -> printResource("Application", appId, loadDomainApplication(appId, readTimestamp))); } } diff --git a/java/google/registry/tools/GetEppResourceCommand.java b/java/google/registry/tools/GetEppResourceCommand.java index 202b5f307..d96eb4f7a 100644 --- a/java/google/registry/tools/GetEppResourceCommand.java +++ b/java/google/registry/tools/GetEppResourceCommand.java @@ -21,7 +21,7 @@ import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.googlecode.objectify.Key; import google.registry.model.EppResource; -import javax.annotation.Nullable; +import java.util.Optional; import org.joda.time.DateTime; /** Abstract command to print one or more resources to stdout. */ @@ -44,17 +44,20 @@ abstract class GetEppResourceCommand implements CommandWithRemoteApi { abstract void runAndPrint(); /** - * Prints a possibly-null resource to stdout, using resourceType and uniqueId to construct a + * Prints a possibly-absent resource to stdout, using resourceType and uniqueId to construct a * nice error message if the resource was null (i.e. doesn't exist). * *

The websafe key is appended to the output for use in e.g. manual mapreduce calls. */ - void printResource(String resourceType, String uniqueId, @Nullable EppResource resource) { - System.out.println(resource != null - ? String.format("%s\n\nWebsafe key: %s", - expand ? resource.toHydratedString() : resource, - Key.create(resource).getString()) - : String.format("%s '%s' does not exist or is deleted\n", resourceType, uniqueId)); + void printResource( + String resourceType, String uniqueId, Optional resource) { + System.out.println( + resource.isPresent() + ? String.format( + "%s\n\nWebsafe key: %s", + expand ? resource.get().toHydratedString() : resource.get(), + Key.create(resource.get()).getString()) + : String.format("%s '%s' does not exist or is deleted\n", resourceType, uniqueId)); } @Override diff --git a/java/google/registry/tools/GetHostCommand.java b/java/google/registry/tools/GetHostCommand.java index 786f68923..dc1f8a195 100644 --- a/java/google/registry/tools/GetHostCommand.java +++ b/java/google/registry/tools/GetHostCommand.java @@ -32,9 +32,7 @@ final class GetHostCommand extends GetEppResourceCommand { @Override public void runAndPrint() { - for (String hostName : mainParameters) { - printResource( - "Host", hostName, loadByForeignKey(HostResource.class, hostName, readTimestamp)); - } + mainParameters.forEach( + h -> printResource("Host", h, loadByForeignKey(HostResource.class, h, readTimestamp))); } } diff --git a/java/google/registry/tools/LockDomainCommand.java b/java/google/registry/tools/LockDomainCommand.java index 5751e6170..a03b8f1a1 100644 --- a/java/google/registry/tools/LockDomainCommand.java +++ b/java/google/registry/tools/LockDomainCommand.java @@ -14,9 +14,9 @@ package google.registry.tools; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.model.EppResourceUtils.loadByForeignKey; +import static google.registry.util.PreconditionsUtils.checkArgumentPresent; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameters; @@ -28,6 +28,7 @@ import com.google.template.soy.data.SoyMapData; import google.registry.model.domain.DomainResource; import google.registry.model.eppcommon.StatusValue; import google.registry.tools.soy.DomainUpdateSoyInfo; +import java.util.Optional; import org.joda.time.DateTime; /** @@ -45,10 +46,11 @@ public class LockDomainCommand extends LockOrUnlockDomainCommand { // Project all domains as of the same time so that argument order doesn't affect behavior. DateTime now = DateTime.now(UTC); for (String domain : getDomains()) { - DomainResource domainResource = loadByForeignKey(DomainResource.class, domain, now); - checkArgument(domainResource != null, "Domain '%s' does not exist", domain); + Optional domainResource = loadByForeignKey(DomainResource.class, domain, now); + checkArgumentPresent(domainResource, "Domain '%s' does not exist or is deleted", domain); ImmutableSet statusesToAdd = - Sets.difference(REGISTRY_LOCK_STATUSES, domainResource.getStatusValues()).immutableCopy(); + Sets.difference(REGISTRY_LOCK_STATUSES, domainResource.get().getStatusValues()) + .immutableCopy(); if (statusesToAdd.isEmpty()) { logger.atInfo().log("Domain '%s' is already locked and needs no updates.", domain); continue; diff --git a/java/google/registry/tools/RenewDomainCommand.java b/java/google/registry/tools/RenewDomainCommand.java index fd1ffcb7b..5dd3ef786 100644 --- a/java/google/registry/tools/RenewDomainCommand.java +++ b/java/google/registry/tools/RenewDomainCommand.java @@ -17,7 +17,7 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.util.CollectionUtils.findDuplicates; -import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; +import static google.registry.util.PreconditionsUtils.checkArgumentPresent; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; @@ -27,6 +27,7 @@ import google.registry.model.domain.DomainResource; import google.registry.tools.soy.RenewDomainSoyInfo; import google.registry.util.Clock; import java.util.List; +import java.util.Optional; import javax.inject.Inject; import org.joda.time.DateTime; import org.joda.time.format.DateTimeFormat; @@ -56,9 +57,11 @@ final class RenewDomainCommand extends MutatingEppToolCommand { checkArgument(period < 10, "Cannot renew domains for 10 or more years"); DateTime now = clock.nowUtc(); for (String domainName : mainParameters) { - DomainResource domain = loadByForeignKey(DomainResource.class, domainName, now); - checkArgumentNotNull(domain, "Domain '%s' does not exist or is deleted", domainName); + Optional domainOptional = + loadByForeignKey(DomainResource.class, domainName, now); + checkArgumentPresent(domainOptional, "Domain '%s' does not exist or is deleted", domainName); setSoyTemplate(RenewDomainSoyInfo.getInstance(), RenewDomainSoyInfo.RENEWDOMAIN); + DomainResource domain = domainOptional.get(); addSoyRecord( domain.getCurrentSponsorClientId(), new SoyMapData( diff --git a/java/google/registry/tools/UniformRapidSuspensionCommand.java b/java/google/registry/tools/UniformRapidSuspensionCommand.java index 9c0b70a0d..4a63d6434 100644 --- a/java/google/registry/tools/UniformRapidSuspensionCommand.java +++ b/java/google/registry/tools/UniformRapidSuspensionCommand.java @@ -20,6 +20,7 @@ import static com.google.common.collect.Sets.difference; import static google.registry.model.EppResourceUtils.checkResourcesExist; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.util.PreconditionsUtils.checkArgumentPresent; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; @@ -37,6 +38,7 @@ import google.registry.tools.soy.UniformRapidSuspensionSoyInfo; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import javax.xml.bind.annotation.adapters.HexBinaryAdapter; import org.joda.time.DateTime; @@ -119,17 +121,17 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand { } catch (ClassCastException | ParseException e) { throw new IllegalArgumentException("Invalid --dsdata JSON", e); } - DomainResource domain = loadByForeignKey(DomainResource.class, domainName, now); - checkArgument(domain != null, "Domain '%s' does not exist", domainName); + Optional domain = loadByForeignKey(DomainResource.class, domainName, now); + checkArgumentPresent(domain, "Domain '%s' does not exist or is deleted", domainName); Set missingHosts = difference(newHostsSet, checkResourcesExist(HostResource.class, newHosts, now)); checkArgument(missingHosts.isEmpty(), "Hosts do not exist: %s", missingHosts); checkArgument( locksToPreserve.isEmpty() || undo, "Locks can only be preserved when running with --undo"); - existingNameservers = getExistingNameservers(domain); - existingLocks = getExistingLocks(domain); - existingDsData = getExistingDsData(domain); + existingNameservers = getExistingNameservers(domain.get()); + existingLocks = getExistingLocks(domain.get()); + existingDsData = getExistingDsData(domain.get()); setSoyTemplate( UniformRapidSuspensionSoyInfo.getInstance(), UniformRapidSuspensionSoyInfo.UNIFORMRAPIDSUSPENSION); diff --git a/java/google/registry/tools/UnlockDomainCommand.java b/java/google/registry/tools/UnlockDomainCommand.java index afd5baffe..4c6a48821 100644 --- a/java/google/registry/tools/UnlockDomainCommand.java +++ b/java/google/registry/tools/UnlockDomainCommand.java @@ -14,9 +14,9 @@ package google.registry.tools; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.model.EppResourceUtils.loadByForeignKey; +import static google.registry.util.PreconditionsUtils.checkArgumentPresent; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameters; @@ -28,6 +28,7 @@ import com.google.template.soy.data.SoyMapData; import google.registry.model.domain.DomainResource; import google.registry.model.eppcommon.StatusValue; import google.registry.tools.soy.DomainUpdateSoyInfo; +import java.util.Optional; import org.joda.time.DateTime; /** @@ -45,10 +46,10 @@ public class UnlockDomainCommand extends LockOrUnlockDomainCommand { // Project all domains as of the same time so that argument order doesn't affect behavior. DateTime now = DateTime.now(UTC); for (String domain : getDomains()) { - DomainResource domainResource = loadByForeignKey(DomainResource.class, domain, now); - checkArgument(domainResource != null, "Domain '%s' does not exist", domain); + Optional domainResource = loadByForeignKey(DomainResource.class, domain, now); + checkArgumentPresent(domainResource, "Domain '%s' does not exist or is deleted", domain); ImmutableSet statusesToRemove = - Sets.intersection(domainResource.getStatusValues(), REGISTRY_LOCK_STATUSES) + Sets.intersection(domainResource.get().getStatusValues(), REGISTRY_LOCK_STATUSES) .immutableCopy(); if (statusesToRemove.isEmpty()) { logger.atInfo().log("Domain '%s' is already unlocked and needs no updates.", domain); diff --git a/java/google/registry/tools/UnrenewDomainCommand.java b/java/google/registry/tools/UnrenewDomainCommand.java index 3de1cf9b4..6f8e2d0e2 100644 --- a/java/google/registry/tools/UnrenewDomainCommand.java +++ b/java/google/registry/tools/UnrenewDomainCommand.java @@ -43,6 +43,7 @@ import google.registry.model.reporting.HistoryEntry.Type; import google.registry.util.Clock; import google.registry.util.NonFinalForTesting; import java.util.List; +import java.util.Optional; import javax.inject.Inject; import org.joda.time.DateTime; @@ -90,16 +91,17 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot domainsNonexistentBuilder.add(domainName); continue; } - DomainResource domain = loadByForeignKey(DomainResource.class, domainName, now); - if (domain == null || domain.getStatusValues().contains(StatusValue.PENDING_DELETE)) { + Optional domain = loadByForeignKey(DomainResource.class, domainName, now); + if (!domain.isPresent() + || domain.get().getStatusValues().contains(StatusValue.PENDING_DELETE)) { domainsDeletingBuilder.add(domainName); continue; } domainsWithDisallowedStatusesBuilder.putAll( - domainName, Sets.intersection(domain.getStatusValues(), DISALLOWED_STATUSES)); + domainName, Sets.intersection(domain.get().getStatusValues(), DISALLOWED_STATUSES)); if (isBeforeOrAt( - leapSafeSubtractYears(domain.getRegistrationExpirationTime(), period), now)) { - domainsExpiringTooSoonBuilder.put(domainName, domain.getRegistrationExpirationTime()); + leapSafeSubtractYears(domain.get().getRegistrationExpirationTime(), period), now)) { + domainsExpiringTooSoonBuilder.put(domainName, domain.get().getRegistrationExpirationTime()); } } @@ -149,13 +151,16 @@ class UnrenewDomainCommand extends ConfirmingCommand implements CommandWithRemot private void unrenewDomain(String domainName) { ofy().assertInTransaction(); DateTime now = ofy().getTransactionTime(); - DomainResource domain = loadByForeignKey(DomainResource.class, domainName, now); + Optional domainOptional = + loadByForeignKey(DomainResource.class, domainName, now); // Transactional sanity checks on the off chance that something changed between init() running // and here. checkState( - domain != null && !domain.getStatusValues().contains(StatusValue.PENDING_DELETE), + domainOptional.isPresent() + && !domainOptional.get().getStatusValues().contains(StatusValue.PENDING_DELETE), "Domain %s was deleted or is pending deletion", domainName); + DomainResource domain = domainOptional.get(); checkState( Sets.intersection(domain.getStatusValues(), DISALLOWED_STATUSES).isEmpty(), "Domain %s has prohibited status values", diff --git a/java/google/registry/tools/UpdateApplicationStatusCommand.java b/java/google/registry/tools/UpdateApplicationStatusCommand.java index e34238c17..d143b50f9 100644 --- a/java/google/registry/tools/UpdateApplicationStatusCommand.java +++ b/java/google/registry/tools/UpdateApplicationStatusCommand.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState; import static google.registry.model.EppResourceUtils.loadDomainApplication; import static google.registry.model.domain.launch.ApplicationStatus.ALLOCATED; import static google.registry.model.ofy.ObjectifyService.ofy; -import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import static google.registry.util.PreconditionsUtils.checkArgumentPresent; import com.beust.jcommander.Parameter; @@ -83,9 +82,12 @@ final class UpdateApplicationStatusCommand extends MutatingCommand { ofy().assertInTransaction(); DateTime now = ofy().getTransactionTime(); - // Load the domain application. - DomainApplication domainApplication = loadDomainApplication(applicationId, now); - checkArgumentNotNull(domainApplication, "Domain application does not exist"); + DomainApplication domainApplication = + loadDomainApplication(applicationId, now) + .orElseThrow( + () -> + new IllegalArgumentException( + "Domain application does not exist or is deleted")); // It's not an error if the application already has the intended status. We want the method // to be idempotent. diff --git a/java/google/registry/tools/UpdateClaimsNoticeCommand.java b/java/google/registry/tools/UpdateClaimsNoticeCommand.java index d00e00a80..41764e9f6 100644 --- a/java/google/registry/tools/UpdateClaimsNoticeCommand.java +++ b/java/google/registry/tools/UpdateClaimsNoticeCommand.java @@ -82,16 +82,20 @@ final class UpdateClaimsNoticeCommand implements CommandWithRemoteApi { DateTime now = ofy().getTransactionTime(); // Load the domain application. - DomainApplication domainApplication = loadDomainApplication(applicationId, now); - checkArgument(domainApplication != null, "Domain application does not exist"); + DomainApplication domainApplication = + loadDomainApplication(applicationId, now) + .orElseThrow( + () -> + new IllegalArgumentException( + "Domain application does not exist or is deleted")); // Make sure this isn't a sunrise application. checkArgument(domainApplication.getEncodedSignedMarks().isEmpty(), "Can't update claims notice on sunrise applications."); // Validate the new launch notice checksum. - String domainLabel = InternetDomainName.from(domainApplication.getFullyQualifiedDomainName()) - .parts().get(0); + String domainLabel = + InternetDomainName.from(domainApplication.getFullyQualifiedDomainName()).parts().get(0); launchNotice.validate(domainLabel); DomainApplication updatedApplication = domainApplication.asBuilder() diff --git a/java/google/registry/tools/UpdateDomainCommand.java b/java/google/registry/tools/UpdateDomainCommand.java index e8421582c..4e7d03431 100644 --- a/java/google/registry/tools/UpdateDomainCommand.java +++ b/java/google/registry/tools/UpdateDomainCommand.java @@ -19,7 +19,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.eppcommon.StatusValue.SERVER_UPDATE_PROHIBITED; import static google.registry.model.ofy.ObjectifyService.ofy; -import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; +import static google.registry.util.PreconditionsUtils.checkArgumentPresent; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; @@ -37,6 +37,7 @@ import google.registry.tools.soy.DomainUpdateSoyInfo; import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.TreeSet; import org.joda.time.DateTime; @@ -172,8 +173,10 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand { if (!nameservers.isEmpty() || !admins.isEmpty() || !techs.isEmpty() || !statuses.isEmpty()) { DateTime now = DateTime.now(UTC); - DomainResource domainResource = loadByForeignKey(DomainResource.class, domain, now); - checkArgumentNotNull(domainResource, "Domain '%s' does not exist", domain); + Optional domainOptional = + loadByForeignKey(DomainResource.class, domain, now); + checkArgumentPresent(domainOptional, "Domain '%s' does not exist or is deleted", domain); + DomainResource domainResource = domainOptional.get(); checkArgument( !domainResource.getStatusValues().contains(SERVER_UPDATE_PROHIBITED), "The domain '%s' has status SERVER_UPDATE_PROHIBITED. Verify that you are allowed " diff --git a/java/google/registry/tools/UpdateSmdCommand.java b/java/google/registry/tools/UpdateSmdCommand.java index 6c516ae5f..ba1c417fa 100644 --- a/java/google/registry/tools/UpdateSmdCommand.java +++ b/java/google/registry/tools/UpdateSmdCommand.java @@ -84,11 +84,16 @@ final class UpdateSmdCommand implements CommandWithRemoteApi { DateTime now = ofy().getTransactionTime(); // Load the domain application. - DomainApplication domainApplication = loadDomainApplication(applicationId, now); - checkArgument(domainApplication != null, "Domain application does not exist"); + DomainApplication domainApplication = + loadDomainApplication(applicationId, now) + .orElseThrow( + () -> + new IllegalArgumentException( + "Domain application does not exist or is deleted")); // Make sure this is a sunrise application. - checkArgument(!domainApplication.getEncodedSignedMarks().isEmpty(), + checkArgument( + !domainApplication.getEncodedSignedMarks().isEmpty(), "Can't update SMD on a landrush application."); // Verify the new SMD. diff --git a/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java b/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java index 48df4d270..d872c0fdf 100644 --- a/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java +++ b/javatests/google/registry/batch/DeleteContactsAndHostsActionTest.java @@ -193,14 +193,14 @@ public class DeleteContactsAndHostsActionTest false); runMapreduce(); ContactResource contactUpdated = - loadByForeignKey(ContactResource.class, "blah8221", clock.nowUtc()); + loadByForeignKey(ContactResource.class, "blah8221", clock.nowUtc()).get(); assertAboutContacts() .that(contactUpdated) .doesNotHaveStatusValue(PENDING_DELETE) .and() .hasDeletionTime(END_OF_TIME); DomainResource domainReloaded = - loadByForeignKey(DomainResource.class, "example.tld", clock.nowUtc()); + loadByForeignKey(DomainResource.class, "example.tld", clock.nowUtc()).get(); assertThat(domainReloaded.getReferencedContacts()).contains(Key.create(contactUpdated)); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(contactUpdated, HistoryEntry.Type.CONTACT_DELETE_FAILURE); @@ -268,7 +268,7 @@ public class DeleteContactsAndHostsActionTest Trid.create(clientTrid.orElse(null), "fakeServerTrid"), false); runMapreduce(); - assertThat(loadByForeignKey(ContactResource.class, "jim919", clock.nowUtc())).isNull(); + assertThat(loadByForeignKey(ContactResource.class, "jim919", clock.nowUtc())).isEmpty(); ContactResource contactAfterDeletion = ofy().load().entity(contact).now(); assertAboutContacts() .that(contactAfterDeletion) @@ -332,10 +332,10 @@ public class DeleteContactsAndHostsActionTest false); runMapreduce(); // Check that the contact is deleted as of now. - assertThat(loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc())).isNull(); + assertThat(loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc())).isEmpty(); // Check that it's still there (it wasn't deleted yesterday) and that it has history. ContactResource softDeletedContact = - loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc().minusDays(1)); + loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc().minusDays(1)).get(); assertAboutContacts() .that(softDeletedContact) .hasOneHistoryEntryEachOfTypes(CONTACT_TRANSFER_REQUEST, CONTACT_DELETE); @@ -393,9 +393,9 @@ public class DeleteContactsAndHostsActionTest Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); - assertThat(loadByForeignKey(ContactResource.class, "blah1234", clock.nowUtc())).isNull(); + assertThat(loadByForeignKey(ContactResource.class, "blah1234", clock.nowUtc())).isEmpty(); ContactResource contactBeforeDeletion = - loadByForeignKey(ContactResource.class, "blah1234", clock.nowUtc().minusDays(1)); + loadByForeignKey(ContactResource.class, "blah1234", clock.nowUtc().minusDays(1)).get(); assertAboutContacts() .that(contactBeforeDeletion) .isNotActiveAt(clock.nowUtc()) @@ -428,7 +428,7 @@ public class DeleteContactsAndHostsActionTest false); runMapreduce(); ContactResource contactAfter = - loadByForeignKey(ContactResource.class, "jane0991", clock.nowUtc()); + loadByForeignKey(ContactResource.class, "jane0991", clock.nowUtc()).get(); assertAboutContacts() .that(contactAfter) .doesNotHaveStatusValue(PENDING_DELETE) @@ -455,7 +455,7 @@ public class DeleteContactsAndHostsActionTest Trid.create("fakeClientTrid", "fakeServerTrid"), true); runMapreduce(); - assertThat(loadByForeignKey(ContactResource.class, "nate007", clock.nowUtc())).isNull(); + assertThat(loadByForeignKey(ContactResource.class, "nate007", clock.nowUtc())).isEmpty(); ContactResource contactAfterDeletion = ofy().load().entity(contact).now(); assertAboutContacts() .that(contactAfterDeletion) @@ -561,9 +561,9 @@ public class DeleteContactsAndHostsActionTest false); enqueueMapreduceOnly(); assertThat(loadByForeignKey(ContactResource.class, "blah2222", clock.nowUtc())) - .isEqualTo(contact); + .hasValue(contact); assertThat(loadByForeignKey(HostResource.class, "rustles.your.jimmies", clock.nowUtc())) - .isEqualTo(host); + .hasValue(host); assertNoTasksEnqueued(QUEUE_ASYNC_DELETE); verify(action.asyncFlowMetrics).recordContactHostDeletionBatchSize(2L); verify(action.asyncFlowMetrics) @@ -610,13 +610,14 @@ public class DeleteContactsAndHostsActionTest false); runMapreduce(); HostResource hostAfter = - loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc()); + loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc()).get(); assertAboutHosts() .that(hostAfter) .doesNotHaveStatusValue(PENDING_DELETE) .and() .hasDeletionTime(END_OF_TIME); - DomainResource domain = loadByForeignKey(DomainResource.class, "example.tld", clock.nowUtc()); + DomainResource domain = + loadByForeignKey(DomainResource.class, "example.tld", clock.nowUtc()).get(); assertThat(domain.getNameservers()).contains(Key.create(hostAfter)); HistoryEntry historyEntry = getOnlyHistoryEntryOfType(hostAfter, HOST_DELETE_FAILURE); assertPollMessageFor( @@ -653,9 +654,9 @@ public class DeleteContactsAndHostsActionTest Trid.create(clientTrid.orElse(null), "fakeServerTrid"), false); runMapreduce(); - assertThat(loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())).isNull(); + assertThat(loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())).isEmpty(); HostResource hostBeforeDeletion = - loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc().minusDays(1)); + loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc().minusDays(1)).get(); assertAboutHosts() .that(hostBeforeDeletion) .isNotActiveAt(clock.nowUtc()) @@ -697,9 +698,9 @@ public class DeleteContactsAndHostsActionTest Trid.create("fakeClientTrid", "fakeServerTrid"), false); runMapreduce(); - assertThat(loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc())).isNull(); + assertThat(loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc())).isEmpty(); HostResource hostBeforeDeletion = - loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc().minusDays(1)); + loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc().minusDays(1)).get(); assertAboutHosts() .that(hostBeforeDeletion) .isNotActiveAt(clock.nowUtc()) @@ -743,15 +744,16 @@ public class DeleteContactsAndHostsActionTest false); runMapreduce(); // Check that the host is deleted as of now. - assertThat(loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())).isNull(); + assertThat(loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())).isEmpty(); assertNoBillingEvents(); assertThat( loadByForeignKey(DomainResource.class, "example.tld", clock.nowUtc()) + .get() .getSubordinateHosts()) .isEmpty(); assertDnsTasksEnqueued("ns2.example.tld"); HostResource hostBeforeDeletion = - loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc().minusDays(1)); + loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc().minusDays(1)).get(); assertAboutHosts() .that(hostBeforeDeletion) .isNotActiveAt(clock.nowUtc()) @@ -782,7 +784,7 @@ public class DeleteContactsAndHostsActionTest false); runMapreduce(); HostResource hostAfter = - loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc()); + loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc()).get(); assertAboutHosts() .that(hostAfter) .doesNotHaveStatusValue(PENDING_DELETE) @@ -809,9 +811,9 @@ public class DeleteContactsAndHostsActionTest Trid.create("fakeClientTrid", "fakeServerTrid"), true); runMapreduce(); - assertThat(loadByForeignKey(HostResource.class, "ns66.example.tld", clock.nowUtc())).isNull(); + assertThat(loadByForeignKey(HostResource.class, "ns66.example.tld", clock.nowUtc())).isEmpty(); HostResource hostBeforeDeletion = - loadByForeignKey(HostResource.class, "ns66.example.tld", clock.nowUtc().minusDays(1)); + loadByForeignKey(HostResource.class, "ns66.example.tld", clock.nowUtc().minusDays(1)).get(); assertAboutHosts() .that(hostBeforeDeletion) .isNotActiveAt(clock.nowUtc()) diff --git a/javatests/google/registry/batch/DeleteProberDataActionTest.java b/javatests/google/registry/batch/DeleteProberDataActionTest.java index 3997e7673..a255819f6 100644 --- a/javatests/google/registry/batch/DeleteProberDataActionTest.java +++ b/javatests/google/registry/batch/DeleteProberDataActionTest.java @@ -15,6 +15,7 @@ package google.registry.batch; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.testing.DatastoreHelper.createTld; @@ -46,6 +47,7 @@ import google.registry.model.registry.Registry.TldType; import google.registry.model.reporting.HistoryEntry; import google.registry.testing.FakeResponse; import google.registry.testing.mapreduce.MapreduceTestCase; +import java.util.Optional; import java.util.Set; import org.joda.money.Money; import org.joda.time.DateTime; @@ -190,7 +192,7 @@ public class DeleteProberDataActionTest extends MapreduceTestCase domain = loadByForeignKey(DomainResource.class, "blah.ib-any.test", DateTime.now(UTC)); - assertThat(domain).isNotNull(); - assertThat(domain.getDeletionTime()).isEqualTo(END_OF_TIME); + assertThat(domain).isPresent(); + assertThat(domain.get().getDeletionTime()).isEqualTo(END_OF_TIME); } @Test diff --git a/javatests/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java b/javatests/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java index 91a8c0b20..641b7a3ce 100644 --- a/javatests/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java +++ b/javatests/google/registry/dns/writer/dnsupdate/DnsUpdateWriterTest.java @@ -250,7 +250,7 @@ public class DnsUpdateWriterTest { newDomainResource("example.tld") .asBuilder() .addSubordinateHost("ns1.example.tld") - .addNameservers(ImmutableSet.of(Key.create(host))) + .addNameserver(Key.create(host)) .build()); writer.publishHost("ns1.example.tld"); @@ -358,7 +358,7 @@ public class DnsUpdateWriterTest { .asBuilder() .addSubordinateHost("ns1.example.tld") .addSubordinateHost("foo.example.tld") - .addNameservers(ImmutableSet.of(Key.create(inBailiwickNameserver))) + .addNameserver(Key.create(inBailiwickNameserver)) .build()); writer.publishDomain("example.tld"); diff --git a/javatests/google/registry/flows/EppLifecycleDomainTest.java b/javatests/google/registry/flows/EppLifecycleDomainTest.java index a43c4f9a4..4f01a52a8 100644 --- a/javatests/google/registry/flows/EppLifecycleDomainTest.java +++ b/javatests/google/registry/flows/EppLifecycleDomainTest.java @@ -107,7 +107,7 @@ public class EppLifecycleDomainTest extends EppTestCase { "EXDATE", "2002-06-01T00:02:00.0Z")); DomainResource domain = - loadByForeignKey(DomainResource.class, "example.tld", createTime.plusHours(1)); + loadByForeignKey(DomainResource.class, "example.tld", createTime.plusHours(1)).get(); // Delete domain example.tld within the add grace period. DateTime deleteTime = createTime.plusDays(1); @@ -184,7 +184,8 @@ public class EppLifecycleDomainTest extends EppTestCase { DomainResource domain = loadByForeignKey( - DomainResource.class, "example.tld", DateTime.parse("2000-08-01T00:02:00Z")); + DomainResource.class, "example.tld", DateTime.parse("2000-08-01T00:02:00Z")) + .get(); // Verify that the autorenew was ended and that the one-time billing event is not canceled. assertBillingEventsForResource( domain, @@ -218,7 +219,8 @@ public class EppLifecycleDomainTest extends EppTestCase { DomainResource domain = loadByForeignKey( - DomainResource.class, "example.tld", DateTime.parse("2000-06-01T00:03:00Z")); + DomainResource.class, "example.tld", DateTime.parse("2000-06-01T00:03:00Z")) + .get(); // Delete domain example.tld within the add grade period. DateTime deleteTime = createTime.plusDays(1); diff --git a/javatests/google/registry/flows/EppLifecycleHostTest.java b/javatests/google/registry/flows/EppLifecycleHostTest.java index 2328424c3..334213ca1 100644 --- a/javatests/google/registry/flows/EppLifecycleHostTest.java +++ b/javatests/google/registry/flows/EppLifecycleHostTest.java @@ -218,9 +218,9 @@ public class EppLifecycleHostTest extends EppTestCase { DateTime timeAfterCreates = DateTime.parse("2000-06-01T00:06:00Z"); HostResource exampleBarFooTldHost = - loadByForeignKey(HostResource.class, "ns1.example.bar.foo.tld", timeAfterCreates); + loadByForeignKey(HostResource.class, "ns1.example.bar.foo.tld", timeAfterCreates).get(); DomainResource exampleBarFooTldDomain = - loadByForeignKey(DomainResource.class, "example.bar.foo.tld", timeAfterCreates); + loadByForeignKey(DomainResource.class, "example.bar.foo.tld", timeAfterCreates).get(); assertAboutHosts() .that(exampleBarFooTldHost) .hasSuperordinateDomain(Key.create(exampleBarFooTldDomain)); @@ -228,18 +228,18 @@ public class EppLifecycleHostTest extends EppTestCase { .containsExactly("ns1.example.bar.foo.tld"); HostResource exampleFooTldHost = - loadByForeignKey(HostResource.class, "ns1.example.foo.tld", timeAfterCreates); + loadByForeignKey(HostResource.class, "ns1.example.foo.tld", timeAfterCreates).get(); DomainResource exampleFooTldDomain = - loadByForeignKey(DomainResource.class, "example.foo.tld", timeAfterCreates); + loadByForeignKey(DomainResource.class, "example.foo.tld", timeAfterCreates).get(); assertAboutHosts() .that(exampleFooTldHost) .hasSuperordinateDomain(Key.create(exampleFooTldDomain)); assertThat(exampleFooTldDomain.getSubordinateHosts()).containsExactly("ns1.example.foo.tld"); HostResource exampleTldHost = - loadByForeignKey(HostResource.class, "ns1.example.tld", timeAfterCreates); + loadByForeignKey(HostResource.class, "ns1.example.tld", timeAfterCreates).get(); DomainResource exampleTldDomain = - loadByForeignKey(DomainResource.class, "example.tld", timeAfterCreates); + loadByForeignKey(DomainResource.class, "example.tld", timeAfterCreates).get(); assertAboutHosts().that(exampleTldHost).hasSuperordinateDomain(Key.create(exampleTldDomain)); assertThat(exampleTldDomain.getSubordinateHosts()).containsExactly("ns1.example.tld"); diff --git a/javatests/google/registry/flows/ResourceFlowTestCase.java b/javatests/google/registry/flows/ResourceFlowTestCase.java index 9a0f19478..dc5fda55f 100644 --- a/javatests/google/registry/flows/ResourceFlowTestCase.java +++ b/javatests/google/registry/flows/ResourceFlowTestCase.java @@ -17,6 +17,7 @@ package google.registry.flows; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.EppResourceUtils.loadByForeignKey; +import static google.registry.model.EppResourceUtils.loadDomainApplication; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.tmch.ClaimsListShardTest.createTestClaimsListShard; import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions; @@ -32,7 +33,6 @@ import com.google.common.testing.TestLogHandler; import com.googlecode.objectify.Key; import google.registry.flows.FlowUtils.NotLoggedInException; import google.registry.model.EppResource; -import google.registry.model.EppResourceUtils; import google.registry.model.domain.DomainApplication; import google.registry.model.domain.launch.ApplicationIdTargetExtension; import google.registry.model.eppcommon.Trid; @@ -46,6 +46,7 @@ import google.registry.testing.TaskQueueHelper.TaskMatcher; import google.registry.util.TypeUtils.TypeInstantiator; import java.util.Optional; import java.util.logging.Level; +import javax.annotation.Nullable; import org.joda.time.DateTime; import org.joda.time.Duration; import org.json.simple.JSONValue; @@ -71,20 +72,22 @@ public abstract class ResourceFlowTestCase T reloadResourceAndCloneAtTime(T resource, DateTime now) { diff --git a/javatests/google/registry/flows/domain/DomainAllocateFlowTest.java b/javatests/google/registry/flows/domain/DomainAllocateFlowTest.java index 35ad7a125..408252b43 100644 --- a/javatests/google/registry/flows/domain/DomainAllocateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainAllocateFlowTest.java @@ -150,7 +150,7 @@ public class DomainAllocateFlowTest boolean sunrushAddGracePeriod = (nameservers == 0); // The application should be marked as allocated, with a new history entry. - DomainApplication application = loadDomainApplication(applicationId, clock.nowUtc()); + DomainApplication application = loadDomainApplication(applicationId, clock.nowUtc()).get(); assertAboutApplications().that(application) .hasApplicationStatus(ApplicationStatus.ALLOCATED).and() .hasHistoryEntryAtIndex(0) diff --git a/javatests/google/registry/flows/domain/DomainApplicationDeleteFlowTest.java b/javatests/google/registry/flows/domain/DomainApplicationDeleteFlowTest.java index 1059a69c3..e3df1d02e 100644 --- a/javatests/google/registry/flows/domain/DomainApplicationDeleteFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainApplicationDeleteFlowTest.java @@ -15,8 +15,10 @@ package google.registry.flows.domain; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.EppResourceUtils.isLinked; import static google.registry.model.EppResourceUtils.loadByForeignKey; +import static google.registry.model.EppResourceUtils.loadDomainApplication; import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.testing.DatastoreHelper.assertNoBillingEvents; import static google.registry.testing.DatastoreHelper.createTld; @@ -72,7 +74,7 @@ public class DomainApplicationDeleteFlowTest clock.advanceOneMilli(); runFlowAssertResponse(loadFile("generic_success_response.xml")); // Check that the domain is fully deleted. - assertThat(reloadDomainApplication()).isNull(); + assertThat(loadDomainApplication(getUniqueIdFromCommand(), clock.nowUtc())).isEmpty(); assertNoBillingEvents(); } @@ -93,11 +95,10 @@ public class DomainApplicationDeleteFlowTest .asBuilder() .setRepoId("1-TLD") .setRegistrant( - Key.create(loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc()))) + Key.create(loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc()).get())) .setNameservers( - ImmutableSet.of( - Key.create( - loadByForeignKey(HostResource.class, "ns1.example.net", clock.nowUtc())))) + Key.create( + loadByForeignKey(HostResource.class, "ns1.example.net", clock.nowUtc()).get())) .build()); doSuccessfulTest(); for (Key key : diff --git a/javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java b/javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java index f671e568a..755f673b4 100644 --- a/javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java @@ -112,16 +112,15 @@ public class DomainApplicationUpdateFlowTest } private DomainApplication persistApplication() { + HostResource host = + loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc()).get(); return persistResource( newApplicationBuilder() .setContacts( ImmutableSet.of( DesignatedContact.create(Type.TECH, Key.create(sh8013Contact)), DesignatedContact.create(Type.ADMIN, Key.create(unusedContact)))) - .setNameservers( - ImmutableSet.of( - Key.create( - loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc())))) + .setNameservers(ImmutableSet.of(Key.create(host))) .build()); } @@ -184,7 +183,8 @@ public class DomainApplicationUpdateFlowTest public void testSuccess_registrantMovedToTechContact() throws Exception { setEppInput("domain_update_sunrise_registrant_to_tech.xml"); persistReferencedEntities(); - ContactResource sh8013 = loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc()); + ContactResource sh8013 = + loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc()).get(); persistResource(newApplicationBuilder().setRegistrant(Key.create(sh8013)).build()); clock.advanceOneMilli(); runFlowAssertResponse(loadFile("generic_success_response.xml")); @@ -194,7 +194,8 @@ public class DomainApplicationUpdateFlowTest public void testSuccess_multipleReferencesToSameContactRemoved() throws Exception { setEppInput("domain_update_sunrise_remove_multiple_contacts.xml"); persistReferencedEntities(); - ContactResource sh8013 = loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc()); + ContactResource sh8013 = + loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc()).get(); Key sh8013Key = Key.create(sh8013); persistResource( newApplicationBuilder() @@ -411,7 +412,8 @@ public class DomainApplicationUpdateFlowTest nameservers.add( Key.create( loadByForeignKey( - HostResource.class, String.format("ns%d.example.tld", i), clock.nowUtc()))); + HostResource.class, String.format("ns%d.example.tld", i), clock.nowUtc()) + .get())); } } persistResource( @@ -546,7 +548,7 @@ public class DomainApplicationUpdateFlowTest DesignatedContact.create( Type.TECH, Key.create( - loadByForeignKey(ContactResource.class, "foo", clock.nowUtc()))))) + loadByForeignKey(ContactResource.class, "foo", clock.nowUtc()).get())))) .build()); EppException thrown = assertThrows(DuplicateContactForRoleException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); @@ -639,7 +641,8 @@ public class DomainApplicationUpdateFlowTest .setNameservers( ImmutableSet.of( Key.create( - loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc())))) + loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc()) + .get()))) .build()); EppException thrown = assertThrows(AddRemoveSameValueException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); @@ -656,7 +659,8 @@ public class DomainApplicationUpdateFlowTest DesignatedContact.create( Type.TECH, Key.create( - loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc()))))) + loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc()) + .get())))) .build()); EppException thrown = assertThrows(AddRemoveSameValueException.class, this::runFlow); assertAboutEppExceptions().that(thrown).marshalsToXml(); @@ -759,10 +763,9 @@ public class DomainApplicationUpdateFlowTest persistResource( reloadDomainApplication() .asBuilder() - .addNameservers( - ImmutableSet.of( - Key.create( - loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())))) + .addNameserver( + Key.create( + loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc()).get())) .build()); persistResource( Registry.get("tld") @@ -833,10 +836,9 @@ public class DomainApplicationUpdateFlowTest persistResource( reloadDomainApplication() .asBuilder() - .addNameservers( - ImmutableSet.of( - Key.create( - loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc())))) + .addNameserver( + Key.create( + loadByForeignKey(HostResource.class, "ns2.example.tld", clock.nowUtc()).get())) .build()); persistResource( Registry.get("tld") diff --git a/javatests/google/registry/flows/domain/DomainDeleteFlowTest.java b/javatests/google/registry/flows/domain/DomainDeleteFlowTest.java index 2a8b2d9bf..676165aa9 100644 --- a/javatests/google/registry/flows/domain/DomainDeleteFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainDeleteFlowTest.java @@ -692,6 +692,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase sh8013Key = Key.create(sh8013); persistResource( newDomainResource(getUniqueIdFromCommand()) @@ -936,11 +939,10 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase newFki = loadHostFki("ns1.example.com"); assertThat(newFki).isNotEqualTo(originalFki); - assertThat( - EppResourceUtils.loadByForeignKey( - HostResource.class, "ns1.example.com", clock.nowUtc())) - .isEqualTo(modifiedHost); + assertThat(loadByForeignKey(HostResource.class, "ns1.example.com", clock.nowUtc())) + .hasValue(modifiedHost); assertThat( ForeignKeyIndex.loadCached( HostResource.class, ImmutableList.of("ns1.example.com"), clock.nowUtc())) diff --git a/javatests/google/registry/tools/EppLifecycleToolsTest.java b/javatests/google/registry/tools/EppLifecycleToolsTest.java index 798d89b22..c17e62a19 100644 --- a/javatests/google/registry/tools/EppLifecycleToolsTest.java +++ b/javatests/google/registry/tools/EppLifecycleToolsTest.java @@ -141,7 +141,8 @@ public class EppLifecycleToolsTest extends EppTestCase { DateTime createTime = DateTime.parse("2000-06-01T00:02:00Z"); DomainResource domain = loadByForeignKey( - DomainResource.class, "example.tld", DateTime.parse("2003-06-02T00:02:00Z")); + DomainResource.class, "example.tld", DateTime.parse("2003-06-02T00:02:00Z")) + .get(); BillingEvent.OneTime renewBillingEvent = new BillingEvent.OneTime.Builder() .setReason(Reason.RENEW) diff --git a/javatests/google/registry/tools/GenerateDnsReportCommandTest.java b/javatests/google/registry/tools/GenerateDnsReportCommandTest.java index af94843ea..0e28ca725 100644 --- a/javatests/google/registry/tools/GenerateDnsReportCommandTest.java +++ b/javatests/google/registry/tools/GenerateDnsReportCommandTest.java @@ -196,7 +196,7 @@ public class GenerateDnsReportCommandTest extends CommandTestCase runCommandForced("--client=NewRegistrar", "missing.tld")); - assertThat(e).hasMessageThat().isEqualTo("Domain 'missing.tld' does not exist"); + assertThat(e).hasMessageThat().isEqualTo("Domain 'missing.tld' does not exist or is deleted"); } @Test diff --git a/javatests/google/registry/tools/UnlockDomainCommandTest.java b/javatests/google/registry/tools/UnlockDomainCommandTest.java index 2a235955b..3c06efede 100644 --- a/javatests/google/registry/tools/UnlockDomainCommandTest.java +++ b/javatests/google/registry/tools/UnlockDomainCommandTest.java @@ -94,7 +94,7 @@ public class UnlockDomainCommandTest extends EppToolCommandTestCase runCommandForced("--client=NewRegistrar", "missing.tld")); - assertThat(e).hasMessageThat().isEqualTo("Domain 'missing.tld' does not exist"); + assertThat(e).hasMessageThat().isEqualTo("Domain 'missing.tld' does not exist or is deleted"); } @Test diff --git a/javatests/google/registry/tools/UnrenewDomainCommandTest.java b/javatests/google/registry/tools/UnrenewDomainCommandTest.java index 680eb3c75..abadd73ff 100644 --- a/javatests/google/registry/tools/UnrenewDomainCommandTest.java +++ b/javatests/google/registry/tools/UnrenewDomainCommandTest.java @@ -79,10 +79,12 @@ public class UnrenewDomainCommandTest extends CommandTestCase