From e577d2f5e95d08b2510d576f70d63a11514a4c11 Mon Sep 17 00:00:00 2001 From: mountford Date: Tue, 16 Jan 2018 12:54:46 -0800 Subject: [PATCH] Add support for RDAP retrieval of all registrars This CL also fixes a bug. Registrars were returned in an arbitrary order. This caused cursor-based pagination to fail. Now we always sort by registrar name (even for handle searches), and use the registrar name in the cursor, to ensure proper behavior. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=182098187 --- .../registry/rdap/RdapEntitySearchAction.java | 50 +++++++++++++----- .../rdap/RdapEntitySearchActionTest.java | 52 +++++++++++++++++++ 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/java/google/registry/rdap/RdapEntitySearchAction.java b/java/google/registry/rdap/RdapEntitySearchAction.java index 5666a6ae0..987063be1 100644 --- a/java/google/registry/rdap/RdapEntitySearchAction.java +++ b/java/google/registry/rdap/RdapEntitySearchAction.java @@ -41,6 +41,7 @@ import google.registry.request.Parameter; import google.registry.request.auth.Auth; import google.registry.util.Clock; import java.util.ArrayList; +import java.util.Comparator; import java.util.List; import java.util.Optional; import javax.inject.Inject; @@ -194,6 +195,7 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { results = searchByHandle( recordWildcardType(RdapSearchPattern.create(handleParam.get(), false)), + cursorType, cursorQueryString, subtype, now); @@ -244,14 +246,17 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { Optional cursorQueryString, Subtype subtype, DateTime now) { - // For wildcard searches, make sure the initial string is long enough, and don't allow suffixes. + // 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"); } + // 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. if (partialStringQuery.getHasWildcard() + && (subtype != Subtype.REGISTRARS) && (partialStringQuery.getInitialString().length() < RdapSearchPattern.MIN_INITIAL_STRING_LENGTH)) { throw new UnprocessableEntityException( @@ -267,6 +272,8 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { } else { registrars = Streams.stream(Registrar.loadAllCached()) + .sorted( + Comparator.comparing(Registrar::getRegistrarName, String.CASE_INSENSITIVE_ORDER)) .filter( registrar -> partialStringQuery.matches(registrar.getRegistrarName()) @@ -320,6 +327,7 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { */ private RdapSearchResults searchByHandle( final RdapSearchPattern partialStringQuery, + CursorType cursorType, Optional cursorQueryString, Subtype subtype, DateTime now) { @@ -358,13 +366,34 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { now); // Handle queries with a wildcard (or including deleted), but no suffix. Because the handle // for registrars is the IANA identifier number, don't allow wildcard searches for registrars, - // by simply not searching for registrars if a wildcard is present. Fetch an extra contact to - // detect result set truncation. + // by simply not searching for registrars if a wildcard is present (unless the request is for + // all registrars, in which case we know what to do). Fetch an extra contact to detect result + // set truncation. } else { - ImmutableList registrars = - ((subtype == Subtype.CONTACTS) || partialStringQuery.getHasWildcard()) - ? ImmutableList.of() - : getMatchingRegistrars(partialStringQuery.getInitialString()); + ImmutableList registrars; + if ((subtype == Subtype.REGISTRARS) + && partialStringQuery.getHasWildcard() + && partialStringQuery.getInitialString().isEmpty()) { + // Even though we are searching by IANA identifier, we should still sort by name, because + // the IANA identifier can by missing, and sorting on that would screw up our cursors. + registrars = + Streams.stream(Registrar.loadAllCached()) + .sorted( + Comparator.comparing( + Registrar::getRegistrarName, String.CASE_INSENSITIVE_ORDER)) + .filter( + registrar -> + ((cursorType != CursorType.REGISTRAR) + || (registrar.getRegistrarName().compareTo(cursorQueryString.get()) + > 0)) + && shouldBeVisible(registrar)) + .limit(rdapResultSetMaxSize + 1) + .collect(toImmutableList()); + } else if ((subtype == Subtype.CONTACTS) || partialStringQuery.getHasWildcard()) { + registrars = ImmutableList.of(); + } else { + registrars = getMatchingRegistrars(partialStringQuery.getInitialString()); + } // Get the contact matches and return the results, fetching an additional contact to detect // truncation. If we are including deleted entries, we must fetch more entries, in case some // get excluded due to permissioning. Any cursor present must be a contact cursor, because we @@ -500,12 +529,7 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { } jsonOutputList.add(rdapJsonFormatter.makeRdapJsonForRegistrar( registrar, false, fullServletPath, rdapWhoisServer, now, outputDataType)); - newCursor = - Optional.of( - REGISTRAR_CURSOR_PREFIX - + ((queryType == QueryType.FULL_NAME) - ? registrar.getRegistrarName() - : registrar.getIanaIdentifier())); + newCursor = Optional.of(REGISTRAR_CURSOR_PREFIX + registrar.getRegistrarName()); } return RdapSearchResults.create( ImmutableList.copyOf(jsonOutputList), diff --git a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java index c76db2d83..c4936e811 100644 --- a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java +++ b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java @@ -851,6 +851,32 @@ public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { "Entity 9")); } + @Test + public void testNameMatchRegistrars_cursorNavigationThroughAll() throws Exception { + createManyContactsAndRegistrars(0, 13, registrarTest); + action.subtypeParam = Optional.of("registrars"); + checkCursorNavigation( + QueryType.FULL_NAME, + "*", + ImmutableList.of( + "Entity 1", + "Entity 10", + "Entity 11", + "Entity 12", + "Entity 13", + "Entity 2", + "Entity 3", + "Entity 4", + "Entity 5", + "Entity 6", + "Entity 7", + "Entity 8", + "Entity 9", + "New Registrar", + "The Registrar", + "Yes Virginia