From f760327ffd969cbc57322b9fbe06615c76dc7dd8 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Tue, 9 Mar 2021 11:11:53 -0500 Subject: [PATCH] Add SQL queries to RdapDomainSearchAction (#982) * Add SQL queries to RdapDomainSearchAction Unfortunately, because ORDER BY uses the locale's sorting functionality, we end up with some weird sort orders in SQL-land (notably, periods are ignored / omitted). As a result, a few of the tests have to be separated out into ofy and SQL versions based on the expected sort order. In addition, there isn't a way to query @Convert-ed fields in Postgres via the standard Hibernate / JPA query language, meaning we have to use a raw Postgres query for that. --- config/presubmits.py | 1 + .../transaction/CriteriaQueryBuilder.java | 35 +- .../registry/rdap/RdapDomainSearchAction.java | 319 ++++++++++---- .../registry/rdap/RdapSearchActionBase.java | 20 +- .../transaction/CriteriaQueryBuilderTest.java | 32 +- .../rdap/RdapDomainSearchActionTest.java | 406 ++++++++++++------ 6 files changed, 572 insertions(+), 241 deletions(-) diff --git a/config/presubmits.py b/config/presubmits.py index 2863e1cc8..1902713ef 100644 --- a/config/presubmits.py +++ b/config/presubmits.py @@ -209,6 +209,7 @@ PRESUBMITS = { "JpaTransactionManagerImpl.java", # CriteriaQueryBuilder is a false positive "CriteriaQueryBuilder.java", + "RdapDomainSearchAction.java", "RdapSearchActionBase.java", }, ): diff --git a/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java b/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java index 1d9e8fd57..ec8cb39ec 100644 --- a/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java +++ b/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java @@ -39,11 +39,6 @@ public class CriteriaQueryBuilder { Predicate predicate(Expression expression, U object); } - /** Functional interface that defines the order-by operator, e.g. {@link CriteriaBuilder#asc}. */ - public interface OrderByClause { - Order order(Expression expression); - } - private final CriteriaQuery query; private final Root root; private final ImmutableList.Builder predicates = new ImmutableList.Builder<>(); @@ -55,7 +50,7 @@ public class CriteriaQueryBuilder { } /** Adds a WHERE clause to the query, given the specified operation, field, and value. */ - public CriteriaQueryBuilder where(WhereClause whereClause, String fieldName, V value) { + public CriteriaQueryBuilder where(String fieldName, WhereClause whereClause, V value) { Expression expression = root.get(fieldName); return where(whereClause.predicate(expression, value)); } @@ -65,10 +60,25 @@ public class CriteriaQueryBuilder { return where(root.get(fieldName).in(values)); } - /** Orders the result by the given operation applied to the given field. */ - public CriteriaQueryBuilder orderBy(OrderByClause orderByClause, String fieldName) { - Expression expression = root.get(fieldName); - return orderBy(orderByClause.order(expression)); + /** + * Adds a WHERE clause to the query specifying that a collection field must contain a particular + * value. + */ + public CriteriaQueryBuilder whereFieldContains(String fieldName, Object value) { + return where( + jpaTm().getEntityManager().getCriteriaBuilder().isMember(value, root.get(fieldName))); + } + + /** Orders the result by the given field ascending. */ + public CriteriaQueryBuilder orderByAsc(String fieldName) { + orders.add(jpaTm().getEntityManager().getCriteriaBuilder().asc(root.get(fieldName))); + return this; + } + + /** Orders the result by the given field descending. */ + public CriteriaQueryBuilder orderByDesc(String fieldName) { + orders.add(jpaTm().getEntityManager().getCriteriaBuilder().desc(root.get(fieldName))); + return this; } /** Builds and returns the query, applying all WHERE and ORDER BY clauses at once. */ @@ -82,11 +92,6 @@ public class CriteriaQueryBuilder { return this; } - private CriteriaQueryBuilder orderBy(Order order) { - orders.add(order); - return this; - } - /** Creates a query builder that will SELECT from the given class. */ public static CriteriaQueryBuilder create(Class clazz) { CriteriaQuery query = jpaTm().getEntityManager().getCriteriaBuilder().createQuery(clazz); diff --git a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java b/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java index 90f23e181..37df31b37 100644 --- a/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/core/src/main/java/google/registry/rdap/RdapDomainSearchAction.java @@ -18,11 +18,15 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static google.registry.model.EppResourceUtils.loadByForeignKey; import static google.registry.model.index.ForeignKeyIndex.loadAndGetKey; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.request.Action.Method.GET; import static google.registry.request.Action.Method.HEAD; +import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DateTimeUtils.START_OF_TIME; +import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Streams; @@ -33,6 +37,7 @@ import com.googlecode.objectify.cmd.Query; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.persistence.VKey; +import google.registry.persistence.transaction.CriteriaQueryBuilder; import google.registry.rdap.RdapJsonFormatter.OutputDataType; import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapMetrics.SearchType; @@ -53,6 +58,8 @@ import java.util.Optional; import java.util.stream.Stream; import java.util.stream.StreamSupport; import javax.inject.Inject; +import javax.persistence.criteria.CriteriaBuilder; +import org.hibernate.Hibernate; /** * RDAP (new WHOIS) action for domain search requests. @@ -75,8 +82,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { static final int RESULT_SET_SIZE_SCALING_FACTOR = 30; - @NonFinalForTesting - static int maxNameserversInFirstStage = 300; + @NonFinalForTesting static int maxNameserversInFirstStage = 300; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -180,9 +186,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { Optional domainBase = loadByForeignKey(DomainBase.class, partialStringQuery.getInitialString(), getRequestTime()); return makeSearchResults( - shouldBeVisible(domainBase) - ? ImmutableList.of(domainBase.get()) - : ImmutableList.of()); + shouldBeVisible(domainBase) ? ImmutableList.of(domainBase.get()) : ImmutableList.of()); } /** Searches for domains by domain name with an initial string, wildcard and possible suffix. */ @@ -198,21 +202,53 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { // domains directly, rather than the foreign keys, because then we have an index on TLD if we // need it. int querySizeLimit = RESULT_SET_SIZE_SCALING_FACTOR * rdapResultSetMaxSize; - Query query = - ofy() - .load() - .type(DomainBase.class) - .filter("fullyQualifiedDomainName <", partialStringQuery.getNextInitialString()) - .filter("fullyQualifiedDomainName >=", partialStringQuery.getInitialString()); - if (cursorString.isPresent()) { - query = query.filter("fullyQualifiedDomainName >", cursorString.get()); + RdapResultSet resultSet; + if (isDatastore()) { + Query query = + ofy() + .load() + .type(DomainBase.class) + .filter("fullyQualifiedDomainName <", partialStringQuery.getNextInitialString()) + .filter("fullyQualifiedDomainName >=", partialStringQuery.getInitialString()); + if (cursorString.isPresent()) { + query = query.filter("fullyQualifiedDomainName >", cursorString.get()); + } + if (partialStringQuery.getSuffix() != null) { + query = query.filter("tld", partialStringQuery.getSuffix()); + } + query = query.limit(querySizeLimit); + // Always check for visibility, because we couldn't look at the deletionTime in the query. + resultSet = getMatchingResources(query, true, querySizeLimit); + } else { + resultSet = + jpaTm() + .transact( + () -> { + CriteriaBuilder criteriaBuilder = + jpaTm().getEntityManager().getCriteriaBuilder(); + CriteriaQueryBuilder queryBuilder = + CriteriaQueryBuilder.create(DomainBase.class) + .where( + "fullyQualifiedDomainName", + criteriaBuilder::like, + String.format("%s%%", partialStringQuery.getInitialString())) + .orderByAsc("fullyQualifiedDomainName"); + if (cursorString.isPresent()) { + queryBuilder = + queryBuilder.where( + "fullyQualifiedDomainName", + criteriaBuilder::greaterThan, + cursorString.get()); + } + if (partialStringQuery.getSuffix() != null) { + queryBuilder = + queryBuilder.where( + "tld", criteriaBuilder::equal, partialStringQuery.getSuffix()); + } + return getMatchingResourcesSql(queryBuilder, true, querySizeLimit); + }); } - if (partialStringQuery.getSuffix() != null) { - query = query.filter("tld", partialStringQuery.getSuffix()); - } - query = query.limit(querySizeLimit); - // Always check for visibility, because we couldn't look at the deletionTime in the query. - return makeSearchResults(getMatchingResources(query, true, querySizeLimit)); + return makeSearchResults(resultSet); } /** Searches for domains by domain name with a TLD suffix. */ @@ -222,16 +258,32 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { // searchByDomainNameWithInitialString, unable to perform an inequality query on deletion time. // Don't use queryItems, because it doesn't handle pending deletes. int querySizeLimit = RESULT_SET_SIZE_SCALING_FACTOR * rdapResultSetMaxSize; - Query query = - ofy() - .load() - .type(DomainBase.class) - .filter("tld", tld); - if (cursorString.isPresent()) { - query = query.filter("fullyQualifiedDomainName >", cursorString.get()); + RdapResultSet resultSet; + if (isDatastore()) { + Query query = ofy().load().type(DomainBase.class).filter("tld", tld); + if (cursorString.isPresent()) { + query = query.filter("fullyQualifiedDomainName >", cursorString.get()); + } + query = query.order("fullyQualifiedDomainName").limit(querySizeLimit); + resultSet = getMatchingResources(query, true, querySizeLimit); + } else { + resultSet = + jpaTm() + .transact( + () -> { + CriteriaQueryBuilder builder = + queryItemsSql( + DomainBase.class, + "tld", + tld, + Optional.of("fullyQualifiedDomainName"), + cursorString, + DeletedItemHandling.INCLUDE) + .orderByAsc("fullyQualifiedDomainName"); + return getMatchingResourcesSql(builder, true, querySizeLimit); + }); } - query = query.order("fullyQualifiedDomainName").limit(querySizeLimit); - return makeSearchResults(getMatchingResources(query, true, querySizeLimit)); + return makeSearchResults(resultSet); } /** @@ -245,7 +297,8 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { */ private DomainSearchResponse searchByNameserverLdhName( final RdapSearchPattern partialStringQuery) { - Iterable> hostKeys = getNameserverRefsByLdhName(partialStringQuery); + ImmutableCollection> hostKeys = + getNameserverRefsByLdhName(partialStringQuery); if (Iterables.isEmpty(hostKeys)) { metricInformationBuilder.setNumHostsRetrieved(0); throw new NotFoundException("No matching nameservers found"); @@ -263,7 +316,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { * initial string is not required (e.g. "*.example.tld" is valid), because we can look up the * domain and just list all of its subordinate hosts. */ - private Iterable> getNameserverRefsByLdhName( + private ImmutableCollection> getNameserverRefsByLdhName( final RdapSearchPattern partialStringQuery) { // Handle queries without a wildcard. if (!partialStringQuery.getHasWildcard()) { @@ -282,25 +335,50 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { // Only return the first maxNameserversInFirstStage nameservers. This could result in an // incomplete result set if a search asks for something like "ns*", but we need to enforce a // limit in order to avoid arbitrarily long-running queries. - Query query = - queryItems( - HostResource.class, - "fullyQualifiedHostName", - partialStringQuery, - Optional.empty(), - DeletedItemHandling.EXCLUDE, - maxNameserversInFirstStage); Optional desiredRegistrar = getDesiredRegistrar(); - if (desiredRegistrar.isPresent()) { - query = query.filter("currentSponsorClientId", desiredRegistrar.get()); + if (isDatastore()) { + Query query = + queryItems( + HostResource.class, + "fullyQualifiedHostName", + partialStringQuery, + Optional.empty(), + DeletedItemHandling.EXCLUDE, + maxNameserversInFirstStage); + if (desiredRegistrar.isPresent()) { + query = query.filter("currentSponsorClientId", desiredRegistrar.get()); + } + return StreamSupport.stream(query.keys().spliterator(), false) + .map(VKey::from) + .collect(toImmutableSet()); + } else { + return jpaTm() + .transact( + () -> { + CriteriaQueryBuilder builder = + queryItemsSql( + HostResource.class, + "fullyQualifiedHostName", + partialStringQuery, + Optional.empty(), + DeletedItemHandling.EXCLUDE); + if (desiredRegistrar.isPresent()) { + builder = + builder.where( + "currentSponsorClientId", + jpaTm().getEntityManager().getCriteriaBuilder()::equal, + desiredRegistrar.get()); + } + return getMatchingResourcesSql(builder, true, maxNameserversInFirstStage) + .resources().stream() + .map(HostResource::createVKey) + .collect(toImmutableSet()); + }); } - return StreamSupport.stream(query.keys().spliterator(), false) - .map(VKey::from) - .collect(toImmutableSet()); } /** Assembles a list of {@link HostResource} keys by name when the pattern has no wildcard. */ - private Iterable> getNameserverRefsByLdhNameWithoutWildcard( + private ImmutableList> getNameserverRefsByLdhNameWithoutWildcard( final RdapSearchPattern partialStringQuery) { // If we need to check the sponsoring registrar, we need to load the resource rather than just // the key. @@ -326,7 +404,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { } /** Assembles a list of {@link HostResource} keys by name using a superordinate domain suffix. */ - private Iterable> getNameserverRefsByLdhNameWithSuffix( + private ImmutableList> getNameserverRefsByLdhNameWithSuffix( final RdapSearchPattern partialStringQuery) { // The suffix must be a domain that we manage. That way, we can look up the domain and search // through the subordinate hosts. This is more efficient, and lets us permit wildcard searches @@ -391,23 +469,66 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { * domains which used to be connected to an undeleted nameserver. */ private DomainSearchResponse searchByNameserverIp(final InetAddress inetAddress) { - Query query = - queryItems( - HostResource.class, - "inetAddresses", - inetAddress.getHostAddress(), - Optional.empty(), - Optional.empty(), - DeletedItemHandling.EXCLUDE, - maxNameserversInFirstStage); Optional desiredRegistrar = getDesiredRegistrar(); - if (desiredRegistrar.isPresent()) { - query = query.filter("currentSponsorClientId", desiredRegistrar.get()); + ImmutableSet> hostKeys; + if (isDatastore()) { + Query query = + queryItems( + HostResource.class, + "inetAddresses", + inetAddress.getHostAddress(), + Optional.empty(), + Optional.empty(), + DeletedItemHandling.EXCLUDE, + maxNameserversInFirstStage); + if (desiredRegistrar.isPresent()) { + query = query.filter("currentSponsorClientId", desiredRegistrar.get()); + } + hostKeys = + StreamSupport.stream(query.keys().spliterator(), false) + .map(VKey::from) + .collect(toImmutableSet()); + } else { + hostKeys = + jpaTm() + .transact( + () -> { + // Hibernate does not allow us to query @Converted array fields directly, either + // in the CriteriaQuery or the raw text format. However, Postgres does -- so we + // use native queries to find hosts where any of the inetAddresses match. + javax.persistence.Query query; + if (desiredRegistrar.isPresent()) { + query = + jpaTm() + .getEntityManager() + .createNativeQuery( + "SELECT h.repo_id FROM \"Host\" h WHERE :address = " + + "ANY(h.inet_addresses) AND " + + "h.current_sponsor_registrar_id = :desiredRegistrar AND " + + "h.deletion_time = CAST(:endOfTime AS timestamptz)") + .setParameter("desiredRegistrar", desiredRegistrar.get()); + } else { + query = + jpaTm() + .getEntityManager() + .createNativeQuery( + "SELECT h.repo_id FROM \"Host\" h WHERE :address = " + + "ANY(h.inet_addresses) AND " + + "h.deletion_time = CAST(:endOfTime AS timestamptz)"); + } + @SuppressWarnings("unchecked") + Stream resultStream = + query + .setParameter("address", InetAddresses.toAddrString(inetAddress)) + .setParameter("endOfTime", END_OF_TIME.toString()) + .setMaxResults(maxNameserversInFirstStage) + .getResultStream(); + return resultStream + .map(repoId -> VKey.create(HostResource.class, repoId)) + .collect(toImmutableSet()); + }); } - return searchByNameserverRefs( - StreamSupport.stream(query.keys().spliterator(), false) - .map(VKey::from) - .collect(toImmutableSet())); + return searchByNameserverRefs(hostKeys); } /** @@ -416,7 +537,8 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { *

This method is called by {@link #searchByNameserverLdhName} and {@link * #searchByNameserverIp} after they assemble the relevant host keys. */ - private DomainSearchResponse searchByNameserverRefs(final Iterable> hostKeys) { + private DomainSearchResponse searchByNameserverRefs( + final ImmutableCollection> hostKeys) { // We must break the query up into chunks, because the in operator is limited to 30 subqueries. // Since it is possible for the same domain to show up more than once in our result list (if // we do a wildcard nameserver search that returns multiple nameservers used by the same @@ -428,24 +550,62 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { int numHostKeysSearched = 0; for (List> chunk : Iterables.partition(hostKeys, 30)) { numHostKeysSearched += chunk.size(); - Query query = - ofy() - .load() - .type(DomainBase.class) - .filter("nsHosts in", chunk.stream().map(VKey::getOfyKey).collect(toImmutableSet())); - if (!shouldIncludeDeleted()) { - query = query.filter("deletionTime >", getRequestTime()); - // If we are not performing an inequality query, we can filter on the cursor in the query. - // Otherwise, we will need to filter the results afterward. - } else if (cursorString.isPresent()) { - query = query.filter("fullyQualifiedDomainName >", cursorString.get()); + if (isDatastore()) { + Query query = + ofy() + .load() + .type(DomainBase.class) + .filter( + "nsHosts in", chunk.stream().map(VKey::getOfyKey).collect(toImmutableSet())); + if (!shouldIncludeDeleted()) { + query = query.filter("deletionTime >", getRequestTime()); + // If we are not performing an inequality query, we can filter on the cursor in the query. + // Otherwise, we will need to filter the results afterward. + } else if (cursorString.isPresent()) { + query = query.filter("fullyQualifiedDomainName >", cursorString.get()); + } + Stream stream = Streams.stream(query).filter(this::isAuthorized); + if (cursorString.isPresent()) { + stream = + stream.filter(domain -> (domain.getDomainName().compareTo(cursorString.get()) > 0)); + } + stream.forEach(domainSetBuilder::add); + } else { + jpaTm() + .transact( + () -> { + for (VKey hostKey : hostKeys) { + CriteriaQueryBuilder queryBuilder = + CriteriaQueryBuilder.create(DomainBase.class) + .whereFieldContains("nsHosts", hostKey) + .orderByAsc("fullyQualifiedDomainName"); + CriteriaBuilder criteriaBuilder = + jpaTm().getEntityManager().getCriteriaBuilder(); + if (!shouldIncludeDeleted()) { + queryBuilder = + queryBuilder.where( + "deletionTime", criteriaBuilder::greaterThan, getRequestTime()); + } + if (cursorString.isPresent()) { + queryBuilder = + queryBuilder.where( + "fullyQualifiedDomainName", + criteriaBuilder::greaterThan, + cursorString.get()); + } + jpaTm() + .getEntityManager() + .createQuery(queryBuilder.build()) + .getResultStream() + .filter(this::isAuthorized) + .forEach( + (domain) -> { + Hibernate.initialize(domain.getDsData()); + domainSetBuilder.add(domain); + }); + } + }); } - Stream stream = Streams.stream(query).filter(domain -> isAuthorized(domain)); - if (cursorString.isPresent()) { - stream = - stream.filter(domain -> (domain.getDomainName().compareTo(cursorString.get()) > 0)); - } - stream.forEach(domainSetBuilder::add); } List domains = domainSetBuilder.build().asList(); metricInformationBuilder.setNumHostsRetrieved(numHostKeysSearched); @@ -489,8 +649,7 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { OutputDataType outputDataType = (domains.size() > 1) ? OutputDataType.SUMMARY : OutputDataType.FULL; DomainSearchResponse.Builder builder = - DomainSearchResponse.builder() - .setIncompletenessWarningType(incompletenessWarningType); + DomainSearchResponse.builder().setIncompletenessWarningType(incompletenessWarningType); Optional newCursor = Optional.empty(); for (DomainBase domain : Iterables.limit(domains, rdapResultSetMaxSize)) { newCursor = Optional.of(domain.getDomainName()); diff --git a/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java b/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java index d8083cbbe..37a5369c6 100644 --- a/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java +++ b/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java @@ -198,8 +198,7 @@ public abstract class RdapSearchActionBase extends RdapActionBase { if (desiredRegistrar.isPresent()) { builder = builder.where( - jpaTm().getEntityManager().getCriteriaBuilder()::equal, - "currentSponsorClientId", + "currentSponsorClientId", jpaTm().getEntityManager().getCriteriaBuilder()::equal, desiredRegistrar.get()); } List queryResult = @@ -413,18 +412,17 @@ public abstract class RdapSearchActionBase extends RdapActionBase { if (partialStringQuery.getHasWildcard()) { builder = builder.where( - criteriaBuilder::like, - filterField, + filterField, criteriaBuilder::like, String.format("%s%%", partialStringQuery.getInitialString())); } else { // no wildcard means we use a standard equals query builder = - builder.where(criteriaBuilder::equal, filterField, partialStringQuery.getInitialString()); + builder.where(filterField, criteriaBuilder::equal, partialStringQuery.getInitialString()); } if (cursorString.isPresent()) { - builder = builder.where(criteriaBuilder::greaterThan, filterField, cursorString.get()); + builder = builder.where(filterField, criteriaBuilder::greaterThan, cursorString.get()); } - builder = builder.orderBy(criteriaBuilder::asc, filterField); + builder = builder.orderByAsc(filterField); return setDeletedItemHandlingSql(builder, deletedItemHandling); } @@ -502,13 +500,13 @@ public abstract class RdapSearchActionBase extends RdapActionBase { jpaTm().assertInTransaction(); CriteriaQueryBuilder builder = CriteriaQueryBuilder.create(clazz); CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder(); - builder = builder.where(criteriaBuilder::equal, filterField, queryString); + builder = builder.where(filterField, criteriaBuilder::equal, queryString); if (cursorString.isPresent()) { if (cursorField.isPresent()) { builder = - builder.where(criteriaBuilder::greaterThan, cursorField.get(), cursorString.get()); + builder.where(cursorField.get(), criteriaBuilder::greaterThan, cursorString.get()); } else { - builder = builder.where(criteriaBuilder::greaterThan, "repoId", cursorString.get()); + builder = builder.where("repoId", criteriaBuilder::greaterThan, cursorString.get()); } } return setDeletedItemHandlingSql(builder, deletedItemHandling); @@ -559,7 +557,7 @@ public abstract class RdapSearchActionBase extends RdapActionBase { if (!Objects.equals(deletedItemHandling, DeletedItemHandling.INCLUDE)) { builder = builder.where( - jpaTm().getEntityManager().getCriteriaBuilder()::equal, "deletionTime", END_OF_TIME); + "deletionTime", jpaTm().getEntityManager().getCriteriaBuilder()::equal, END_OF_TIME); } return builder; } diff --git a/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java b/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java index eb0da272f..728d6d260 100644 --- a/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java +++ b/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java @@ -77,8 +77,7 @@ class CriteriaQueryBuilderTest { CriteriaQuery query = CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) .where( - jpaTm().getEntityManager().getCriteriaBuilder()::equal, - "data", + "data", jpaTm().getEntityManager().getCriteriaBuilder()::equal, "zztz") .build(); return jpaTm().getEntityManager().createQuery(query).getResultList(); @@ -95,7 +94,7 @@ class CriteriaQueryBuilderTest { CriteriaQuery query = CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) .where( - jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "a%") + "data", jpaTm().getEntityManager().getCriteriaBuilder()::like, "a%") .build(); return jpaTm().getEntityManager().createQuery(query).getResultList(); }); @@ -111,7 +110,7 @@ class CriteriaQueryBuilderTest { CriteriaQuery query = CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) .where( - jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "%a%") + "data", jpaTm().getEntityManager().getCriteriaBuilder()::like, "%a%") .build(); return jpaTm().getEntityManager().createQuery(query).getResultList(); }); @@ -128,10 +127,10 @@ class CriteriaQueryBuilderTest { CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) // first "where" matches 1 and 3 .where( - jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "%a%") + "data", jpaTm().getEntityManager().getCriteriaBuilder()::like, "%a%") // second "where" matches 1 and 2 .where( - jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "%t%") + "data", jpaTm().getEntityManager().getCriteriaBuilder()::like, "%t%") .build(); return jpaTm().getEntityManager().createQuery(query).getResultList(); }); @@ -169,22 +168,37 @@ class CriteriaQueryBuilderTest { } @Test - void testSuccess_orderBy() { + void testSuccess_orderByAsc() { List result = jpaTm() .transact( () -> { CriteriaQuery query = CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) - .orderBy(jpaTm().getEntityManager().getCriteriaBuilder()::asc, "data") + .orderByAsc("data") .where( - jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "%a%") + "data", jpaTm().getEntityManager().getCriteriaBuilder()::like, "%a%") .build(); return jpaTm().getEntityManager().createQuery(query).getResultList(); }); assertThat(result).containsExactly(entity3, entity1).inOrder(); } + @Test + void testSuccess_orderByDesc() { + List result = + jpaTm() + .transact( + () -> { + CriteriaQuery query = + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + .orderByDesc("data") + .build(); + return jpaTm().getEntityManager().createQuery(query).getResultList(); + }); + assertThat(result).containsExactly(entity2, entity1, entity3).inOrder(); + } + @Entity(name = "CriteriaQueryBuilderTestEntity") private static class CriteriaQueryBuilderTestEntity extends ImmutableObject { @Id private String name; diff --git a/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java b/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java index 98b954b6d..b06f5ed87 100644 --- a/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/core/src/test/java/google/registry/rdap/RdapDomainSearchActionTest.java @@ -49,15 +49,19 @@ import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapMetrics.SearchType; import google.registry.rdap.RdapMetrics.WildcardType; import google.registry.rdap.RdapSearchResults.IncompletenessWarningType; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.FakeResponse; +import google.registry.testing.TestOfyAndSql; +import google.registry.testing.TestOfyOnly; +import google.registry.testing.TestSqlOnly; import java.net.URLDecoder; import java.util.HashMap; import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; /** Unit tests for {@link RdapDomainSearchAction}. */ +@DualDatabaseTest class RdapDomainSearchActionTest extends RdapSearchActionTestCase { RdapDomainSearchActionTest() { @@ -386,13 +390,20 @@ class RdapDomainSearchActionTest extends RdapSearchActionTestCase