diff --git a/core/src/main/java/google/registry/model/registrar/Registrar.java b/core/src/main/java/google/registry/model/registrar/Registrar.java index d27f6b870..4e235a273 100644 --- a/core/src/main/java/google/registry/model/registrar/Registrar.java +++ b/core/src/main/java/google/registry/model/registrar/Registrar.java @@ -996,6 +996,13 @@ public class Registrar extends ImmutableObject return CACHE_BY_CLIENT_ID.get().values(); } + /** Loads all registrar keys using an in-memory cache. */ + public static ImmutableSet> loadAllKeysCached() { + return CACHE_BY_CLIENT_ID.get().keySet().stream() + .map(Registrar::createVKey) + .collect(toImmutableSet()); + } + /** Loads and returns a registrar entity by its client id directly from Datastore. */ public static Optional loadByClientId(String clientId) { checkArgument(!Strings.isNullOrEmpty(clientId), "clientId must be specified"); diff --git a/core/src/main/java/google/registry/rdap/RdapEntityAction.java b/core/src/main/java/google/registry/rdap/RdapEntityAction.java index e57f786a9..6a4bfddfb 100644 --- a/core/src/main/java/google/registry/rdap/RdapEntityAction.java +++ b/core/src/main/java/google/registry/rdap/RdapEntityAction.java @@ -14,7 +14,8 @@ package google.registry.rdap; -import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import static google.registry.rdap.RdapUtils.getRegistrarByIanaIdentifier; import static google.registry.rdap.RdapUtils.getRegistrarByName; import static google.registry.request.Action.Method.GET; @@ -23,9 +24,9 @@ import static google.registry.request.Action.Method.HEAD; import com.google.common.collect.ImmutableSet; import com.google.common.primitives.Longs; import com.google.re2j.Pattern; -import com.googlecode.objectify.Key; import google.registry.model.contact.ContactResource; import google.registry.model.registrar.Registrar; +import google.registry.persistence.VKey; import google.registry.rdap.RdapJsonFormatter.OutputDataType; import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapObjectClasses.RdapEntity; @@ -69,13 +70,14 @@ public class RdapEntityAction extends RdapActionBase { // RDAP Technical Implementation Guide 2.3.1 - MUST support contact entity lookup using the // handle if (ROID_PATTERN.matcher(pathSearchString).matches()) { - Key contactKey = Key.create(ContactResource.class, pathSearchString); - ContactResource contactResource = ofy().load().key(contactKey).now(); + VKey contactVKey = VKey.create(ContactResource.class, pathSearchString); + Optional contactResource = + transactIfJpaTm(() -> tm().loadByKeyIfPresent(contactVKey)); // As per Andy Newton on the regext mailing list, contacts by themselves have no role, since // they are global, and might have different roles for different domains. - if (contactResource != null && isAuthorized(contactResource)) { + if (contactResource.isPresent() && isAuthorized(contactResource.get())) { return rdapJsonFormatter.createRdapContactEntity( - contactResource, ImmutableSet.of(), OutputDataType.FULL); + contactResource.get(), ImmutableSet.of(), OutputDataType.FULL); } } diff --git a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java index 018a90800..7a98cbc6a 100644 --- a/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java +++ b/core/src/main/java/google/registry/rdap/RdapJsonFormatter.java @@ -20,8 +20,8 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; import static google.registry.model.EppResourceUtils.isLinked; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import static google.registry.rdap.RdapIcannStandardInformation.CONTACT_REDACTED_VALUE; import static google.registry.util.CollectionUtils.union; @@ -35,7 +35,6 @@ import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; import com.google.common.net.InetAddresses; import com.google.gson.JsonArray; -import com.googlecode.objectify.Key; import google.registry.config.RegistryConfig.Config; import google.registry.model.EppResource; import google.registry.model.contact.ContactAddress; @@ -77,7 +76,6 @@ import java.net.URI; import java.nio.file.Paths; import java.util.HashMap; import java.util.Locale; -import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -343,15 +341,11 @@ public class RdapJsonFormatter { // Kick off the database loads of the nameservers that we will need, so it can load // asynchronously while we load and process the contacts. ImmutableSet loadedHosts = - ImmutableSet.copyOf(tm().loadByKeys(domainBase.getNameservers()).values()); + transactIfJpaTm( + () -> ImmutableSet.copyOf(tm().loadByKeys(domainBase.getNameservers()).values())); // Load the registrant and other contacts and add them to the data. - Map, ContactResource> loadedContacts = - ofy() - .load() - .keys( - domainBase.getReferencedContacts().stream() - .map(VKey::getOfyKey) - .collect(toImmutableSet())); + ImmutableMap, ContactResource> loadedContacts = + transactIfJpaTm(() -> tm().loadByKeysIfPresent(domainBase.getReferencedContacts())); // RDAP Response Profile 2.7.3, A domain MUST have the REGISTRANT, ADMIN, TECH roles and MAY // have others. We also add the BILLING. // @@ -359,16 +353,16 @@ public class RdapJsonFormatter { // fields we don't want to show (as opposed to not having contacts at all) because of GDPR etc. // // the GDPR redaction is handled in createRdapContactEntity - ImmutableSetMultimap, Type> contactsToRoles = + ImmutableSetMultimap, Type> contactsToRoles = Streams.concat( domainBase.getContacts().stream(), Stream.of(DesignatedContact.create(Type.REGISTRANT, domainBase.getRegistrant()))) .sorted(DESIGNATED_CONTACT_ORDERING) .collect( toImmutableSetMultimap( - contact -> contact.getContactKey().getOfyKey(), DesignatedContact::getType)); + DesignatedContact::getContactKey, DesignatedContact::getType)); - for (Key contactKey : contactsToRoles.keySet()) { + for (VKey contactKey : contactsToRoles.keySet()) { Set roles = contactsToRoles.get(contactKey).stream() .map(RdapJsonFormatter::convertContactTypeToRdapRole) @@ -430,10 +424,12 @@ public class RdapJsonFormatter { statuses.add(StatusValue.LINKED); } if (hostResource.isSubordinate() - && tm().loadByKey(hostResource.getSuperordinateDomain()) - .cloneProjectedAtTime(getRequestTime()) - .getStatusValues() - .contains(StatusValue.PENDING_TRANSFER)) { + && transactIfJpaTm( + () -> + tm().loadByKey(hostResource.getSuperordinateDomain()) + .cloneProjectedAtTime(getRequestTime()) + .getStatusValues() + .contains(StatusValue.PENDING_TRANSFER))) { statuses.add(StatusValue.PENDING_TRANSFER); } builder diff --git a/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java b/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java index 4392ba085..b1d844b60 100644 --- a/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java +++ b/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java @@ -392,22 +392,6 @@ public abstract class RdapSearchActionBase extends RdapActionBase { return setOtherQueryAttributes(query, deletedItemHandling, resultSetMaxSize); } - /** Handles searches by key using a simple string. */ - static Query queryItemsByKey( - Class clazz, - String queryString, - DeletedItemHandling deletedItemHandling, - int resultSetMaxSize) { - if (queryString.length() < RdapSearchPattern.MIN_INITIAL_STRING_LENGTH) { - throw new UnprocessableEntityException( - String.format( - "Initial search string must be at least %d characters", - RdapSearchPattern.MIN_INITIAL_STRING_LENGTH)); - } - Query query = ofy().load().type(clazz).filterKey("=", Key.create(clazz, queryString)); - return setOtherQueryAttributes(query, deletedItemHandling, resultSetMaxSize); - } - static Query setOtherQueryAttributes( Query query, DeletedItemHandling deletedItemHandling, int resultSetMaxSize) { if (deletedItemHandling != DeletedItemHandling.INCLUDE) { diff --git a/core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java b/core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java index 6974713f5..ec3c12a0a 100644 --- a/core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java +++ b/core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java @@ -16,7 +16,6 @@ package google.registry.rdap; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static java.nio.charset.StandardCharsets.UTF_8; @@ -34,7 +33,6 @@ import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; -import com.googlecode.objectify.Key; import google.registry.keyring.api.KeyModule; import google.registry.model.registrar.Registrar; import google.registry.model.registry.Registries; @@ -197,37 +195,37 @@ public final class UpdateRegistrarRdapBaseUrlsAction implements Runnable { @Override public void run() { ImmutableSetMultimap ianaToBaseUrls = getRdapBaseUrlsPerIanaId(); - - for (Key registrarKey : ofy().load().type(Registrar.class).keys()) { - tm() - .transact( - () -> { - Registrar registrar = ofy().load().key(registrarKey).now(); - // Has the registrar been deleted since we loaded the key? (unlikly, especially - // given we don't delete registrars...) - if (registrar == null) { - return; - } - // Only update REAL registrars - if (registrar.getType() != Registrar.Type.REAL) { - return; - } - String ianaId = String.valueOf(registrar.getIanaIdentifier()); - ImmutableSet baseUrls = ianaToBaseUrls.get(ianaId); - // If this registrar already has these values, skip it - if (registrar.getRdapBaseUrls().equals(baseUrls)) { - logger.atInfo().log( - "No change in RdapBaseUrls for registrar %s (ianaId %s)", - registrar.getClientId(), ianaId); - return; - } - logger.atInfo().log( - "Updating RdapBaseUrls for registrar %s (ianaId %s) from %s to %s", - registrar.getClientId(), ianaId, registrar.getRdapBaseUrls(), baseUrls); - ofy() - .save() - .entity(registrar.asBuilder().setRdapBaseUrls(baseUrls).build()); - }); - } + Registrar.loadAllKeysCached() + .forEach( + (key) -> + tm().transact( + () -> { + Registrar registrar = tm().loadByKey(key); + // Has the registrar been deleted since we loaded the key? (unlikely, + // especially given we don't delete registrars...) + if (registrar == null) { + return; + } + // Only update REAL registrars + if (registrar.getType() != Registrar.Type.REAL) { + return; + } + String ianaId = String.valueOf(registrar.getIanaIdentifier()); + ImmutableSet baseUrls = ianaToBaseUrls.get(ianaId); + // If this registrar already has these values, skip it + if (registrar.getRdapBaseUrls().equals(baseUrls)) { + logger.atInfo().log( + "No change in RdapBaseUrls for registrar %s (ianaId %s)", + registrar.getClientId(), ianaId); + return; + } + logger.atInfo().log( + "Updating RdapBaseUrls for registrar %s (ianaId %s) from %s to %s", + registrar.getClientId(), + ianaId, + registrar.getRdapBaseUrls(), + baseUrls); + tm().put(registrar.asBuilder().setRdapBaseUrls(baseUrls).build()); + })); } } diff --git a/core/src/test/java/google/registry/rdap/RdapEntityActionTest.java b/core/src/test/java/google/registry/rdap/RdapEntityActionTest.java index 7836319b2..6790f67da 100644 --- a/core/src/test/java/google/registry/rdap/RdapEntityActionTest.java +++ b/core/src/test/java/google/registry/rdap/RdapEntityActionTest.java @@ -38,12 +38,14 @@ import google.registry.rdap.RdapMetrics.SearchType; import google.registry.rdap.RdapMetrics.WildcardType; import google.registry.rdap.RdapSearchResults.IncompletenessWarningType; import google.registry.request.Action; +import google.registry.testing.DualDatabaseTest; +import google.registry.testing.TestOfyAndSql; import java.util.Optional; import javax.annotation.Nullable; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; /** Unit tests for {@link RdapEntityAction}. */ +@DualDatabaseTest class RdapEntityActionTest extends RdapActionBaseTestCase { RdapEntityActionTest() { @@ -187,35 +189,35 @@ class RdapEntityActionTest extends RdapActionBaseTestCase { assertThat(response.getStatus()).isEqualTo(404); } - @Test + @TestOfyAndSql void testUnknownEntity_RoidPattern_notFound() { runNotFoundTest("_MISSING-ENTITY_"); } - @Test + @TestOfyAndSql void testUnknownEntity_IanaPattern_notFound() { runNotFoundTest("123"); } - @Test + @TestOfyAndSql void testUnknownEntity_notRoidNotIana_notFound() { // Since we allow search by registrar name, every string is a possible name runNotFoundTest("some,random,string"); } - @Test + @TestOfyAndSql void testValidRegistrantContact_works() { login("evilregistrar"); runSuccessfulHandleTest(registrant.getRepoId(), "rdap_associated_contact.json"); } - @Test + @TestOfyAndSql void testValidRegistrantContact_found_asAdministrator() { loginAsAdmin(); runSuccessfulHandleTest(registrant.getRepoId(), "rdap_associated_contact.json"); } - @Test + @TestOfyAndSql void testValidRegistrantContact_found_notLoggedIn() { runSuccessfulHandleTest( registrant.getRepoId(), @@ -225,7 +227,7 @@ class RdapEntityActionTest extends RdapActionBaseTestCase { "rdap_associated_contact_no_personal_data.json"); } - @Test + @TestOfyAndSql void testValidRegistrantContact_found_loggedInAsOtherRegistrar() { login("otherregistrar"); runSuccessfulHandleTest( @@ -236,49 +238,49 @@ class RdapEntityActionTest extends RdapActionBaseTestCase { "rdap_associated_contact_no_personal_data.json"); } - @Test + @TestOfyAndSql void testValidAdminContact_works() { login("evilregistrar"); runSuccessfulHandleTest(adminContact.getRepoId(), "rdap_associated_contact.json"); } - @Test + @TestOfyAndSql void testValidTechContact_works() { login("evilregistrar"); runSuccessfulHandleTest(techContact.getRepoId(), "rdap_associated_contact.json"); } - @Test + @TestOfyAndSql void testValidDisconnectedContact_works() { login("evilregistrar"); runSuccessfulHandleTest(disconnectedContact.getRepoId(), "rdap_contact.json"); } - @Test + @TestOfyAndSql void testDeletedContact_notFound() { runNotFoundTest(deletedContact.getRepoId()); } - @Test + @TestOfyAndSql void testDeletedContact_notFound_includeDeletedSetFalse() { action.includeDeletedParam = Optional.of(false); runNotFoundTest(deletedContact.getRepoId()); } - @Test + @TestOfyAndSql void testDeletedContact_notFound_notLoggedIn() { action.includeDeletedParam = Optional.of(true); runNotFoundTest(deletedContact.getRepoId()); } - @Test + @TestOfyAndSql void testDeletedContact_notFound_loggedInAsDifferentRegistrar() { login("idnregistrar"); action.includeDeletedParam = Optional.of(true); runNotFoundTest(deletedContact.getRepoId()); } - @Test + @TestOfyAndSql void testDeletedContact_found_loggedInAsCorrectRegistrar() { login("evilregistrar"); action.includeDeletedParam = Optional.of(true); @@ -290,7 +292,7 @@ class RdapEntityActionTest extends RdapActionBaseTestCase { "rdap_contact_deleted.json"); } - @Test + @TestOfyAndSql void testDeletedContact_found_loggedInAsAdmin() { loginAsAdmin(); action.includeDeletedParam = Optional.of(true); @@ -302,12 +304,12 @@ class RdapEntityActionTest extends RdapActionBaseTestCase { "rdap_contact_deleted.json"); } - @Test + @TestOfyAndSql void testRegistrar_found() { runSuccessfulHandleTest("101", "Yes Virginia