diff --git a/java/google/registry/rdap/RdapEntitySearchAction.java b/java/google/registry/rdap/RdapEntitySearchAction.java index 64ecaa8ad..0405e146f 100644 --- a/java/google/registry/rdap/RdapEntitySearchAction.java +++ b/java/google/registry/rdap/RdapEntitySearchAction.java @@ -31,6 +31,7 @@ import google.registry.model.registrar.Registrar; import google.registry.rdap.RdapJsonFormatter.BoilerplateType; import google.registry.rdap.RdapJsonFormatter.OutputDataType; import google.registry.rdap.RdapMetrics.EndpointType; +import google.registry.rdap.RdapMetrics.SearchType; import google.registry.rdap.RdapSearchResults.IncompletenessWarningType; import google.registry.request.Action; import google.registry.request.HttpException.BadRequestException; @@ -99,13 +100,18 @@ public class RdapEntitySearchAction extends RdapActionBase { } RdapSearchResults results; if (fnParam.isPresent()) { + metricInformationBuilder.setSearchType(SearchType.BY_FULL_NAME); // syntax: /rdap/entities?fn=Bobby%20Joe* // The name is the contact name or registrar name (not registrar contact name). - results = searchByName(RdapSearchPattern.create(fnParam.get(), false), now); + results = + searchByName(recordWildcardType(RdapSearchPattern.create(fnParam.get(), false)), now); } else { + metricInformationBuilder.setSearchType(SearchType.BY_HANDLE); // syntax: /rdap/entities?handle=12345-* // The handle is either the contact roid or the registrar clientId. - results = searchByHandle(RdapSearchPattern.create(handleParam.get(), false), now); + results = + searchByHandle( + recordWildcardType(RdapSearchPattern.create(handleParam.get(), false)), now); } if (results.jsonList().isEmpty()) { throw new NotFoundException("No entities found"); @@ -213,11 +219,14 @@ public class RdapEntitySearchAction extends RdapActionBase { .type(ContactResource.class) .id(partialStringQuery.getInitialString()) .now(); - return makeSearchResults( + ImmutableList contactResourceList = ((contactResource != null) && shouldBeVisible(contactResource, now)) ? ImmutableList.of(contactResource) - : ImmutableList.of(), + : ImmutableList.of(); + return makeSearchResults( + contactResourceList, IncompletenessWarningType.COMPLETE, + contactResourceList.size(), getMatchingRegistrars(partialStringQuery.getInitialString()), now); // Handle queries with a wildcard (or including deleted), but no suffix. Because the handle @@ -235,10 +244,7 @@ public class RdapEntitySearchAction extends RdapActionBase { int querySizeLimit = getStandardQuerySizeLimit(); Query query = queryItemsByKey( - ContactResource.class, - partialStringQuery, - shouldIncludeDeleted(), - querySizeLimit); + ContactResource.class, partialStringQuery, shouldIncludeDeleted(), querySizeLimit); return makeSearchResults( getMatchingResources(query, shouldIncludeDeleted(), now, querySizeLimit), registrars, @@ -261,23 +267,30 @@ public class RdapEntitySearchAction extends RdapActionBase { /** * Builds a JSON array of entity info maps based on the specified contacts and registrars. * - *

This is a convenience wrapper for the four-argument makeSearchResults; it unpacks the two - * properties of the ContactsAndIncompletenessWarningType structure and passes them as separate - * arguments. + *

This is a convenience wrapper for the four-argument makeSearchResults; it unpacks the + * properties of the {@link RdapResultSet} structure and passes them as separate arguments. */ private RdapSearchResults makeSearchResults( RdapResultSet resultSet, List registrars, DateTime now) { return makeSearchResults( - resultSet.resources(), resultSet.incompletenessWarningType(), registrars, now); + resultSet.resources(), + resultSet.incompletenessWarningType(), + resultSet.numResourcesRetrieved(), + registrars, + now); } /** * Builds a JSON array of entity info maps based on the specified contacts and registrars. * + *

The number of contacts retrieved is recorded for use by the metrics. + * * @param contacts the list of contacts which can be returned * @param incompletenessWarningType MIGHT_BE_INCOMPLETE if the list of contacts might be * incomplete; this only matters if the total count of contacts and registrars combined is * less than a full result set's worth + * @param numContactsRetrieved the number of contacts retrieved in the process of generating the + * results * @param registrars the list of registrars which can be returned * @param now the current date and time * @return an {@link RdapSearchResults} object @@ -285,9 +298,12 @@ public class RdapEntitySearchAction extends RdapActionBase { private RdapSearchResults makeSearchResults( List contacts, IncompletenessWarningType incompletenessWarningType, + int numContactsRetrieved, List registrars, DateTime now) { + metricInformationBuilder.setNumContactsRetrieved(numContactsRetrieved); + // Determine what output data type to use, depending on whether more than one entity will be // returned. OutputDataType outputDataType = diff --git a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java index b1610a350..6c80aa104 100644 --- a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java @@ -17,7 +17,6 @@ package google.registry.rdap; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.truth.Truth.assertThat; import static google.registry.rdap.RdapAuthorization.Role.ADMINISTRATOR; -import static google.registry.rdap.RdapAuthorization.Role.PUBLIC; import static google.registry.rdap.RdapAuthorization.Role.REGISTRAR; import static google.registry.request.Action.Method.POST; import static google.registry.testing.DatastoreHelper.createTld; @@ -33,7 +32,6 @@ import static google.registry.testing.FullFieldsTestEntityHelper.makeRegistrar; import static google.registry.testing.FullFieldsTestEntityHelper.makeRegistrarContacts; import static google.registry.testing.TestDataHelper.loadFileWithSubstitutions; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.appengine.api.users.User; @@ -84,7 +82,7 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link RdapDomainSearchAction}. */ @RunWith(JUnit4.class) -public class RdapDomainSearchActionTest { +public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { @Rule public final AppEngineRule appEngine = AppEngineRule.builder() @@ -101,11 +99,7 @@ public class RdapDomainSearchActionTest { private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); private final UserAuthInfo adminUserAuthInfo = UserAuthInfo.create(user, true); - private RdapAuthorization.Role metricRole = PUBLIC; - private WildcardType metricWildcardType = WildcardType.INVALID; - private int metricPrefixLength = 0; private final RdapDomainSearchAction action = new RdapDomainSearchAction(); - private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); private Registrar registrar; private DomainResource domainCatLol; @@ -675,26 +669,6 @@ public class RdapDomainSearchActionTest { assertThat(response.getStatus()).isEqualTo(404); } - private void rememberWildcardType(String queryString) { - try { - RdapSearchPattern partialStringQuery = - RdapSearchPattern.create(Idn.toASCII(queryString), true); - if (!partialStringQuery.getHasWildcard()) { - metricWildcardType = WildcardType.NO_WILDCARD; - } else if (partialStringQuery.getSuffix() == null) { - metricWildcardType = WildcardType.PREFIX; - } else if (partialStringQuery.getInitialString().isEmpty()) { - metricWildcardType = WildcardType.SUFFIX; - } else { - metricWildcardType = WildcardType.PREFIX_AND_SUFFIX; - } - metricPrefixLength = partialStringQuery.getInitialString().length(); - } catch (Exception e) { - metricWildcardType = WildcardType.INVALID; - metricPrefixLength = 0; - } - } - private void verifyMetrics(SearchType searchType, Optional numDomainsRetrieved) { verifyMetrics( searchType, numDomainsRetrieved, Optional.empty(), IncompletenessWarningType.COMPLETE); @@ -727,25 +701,16 @@ public class RdapDomainSearchActionTest { Optional numDomainsRetrieved, Optional numHostsRetrieved, IncompletenessWarningType incompletenessWarningType) { - RdapMetrics.RdapMetricInformation.Builder builder = - RdapMetrics.RdapMetricInformation.builder() - .setEndpointType(EndpointType.DOMAINS) - .setSearchType(searchType) - .setWildcardType(metricWildcardType) - .setPrefixLength(metricPrefixLength) - .setIncludeDeleted(action.includeDeletedParam.isPresent()) - .setRegistrarSpecified(action.registrarParam.isPresent()) - .setRole(metricRole) - .setRequestMethod(POST) - .setStatusCode(200) - .setIncompletenessWarningType(incompletenessWarningType); - if (numDomainsRetrieved.isPresent()) { - builder.setNumDomainsRetrieved(numDomainsRetrieved.get()); - } - if (numHostsRetrieved.isPresent()) { - builder.setNumHostsRetrieved(numHostsRetrieved.get()); - } - verify(rdapMetrics).updateMetrics(builder.build()); + metricSearchType = searchType; + verifyMetrics( + EndpointType.DOMAINS, + POST, + action.includeDeletedParam.orElse(false), + action.registrarParam.isPresent(), + numDomainsRetrieved, + numHostsRetrieved, + Optional.empty(), + incompletenessWarningType); } private void verifyErrorMetrics(SearchType searchType) { @@ -762,25 +727,8 @@ public class RdapDomainSearchActionTest { Optional numDomainsRetrieved, Optional numHostsRetrieved, int statusCode) { - RdapMetrics.RdapMetricInformation.Builder builder = - RdapMetrics.RdapMetricInformation.builder() - .setEndpointType(EndpointType.DOMAINS) - .setSearchType(searchType) - .setWildcardType(metricWildcardType) - .setPrefixLength(metricPrefixLength) - .setIncludeDeleted(action.includeDeletedParam.isPresent()) - .setRegistrarSpecified(action.registrarParam.isPresent()) - .setRole(metricRole) - .setRequestMethod(POST) - .setStatusCode(statusCode) - .setIncompletenessWarningType(IncompletenessWarningType.COMPLETE); - if (numDomainsRetrieved.isPresent()) { - builder.setNumDomainsRetrieved(numDomainsRetrieved.get()); - } - if (numHostsRetrieved.isPresent()) { - builder.setNumHostsRetrieved(numHostsRetrieved.get()); - } - verify(rdapMetrics).updateMetrics(builder.build()); + metricStatusCode = statusCode; + verifyMetrics(searchType, numDomainsRetrieved, numHostsRetrieved); } @Test @@ -934,6 +882,7 @@ public class RdapDomainSearchActionTest { ImmutableList.of("ns1.cat.xn--q9jyb4c", "ns2.cat.xn--q9jyb4c"), "rdap_domain_unicode_no_contacts_with_remark.json"); // The unicode gets translated to ASCII before getting parsed into a search pattern. + metricPrefixLength = 15; verifyMetrics(SearchType.BY_DOMAIN_NAME, Optional.of(1L)); } diff --git a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java index 9dd4059f1..2dac475c5 100644 --- a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java +++ b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java @@ -16,6 +16,9 @@ package google.registry.rdap; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.truth.Truth.assertThat; +import static google.registry.rdap.RdapAuthorization.Role.ADMINISTRATOR; +import static google.registry.rdap.RdapAuthorization.Role.REGISTRAR; +import static google.registry.request.Action.Method.GET; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.persistResource; import static google.registry.testing.DatastoreHelper.persistResources; @@ -35,6 +38,9 @@ import com.google.common.collect.ImmutableMap; import google.registry.model.ImmutableObject; import google.registry.model.ofy.Ofy; import google.registry.model.registrar.Registrar; +import google.registry.rdap.RdapMetrics.EndpointType; +import google.registry.rdap.RdapMetrics.SearchType; +import google.registry.rdap.RdapSearchResults.IncompletenessWarningType; import google.registry.request.Action; import google.registry.request.auth.AuthLevel; import google.registry.request.auth.AuthResult; @@ -60,7 +66,7 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link RdapEntitySearchAction}. */ @RunWith(JUnit4.class) -public class RdapEntitySearchActionTest { +public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build(); @Rule public final InjectRule inject = new InjectRule(); @@ -72,7 +78,6 @@ public class RdapEntitySearchActionTest { private final User user = new User("rdap.user@example.com", "gmail.com", "12345"); private final UserAuthInfo userAuthInfo = UserAuthInfo.create(user, false); private final UserAuthInfo adminUserAuthInfo = UserAuthInfo.create(user, true); - private final RdapMetrics rdapMetrics = mock(RdapMetrics.class); private final RdapEntitySearchAction action = new RdapEntitySearchAction(); private Registrar registrarDeleted; @@ -80,12 +85,14 @@ public class RdapEntitySearchActionTest { private Registrar registrarTest; private Object generateActualJsonWithFullName(String fn) { + metricSearchType = SearchType.BY_FULL_NAME; action.fnParam = Optional.of(fn); action.run(); return JSONValue.parse(response.getPayload()); } private Object generateActualJsonWithHandle(String handle) { + metricSearchType = SearchType.BY_HANDLE; action.handleParam = Optional.of(handle); action.run(); return JSONValue.parse(response.getPayload()); @@ -161,12 +168,14 @@ public class RdapEntitySearchActionTest { private void login(String registrar) { when(sessionUtils.checkRegistrarConsoleLogin(request, userAuthInfo)).thenReturn(true); when(sessionUtils.getRegistrarClientId(request)).thenReturn(registrar); + metricRole = REGISTRAR; } private void loginAsAdmin() { action.authResult = AuthResult.create(AuthLevel.USER, adminUserAuthInfo); when(sessionUtils.checkRegistrarConsoleLogin(request, adminUserAuthInfo)).thenReturn(true); when(sessionUtils.getRegistrarClientId(request)).thenReturn("noregistrar"); + metricRole = ADMINISTRATOR; } private Object generateExpectedJson(String expectedOutputFile) { @@ -247,6 +256,32 @@ public class RdapEntitySearchActionTest { persistResources(resourcesBuilder.build()); } + private void verifyMetrics(long numContactsRetrieved) { + verifyMetrics(Optional.of(numContactsRetrieved)); + } + + private void verifyMetrics(Optional numContactsRetrieved) { + verifyMetrics( + EndpointType.ENTITIES, + GET, + action.includeDeletedParam.orElse(false), + action.registrarParam.isPresent(), + Optional.empty(), + Optional.empty(), + numContactsRetrieved, + IncompletenessWarningType.COMPLETE); + } + + private void verifyErrorMetrics(long numContactsRetrieved) { + metricStatusCode = 404; + verifyMetrics(numContactsRetrieved); + } + + private void verifyErrorMetrics(Optional numContactsRetrieved, int statusCode) { + metricStatusCode = statusCode; + verifyMetrics(numContactsRetrieved); + } + private void checkNumberOfEntitiesInResult(Object obj, int expected) { assertThat(obj).isInstanceOf(Map.class); @@ -254,9 +289,9 @@ public class RdapEntitySearchActionTest { Map map = (Map) obj; @SuppressWarnings("unchecked") - List domains = (List) map.get("entitySearchResults"); + List entities = (List) map.get("entitySearchResults"); - assertThat(domains).hasSize(expected); + assertThat(entities).hasSize(expected); } private void runSuccessfulNameTestWithBlinky(String queryString, String fileName) { @@ -286,14 +321,16 @@ public class RdapEntitySearchActionTest { @Nullable String email, @Nullable String address, String fileName) { + rememberWildcardType(queryString); assertThat(generateActualJsonWithFullName(queryString)) .isEqualTo( generateExpectedJsonForEntity(handle, fullName, status, email, address, fileName)); assertThat(response.getStatus()).isEqualTo(200); } - private void runNotFoundNameTest(String fullName) { - assertThat(generateActualJsonWithFullName(fullName)) + private void runNotFoundNameTest(String queryString) { + rememberWildcardType(queryString); + assertThat(generateActualJsonWithFullName(queryString)) .isEqualTo(generateExpectedJson("No entities found", "rdap_error_404.json")); assertThat(response.getStatus()).isEqualTo(404); } @@ -325,14 +362,16 @@ public class RdapEntitySearchActionTest { @Nullable String email, @Nullable String address, String fileName) { + rememberWildcardType(queryString); assertThat(generateActualJsonWithHandle(queryString)) .isEqualTo( generateExpectedJsonForEntity(handle, fullName, status, email, address, fileName)); assertThat(response.getStatus()).isEqualTo(200); } - private void runNotFoundHandleTest(String handle) { - assertThat(generateActualJsonWithHandle(handle)) + private void runNotFoundHandleTest(String queryString) { + rememberWildcardType(queryString); + assertThat(generateActualJsonWithHandle(queryString)) .isEqualTo(generateExpectedJson("No entities found", "rdap_error_404.json")); assertThat(response.getStatus()).isEqualTo(404); } @@ -342,6 +381,7 @@ public class RdapEntitySearchActionTest { action.requestPath = RdapEntitySearchAction.PATH + "/path"; action.run(); assertThat(response.getStatus()).isEqualTo(400); + verifyErrorMetrics(Optional.empty(), 400); } @Test @@ -352,6 +392,7 @@ public class RdapEntitySearchActionTest { generateExpectedJson( "You must specify either fn=XXXX or handle=YYYY", "rdap_error_400.json")); assertThat(response.getStatus()).isEqualTo(400); + verifyErrorMetrics(Optional.empty(), 400); } @Test @@ -360,6 +401,7 @@ public class RdapEntitySearchActionTest { .isEqualTo( generateExpectedJson("Suffix not allowed after wildcard", "rdap_error_422.json")); assertThat(response.getStatus()).isEqualTo(422); + verifyErrorMetrics(Optional.empty(), 422); } @Test @@ -368,6 +410,7 @@ public class RdapEntitySearchActionTest { .isEqualTo( generateExpectedJson("Suffix not allowed after wildcard", "rdap_error_422.json")); assertThat(response.getStatus()).isEqualTo(422); + verifyErrorMetrics(Optional.empty(), 422); } @Test @@ -375,32 +418,38 @@ public class RdapEntitySearchActionTest { assertThat(generateActualJsonWithHandle("*.*")) .isEqualTo(generateExpectedJson("Only one wildcard allowed", "rdap_error_422.json")); assertThat(response.getStatus()).isEqualTo(422); + verifyErrorMetrics(Optional.empty(), 422); } @Test public void testNoCharactersToMatch_rejected() throws Exception { + rememberWildcardType("*"); assertThat(generateActualJsonWithHandle("*")) .isEqualTo( generateExpectedJson( "Initial search string must be at least 2 characters", "rdap_error_422.json")); assertThat(response.getStatus()).isEqualTo(422); + verifyErrorMetrics(Optional.empty(), 422); } @Test public void testFewerThanTwoCharactersToMatch_rejected() throws Exception { + rememberWildcardType("a*"); assertThat(generateActualJsonWithHandle("a*")) .isEqualTo( generateExpectedJson( "Initial search string must be at least 2 characters", "rdap_error_422.json")); assertThat(response.getStatus()).isEqualTo(422); + verifyErrorMetrics(Optional.empty(), 422); } @Test public void testNameMatchContact_found() throws Exception { login("2-RegistrarTest"); runSuccessfulNameTestWithBlinky("Blinky (赤ベイ)", "rdap_contact.json"); + verifyMetrics(1); } @Test @@ -408,6 +457,7 @@ public class RdapEntitySearchActionTest { login("2-RegistrarTest"); action.registrarParam = Optional.of("2-RegistrarTest"); runSuccessfulNameTestWithBlinky("Blinky (赤ベイ)", "rdap_contact.json"); + verifyMetrics(1); } @Test @@ -415,29 +465,35 @@ public class RdapEntitySearchActionTest { login("2-RegistrarTest"); action.registrarParam = Optional.of("2-RegistrarInact"); runNotFoundNameTest("Blinky (赤ベイ)"); + verifyErrorMetrics(0); } @Test public void testNameMatchContact_found_asAdministrator() throws Exception { loginAsAdmin(); + rememberWildcardType("Blinky (赤ベイ)"); runSuccessfulNameTestWithBlinky("Blinky (赤ベイ)", "rdap_contact.json"); + verifyMetrics(1); } @Test public void testNameMatchContact_notFound_notLoggedIn() throws Exception { runNotFoundNameTest("Blinky (赤ベイ)"); + verifyErrorMetrics(0); } @Test public void testNameMatchContact_notFound_loggedInAsOtherRegistrar() throws Exception { login("2-Registrar"); runNotFoundNameTest("Blinky (赤ベイ)"); + verifyErrorMetrics(0); } @Test public void testNameMatchContact_found_wildcard() throws Exception { login("2-RegistrarTest"); runSuccessfulNameTestWithBlinky("Blinky*", "rdap_contact.json"); + verifyMetrics(1); } @Test @@ -445,6 +501,7 @@ public class RdapEntitySearchActionTest { login("2-RegistrarTest"); action.registrarParam = Optional.of("2-RegistrarTest"); runSuccessfulNameTestWithBlinky("Blinky*", "rdap_contact.json"); + verifyMetrics(1); } @Test @@ -452,20 +509,24 @@ public class RdapEntitySearchActionTest { login("2-RegistrarTest"); action.registrarParam = Optional.of("2-RegistrarInact"); runNotFoundNameTest("Blinky*"); + verifyErrorMetrics(0); } @Test public void testNameMatchContact_found_wildcardBoth() throws Exception { login("2-RegistrarTest"); + rememberWildcardType("Blin*"); assertThat(generateActualJsonWithFullName("Blin*")) .isEqualTo(generateExpectedJson("rdap_multiple_contacts2.json")); assertThat(response.getStatus()).isEqualTo(200); + verifyMetrics(2); } @Test public void testNameMatchContact_notFound_deleted() throws Exception { login("2-RegistrarTest"); runNotFoundNameTest("Cl*"); + verifyErrorMetrics(0); } @Test @@ -473,6 +534,7 @@ public class RdapEntitySearchActionTest { login("2-RegistrarTest"); action.includeDeletedParam = Optional.of(true); runNotFoundNameTest("Cl*"); + verifyErrorMetrics(0); } @Test @@ -480,6 +542,7 @@ public class RdapEntitySearchActionTest { login("2-Registrar"); action.includeDeletedParam = Optional.of(true); runNotFoundNameTest("Cl*"); + verifyErrorMetrics(0); } @Test @@ -487,6 +550,7 @@ public class RdapEntitySearchActionTest { loginAsAdmin(); action.includeDeletedParam = Optional.of(true); runNotFoundNameTest("Cl*"); + verifyErrorMetrics(0); } @Test @@ -494,6 +558,7 @@ public class RdapEntitySearchActionTest { login("2-RegistrarTest"); runSuccessfulNameTest( "Yes Virginia