diff --git a/java/google/registry/export/PublishDetailReportAction.java b/java/google/registry/export/PublishDetailReportAction.java index 8f863f8b2..769feb90c 100644 --- a/java/google/registry/export/PublishDetailReportAction.java +++ b/java/google/registry/export/PublishDetailReportAction.java @@ -91,7 +91,7 @@ public final class PublishDetailReportAction implements Runnable, JsonAction { try { logger.infofmt("Publishing detail report for parameters: %s", json); String registrarId = getParam(json, REGISTRAR_ID_PARAM); - Registrar registrar = checkArgumentNotNull(Registrar.loadByClientId(registrarId), + Registrar registrar = checkArgumentNotNull(Registrar.loadByClientIdCached(registrarId), "Registrar %s not found", registrarId); String driveFolderId = checkArgumentNotNull(registrar.getDriveFolderId(), "No drive folder associated with registrar " + registrarId); diff --git a/java/google/registry/export/SyncGroupMembersAction.java b/java/google/registry/export/SyncGroupMembersAction.java index fb93a03ff..e4392760d 100644 --- a/java/google/registry/export/SyncGroupMembersAction.java +++ b/java/google/registry/export/SyncGroupMembersAction.java @@ -113,13 +113,13 @@ public final class SyncGroupMembersAction implements Runnable { */ @Override public void run() { - List dirtyRegistrars = Registrar - .loadAllActive() + List dirtyRegistrars = FluentIterable.from(Registrar.loadAllCached()) .filter(new Predicate() { @Override public boolean apply(Registrar registrar) { - // Only grab registrars that require syncing and are of the correct type. - return registrar.getContactsRequireSyncing() + // Only grab active registrars that require syncing and are of the correct type. + return registrar.isActive() + && registrar.getContactsRequireSyncing() && registrar.getType() == Registrar.Type.REAL; }}) .toList(); diff --git a/java/google/registry/export/sheet/SyncRegistrarsSheet.java b/java/google/registry/export/sheet/SyncRegistrarsSheet.java index 0ea823c90..f6e8eb4f7 100644 --- a/java/google/registry/export/sheet/SyncRegistrarsSheet.java +++ b/java/google/registry/export/sheet/SyncRegistrarsSheet.java @@ -65,7 +65,7 @@ class SyncRegistrarsSheet { boolean wereRegistrarsModified() { Cursor cursor = ofy().load().key(Cursor.createGlobalKey(SYNC_REGISTRAR_SHEET)).now(); DateTime lastUpdateTime = (cursor == null) ? START_OF_TIME : cursor.getCursorTime(); - for (Registrar registrar : Registrar.loadAll()) { + for (Registrar registrar : Registrar.loadAllCached()) { if (DateTimeUtils.isAtOrAfter(registrar.getLastUpdateTime(), lastUpdateTime)) { return true; } @@ -85,7 +85,7 @@ class SyncRegistrarsSheet { public int compare(Registrar left, Registrar right) { return left.getClientId().compareTo(right.getClientId()); } - }.immutableSortedCopy(Registrar.loadAll())) + }.immutableSortedCopy(Registrar.loadAllCached())) .filter( new Predicate() { @Override diff --git a/java/google/registry/flows/domain/DomainFlowUtils.java b/java/google/registry/flows/domain/DomainFlowUtils.java index 2527cc40c..b7c681733 100644 --- a/java/google/registry/flows/domain/DomainFlowUtils.java +++ b/java/google/registry/flows/domain/DomainFlowUtils.java @@ -230,7 +230,7 @@ public class DomainFlowUtils { /** Check if the registrar running the flow has access to the TLD in question. */ public static void checkAllowedAccessToTld(String clientId, String tld) throws EppException { - if (!Registrar.loadByClientId(clientId).getAllowedTlds().contains(tld)) { + if (!Registrar.loadByClientIdCached(clientId).getAllowedTlds().contains(tld)) { throw new DomainFlowUtils.NotAuthorizedForTldException(tld); } } @@ -434,7 +434,7 @@ public class DomainFlowUtils { if (isDomainPremium(domainName, priceTime)) { // NB: The load of the Registar object is transactionless, which means that it should hit // memcache most of the time. - if (Registrar.loadByClientId(clientId).getBlockPremiumNames()) { + if (Registrar.loadByClientIdCached(clientId).getBlockPremiumNames()) { throw new PremiumNameBlockedException(); } } diff --git a/java/google/registry/flows/session/LoginFlow.java b/java/google/registry/flows/session/LoginFlow.java index b4b9bfffa..eededa555 100644 --- a/java/google/registry/flows/session/LoginFlow.java +++ b/java/google/registry/flows/session/LoginFlow.java @@ -116,7 +116,7 @@ public class LoginFlow implements Flow { } serviceExtensionUrisBuilder.add(uri); } - Registrar registrar = Registrar.loadByClientId(login.getClientId()); + Registrar registrar = Registrar.loadByClientIdCached(login.getClientId()); if (registrar == null) { throw new BadRegistrarClientIdException(login.getClientId()); } diff --git a/java/google/registry/model/registrar/Registrar.java b/java/google/registry/model/registrar/Registrar.java index 8f2c4b087..a4fe2f02b 100644 --- a/java/google/registry/model/registrar/Registrar.java +++ b/java/google/registry/model/registrar/Registrar.java @@ -16,7 +16,6 @@ package google.registry.model.registrar; import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Predicates.equalTo; import static com.google.common.base.Predicates.in; import static com.google.common.base.Predicates.notNull; @@ -26,6 +25,7 @@ import static com.google.common.collect.Sets.immutableEnumSet; import static com.google.common.io.BaseEncoding.base64; import static google.registry.config.RegistryConfig.getDefaultRegistrarReferralUrl; import static google.registry.config.RegistryConfig.getDefaultRegistrarWhoisServer; +import static google.registry.model.CacheUtils.memoizeWithShortExpiration; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.ofy.Ofy.RECOMMENDED_MEMCACHE_EXPIRATION; @@ -45,6 +45,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.Range; import com.google.common.collect.Sets; import com.google.re2j.Pattern; import com.googlecode.objectify.Key; @@ -200,6 +201,27 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable } }; + /** + * A caching {@link Supplier} of a clientId to {@link Registrar} map. + * + *

The supplier's get() method enters a transactionless context briefly to avoid enrolling the + * query inside an unrelated client-affecting transaction. + */ + private static final Supplier> CACHE_BY_CLIENT_ID = + memoizeWithShortExpiration(new Supplier>() { + @Override + public ImmutableMap get() { + return ofy().doTransactionless(new Work>() { + @Override + public ImmutableMap run() { + ImmutableMap.Builder builder = new ImmutableMap.Builder<>(); + for (Registrar registrar : loadAll()) { + builder.put(registrar.getClientId(), registrar); + } + return builder.build(); + }}); + }}); + @Parent Key parent = getCrossTldKey(); @@ -306,9 +328,11 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable * @see Registrar IDs */ @Index + @Nullable Long ianaIdentifier; /** Identifier of registrar used in external billing system (e.g. Oracle). */ + @Nullable Long billingIdentifier; /** @@ -413,10 +437,12 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable return creationTime.getTimestamp(); } + @Nullable public Long getIanaIdentifier() { return ianaIdentifier; } + @Nullable public Long getBillingIdentifier() { return billingIdentifier; } @@ -647,7 +673,8 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable public Builder setClientId(String clientId) { // Client id must be [3,16] chars long. See "clIDType" in the base EPP schema of RFC 5730. // (Need to validate this here as there's no matching EPP XSD for validation.) - checkArgument(clientId.length() >= 3 && clientId.length() <= 16, + checkArgument( + Range.closed(3, 16).contains(clientId.length()), "Client identifier must be 3-16 characters long."); getInstance().clientIdentifier = clientId; return this; @@ -831,8 +858,9 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable public Builder setPassword(String password) { // Passwords must be [6,16] chars long. See "pwType" in the base EPP schema of RFC 5730. - checkArgument(password != null && password.length() >= 6 && password.length() <= 16, - "Password must be [6,16] characters long."); + checkArgument( + Range.closed(6, 16).contains(nullToEmpty(password).length()), + "Password must be 6-16 characters long."); getInstance().salt = base64().encode(saltSupplier.get()); getInstance().passwordHash = getInstance().hashPassword(password); return this; @@ -862,124 +890,25 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable } } - /** Load a registrar entity by its client id outside of a transaction. */ - @Nullable - public static Registrar loadByClientId(final String clientId) { - checkNotNull(clientId, "Client ID cannot be null"); - return ofy().doTransactionless(new Work() { - @Override - public Registrar run() { - return ofy().loadWithMemcache() - .type(Registrar.class) - .parent(getCrossTldKey()) - .id(clientId) - .now(); - }}); - } - - /** - * Load registrar entities by client id range outside of a transaction. - * - * @param clientIdStart returned registrars will have a client id greater than or equal to this - * @param clientIdAfterEnd returned registrars will have a client id less than this - * @param resultSetMaxSize the maximum number of registrar entities to be returned - */ - public static Iterable loadByClientIdRange( - final String clientIdStart, final String clientIdAfterEnd, final int resultSetMaxSize) { - return ofy().doTransactionless(new Work>() { - @Override - public Iterable run() { - return ofy().load() - .type(Registrar.class) - .filterKey(">=", Key.create(getCrossTldKey(), Registrar.class, clientIdStart)) - .filterKey("<", Key.create(getCrossTldKey(), Registrar.class, clientIdAfterEnd)) - .limit(resultSetMaxSize); - }}); - } - - /** Load a registrar entity by its name outside of a transaction. */ - @Nullable - public static Registrar loadByName(final String name) { - return ofy().doTransactionless(new Work() { - @Override - public Registrar run() { - return ofy().load() - .type(Registrar.class) - .filter("registrarName", name) - .first() - .now(); - }}); - } - - /** - * Load registrar entities by registrar name range, inclusive of the start but not the end, - * outside of a transaction. - * - * @param nameStart returned registrars will have a name greater than or equal to this - * @param nameAfterEnd returned registrars will have a name less than this - * @param resultSetMaxSize the maximum number of registrar entities to be returned - */ - public static Iterable loadByNameRange( - final String nameStart, final String nameAfterEnd, final int resultSetMaxSize) { - return ofy().doTransactionless(new Work>() { - @Override - public Iterable run() { - return ofy().load() - .type(Registrar.class) - .filter("registrarName >=", nameStart) - .filter("registrarName <", nameAfterEnd) - .limit(resultSetMaxSize); - }}); - } - - /** - * Load registrar entities by IANA identifier range outside of a transaction. - * - * @param ianaIdentifierStart returned registrars will have an IANA id greater than or equal to - * this - * @param ianaIdentifierAfterEnd returned registrars will have an IANA id less than this - * @param resultSetMaxSize the maximum number of registrar entities to be returned - */ - public static Iterable loadByIanaIdentifierRange( - final Long ianaIdentifierStart, - final Long ianaIdentifierAfterEnd, - final int resultSetMaxSize) { - return ofy().doTransactionless(new Work>() { - @Override - public Iterable run() { - return ofy().load() - .type(Registrar.class) - .filter("ianaIdentifier >=", ianaIdentifierStart) - .filter("ianaIdentifier <", ianaIdentifierAfterEnd) - .limit(resultSetMaxSize); - }}); - } - - /** Loads all registrar entities. */ + /** Loads all registrar entities directly from Datastore. */ public static Iterable loadAll() { - return ofy().load().type(Registrar.class).ancestor(getCrossTldKey()); + return ImmutableList.copyOf(ofy().load().type(Registrar.class).ancestor(getCrossTldKey())); } - /** Loads all active registrar entities. */ - public static FluentIterable loadAllActive() { - return FluentIterable.from(loadAll()).filter(IS_ACTIVE); + /** Loads all registrar entities using an in-memory cache. */ + public static Iterable loadAllCached() { + return CACHE_BY_CLIENT_ID.get().values(); } - private static final Predicate IS_ACTIVE = new Predicate() { - @Override - public boolean apply(Registrar registrar) { - return registrar.isActive(); - }}; - - /** Loads all active registrar entities. */ - public static FluentIterable loadAllActiveAndPubliclyVisible() { - return FluentIterable.from(loadAll()).filter(IS_ACTIVE_AND_PUBLICLY_VISIBLE); + /** Load a registrar entity by its client id directly from Datastore. */ + @Nullable + public static Registrar loadByClientId(String clientId) { + return ofy().load().type(Registrar.class).parent(getCrossTldKey()).id(clientId).now(); } - private static final Predicate IS_ACTIVE_AND_PUBLICLY_VISIBLE = - new Predicate() { - @Override - public boolean apply(Registrar registrar) { - return registrar.isActiveAndPubliclyVisible(); - }}; + /** Load a registrar entity by its client id using an in-memory cache. */ + @Nullable + public static Registrar loadByClientIdCached(String clientId) { + return CACHE_BY_CLIENT_ID.get().get(clientId); + } } diff --git a/java/google/registry/rdap/RdapEntityAction.java b/java/google/registry/rdap/RdapEntityAction.java index 0967fc56b..0fb1a38f4 100644 --- a/java/google/registry/rdap/RdapEntityAction.java +++ b/java/google/registry/rdap/RdapEntityAction.java @@ -15,12 +15,13 @@ package google.registry.rdap; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.rdap.RdapUtils.getRegistrarByIanaIdentifier; import static google.registry.request.Action.Method.GET; import static google.registry.request.Action.Method.HEAD; import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Iterables; +import com.google.common.primitives.Longs; import com.google.re2j.Pattern; import com.googlecode.objectify.Key; import google.registry.model.contact.ContactResource; @@ -96,18 +97,14 @@ public class RdapEntityAction extends RdapActionBase { OutputDataType.FULL); } } - try { - Long ianaIdentifier = Long.parseLong(pathSearchString); + Long ianaIdentifier = Longs.tryParse(pathSearchString); + if (ianaIdentifier != null) { wasValidKey = true; - Registrar registrar = Iterables.getOnlyElement( - Registrar.loadByIanaIdentifierRange(ianaIdentifier, ianaIdentifier + 1, 1), null); - if ((registrar != null) && registrar.isActiveAndPubliclyVisible()) { + Optional registrar = getRegistrarByIanaIdentifier(ianaIdentifier); + if ((registrar.isPresent()) && registrar.get().isActiveAndPubliclyVisible()) { return rdapJsonFormatter.makeRdapJsonForRegistrar( - registrar, true, rdapLinkBase, rdapWhoisServer, now, OutputDataType.FULL); + registrar.get(), true, rdapLinkBase, rdapWhoisServer, now, OutputDataType.FULL); } - } catch (NumberFormatException e) { - // Although the search string was not a valid IANA identifier, it might still have been a - // valid ROID. } // At this point, we have failed to find either a contact or a registrar. throw wasValidKey diff --git a/java/google/registry/rdap/RdapEntitySearchAction.java b/java/google/registry/rdap/RdapEntitySearchAction.java index 237575dff..4c4d07936 100644 --- a/java/google/registry/rdap/RdapEntitySearchAction.java +++ b/java/google/registry/rdap/RdapEntitySearchAction.java @@ -16,14 +16,18 @@ package google.registry.rdap; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.rdap.RdapIcannStandardInformation.TRUNCATION_NOTICES; +import static google.registry.rdap.RdapUtils.getRegistrarByIanaIdentifier; 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 com.google.common.base.Optional; +import com.google.common.base.Predicate; +import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.primitives.Booleans; +import com.google.common.primitives.Longs; import com.googlecode.objectify.Key; import google.registry.config.RegistryConfig.Config; import google.registry.model.contact.ContactResource; @@ -138,19 +142,16 @@ public class RdapEntitySearchAction extends RdapActionBase { throw new UnprocessableEntityException("Suffixes not allowed in entity name searches"); } // Get the registrar matches, depending on whether there's a wildcard. - ImmutableList registrarMatches; - if (!partialStringQuery.getHasWildcard()) { - Registrar registrar = Registrar.loadByName(partialStringQuery.getInitialString()); - registrarMatches = (registrar == null) - ? ImmutableList.of() - : ImmutableList.of(registrar); - } else { - // Fetch an additional registrar, so we can detect result set truncation. - registrarMatches = ImmutableList.copyOf(Registrar.loadByNameRange( - partialStringQuery.getInitialString(), - partialStringQuery.getNextInitialString(), - rdapResultSetMaxSize + 1)); - } + ImmutableList registrarMatches = + FluentIterable.from(Registrar.loadAllCached()) + .filter( + new Predicate() { + @Override + public boolean apply(Registrar registrar) { + return partialStringQuery.matches(registrar.getRegistrarName()); + }}) + .limit(rdapResultSetMaxSize + 1) + .toList(); // Get the contact matches and return the results, fetching an additional contact to detect // truncation. return makeSearchResults( @@ -169,7 +170,8 @@ public class RdapEntitySearchAction extends RdapActionBase { .type(ContactResource.class) .id(partialStringQuery.getInitialString()) .now(); - ImmutableList registrars = getMatchingRegistrars(partialStringQuery); + ImmutableList registrars = + getMatchingRegistrars(partialStringQuery.getInitialString()); return makeSearchResults( ((contactResource == null) || !contactResource.getDeletionTime().isEqual(END_OF_TIME)) ? ImmutableList.of() : ImmutableList.of(contactResource), @@ -201,17 +203,15 @@ public class RdapEntitySearchAction extends RdapActionBase { } /** Looks up registrars by handle (i.e. IANA identifier). */ - private ImmutableList - getMatchingRegistrars(final RdapSearchPattern partialStringQuery) { - Long ianaIdentifier; - try { - ianaIdentifier = Long.parseLong(partialStringQuery.getInitialString()); - } catch (NumberFormatException e) { - return ImmutableList.of(); + private ImmutableList getMatchingRegistrars(final String ianaIdentifierString) { + Long ianaIdentifier = Longs.tryParse(ianaIdentifierString); + if (ianaIdentifier != null) { + Optional registrar = getRegistrarByIanaIdentifier(ianaIdentifier); + if (registrar.isPresent()) { + return ImmutableList.of(registrar.get()); + } } - // Fetch an additional registrar to detect result set truncation. - return ImmutableList.copyOf(Registrar.loadByIanaIdentifierRange( - ianaIdentifier, ianaIdentifier + 1, rdapResultSetMaxSize + 1)); + return ImmutableList.of(); } /** Builds a JSON array of entity info maps based on the specified contacts and registrars. */ diff --git a/java/google/registry/rdap/RdapUtils.java b/java/google/registry/rdap/RdapUtils.java new file mode 100644 index 000000000..dd04512ee --- /dev/null +++ b/java/google/registry/rdap/RdapUtils.java @@ -0,0 +1,38 @@ +// Copyright 2017 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.rdap; + +import static com.google.common.collect.Iterables.tryFind; + +import com.google.common.base.Optional; +import com.google.common.base.Predicate; +import google.registry.model.registrar.Registrar; + +/** Utility functions for RDAP. */ +public final class RdapUtils { + + private RdapUtils() {} + + /** Looks up a registrar by its IANA identifier. */ + static Optional getRegistrarByIanaIdentifier(final long ianaIdentifier) { + return tryFind( + Registrar.loadAllCached(), + new Predicate() { + @Override + public boolean apply(Registrar registrar) { + return registrar.getIanaIdentifier() == ianaIdentifier; + }}); + } +} diff --git a/java/google/registry/rde/RdeStagingMapper.java b/java/google/registry/rde/RdeStagingMapper.java index a26b75b2e..6eee05a06 100644 --- a/java/google/registry/rde/RdeStagingMapper.java +++ b/java/google/registry/rde/RdeStagingMapper.java @@ -62,7 +62,7 @@ public final class RdeStagingMapper extends Mapper getAllRegistrarNames() { - return Registrar.loadAllActive() + return FluentIterable.from(Registrar.loadAll()) .transform(new Function() { @Override public String apply(Registrar registrar) { + if (!registrar.isActive()) { + return null; + } String name = registrar.getClientId(); // Look for names of the form "regname-1", "regname-2", etc. and strip the -# suffix. String replacedName = name.replaceFirst("^(.*)-[1234]$", "$1"); diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index 420d64c2a..0711d9878 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/java/google/registry/ui/server/registrar/ConsoleUiAction.java @@ -115,7 +115,7 @@ public final class ConsoleUiAction implements Runnable { .render()); return; } - Registrar registrar = Registrar.loadByClientId(sessionUtils.getRegistrarClientId(req)); + Registrar registrar = Registrar.loadByClientIdCached(sessionUtils.getRegistrarClientId(req)); data.put( "xsrfToken", xsrfTokenManager.generateToken(userService.getCurrentUser().getEmail())); diff --git a/java/google/registry/ui/server/registrar/SessionUtils.java b/java/google/registry/ui/server/registrar/SessionUtils.java index 9c3762117..3052fb3b0 100644 --- a/java/google/registry/ui/server/registrar/SessionUtils.java +++ b/java/google/registry/ui/server/registrar/SessionUtils.java @@ -122,7 +122,9 @@ public class SessionUtils { if (contact == null) { return Optional.absent(); } - Optional result = Optional.fromNullable(ofy().load().key(contact.getParent()).now()); + String registrarClientId = contact.getParent().getName(); + Optional result = + Optional.fromNullable(Registrar.loadByClientIdCached(registrarClientId)); if (!result.isPresent()) { logger.severefmt( "A contact record exists for non-existent registrar: %s.", Key.create(contact)); @@ -132,7 +134,7 @@ public class SessionUtils { /** @see #hasAccessToRegistrar(Registrar, String) */ private static boolean hasAccessToRegistrar(String clientId, final String gaeUserId) { - Registrar registrar = Registrar.loadByClientId(clientId); + Registrar registrar = Registrar.loadByClientIdCached(clientId); if (registrar == null) { logger.warningfmt("Registrar '%s' disappeared from Datastore!", clientId); return false; diff --git a/java/google/registry/whois/RegistrarLookupCommand.java b/java/google/registry/whois/RegistrarLookupCommand.java index 4393697bc..8f26dd0e8 100644 --- a/java/google/registry/whois/RegistrarLookupCommand.java +++ b/java/google/registry/whois/RegistrarLookupCommand.java @@ -25,6 +25,7 @@ import com.google.common.base.CharMatcher; import com.google.common.base.Joiner; import com.google.common.base.Splitter; import com.google.common.base.Supplier; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import google.registry.model.registrar.Registrar; import google.registry.util.FormattingLogger; @@ -48,10 +49,9 @@ final class RegistrarLookupCommand implements WhoisCommand { @Override public Map get() { Map map = new HashMap<>(); - // Use the normalized registrar name as a key. - Iterable registrars = Registrar.loadAllActiveAndPubliclyVisible(); - for (Registrar registrar : registrars) { - if (registrar.getRegistrarName() == null) { + // Use the normalized registrar name as a key, and ignore inactive and hidden registrars. + for (Registrar registrar : Registrar.loadAllCached()) { + if (!registrar.isActiveAndPubliclyVisible() || registrar.getRegistrarName() == null) { continue; } String normalized = normalizeRegistrarName(registrar.getRegistrarName()); @@ -65,7 +65,7 @@ final class RegistrarLookupCommand implements WhoisCommand { // if there isn't already a mapping for this string, so that if there's a registrar with a // two word name (Go Daddy) and no business-type suffix and another registrar with just // that first word as its name (Go), the latter will win. - for (Registrar registrar : registrars) { + for (Registrar registrar : ImmutableList.copyOf(map.values())) { if (registrar.getRegistrarName() == null) { continue; } diff --git a/javatests/google/registry/model/registrar/RegistrarTest.java b/javatests/google/registry/model/registrar/RegistrarTest.java index 801d8ba7d..389a970fc 100644 --- a/javatests/google/registry/model/registrar/RegistrarTest.java +++ b/javatests/google/registry/model/registrar/RegistrarTest.java @@ -142,19 +142,19 @@ public class RegistrarTest extends EntityTestCase { @Test public void testFailure_passwordNull() throws Exception { - thrown.expect(IllegalArgumentException.class, "Password must be [6,16] characters long."); + thrown.expect(IllegalArgumentException.class, "Password must be 6-16 characters long."); new Registrar.Builder().setPassword(null); } @Test public void testFailure_passwordTooShort() throws Exception { - thrown.expect(IllegalArgumentException.class, "Password must be [6,16] characters long."); + thrown.expect(IllegalArgumentException.class, "Password must be 6-16 characters long."); new Registrar.Builder().setPassword("abcde"); } @Test public void testFailure_passwordTooLong() throws Exception { - thrown.expect(IllegalArgumentException.class, "Password must be [6,16] characters long."); + thrown.expect(IllegalArgumentException.class, "Password must be 6-16 characters long."); new Registrar.Builder().setPassword("abcdefghijklmnopq"); } @@ -397,11 +397,11 @@ public class RegistrarTest extends EntityTestCase { } @Test - public void testLoadByClientId_isTransactionless() { + public void testLoadByClientIdCached_isTransactionless() { ofy().transact(new VoidWork() { @Override public void vrun() { - assertThat(Registrar.loadByClientId("registrar")).isNotNull(); + assertThat(Registrar.loadByClientIdCached("registrar")).isNotNull(); // Load something as a control to make sure we are seeing loaded keys in the session cache. ofy().load().entity(abuseAdminContact).now(); assertThat(ofy().getSessionKeys()).contains(Key.create(abuseAdminContact)); @@ -409,7 +409,7 @@ public class RegistrarTest extends EntityTestCase { }}); ofy().clearSessionCache(); // Conversely, loads outside of a transaction should end up in the session cache. - assertThat(Registrar.loadByClientId("registrar")).isNotNull(); + assertThat(Registrar.loadByClientIdCached("registrar")).isNotNull(); assertThat(ofy().getSessionKeys()).contains(Key.create(registrar)); } }