From f78b64d93cefff2c47606d48ce107747ee19c852 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Tue, 2 Mar 2021 13:13:55 -0500 Subject: [PATCH] Add SQL searching to RdapEntitySearchAction and RdapSearchActionBase (#969) - Adds a CriteriaQueryBuilder class that allows us to build CriteriaQuery objects with sane and modular WHERE and ORDER BY clauses. CriteriaQuery requires that all WHERE and ORDER BY clauses be specified at the same time (else later ones will overwrite the earlier ones) so in order to have a proper builder pattern we need to wait to build the query object until we are done adding clauses. - In addition, encapsulating the query logic in the CriteriaQueryBuilder class means that we don't need to deal with the complicated Root/Path branching, otherwise we'd have to keep track of CriteriaQuery and Root objects everywhere. - Added a REPLAYED_ENTITIES TransitionId that will represent all replayed entities, e.g. EppResources. Also sets this, by default, to always be CLOUD_SQL if we're using the SQL transaction manager in tests. - Added branching logic in RdapEntitySearchAction based on that transition ID that determines whether we do the existing ofy query logic or JPA logic. --- config/presubmits.py | 3 + .../common/DatabaseTransitionSchedule.java | 2 + .../transaction/CriteriaQueryBuilder.java | 97 +++++++++ .../google/registry/rdap/RdapActionBase.java | 11 + .../registry/rdap/RdapEntitySearchAction.java | 110 ++++++---- .../registry/rdap/RdapSearchActionBase.java | 198 +++++++++++++++-- .../transaction/CriteriaQueryBuilderTest.java | 202 ++++++++++++++++++ .../rdap/RdapEntitySearchActionTest.java | 181 ++++++++-------- .../registry/testing/AppEngineExtension.java | 16 ++ 9 files changed, 681 insertions(+), 139 deletions(-) create mode 100644 core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java create mode 100644 core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java diff --git a/config/presubmits.py b/config/presubmits.py index c1f1ca44a..2863e1cc8 100644 --- a/config/presubmits.py +++ b/config/presubmits.py @@ -207,6 +207,9 @@ PRESUBMITS = { "ForeignKeyIndex.java", "HistoryEntryDao.java", "JpaTransactionManagerImpl.java", + # CriteriaQueryBuilder is a false positive + "CriteriaQueryBuilder.java", + "RdapSearchActionBase.java", }, ): "The first String parameter to EntityManager.create(Native)Query " diff --git a/core/src/main/java/google/registry/model/common/DatabaseTransitionSchedule.java b/core/src/main/java/google/registry/model/common/DatabaseTransitionSchedule.java index 0d2ac29a9..1f536044c 100644 --- a/core/src/main/java/google/registry/model/common/DatabaseTransitionSchedule.java +++ b/core/src/main/java/google/registry/model/common/DatabaseTransitionSchedule.java @@ -62,6 +62,8 @@ public class DatabaseTransitionSchedule extends ImmutableObject implements Datas DOMAIN_LABEL_LISTS, /** The schedule for the migration of the {@link SignedMarkRevocationList} entity. */ SIGNED_MARK_REVOCATION_LIST, + /** The schedule for all asynchronously-replayed entities, ones not dually-written. */ + REPLAYED_ENTITIES, } /** diff --git a/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java b/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java new file mode 100644 index 000000000..1d9e8fd57 --- /dev/null +++ b/core/src/main/java/google/registry/persistence/transaction/CriteriaQueryBuilder.java @@ -0,0 +1,97 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.persistence.transaction; + +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; + +import com.google.common.collect.ImmutableList; +import java.util.Collection; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Expression; +import javax.persistence.criteria.Order; +import javax.persistence.criteria.Predicate; +import javax.persistence.criteria.Root; + +/** + * An extension of {@link CriteriaQuery} that uses a Builder-style pattern when adding "WHERE" + * and/or "ORDER BY" clauses. + * + *

{@link CriteriaQuery}, as is, requires that all clauses must be passed in at once -- if one + * calls "WHERE" multiple times, the later call overwrites the earlier call. + */ +public class CriteriaQueryBuilder { + + /** Functional interface that defines the 'where' operator, e.g. {@link CriteriaBuilder#equal}. */ + public interface WhereClause { + 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<>(); + private final ImmutableList.Builder orders = new ImmutableList.Builder<>(); + + private CriteriaQueryBuilder(CriteriaQuery query, Root root) { + this.query = query; + this.root = root; + } + + /** Adds a WHERE clause to the query, given the specified operation, field, and value. */ + public CriteriaQueryBuilder where(WhereClause whereClause, String fieldName, V value) { + Expression expression = root.get(fieldName); + return where(whereClause.predicate(expression, value)); + } + + /** Adds a WHERE clause to the query specifying that a value must be in the given collection. */ + public CriteriaQueryBuilder whereFieldIsIn(String fieldName, Collection values) { + 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)); + } + + /** Builds and returns the query, applying all WHERE and ORDER BY clauses at once. */ + public CriteriaQuery build() { + Predicate[] predicateArray = predicates.build().toArray(new Predicate[0]); + return query.where(predicateArray).orderBy(orders.build()); + } + + private CriteriaQueryBuilder where(Predicate predicate) { + predicates.add(predicate); + 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); + Root root = query.from(clazz); + query = query.select(root); + return new CriteriaQueryBuilder<>(query, root); + } +} diff --git a/core/src/main/java/google/registry/rdap/RdapActionBase.java b/core/src/main/java/google/registry/rdap/RdapActionBase.java index e95c0a16a..b00738a10 100644 --- a/core/src/main/java/google/registry/rdap/RdapActionBase.java +++ b/core/src/main/java/google/registry/rdap/RdapActionBase.java @@ -17,6 +17,7 @@ package google.registry.rdap; import static com.google.common.base.Charsets.UTF_8; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.request.Actions.getPathForAction; import static google.registry.util.DomainNameUtils.canonicalizeDomainName; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; @@ -28,7 +29,10 @@ import com.google.common.net.MediaType; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import google.registry.config.RegistryConfig.Config; +import google.registry.model.DatabaseMigrationUtils; import google.registry.model.EppResource; +import google.registry.model.common.DatabaseTransitionSchedule.PrimaryDatabase; +import google.registry.model.common.DatabaseTransitionSchedule.TransitionId; import google.registry.model.registrar.Registrar; import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapObjectClasses.ErrorResponse; @@ -256,4 +260,11 @@ public abstract class RdapActionBase implements Runnable { DateTime getRequestTime() { return rdapJsonFormatter.getRequestTime(); } + + static boolean isDatastore() { + return tm().transact( + () -> + DatabaseMigrationUtils.getPrimaryDatabase(TransitionId.REPLAYED_ENTITIES) + .equals(PrimaryDatabase.DATASTORE)); + } } diff --git a/core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java b/core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java index b464675f8..93419aef7 100644 --- a/core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java +++ b/core/src/main/java/google/registry/rdap/RdapEntitySearchAction.java @@ -15,7 +15,9 @@ package google.registry.rdap; import static com.google.common.collect.ImmutableList.toImmutableList; -import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; +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.request.Action.Method.GET; import static google.registry.request.Action.Method.HEAD; @@ -29,6 +31,9 @@ import com.google.common.primitives.Longs; import com.googlecode.objectify.cmd.Query; import google.registry.model.contact.ContactResource; import google.registry.model.registrar.Registrar; +import google.registry.persistence.VKey; +import google.registry.persistence.transaction.CriteriaQueryBuilder; +import google.registry.rdap.RdapAuthorization.Role; import google.registry.rdap.RdapJsonFormatter.OutputDataType; import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapMetrics.SearchType; @@ -112,7 +117,6 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { /** Parses the parameters and calls the appropriate search function. */ @Override public EntitySearchResponse getSearchResponse(boolean isHeadRequest) { - // RDAP syntax example: /rdap/entities?fn=Bobby%20Joe*. if (Booleans.countTrue(fnParam.isPresent(), handleParam.isPresent()) != 1) { throw new BadRequestException("You must specify either fn=XXXX or handle=YYYY"); @@ -214,9 +218,7 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { // Don't allow wildcard suffixes when searching for entities. if (partialStringQuery.getHasWildcard() && (partialStringQuery.getSuffix() != null)) { throw new UnprocessableEntityException( - partialStringQuery.getHasWildcard() - ? "Suffixes not allowed in wildcard entity name searches" - : "Suffixes not allowed when searching for deleted entities"); + "Suffixes not allowed in wildcard entity name searches"); } // For wildcards, make sure the initial string is long enough, except in the special case of // searching for all registrars, where we aren't worried about inefficient searches. @@ -225,9 +227,7 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { && (partialStringQuery.getInitialString().length() < RdapSearchPattern.MIN_INITIAL_STRING_LENGTH)) { throw new UnprocessableEntityException( - partialStringQuery.getHasWildcard() - ? "Initial search string required in wildcard entity name searches" - : "Initial search string required when searching for deleted entities"); + "Initial search string required in wildcard entity name searches"); } // Get the registrar matches. If we have a registrar cursor, weed out registrars up to and // including the one we ended with last time. We can skip registrars if subtype is CONTACTS. @@ -262,18 +262,39 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { || (cursorType == CursorType.REGISTRAR)) { resultSet = RdapResultSet.create(ImmutableList.of()); } else { - Query query = - queryItems( - ContactResource.class, - "searchName", - partialStringQuery, - cursorQueryString, // if we get this far, and there's a cursor, it must be a contact - DeletedItemHandling.EXCLUDE, - rdapResultSetMaxSize + 1); - if (rdapAuthorization.role() != RdapAuthorization.Role.ADMINISTRATOR) { - query = query.filter("currentSponsorClientId in", rdapAuthorization.clientIds()); + if (isDatastore()) { + Query query = + queryItems( + ContactResource.class, + "searchName", + partialStringQuery, + cursorQueryString, // if we get here and there's a cursor, it must be a contact + DeletedItemHandling.EXCLUDE, + rdapResultSetMaxSize + 1); + if (!rdapAuthorization.role().equals(Role.ADMINISTRATOR)) { + query = query.filter("currentSponsorClientId in", rdapAuthorization.clientIds()); + } + resultSet = getMatchingResources(query, false, rdapResultSetMaxSize + 1); + } else { + resultSet = + jpaTm() + .transact( + () -> { + CriteriaQueryBuilder builder = + queryItemsSql( + ContactResource.class, + "searchName", + partialStringQuery, + cursorQueryString, + DeletedItemHandling.EXCLUDE); + if (!rdapAuthorization.role().equals(Role.ADMINISTRATOR)) { + builder = + builder.whereFieldIsIn( + "currentSponsorClientId", rdapAuthorization.clientIds()); + } + return getMatchingResourcesSql(builder, false, rdapResultSetMaxSize + 1); + }); } - resultSet = getMatchingResources(query, false, rdapResultSetMaxSize + 1); } } return makeSearchResults(resultSet, registrars, QueryType.FULL_NAME); @@ -303,15 +324,15 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { if (subtype == Subtype.REGISTRARS) { contactResourceList = ImmutableList.of(); } else { - ContactResource contactResource = - ofy() - .load() - .type(ContactResource.class) - .id(partialStringQuery.getInitialString()) - .now(); + Optional contactResource = + transactIfJpaTm( + () -> + tm().loadByKeyIfPresent( + VKey.create( + ContactResource.class, partialStringQuery.getInitialString()))); contactResourceList = - ((contactResource != null) && shouldBeVisible(contactResource)) - ? ImmutableList.of(contactResource) + (contactResource.isPresent() && shouldBeVisible(contactResource.get())) + ? ImmutableList.of(contactResource.get()) : ImmutableList.of(); } ImmutableList registrarList; @@ -365,16 +386,31 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { if (subtype == Subtype.REGISTRARS) { contactResultSet = RdapResultSet.create(ImmutableList.of()); } else { - contactResultSet = - getMatchingResources( - queryItemsByKey( - ContactResource.class, - partialStringQuery, - cursorQueryString, - getDeletedItemHandling(), - querySizeLimit), - shouldIncludeDeleted(), - querySizeLimit); + if (isDatastore()) { + contactResultSet = + getMatchingResources( + queryItemsByKey( + ContactResource.class, + partialStringQuery, + cursorQueryString, + getDeletedItemHandling(), + querySizeLimit), + shouldIncludeDeleted(), + querySizeLimit); + } else { + contactResultSet = + jpaTm() + .transact( + () -> + getMatchingResourcesSql( + queryItemsByKeySql( + ContactResource.class, + partialStringQuery, + cursorQueryString, + getDeletedItemHandling()), + shouldIncludeDeleted(), + querySizeLimit)); + } } return makeSearchResults(contactResultSet, registrars, QueryType.HANDLE); } diff --git a/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java b/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java index b1d844b60..d8083cbbe 100644 --- a/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java +++ b/core/src/main/java/google/registry/rdap/RdapSearchActionBase.java @@ -16,6 +16,7 @@ package google.registry.rdap; import static com.google.common.base.Charsets.UTF_8; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.util.DateTimeUtils.END_OF_TIME; import com.google.common.collect.ImmutableList; @@ -24,6 +25,7 @@ import com.googlecode.objectify.Key; import com.googlecode.objectify.cmd.Query; import google.registry.model.EppResource; import google.registry.model.registrar.Registrar; +import google.registry.persistence.transaction.CriteriaQueryBuilder; import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapMetrics.WildcardType; import google.registry.rdap.RdapSearchResults.BaseSearchResponse; @@ -40,8 +42,10 @@ import java.util.ArrayList; import java.util.Base64; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import javax.inject.Inject; +import javax.persistence.criteria.CriteriaBuilder; /** * Base RDAP (new WHOIS) action for domain, nameserver and entity search requests. @@ -161,14 +165,63 @@ public abstract class RdapSearchActionBase extends RdapActionBase { if (desiredRegistrar.isPresent()) { query = query.filter("currentSponsorClientId", desiredRegistrar.get()); } - if (!checkForVisibility) { - return RdapResultSet.create(query.list()); + List queryResult = query.list(); + if (checkForVisibility) { + return filterResourcesByVisibility(queryResult, querySizeLimit); + } else { + return RdapResultSet.create(queryResult); } + } + + /** + * In Cloud SQL, builds and runs the given query, and checks for permissioning if necessary. + * + * @param builder a query builder that represents the various SELECT FROM, WHERE, ORDER BY and + * (etc) clauses that make up this SQL query + * @param checkForVisibility true if the results should be checked to make sure they are visible; + * normally this should be equal to the shouldIncludeDeleted setting, but in cases where the + * query could not check deletion status (due to Datastore limitations such as the limit of + * one field queried for inequality, for instance), it may need to be set to true even when + * not including deleted records + * @param querySizeLimit the maximum number of items the query is expected to return, usually + * because the limit has been set + * @return an {@link RdapResultSet} object containing the list of resources and an incompleteness + * warning flag, which is set to MIGHT_BE_INCOMPLETE iff any resources were excluded due to + * lack of visibility, and the resulting list of resources is less than the maximum allowable, + * and the number of items returned by the query is greater than or equal to the maximum + * number we might have expected + */ + RdapResultSet getMatchingResourcesSql( + CriteriaQueryBuilder builder, boolean checkForVisibility, int querySizeLimit) { + jpaTm().assertInTransaction(); + Optional desiredRegistrar = getDesiredRegistrar(); + if (desiredRegistrar.isPresent()) { + builder = + builder.where( + jpaTm().getEntityManager().getCriteriaBuilder()::equal, + "currentSponsorClientId", + desiredRegistrar.get()); + } + List queryResult = + jpaTm() + .getEntityManager() + .createQuery(builder.build()) + .setMaxResults(querySizeLimit) + .getResultList(); + if (checkForVisibility) { + return filterResourcesByVisibility(queryResult, querySizeLimit); + } else { + return RdapResultSet.create(queryResult); + } + } + + private RdapResultSet filterResourcesByVisibility( + List queryResult, int querySizeLimit) { // If we are including deleted resources, we need to check that we're authorized for each one. List resources = new ArrayList<>(); int numResourcesQueried = 0; boolean someExcluded = false; - for (T resource : query) { + for (T resource : queryResult) { if (shouldBeVisible(resource)) { resources.add(resource); } else { @@ -268,8 +321,8 @@ public abstract class RdapSearchActionBase extends RdapActionBase { // to be (more) sure we got everything. int getStandardQuerySizeLimit() { return shouldIncludeDeleted() - ? (RESULT_SET_SIZE_SCALING_FACTOR * (rdapResultSetMaxSize + 1)) - : (rdapResultSetMaxSize + 1); + ? (RESULT_SET_SIZE_SCALING_FACTOR * (rdapResultSetMaxSize + 1)) + : (rdapResultSetMaxSize + 1); } /** @@ -311,9 +364,10 @@ public abstract class RdapSearchActionBase extends RdapActionBase { query = query.filter(filterField, partialStringQuery.getInitialString()); } else { // Ignore the suffix; the caller will need to filter on the suffix, if any. - query = query - .filter(filterField + " >=", partialStringQuery.getInitialString()) - .filter(filterField + " <", partialStringQuery.getNextInitialString()); + query = + query + .filter(filterField + " >=", partialStringQuery.getInitialString()) + .filter(filterField + " <", partialStringQuery.getNextInitialString()); } if (cursorString.isPresent()) { query = query.filter(filterField + " >", cursorString.get()); @@ -321,6 +375,59 @@ public abstract class RdapSearchActionBase extends RdapActionBase { return setOtherQueryAttributes(query, deletedItemHandling, resultSetMaxSize); } + /** + * In Cloud SQL, handles prefix searches in cases where, if we need to filter out deleted items, + * there are no pending deletes. + * + *

In such cases, it is sufficient to check whether {@code deletionTime} is equal to {@code + * END_OF_TIME}, because any other value means it has already been deleted. This allows us to use + * an equality query for the deletion time. + * + * @param clazz the type of resource to be queried + * @param filterField the database field of interest + * @param partialStringQuery the details of the search string; if there is no wildcard, an + * equality query is used; if there is a wildcard, a range query is used instead; the initial + * string should not be empty, and any search suffix will be ignored, so the caller must + * filter the results if a suffix is specified + * @param cursorString if a cursor is present, this parameter should specify the cursor string, to + * skip any results up to and including the string; empty() if there is no cursor + * @param deletedItemHandling whether to include or exclude deleted items + * @return a {@link CriteriaQueryBuilder} object representing the query so far + */ + static CriteriaQueryBuilder queryItemsSql( + Class clazz, + String filterField, + RdapSearchPattern partialStringQuery, + Optional cursorString, + DeletedItemHandling deletedItemHandling) { + jpaTm().assertInTransaction(); + if (partialStringQuery.getInitialString().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)); + } + CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder(); + CriteriaQueryBuilder builder = CriteriaQueryBuilder.create(clazz); + if (partialStringQuery.getHasWildcard()) { + builder = + builder.where( + criteriaBuilder::like, + filterField, + String.format("%s%%", partialStringQuery.getInitialString())); + } else { + // no wildcard means we use a standard equals query + builder = + builder.where(criteriaBuilder::equal, filterField, partialStringQuery.getInitialString()); + } + if (cursorString.isPresent()) { + builder = builder.where(criteriaBuilder::greaterThan, filterField, cursorString.get()); + } + builder = builder.orderBy(criteriaBuilder::asc, filterField); + return setDeletedItemHandlingSql(builder, deletedItemHandling); + } + /** * Handles searches using a simple string rather than an {@link RdapSearchPattern}. * @@ -331,9 +438,9 @@ public abstract class RdapSearchActionBase extends RdapActionBase { * @param filterField the database field of interest * @param queryString the search string * @param cursorField the field which should be compared to the cursor string, or empty() if the - * key should be compared to a key created from the cursor string + * key should be compared to a key created from the cursor string * @param cursorString if a cursor is present, this parameter should specify the cursor string, to - * skip any results up to and including the string; empty() if there is no cursor + * skip any results up to and including the string; empty() if there is no cursor * @param deletedItemHandling whether to include or exclude deleted items * @param resultSetMaxSize the maximum number of results to return * @return the query object @@ -363,6 +470,50 @@ public abstract class RdapSearchActionBase extends RdapActionBase { return setOtherQueryAttributes(query, deletedItemHandling, resultSetMaxSize); } + /** + * In Cloud SQL, handles searches using a simple string rather than an {@link RdapSearchPattern}. + * + *

Since the filter is not an inequality, we can support also checking a cursor string against + * a different field (which involves an inequality on that field). + * + * @param clazz the type of resource to be queried + * @param filterField the database field of interest + * @param queryString the search string + * @param cursorField the field which should be compared to the cursor string, or empty() if the + * key should be compared to a key created from the cursor string + * @param cursorString if a cursor is present, this parameter should specify the cursor string, to + * skip any results up to and including the string; empty() if there is no cursor + * @param deletedItemHandling whether to include or exclude deleted items + * @return a {@link CriteriaQueryBuilder} object representing the query so far + */ + static CriteriaQueryBuilder queryItemsSql( + Class clazz, + String filterField, + String queryString, + Optional cursorField, + Optional cursorString, + DeletedItemHandling deletedItemHandling) { + 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)); + } + jpaTm().assertInTransaction(); + CriteriaQueryBuilder builder = CriteriaQueryBuilder.create(clazz); + CriteriaBuilder criteriaBuilder = jpaTm().getEntityManager().getCriteriaBuilder(); + builder = builder.where(criteriaBuilder::equal, filterField, queryString); + if (cursorString.isPresent()) { + if (cursorField.isPresent()) { + builder = + builder.where(criteriaBuilder::greaterThan, cursorField.get(), cursorString.get()); + } else { + builder = builder.where(criteriaBuilder::greaterThan, "repoId", cursorString.get()); + } + } + return setDeletedItemHandlingSql(builder, deletedItemHandling); + } + /** Handles searches where the field to be searched is the key. */ static Query queryItemsByKey( Class clazz, @@ -382,9 +533,10 @@ public abstract class RdapSearchActionBase extends RdapActionBase { query = query.filterKey("=", Key.create(clazz, partialStringQuery.getInitialString())); } else { // Ignore the suffix; the caller will need to filter on the suffix, if any. - query = query - .filterKey(">=", Key.create(clazz, partialStringQuery.getInitialString())) - .filterKey("<", Key.create(clazz, partialStringQuery.getNextInitialString())); + query = + query + .filterKey(">=", Key.create(clazz, partialStringQuery.getInitialString())) + .filterKey("<", Key.create(clazz, partialStringQuery.getNextInitialString())); } if (cursorString.isPresent()) { query = query.filterKey(">", Key.create(clazz, cursorString.get())); @@ -392,6 +544,26 @@ public abstract class RdapSearchActionBase extends RdapActionBase { return setOtherQueryAttributes(query, deletedItemHandling, resultSetMaxSize); } + /** In Cloud SQL, handles searches where the field to be searched is the key. */ + static CriteriaQueryBuilder queryItemsByKeySql( + Class clazz, + RdapSearchPattern partialStringQuery, + Optional cursorString, + DeletedItemHandling deletedItemHandling) { + jpaTm().assertInTransaction(); + return queryItemsSql(clazz, "repoId", partialStringQuery, cursorString, deletedItemHandling); + } + + static CriteriaQueryBuilder setDeletedItemHandlingSql( + CriteriaQueryBuilder builder, DeletedItemHandling deletedItemHandling) { + if (!Objects.equals(deletedItemHandling, DeletedItemHandling.INCLUDE)) { + builder = + builder.where( + jpaTm().getEntityManager().getCriteriaBuilder()::equal, "deletionTime", END_OF_TIME); + } + return builder; + } + static Query setOtherQueryAttributes( Query query, DeletedItemHandling deletedItemHandling, int resultSetMaxSize) { if (deletedItemHandling != DeletedItemHandling.INCLUDE) { diff --git a/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java b/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java new file mode 100644 index 000000000..eb0da272f --- /dev/null +++ b/core/src/test/java/google/registry/persistence/transaction/CriteriaQueryBuilderTest.java @@ -0,0 +1,202 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.persistence.transaction; + +import static com.google.common.truth.Truth.assertThat; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; + +import com.google.common.collect.ImmutableList; +import google.registry.model.ImmutableObject; +import google.registry.persistence.transaction.JpaTestRules.JpaUnitTestExtension; +import google.registry.testing.FakeClock; +import java.util.List; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.criteria.CriteriaQuery; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +/** Tests for {@link CriteriaQueryBuilder}. */ +class CriteriaQueryBuilderTest { + + private final FakeClock fakeClock = new FakeClock(); + + private CriteriaQueryBuilderTestEntity entity1 = + new CriteriaQueryBuilderTestEntity("name1", "data"); + private CriteriaQueryBuilderTestEntity entity2 = + new CriteriaQueryBuilderTestEntity("name2", "zztz"); + private CriteriaQueryBuilderTestEntity entity3 = new CriteriaQueryBuilderTestEntity("zzz", "aaa"); + + @RegisterExtension + final JpaUnitTestExtension jpaExtension = + new JpaTestRules.Builder() + .withClock(fakeClock) + .withEntityClass(CriteriaQueryBuilderTestEntity.class) + .buildUnitTestRule(); + + @BeforeEach + void beforeEach() { + jpaTm().transact(() -> jpaTm().putAll(ImmutableList.of(entity1, entity2, entity3))); + } + + @Test + void testSuccess_noWhereClause() { + assertThat( + jpaTm() + .transact( + () -> + jpaTm() + .getEntityManager() + .createQuery( + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + .build()) + .getResultList())) + .containsExactly(entity1, entity2, entity3) + .inOrder(); + } + + @Test + void testSuccess_where_exactlyOne() { + List result = + jpaTm() + .transact( + () -> { + CriteriaQuery query = + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + .where( + jpaTm().getEntityManager().getCriteriaBuilder()::equal, + "data", + "zztz") + .build(); + return jpaTm().getEntityManager().createQuery(query).getResultList(); + }); + assertThat(result).containsExactly(entity2); + } + + @Test + void testSuccess_where_like_oneResult() { + List result = + jpaTm() + .transact( + () -> { + CriteriaQuery query = + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + .where( + jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "a%") + .build(); + return jpaTm().getEntityManager().createQuery(query).getResultList(); + }); + assertThat(result).containsExactly(entity3); + } + + @Test + void testSuccess_where_like_twoResults() { + List result = + jpaTm() + .transact( + () -> { + CriteriaQuery query = + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + .where( + jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "%a%") + .build(); + return jpaTm().getEntityManager().createQuery(query).getResultList(); + }); + assertThat(result).containsExactly(entity1, entity3).inOrder(); + } + + @Test + void testSuccess_multipleWheres() { + List result = + jpaTm() + .transact( + () -> { + CriteriaQuery query = + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + // first "where" matches 1 and 3 + .where( + jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "%a%") + // second "where" matches 1 and 2 + .where( + jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "%t%") + .build(); + return jpaTm().getEntityManager().createQuery(query).getResultList(); + }); + assertThat(result).containsExactly(entity1); + } + + @Test + void testSuccess_where_in_oneResult() { + List result = + jpaTm() + .transact( + () -> { + CriteriaQuery query = + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + .whereFieldIsIn("data", ImmutableList.of("aaa", "bbb")) + .build(); + return jpaTm().getEntityManager().createQuery(query).getResultList(); + }); + assertThat(result).containsExactly(entity3).inOrder(); + } + + @Test + void testSuccess_where_in_twoResults() { + List result = + jpaTm() + .transact( + () -> { + CriteriaQuery query = + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + .whereFieldIsIn("data", ImmutableList.of("aaa", "bbb", "data")) + .build(); + return jpaTm().getEntityManager().createQuery(query).getResultList(); + }); + assertThat(result).containsExactly(entity1, entity3).inOrder(); + } + + @Test + void testSuccess_orderBy() { + List result = + jpaTm() + .transact( + () -> { + CriteriaQuery query = + CriteriaQueryBuilder.create(CriteriaQueryBuilderTestEntity.class) + .orderBy(jpaTm().getEntityManager().getCriteriaBuilder()::asc, "data") + .where( + jpaTm().getEntityManager().getCriteriaBuilder()::like, "data", "%a%") + .build(); + return jpaTm().getEntityManager().createQuery(query).getResultList(); + }); + assertThat(result).containsExactly(entity3, entity1).inOrder(); + } + + @Entity(name = "CriteriaQueryBuilderTestEntity") + private static class CriteriaQueryBuilderTestEntity extends ImmutableObject { + @Id private String name; + + @SuppressWarnings("unused") + private String data; + + private CriteriaQueryBuilderTestEntity() {} + + private CriteriaQueryBuilderTestEntity(String name, String data) { + this.name = name; + this.data = data; + } + } +} diff --git a/core/src/test/java/google/registry/rdap/RdapEntitySearchActionTest.java b/core/src/test/java/google/registry/rdap/RdapEntitySearchActionTest.java index e32c2cf90..1f84b67b0 100644 --- a/core/src/test/java/google/registry/rdap/RdapEntitySearchActionTest.java +++ b/core/src/test/java/google/registry/rdap/RdapEntitySearchActionTest.java @@ -43,14 +43,16 @@ import google.registry.model.reporting.HistoryEntry; import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.rdap.RdapMetrics.SearchType; import google.registry.rdap.RdapSearchResults.IncompletenessWarningType; +import google.registry.testing.DualDatabaseTest; import google.registry.testing.FakeResponse; +import google.registry.testing.TestOfyAndSql; import java.net.URLDecoder; import java.util.Optional; import javax.annotation.Nullable; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; /** Unit tests for {@link RdapEntitySearchAction}. */ +@DualDatabaseTest class RdapEntitySearchActionTest extends RdapSearchActionTestCase { RdapEntitySearchActionTest() { @@ -209,9 +211,9 @@ class RdapEntitySearchActionTest extends RdapSearchActionTestCase"); verifyErrorMetrics(0); } - @Test + @TestOfyAndSql void testNameMatchContacts_nonTruncated() { login("2-RegistrarTest"); createManyContactsAndRegistrars(4, 0, registrarTest); rememberWildcardType("Entity *"); + // JsonObject foo = generateActualJsonWithFullName("Entity *"); assertThat(generateActualJsonWithFullName("Entity *")) .isEqualTo(generateExpectedJson("rdap_nontruncated_contacts.json")); assertThat(response.getStatus()).isEqualTo(200); verifyMetrics(4); } - @Test + @TestOfyAndSql void testNameMatchContacts_truncated() { login("2-RegistrarTest"); createManyContactsAndRegistrars(5, 0, registrarTest); @@ -683,7 +686,7 @@ class RdapEntitySearchActionTest extends RdapSearchActionTestCase")); } - @Test + @TestOfyAndSql void testNameMatchMix_truncated() { login("2-RegistrarTest"); createManyContactsAndRegistrars(3, 3, registrarTest); @@ -811,7 +814,7 @@ class RdapEntitySearchActionTest extends RdapSearchActionTestCase", "rdap_registrar.json"); verifyMetrics(0); } - @Test + @TestOfyAndSql void testHandleMatchRegistrar_found_subtypeAll() { action.subtypeParam = Optional.of("all"); runSuccessfulHandleTest("20", "20", "Yes Virginia