Optimize RDAP entity event query (#1635)

* Optimize RDAP entity event query

For each EPP entity, directly load the latest HistoryEntry per event type
instead of loading all events through the HistoryEntryDao.

Although most entities have a small number of history entries, there are
a few entities with many entries, enough to cause OutOfMemory error.

core/src/main/java/google/registry/batch/ExpandRecurringBillingEventsAction.java

@@ -17,6 +17,7 @@ package google.registry.batch;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.collect.Sets.difference;
+import static com.google.common.collect.Sets.newHashSet;
 import static google.registry.mapreduce.MapreduceRunner.PARAM_DRY_RUN;
 import static google.registry.mapreduce.inputs.EppResourceInputs.createChildEntityInput;
 import static google.registry.model.common.Cursor.CursorType.RECURRING_BILLING;
@@ -36,11 +37,13 @@ import static google.registry.util.DomainNameUtils.getTldFromDomainName;
 import com.google.appengine.tools.mapreduce.Mapper;
 import com.google.appengine.tools.mapreduce.Reducer;
 import com.google.appengine.tools.mapreduce.ReducerInput;
+import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Range;
 import com.google.common.collect.Streams;
 import com.google.common.flogger.FluentLogger;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.mapreduce.MapreduceRunner;
 import google.registry.mapreduce.inputs.NullInput;
 import google.registry.model.ImmutableObject;
@@ -61,6 +64,7 @@ import google.registry.request.Parameter;
 import google.registry.request.Response;
 import google.registry.request.auth.Auth;
 import google.registry.util.Clock;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import javax.inject.Inject;
@@ -86,6 +90,11 @@ public class ExpandRecurringBillingEventsAction implements Runnable {
 
   @Inject Clock clock;
   @Inject MapreduceRunner mrRunner;
+
+  @Inject
+  @Config("jdbcBatchSize")
+  int batchSize;
+
   @Inject @Parameter(PARAM_DRY_RUN) boolean isDryRun;
   @Inject @Parameter(PARAM_CURSOR_TIME) Optional<DateTime> cursorTimeParam;
   @Inject Response response;
@@ -120,61 +129,116 @@ public class ExpandRecurringBillingEventsAction implements Runnable {
                       ImmutableSet.of(DomainBase.class), ImmutableSet.of(Recurring.class))))
           .sendLinkToMapreduceConsole(response);
     } else {
-      int numBillingEventsSaved =
-          jpaTm()
-              .transact(
-                  () ->
-                      jpaTm()
-                          .query(
-                              "FROM BillingRecurrence "
-                                  + "WHERE eventTime <= :executeTime "
-                                  + "AND eventTime < recurrenceEndTime "
-                                  + "ORDER BY id ASC",
-                              Recurring.class)
-                          .setParameter("executeTime", executeTime)
-                          // Need to get a list from the transaction and then convert it to a stream
-                          // for further processing. If we get a stream directly, each elements gets
-                          // processed downstream eagerly but Hibernate returns a
-                          // ScrollableResultsIterator that cannot be advanced outside the
-                          // transaction, resulting in an exception.
-                          .getResultList())
-              .stream()
-              .map(
-                  recurring ->
-                      jpaTm()
-                          .transact(
-                              () ->
-                                  expandBillingEvent(recurring, executeTime, cursorTime, isDryRun)))
-              .reduce(0, Integer::sum);
-
-      if (!isDryRun) {
-        logger.atInfo().log("Saved OneTime billing events.", numBillingEventsSaved);
-      } else {
-        logger.atInfo().log("Generated OneTime billing events (dry run).", numBillingEventsSaved);
-      }
-      logger.atInfo().log(
-          "Recurring event expansion %s complete for billing event range [%s, %s).",
-          isDryRun ? "(dry run) " : "", cursorTime, executeTime);
-      tm().transact(
-              () -> {
-                // Check for the unlikely scenario where the cursor has been altered during the
-                // expansion.
-                DateTime currentCursorTime =
-                    tm().loadByKeyIfPresent(Cursor.createGlobalVKey(RECURRING_BILLING))
-                        .orElse(Cursor.createGlobal(RECURRING_BILLING, START_OF_TIME))
-                        .getCursorTime();
-                if (!currentCursorTime.equals(persistedCursorTime)) {
-                  throw new IllegalStateException(
-                      String.format(
-                          "Current cursor position %s does not match persisted cursor position %s.",
-                          currentCursorTime, persistedCursorTime));
-                }
-                if (!isDryRun) {
-                  tm().put(Cursor.createGlobal(RECURRING_BILLING, executeTime));
-                }
-              });
+      expandSqlBillingEventsInBatches(executeTime, cursorTime, persistedCursorTime);
     }
   }
+
+  private void expandSqlBillingEventsInBatches(
+      DateTime executeTime, DateTime cursorTime, DateTime persistedCursorTime) {
+    int totalBillingEventsSaved = 0;
+    long maxProcessedRecurrenceId = 0;
+    SqlBatchResults sqlBatchResults;
+
+    do {
+      final long prevMaxProcessedRecurrenceId = maxProcessedRecurrenceId;
+      sqlBatchResults =
+          jpaTm()
+              .transact(
+                  () -> {
+                    Set<String> expandedDomains = newHashSet();
+                    int batchBillingEventsSaved = 0;
+                    long maxRecurrenceId = prevMaxProcessedRecurrenceId;
+                    List<Recurring> recurrings =
+                        jpaTm()
+                            .query(
+                                "FROM BillingRecurrence "
+                                    + "WHERE eventTime <= :executeTime "
+                                    + "AND eventTime < recurrenceEndTime "
+                                    + "AND id > :maxProcessedRecurrenceId "
+                                    + "ORDER BY id ASC",
+                                Recurring.class)
+                            .setParameter("executeTime", executeTime)
+                            .setParameter("maxProcessedRecurrenceId", prevMaxProcessedRecurrenceId)
+                            .setMaxResults(batchSize)
+                            .getResultList();
+                    for (Recurring recurring : recurrings) {
+                      if (expandedDomains.contains(recurring.getTargetId())) {
+                        // On the off chance this batch contains multiple recurrences for the same
+                        // domain (which is actually possible if a given domain is quickly renewed
+                        // multiple times in a row), then short-circuit after the first one is
+                        // processed that involves actually expanding a billing event. This is
+                        // necessary because otherwise we get an "Inserted/updated object reloaded"
+                        // error from Hibernate when those billing events would be loaded
+                        // inside a transaction where they were already written. Note, there is no
+                        // actual further work to be done in this case anyway, not unless it has
+                        // somehow been over a year since this action last ran successfully (and if
+                        // that were somehow true, the remaining billing events would still be
+                        // expanded on subsequent runs).
+                        continue;
+                      }
+                      int billingEventsSaved =
+                          expandBillingEvent(recurring, executeTime, cursorTime, isDryRun);
+                      batchBillingEventsSaved += billingEventsSaved;
+                      if (billingEventsSaved > 0) {
+                        expandedDomains.add(recurring.getTargetId());
+                      }
+                      maxRecurrenceId = Math.max(maxRecurrenceId, recurring.getId());
+                    }
+                    return SqlBatchResults.create(
+                        batchBillingEventsSaved,
+                        maxRecurrenceId,
+                        maxRecurrenceId > prevMaxProcessedRecurrenceId);
+                  });
+      totalBillingEventsSaved += sqlBatchResults.batchBillingEventsSaved();
+      maxProcessedRecurrenceId = sqlBatchResults.maxProcessedRecurrenceId();
+      logger.atInfo().log(
+          "Saved %d billing events in batch with max recurrence id %d.",
+          sqlBatchResults.batchBillingEventsSaved(), maxProcessedRecurrenceId);
+    } while (sqlBatchResults.shouldContinue());
+
+    if (!isDryRun) {
+      logger.atInfo().log("Saved OneTime billing events.", totalBillingEventsSaved);
+    } else {
+      logger.atInfo().log("Generated OneTime billing events (dry run).", totalBillingEventsSaved);
+    }
+    logger.atInfo().log(
+        "Recurring event expansion %s complete for billing event range [%s, %s).",
+        isDryRun ? "(dry run) " : "", cursorTime, executeTime);
+    tm().transact(
+            () -> {
+              // Check for the unlikely scenario where the cursor has been altered during the
+              // expansion.
+              DateTime currentCursorTime =
+                  tm().loadByKeyIfPresent(Cursor.createGlobalVKey(RECURRING_BILLING))
+                      .orElse(Cursor.createGlobal(RECURRING_BILLING, START_OF_TIME))
+                      .getCursorTime();
+              if (!currentCursorTime.equals(persistedCursorTime)) {
+                throw new IllegalStateException(
+                    String.format(
+                        "Current cursor position %s does not match persisted cursor position %s.",
+                        currentCursorTime, persistedCursorTime));
+              }
+              if (!isDryRun) {
+                tm().put(Cursor.createGlobal(RECURRING_BILLING, executeTime));
+              }
+            });
+  }
+
+  @AutoValue
+  abstract static class SqlBatchResults {
+    abstract int batchBillingEventsSaved();
+
+    abstract long maxProcessedRecurrenceId();
+
+    abstract boolean shouldContinue();
+
+    static SqlBatchResults create(
+        int batchBillingEventsSaved, long maxProcessedRecurrenceId, boolean shouldContinue) {
+      return new AutoValue_ExpandRecurringBillingEventsAction_SqlBatchResults(
+          batchBillingEventsSaved, maxProcessedRecurrenceId, shouldContinue);
+    }
+  }
+
   /** Mapper to expand {@link Recurring} billing events into synthetic {@link OneTime} events. */
   public static class ExpandRecurringBillingEventsMapper
       extends Mapper<Recurring, DateTime, DateTime> {

core/src/main/java/google/registry/config/RegistryConfig.java

@@ -688,7 +688,7 @@ public final class RegistryConfig {
     @Provides
     @Config("defaultShouldPublishInvoices")
     public static boolean provideDefaultShouldPublishInvoices() {
-      return true;
+      return false;
     }
 
     /**
@@ -1351,6 +1351,12 @@ public final class RegistryConfig {
     public static int provideWipeOutQueryBatchSize(RegistryConfigSettings config) {
       return config.contactHistory.wipeOutQueryBatchSize;
     }
+
+    @Provides
+    @Config("jdbcBatchSize")
+    public static int provideHibernateJdbcBatchSize(RegistryConfigSettings config) {
+      return config.hibernate.jdbcBatchSize;
+    }
   }
 
   /** Returns the App Engine project ID, which is based off the environment name. */
@@ -1555,7 +1561,7 @@ public final class RegistryConfig {
    * https://docs.jboss.org/hibernate/orm/5.6/userguide/html_single/Hibernate_User_Guide.html,
    * recommend between 10 and 50.
    */
-  public static String getHibernateJdbcBatchSize() {
+  public static int getHibernateJdbcBatchSize() {
     return CONFIG_SETTINGS.get().hibernate.jdbcBatchSize;
   }
 

core/src/main/java/google/registry/config/RegistryConfigSettings.java

@@ -120,7 +120,7 @@ public class RegistryConfigSettings {
     public String hikariMinimumIdle;
     public String hikariMaximumPoolSize;
     public String hikariIdleTimeout;
-    public String jdbcBatchSize;
+    public int jdbcBatchSize;
     public String jdbcFetchSize;
   }
 

core/src/main/java/google/registry/env/production/default/WEB-INF/cron.xml

@@ -255,7 +255,7 @@
   </cron>
 
   <cron>
-    <url><![CDATA[/_dr/cron/fanout?queue=retryable-cron-tasks&endpoint=/_dr/task/generateInvoices&runInEmpty]]></url>
+    <url><![CDATA[/_dr/cron/fanout?queue=retryable-cron-tasks&endpoint=/_dr/task/generateInvoices?shouldPublish=true&runInEmpty]]></url>
     <description>
       Starts the beam/invoicing/InvoicingPipeline Dataflow template, which creates the overall invoice and
       detail report CSVs for last month, storing them in gs://[PROJECT-ID]-billing/invoices/yyyy-MM.

core/src/main/java/google/registry/export/sheet/SyncRegistrarsSheet.java

@@ -120,7 +120,6 @@ class SyncRegistrarsSheet {
                   builder.put("registrarName", convert(registrar.getRegistrarName()));
                   builder.put("state", convert(registrar.getState()));
                   builder.put("ianaIdentifier", convert(registrar.getIanaIdentifier()));
-                  builder.put("billingIdentifier", convert(registrar.getBillingIdentifier()));
                   builder.put("billingAccountMap", convert(registrar.getBillingAccountMap()));
                   builder.put("primaryContacts", convertContacts(contacts, byType(ADMIN)));
                   builder.put("techContacts", convertContacts(contacts, byType(TECH)));

core/src/main/java/google/registry/flows/custom/DomainPricingCustomLogic.java

@@ -33,7 +33,7 @@ import org.joda.time.DateTime;
  */
 public class DomainPricingCustomLogic extends BaseFlowCustomLogic {
 
-  protected DomainPricingCustomLogic(
+  public DomainPricingCustomLogic(
       EppInput eppInput, SessionMetadata sessionMetadata, FlowMetadata flowMetadata) {
     super(eppInput, sessionMetadata, flowMetadata);
   }

core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java

@@ -680,7 +680,7 @@ public class DomainFlowUtils {
         break;
       case RENEW:
         builder.setAvailIfSupported(true);
-        fees = pricingLogic.getRenewPrice(registry, domainNameString, now, years).getFees();
+        fees = pricingLogic.getRenewPrice(registry, domainNameString, now, years, null).getFees();
         break;
       case RESTORE:
         // The minimum allowable period per the EPP spec is 1, so, strangely, 1 year still has to be

core/src/main/java/google/registry/flows/domain/DomainPricingLogic.java

@@ -14,8 +14,11 @@
 
 package google.registry.flows.domain;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static google.registry.flows.domain.DomainFlowUtils.zeroInCurrency;
 import static google.registry.pricing.PricingEngineProxy.getPricesForDomainName;
+import static google.registry.util.DomainNameUtils.getTldFromDomainName;
+import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
 
 import com.google.common.net.InternetDomainName;
 import google.registry.flows.EppException;
@@ -27,15 +30,16 @@ import google.registry.flows.custom.DomainPricingCustomLogic.RenewPriceParameter
 import google.registry.flows.custom.DomainPricingCustomLogic.RestorePriceParameters;
 import google.registry.flows.custom.DomainPricingCustomLogic.TransferPriceParameters;
 import google.registry.flows.custom.DomainPricingCustomLogic.UpdatePriceParameters;
+import google.registry.model.billing.BillingEvent.Recurring;
 import google.registry.model.domain.fee.BaseFee;
 import google.registry.model.domain.fee.BaseFee.FeeType;
 import google.registry.model.domain.fee.Fee;
 import google.registry.model.domain.token.AllocationToken;
 import google.registry.model.pricing.PremiumPricingEngine.DomainPrices;
 import google.registry.model.tld.Registry;
-import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.util.Optional;
+import javax.annotation.Nullable;
 import javax.inject.Inject;
 import org.joda.money.CurrencyUnit;
 import org.joda.money.Money;
@@ -102,18 +106,63 @@ public final class DomainPricingLogic {
             .build());
   }
 
-  /** Returns a new renew price for the pricer. */
-  @SuppressWarnings("unused")
-  FeesAndCredits getRenewPrice(Registry registry, String domainName, DateTime dateTime, int years)
+  /** Returns a new renewal cost for the pricer. */
+  FeesAndCredits getRenewPrice(
+      Registry registry,
+      String domainName,
+      DateTime dateTime,
+      int years,
+      @Nullable Recurring recurringBillingEvent)
       throws EppException {
-    DomainPrices domainPrices = getPricesForDomainName(domainName, dateTime);
-    BigDecimal renewCost = domainPrices.getRenewCost().multipliedBy(years).getAmount();
+    checkArgument(years > 0, "Number of years must be positive");
+    Money renewCost;
+    boolean isRenewCostPremiumPrice;
+    // recurring billing event is null if the domain is still available. Billing events are created
+    // in the process of domain creation.
+    if (recurringBillingEvent == null) {
+      DomainPrices domainPrices = getPricesForDomainName(domainName, dateTime);
+      renewCost = domainPrices.getRenewCost().multipliedBy(years);
+      isRenewCostPremiumPrice = domainPrices.isPremium();
+    } else {
+      switch (recurringBillingEvent.getRenewalPriceBehavior()) {
+        case DEFAULT:
+          DomainPrices domainPrices = getPricesForDomainName(domainName, dateTime);
+          renewCost = domainPrices.getRenewCost().multipliedBy(years);
+          isRenewCostPremiumPrice = domainPrices.isPremium();
+          break;
+          // if the renewal price behavior is specified, then the renewal price should be the same
+          // as the creation price, which is stored in the billing event as the renewal price
+        case SPECIFIED:
+          checkArgumentPresent(
+              recurringBillingEvent.getRenewalPrice(),
+              "Unexpected behavior: renewal price cannot be null when renewal behavior is"
+                  + " SPECIFIED");
+          renewCost = recurringBillingEvent.getRenewalPrice().get().multipliedBy(years);
+          isRenewCostPremiumPrice = false;
+          break;
+          // if the renewal price behavior is nonpremium, it means that the domain should be renewed
+          // at standard price of domains at the time, even if the domain is premium
+        case NONPREMIUM:
+          renewCost =
+              Registry.get(getTldFromDomainName(domainName))
+                  .getStandardRenewCost(dateTime)
+                  .multipliedBy(years);
+          isRenewCostPremiumPrice = false;
+          break;
+        default:
+          throw new IllegalArgumentException(
+              String.format(
+                  "Unknown RenewalPriceBehavior enum value: %s",
+                  recurringBillingEvent.getRenewalPriceBehavior()));
+      }
+    }
     return customLogic.customizeRenewPrice(
         RenewPriceParameters.newBuilder()
             .setFeesAndCredits(
                 new FeesAndCredits.Builder()
-                    .setCurrency(registry.getCurrency())
-                    .addFeeOrCredit(Fee.create(renewCost, FeeType.RENEW, domainPrices.isPremium()))
+                    .setCurrency(renewCost.getCurrencyUnit())
+                    .addFeeOrCredit(
+                        Fee.create(renewCost.getAmount(), FeeType.RENEW, isRenewCostPremiumPrice))
                     .build())
             .setRegistry(registry)
             .setDomainName(InternetDomainName.from(domainName))

core/src/main/java/google/registry/flows/domain/DomainRenewFlow.java

@@ -155,7 +155,8 @@ public final class DomainRenewFlow implements TransactionalFlow {
     Optional<FeeRenewCommandExtension> feeRenew =
         eppInput.getSingleExtension(FeeRenewCommandExtension.class);
     FeesAndCredits feesAndCredits =
-        pricingLogic.getRenewPrice(Registry.get(existingDomain.getTld()), targetId, now, years);
+        pricingLogic.getRenewPrice(
+            Registry.get(existingDomain.getTld()), targetId, now, years, null);
     validateFeeChallenge(targetId, now, feeRenew, feesAndCredits);
     flowCustomLogic.afterValidation(
         AfterValidationParameters.newBuilder()

core/src/main/java/google/registry/model/AppEngineEnvironment.java

@@ -116,4 +116,9 @@ public class AppEngineEnvironment {
               }
             });
   }
+
+  /** Returns true if the current thread is in an App Engine Environment. */
+  public static boolean isInAppEngineEnvironment() {
+    return ApiProxy.getCurrentEnvironment() != null;
+  }
 }

core/src/main/java/google/registry/model/CacheUtils.java

@@ -18,9 +18,13 @@ import static com.google.common.base.Suppliers.memoizeWithExpiration;
 import static google.registry.config.RegistryConfig.getSingletonCacheRefreshDuration;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
+import com.github.benmanes.caffeine.cache.CacheLoader;
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.google.common.base.Supplier;
+import google.registry.model.annotations.DeleteAfterMigration;
 import java.time.Duration;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /** Utility methods related to caching Datastore entities. */
 public class CacheUtils {
@@ -28,8 +32,8 @@ public class CacheUtils {
   /**
    * Memoize a supplier, with a short expiration specified in the environment config.
    *
-   * <p>Use this for things that might change while code is running. (For example, the various
-   * lists downloaded from the TMCH get updated in Datastore and the caches need to be refreshed.)
+   * <p>Use this for things that might change while code is running. (For example, the various lists
+   * downloaded from the TMCH get updated in Datastore and the caches need to be refreshed.)
    */
   public static <T> Supplier<T> memoizeWithShortExpiration(Supplier<T> original) {
     return tryMemoizeWithExpiration(getSingletonCacheRefreshDuration(), original);
@@ -73,4 +77,29 @@ public class CacheUtils {
     }
     return caffeine;
   }
+
+  /**
+   * A {@link CacheLoader} that automatically masquerade the background thread where the refresh
+   * action runs in to be an GAE thread.
+   */
+  @DeleteAfterMigration
+  public abstract static class AppEngineEnvironmentCacheLoader<K, V> implements CacheLoader<K, V> {
+
+    private static final AppEngineEnvironment environment = new AppEngineEnvironment();
+
+    @Override
+    public @Nullable V reload(@NonNull K key, @NonNull V oldValue) throws Exception {
+      V value;
+      boolean isMasqueraded = false;
+      if (!AppEngineEnvironment.isInAppEngineEnvironment()) {
+        environment.setEnvironmentForCurrentThread();
+        isMasqueraded = true;
+      }
+      value = load(key);
+      if (isMasqueraded) {
+        environment.unsetEnvironmentForCurrentThread();
+      }
+      return value;
+    }
+  }
 }

core/src/main/java/google/registry/model/EppResource.java

@@ -37,6 +37,7 @@ import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Id;
 import com.googlecode.objectify.annotation.Index;
 import google.registry.config.RegistryConfig;
+import google.registry.model.CacheUtils.AppEngineEnvironmentCacheLoader;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.index.EppResourceIndex;
 import google.registry.model.index.ForeignKeyIndex;
@@ -385,7 +386,7 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
   }
 
   static final CacheLoader<VKey<? extends EppResource>, EppResource> CACHE_LOADER =
-      new CacheLoader<VKey<? extends EppResource>, EppResource>() {
+      new AppEngineEnvironmentCacheLoader<VKey<? extends EppResource>, EppResource>() {
 
         @Override
         public EppResource load(VKey<? extends EppResource> key) {

core/src/main/java/google/registry/model/index/ForeignKeyIndex.java

@@ -43,6 +43,7 @@ import com.googlecode.objectify.annotation.Index;
 import google.registry.config.RegistryConfig;
 import google.registry.model.BackupGroupRoot;
 import google.registry.model.CacheUtils;
+import google.registry.model.CacheUtils.AppEngineEnvironmentCacheLoader;
 import google.registry.model.EppResource;
 import google.registry.model.annotations.DeleteAfterMigration;
 import google.registry.model.annotations.ReportedOn;
@@ -256,7 +257,8 @@ public abstract class ForeignKeyIndex<E extends EppResource> extends BackupGroup
   }
 
   static final CacheLoader<VKey<ForeignKeyIndex<?>>, Optional<ForeignKeyIndex<?>>> CACHE_LOADER =
-      new CacheLoader<VKey<ForeignKeyIndex<?>>, Optional<ForeignKeyIndex<?>>>() {
+      new AppEngineEnvironmentCacheLoader<
+          VKey<ForeignKeyIndex<?>>, Optional<ForeignKeyIndex<?>>>() {
 
         @Override
         public Optional<ForeignKeyIndex<?>> load(VKey<ForeignKeyIndex<?>> key) {

core/src/main/java/google/registry/model/registrar/Registrar.java

@@ -62,6 +62,7 @@ import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Embed;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
+import com.googlecode.objectify.annotation.Ignore;
 import com.googlecode.objectify.annotation.IgnoreSave;
 import com.googlecode.objectify.annotation.Index;
 import com.googlecode.objectify.annotation.Mapify;
@@ -75,6 +76,7 @@ import google.registry.model.JsonMapBuilder;
 import google.registry.model.Jsonifiable;
 import google.registry.model.UnsafeSerializable;
 import google.registry.model.UpdateAutoTimestamp;
+import google.registry.model.annotations.DeleteAfterMigration;
 import google.registry.model.annotations.InCrossTld;
 import google.registry.model.annotations.ReportedOn;
 import google.registry.model.common.EntityGroupRoot;
@@ -123,24 +125,24 @@ public class Registrar extends ImmutableObject
 
   /** Represents the type of a registrar entity. */
   public enum Type {
-    /** A real-world, third-party registrar. Should have non-null IANA and billing IDs. */
+    /** A real-world, third-party registrar. Should have non-null IANA and billing account IDs. */
     REAL(Objects::nonNull),
 
     /**
      * A registrar account used by a real third-party registrar undergoing operational testing and
-     * evaluation. Should only be created in sandbox, and should have null IANA/billing IDs.
+     * evaluation. Should only be created in sandbox, and should have null IANA/billing account IDs.
      */
     OTE(Objects::isNull),
 
     /**
-     * A registrar used for predelegation testing. Should have a null billing ID. The IANA ID should
-     * be either 9995 or 9996, which are reserved for predelegation testing.
+     * A registrar used for predelegation testing. Should have a null billing account ID. The IANA
+     * ID should be either 9995 or 9996, which are reserved for predelegation testing.
      */
     PDT(n -> ImmutableSet.of(9995L, 9996L).contains(n)),
 
     /**
      * A registrar used for external monitoring by ICANN. Should have IANA ID 9997 and a null
-     * billing ID.
+     * billing account ID.
      */
     EXTERNAL_MONITORING(isEqual(9997L)),
 
@@ -148,13 +150,13 @@ public class Registrar extends ImmutableObject
      * A registrar used for when the registry acts as a registrar. Must have either IANA ID 9998
      * (for billable transactions) or 9999 (for non-billable transactions).
      */
-    // TODO(b/13786188): determine what billing ID for this should be, if any.
+    // TODO(b/13786188): determine what billing account ID for this should be, if any.
     INTERNAL(n -> ImmutableSet.of(9998L, 9999L).contains(n)),
 
-    /** A registrar used for internal monitoring. Should have null IANA/billing IDs. */
+    /** A registrar used for internal monitoring. Should have null IANA/billing account IDs. */
     MONITORING(Objects::isNull),
 
-    /** A registrar used for internal testing. Should have null IANA/billing IDs. */
+    /** A registrar used for internal testing. Should have null IANA/billing account IDs. */
     TEST(Objects::isNull);
 
     /**
@@ -388,7 +390,8 @@ public class Registrar extends ImmutableObject
   @Index @Nullable Long ianaIdentifier;
 
   /** Identifier of registrar used in external billing system (e.g. Oracle). */
-  @Nullable Long billingIdentifier;
+  // TODO(sarahbot@): Drop this column from the table in a flyway script in a follow up PR.
+  @DeleteAfterMigration @Nullable @Deprecated @Ignore Long billingIdentifier;
 
   /** Purchase Order number used for invoices in external billing system, if applicable. */
   @Nullable String poNumber;
@@ -496,11 +499,6 @@ public class Registrar extends ImmutableObject
     return ianaIdentifier;
   }
 
-  @Nullable
-  public Long getBillingIdentifier() {
-    return billingIdentifier;
-  }
-
   public Optional<String> getPoNumber() {
     return Optional.ofNullable(poNumber);
   }
@@ -688,7 +686,6 @@ public class Registrar extends ImmutableObject
     return new JsonMapBuilder()
         .put("clientIdentifier", clientIdentifier)
         .put("ianaIdentifier", ianaIdentifier)
-        .put("billingIdentifier", billingIdentifier)
         .putString("creationTime", creationTime.getTimestamp())
         .putString("lastUpdateTime", lastUpdateTime.getTimestamp())
         .putString("lastCertificateUpdateTime", lastCertificateUpdateTime)
@@ -785,14 +782,6 @@ public class Registrar extends ImmutableObject
       return this;
     }
 
-    public Builder setBillingIdentifier(@Nullable Long billingIdentifier) {
-      checkArgument(
-          billingIdentifier == null || billingIdentifier > 0,
-          "Billing ID must be a positive number");
-      getInstance().billingIdentifier = billingIdentifier;
-      return this;
-    }
-
     public Builder setPoNumber(Optional<String> poNumber) {
       getInstance().poNumber = poNumber.orElse(null);
       return this;

core/src/main/java/google/registry/model/reporting/HistoryEntryDao.java

@@ -211,7 +211,7 @@ public class HistoryEntryDao {
         jpaTm().criteriaQuery(criteriaQuery).getResultList());
   }
 
-  private static Class<? extends HistoryEntry> getHistoryClassFromParent(
+  public static Class<? extends HistoryEntry> getHistoryClassFromParent(
       Class<? extends EppResource> parent) {
     if (!RESOURCE_TYPES_TO_HISTORY_TYPES.containsKey(parent)) {
       throw new IllegalArgumentException(
@@ -220,7 +220,7 @@ public class HistoryEntryDao {
     return RESOURCE_TYPES_TO_HISTORY_TYPES.get(parent);
   }
 
-  private static String getRepoIdFieldNameFromHistoryClass(
+  public static String getRepoIdFieldNameFromHistoryClass(
       Class<? extends HistoryEntry> historyClass) {
     if (!REPO_ID_FIELD_NAMES.containsKey(historyClass)) {
       throw new IllegalArgumentException(

core/src/main/java/google/registry/model/tld/Registry.java

@@ -29,7 +29,6 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 import static org.joda.money.CurrencyUnit.USD;
 
-import com.github.benmanes.caffeine.cache.CacheLoader;
 import com.github.benmanes.caffeine.cache.LoadingCache;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
@@ -50,6 +49,7 @@ import com.googlecode.objectify.annotation.OnSave;
 import com.googlecode.objectify.annotation.Parent;
 import google.registry.model.Buildable;
 import google.registry.model.CacheUtils;
+import google.registry.model.CacheUtils.AppEngineEnvironmentCacheLoader;
 import google.registry.model.CreateAutoTimestamp;
 import google.registry.model.ImmutableObject;
 import google.registry.model.UnsafeSerializable;
@@ -268,7 +268,7 @@ public class Registry extends ImmutableObject
   private static final LoadingCache<String, Optional<Registry>> CACHE =
       CacheUtils.newCacheBuilder(getSingletonCacheRefreshDuration())
           .build(
-              new CacheLoader<String, Optional<Registry>>() {
+              new AppEngineEnvironmentCacheLoader<String, Optional<Registry>>() {
                 @Override
                 public Optional<Registry> load(final String tld) {
                   // Enter a transaction-less context briefly; we don't want to enroll every TLD in

core/src/main/java/google/registry/persistence/PersistenceModule.java

@@ -107,7 +107,7 @@ public abstract class PersistenceModule {
     properties.put(HIKARI_MAXIMUM_POOL_SIZE, getHibernateHikariMaximumPoolSize());
     properties.put(HIKARI_IDLE_TIMEOUT, getHibernateHikariIdleTimeout());
     properties.put(Environment.DIALECT, NomulusPostgreSQLDialect.class.getName());
-    properties.put(JDBC_BATCH_SIZE, getHibernateJdbcBatchSize());
+    properties.put(JDBC_BATCH_SIZE, Integer.toString(getHibernateJdbcBatchSize()));
     properties.put(JDBC_FETCH_SIZE, getHibernateJdbcFetchSize());
     return properties.build();
   }

core/src/main/java/google/registry/rdap/RdapJsonFormatter.java

@@ -20,11 +20,14 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
 import static google.registry.model.EppResourceUtils.isLinked;
+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.RdapIcannStandardInformation.CONTACT_REDACTED_VALUE;
 import static google.registry.util.CollectionUtils.union;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -81,6 +84,7 @@ import java.util.Set;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
+import javax.persistence.Entity;
 import org.joda.time.DateTime;
 
 /**
@@ -150,6 +154,19 @@ public class RdapJsonFormatter {
     INTERNAL
   }
 
+  /**
+   * JPQL query template for finding the latest history entry per event type for an EPP entity.
+   *
+   * <p>User should replace '%entityName%', '%repoIdField%', and '%repoIdValue%' with valid values.
+   * A DomainHistory query may look like below: {@code select e from DomainHistory e where
+   * domainRepoId = '17-Q9JYB4C' and modificationTime in (select max(modificationTime) from
+   * DomainHistory where domainRepoId = '17-Q9JYB4C' and type is not null group by type)}
+   */
+  private static final String GET_LAST_HISTORY_BY_TYPE_JPQL_TEMPLATE =
+      "select e from %entityName% e where %repoIdField% = '%repoIdValue%' and modificationTime in "
+          + " (select max(modificationTime) from %entityName% where "
+          + " %repoIdField% = '%repoIdValue%' and type is not null group by type)";
+
   /** Map of EPP status values to the RDAP equivalents. */
   private static final ImmutableMap<StatusValue, RdapStatus> STATUS_TO_RDAP_STATUS_MAP =
       new ImmutableMap.Builder<StatusValue, RdapStatus>()
@@ -855,17 +872,8 @@ public class RdapJsonFormatter {
     return rolesBuilder.build();
   }
 
-  /**
-   * Creates the list of optional events to list in domain, nameserver, or contact replies.
-   *
-   * <p>Only has entries for optional events that won't be shown in "SUMMARY" versions of these
-   * objects. These are either stated as optional in the RDAP Response Profile 15feb19, or not
-   * mentioned at all but thought to be useful anyway.
-   *
-   * <p>Any required event should be added elsewhere, preferably without using HistoryEntries (so
-   * that we don't need to load HistoryEntries for "summary" responses).
-   */
-  private ImmutableList<Event> makeOptionalEvents(EppResource resource) {
+  @VisibleForTesting
+  ImmutableMap<EventAction, HistoryEntry> getLastHistoryEntryByType(EppResource resource) {
     HashMap<EventAction, HistoryEntry> lastEntryOfType = Maps.newHashMap();
     // Events (such as transfer, but also create) can appear multiple times. We only want the last
     // time they appeared.
@@ -878,8 +886,26 @@ public class RdapJsonFormatter {
     // 2.3.2.3 An event of *eventAction* type *transfer*, with the last date and time that the
     // domain was transferred. The event of *eventAction* type *transfer* MUST be omitted if the
     // domain name has not been transferred since it was created.
-    Iterable<? extends HistoryEntry> historyEntries =
-        HistoryEntryDao.loadHistoryObjectsForResource(resource.createVKey());
+    Iterable<? extends HistoryEntry> historyEntries;
+    if (tm().isOfy()) {
+      historyEntries = HistoryEntryDao.loadHistoryObjectsForResource(resource.createVKey());
+    } else {
+      VKey<? extends EppResource> resourceVkey = resource.createVKey();
+      Class<? extends HistoryEntry> historyClass =
+          HistoryEntryDao.getHistoryClassFromParent(resourceVkey.getKind());
+      String entityName = historyClass.getAnnotation(Entity.class).name();
+      if (Strings.isNullOrEmpty(entityName)) {
+        entityName = historyClass.getSimpleName();
+      }
+      String repoIdFieldName = HistoryEntryDao.getRepoIdFieldNameFromHistoryClass(historyClass);
+      String jpql =
+          GET_LAST_HISTORY_BY_TYPE_JPQL_TEMPLATE
+              .replace("%entityName%", entityName)
+              .replace("%repoIdField%", repoIdFieldName)
+              .replace("%repoIdValue%", resourceVkey.getSqlKey().toString());
+      historyEntries =
+          jpaTm().transact(() -> jpaTm().getEntityManager().createQuery(jpql).getResultList());
+    }
     for (HistoryEntry historyEntry : historyEntries) {
       EventAction rdapEventAction =
           HISTORY_ENTRY_TYPE_TO_RDAP_EVENT_ACTION_MAP.get(historyEntry.getType());
@@ -889,6 +915,21 @@ public class RdapJsonFormatter {
       }
       lastEntryOfType.put(rdapEventAction, historyEntry);
     }
+    return ImmutableMap.copyOf(lastEntryOfType);
+  }
+
+  /**
+   * Creates the list of optional events to list in domain, nameserver, or contact replies.
+   *
+   * <p>Only has entries for optional events that won't be shown in "SUMMARY" versions of these
+   * objects. These are either stated as optional in the RDAP Response Profile 15feb19, or not
+   * mentioned at all but thought to be useful anyway.
+   *
+   * <p>Any required event should be added elsewhere, preferably without using HistoryEntries (so
+   * that we don't need to load HistoryEntries for "summary" responses).
+   */
+  private ImmutableList<Event> makeOptionalEvents(EppResource resource) {
+    ImmutableMap<EventAction, HistoryEntry> lastEntryOfType = getLastHistoryEntryByType(resource);
     ImmutableList.Builder<Event> eventsBuilder = new ImmutableList.Builder<>();
     DateTime creationTime = resource.getCreationTime();
     DateTime lastChangeTime =

core/src/main/java/google/registry/tmch/TmchCertificateAuthority.java

@@ -19,12 +19,12 @@ import static google.registry.config.RegistryConfig.ConfigModule.TmchCaMode.PROD
 import static google.registry.config.RegistryConfig.getSingletonCacheRefreshDuration;
 import static google.registry.util.ResourceUtils.readResourceUtf8;
 
-import com.github.benmanes.caffeine.cache.CacheLoader;
 import com.github.benmanes.caffeine.cache.LoadingCache;
 import com.google.common.collect.ImmutableMap;
 import google.registry.config.RegistryConfig.Config;
 import google.registry.config.RegistryConfig.ConfigModule.TmchCaMode;
 import google.registry.model.CacheUtils;
+import google.registry.model.CacheUtils.AppEngineEnvironmentCacheLoader;
 import google.registry.model.tmch.TmchCrl;
 import google.registry.util.Clock;
 import google.registry.util.X509Utils;
@@ -78,7 +78,7 @@ public final class TmchCertificateAuthority {
   private static final LoadingCache<TmchCaMode, X509CRL> CRL_CACHE =
       CacheUtils.newCacheBuilder(getSingletonCacheRefreshDuration())
           .build(
-              new CacheLoader<TmchCaMode, X509CRL>() {
+              new AppEngineEnvironmentCacheLoader<TmchCaMode, X509CRL>() {
                 @Override
                 public X509CRL load(final TmchCaMode tmchCaMode) throws GeneralSecurityException {
                   Optional<TmchCrl> storedCrl = TmchCrl.get();

core/src/main/java/google/registry/tools/CreateOrUpdateRegistrarCommand.java

@@ -150,14 +150,6 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
       validateWith = OptionalLongParameter.class)
   Optional<Long> ianaId;
 
-  @Nullable
-  @Parameter(
-      names = "--billing_id",
-      description = "Registrar Billing ID (i.e. Oracle #)",
-      converter = OptionalLongParameter.class,
-      validateWith = OptionalLongParameter.class)
-  private Optional<Long> billingId;
-
   @Nullable
   @Parameter(
       names = "--po_number",
@@ -173,7 +165,8 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
           "Registrar Billing Account key-value pairs (formatted as key=value[,key=value...]), "
               + "where key is a currency unit (USD, JPY, etc) and value is the registrar's billing "
               + "account id for that currency. During update, only the pairs that need updating "
-              + "need to be provided.",
+              + "need to be provided, except when an empty string is provided, in which case the"
+              + "entire map is nullified.",
       converter = CurrencyUnitToStringMap.class,
       validateWith = CurrencyUnitToStringMap.class)
   private Map<CurrencyUnit, String> billingAccountMap;
@@ -362,13 +355,12 @@ abstract class CreateOrUpdateRegistrarCommand extends MutatingCommand {
       if (ianaId != null) {
         builder.setIanaIdentifier(ianaId.orElse(null));
       }
-      if (billingId != null) {
-        builder.setBillingIdentifier(billingId.orElse(null));
-      }
       Optional.ofNullable(poNumber).ifPresent(builder::setPoNumber);
       if (billingAccountMap != null) {
         LinkedHashMap<CurrencyUnit, String> newBillingAccountMap = new LinkedHashMap<>();
-        if (oldRegistrar != null && oldRegistrar.getBillingAccountMap() != null) {
+        if (oldRegistrar != null
+            && oldRegistrar.getBillingAccountMap() != null
+            && !billingAccountMap.isEmpty()) {
           newBillingAccountMap.putAll(oldRegistrar.getBillingAccountMap());
         }
         newBillingAccountMap.putAll(billingAccountMap);

core/src/test/java/google/registry/batch/ExpandRecurringBillingEventsActionTest.java

@@ -90,6 +90,7 @@ public class ExpandRecurringBillingEventsActionTest
     action.mrRunner = makeDefaultRunner();
     action.clock = clock;
     action.cursorTimeParam = Optional.empty();
+    action.batchSize = 2;
     createTld("tld");
     domain =
         persistResource(
@@ -279,11 +280,12 @@ public class ExpandRecurringBillingEventsActionTest
     assertHistoryEntryMatches(
         domain, persistedEntry, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected = defaultOneTimeBuilder().setParent(persistedEntry).build();
-    // Persist an otherwise identical billing event that differs only in billing time.
+    // Persist an otherwise identical billing event that differs only in billing time (and ID).
     BillingEvent.OneTime persisted =
         persistResource(
             expected
                 .asBuilder()
+                .setId(15891L)
                 .setBillingTime(DateTime.parse("1999-02-19T00:00:00Z"))
                 .setEventTime(DateTime.parse("1999-01-05T00:00:00Z"))
                 .build());
@@ -639,43 +641,95 @@ public class ExpandRecurringBillingEventsActionTest
   @TestOfyAndSql
   void testSuccess_expandMultipleEvents() throws Exception {
     persistResource(recurring);
+    DomainBase domain2 =
+        persistResource(
+            newDomainBase("example2.tld")
+                .asBuilder()
+                .setCreationTimeForTest(DateTime.parse("1999-04-05T00:00:00Z"))
+                .build());
+    DomainHistory historyEntry2 =
+        persistResource(
+            new DomainHistory.Builder()
+                .setRegistrarId(domain2.getCreationRegistrarId())
+                .setType(HistoryEntry.Type.DOMAIN_CREATE)
+                .setModificationTime(DateTime.parse("1999-04-05T00:00:00Z"))
+                .setDomain(domain2)
+                .build());
     BillingEvent.Recurring recurring2 =
         persistResource(
-            recurring
+            new BillingEvent.Recurring.Builder()
+                .setParent(historyEntry2)
+                .setRegistrarId(domain2.getCreationRegistrarId())
+                .setEventTime(DateTime.parse("2000-04-05T00:00:00Z"))
+                .setFlags(ImmutableSet.of(Flag.AUTO_RENEW))
+                .setReason(Reason.RENEW)
+                .setRecurrenceEndTime(END_OF_TIME)
+                .setTargetId(domain2.getDomainName())
+                .build());
+    DomainBase domain3 =
+        persistResource(
+            newDomainBase("example3.tld")
                 .asBuilder()
-                .setEventTime(recurring.getEventTime().plusMonths(3))
-                .setId(3L)
+                .setCreationTimeForTest(DateTime.parse("1999-06-05T00:00:00Z"))
+                .build());
+    DomainHistory historyEntry3 =
+        persistResource(
+            new DomainHistory.Builder()
+                .setRegistrarId(domain3.getCreationRegistrarId())
+                .setType(HistoryEntry.Type.DOMAIN_CREATE)
+                .setModificationTime(DateTime.parse("1999-06-05T00:00:00Z"))
+                .setDomain(domain3)
+                .build());
+    BillingEvent.Recurring recurring3 =
+        persistResource(
+            new BillingEvent.Recurring.Builder()
+                .setParent(historyEntry3)
+                .setRegistrarId(domain3.getCreationRegistrarId())
+                .setEventTime(DateTime.parse("2000-06-05T00:00:00Z"))
+                .setFlags(ImmutableSet.of(Flag.AUTO_RENEW))
+                .setReason(Reason.RENEW)
+                .setRecurrenceEndTime(END_OF_TIME)
+                .setTargetId(domain3.getDomainName())
                 .build());
     action.cursorTimeParam = Optional.of(START_OF_TIME);
     runAction();
-    List<DomainHistory> persistedEntries =
-        getHistoryEntriesOfType(domain, DOMAIN_AUTORENEW, DomainHistory.class);
-    assertThat(persistedEntries).hasSize(2);
+
+    DomainHistory persistedHistory1 =
+        getOnlyHistoryEntryOfType(domain, DOMAIN_AUTORENEW, DomainHistory.class);
     assertHistoryEntryMatches(
-        domain,
-        persistedEntries.get(0),
-        "TheRegistrar",
-        DateTime.parse("2000-02-19T00:00:00Z"),
-        true);
+        domain, persistedHistory1, "TheRegistrar", DateTime.parse("2000-02-19T00:00:00Z"), true);
     BillingEvent.OneTime expected =
         defaultOneTimeBuilder()
-            .setParent(persistedEntries.get(0))
+            .setParent(persistedHistory1)
             .setCancellationMatchingBillingEvent(recurring.createVKey())
             .build();
+    DomainHistory persistedHistory2 =
+        getOnlyHistoryEntryOfType(domain2, DOMAIN_AUTORENEW, DomainHistory.class);
     assertHistoryEntryMatches(
-        domain,
-        persistedEntries.get(1),
-        "TheRegistrar",
-        DateTime.parse("2000-05-20T00:00:00Z"),
-        true);
+        domain2, persistedHistory2, "TheRegistrar", DateTime.parse("2000-05-20T00:00:00Z"), true);
     BillingEvent.OneTime expected2 =
         defaultOneTimeBuilder()
             .setBillingTime(DateTime.parse("2000-05-20T00:00:00Z"))
             .setEventTime(DateTime.parse("2000-04-05T00:00:00Z"))
-            .setParent(persistedEntries.get(1))
+            .setParent(persistedHistory2)
+            .setTargetId(domain2.getDomainName())
             .setCancellationMatchingBillingEvent(recurring2.createVKey())
             .build();
-    assertBillingEventsForResource(domain, expected, expected2, recurring, recurring2);
+    DomainHistory persistedHistory3 =
+        getOnlyHistoryEntryOfType(domain3, DOMAIN_AUTORENEW, DomainHistory.class);
+    assertHistoryEntryMatches(
+        domain3, persistedHistory3, "TheRegistrar", DateTime.parse("2000-07-20T00:00:00Z"), true);
+    BillingEvent.OneTime expected3 =
+        defaultOneTimeBuilder()
+            .setBillingTime(DateTime.parse("2000-07-20T00:00:00Z"))
+            .setEventTime(DateTime.parse("2000-06-05T00:00:00Z"))
+            .setTargetId(domain3.getDomainName())
+            .setParent(persistedHistory3)
+            .setCancellationMatchingBillingEvent(recurring3.createVKey())
+            .build();
+    assertBillingEventsForResource(domain, expected, recurring);
+    assertBillingEventsForResource(domain2, expected2, recurring2);
+    assertBillingEventsForResource(domain3, expected3, recurring3);
     assertCursorAt(currentTestTime);
   }
 

core/src/test/java/google/registry/export/sheet/SyncRegistrarsSheetTest.java

@@ -204,7 +204,6 @@ public class SyncRegistrarsSheetTest {
     assertThat(row).containsEntry("registrarName", "AAA Registrar Inc.");
     assertThat(row).containsEntry("state", "SUSPENDED");
     assertThat(row).containsEntry("ianaIdentifier", "8");
-    assertThat(row).containsEntry("billingIdentifier", "");
     assertThat(row)
         .containsEntry(
             "primaryContacts",
@@ -300,7 +299,6 @@ public class SyncRegistrarsSheetTest {
     assertThat(row).containsEntry("registrarName", "Another Registrar LLC");
     assertThat(row).containsEntry("state", "ACTIVE");
     assertThat(row).containsEntry("ianaIdentifier", "1");
-    assertThat(row).containsEntry("billingIdentifier", "");
     assertThat(row).containsEntry("primaryContacts", "");
     assertThat(row).containsEntry("techContacts", "");
     assertThat(row).containsEntry("marketingContacts", "");
@@ -350,7 +348,6 @@ public class SyncRegistrarsSheetTest {
     assertThat(row).containsEntry("registrarName", "Some Registrar");
     assertThat(row).containsEntry("state", "ACTIVE");
     assertThat(row).containsEntry("ianaIdentifier", "8");
-    assertThat(row).containsEntry("billingIdentifier", "");
     assertThat(row).containsEntry("primaryContacts", "");
     assertThat(row).containsEntry("techContacts", "");
     assertThat(row).containsEntry("marketingContacts", "");

core/src/test/java/google/registry/flows/domain/DomainFlowUtilsTest.java

@@ -45,6 +45,7 @@ import google.registry.testing.TestOfyAndSql;
 import org.joda.money.Money;
 import org.junit.jupiter.api.BeforeEach;
 
+/** Unit tests for {@link DomainFlowUtils}. */
 @DualDatabaseTest
 class DomainFlowUtilsTest extends ResourceFlowTestCase<DomainInfoFlow, DomainBase> {
 

core/src/test/java/google/registry/flows/domain/DomainPricingLogicTest.java

@@ -0,0 +1,405 @@
+// Copyright 2022 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.flows.domain;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+import static google.registry.model.billing.BillingEvent.Flag.AUTO_RENEW;
+import static google.registry.model.billing.BillingEvent.RenewalPriceBehavior.DEFAULT;
+import static google.registry.model.billing.BillingEvent.RenewalPriceBehavior.NONPREMIUM;
+import static google.registry.model.billing.BillingEvent.RenewalPriceBehavior.SPECIFIED;
+import static google.registry.model.domain.fee.BaseFee.FeeType.RENEW;
+import static google.registry.model.reporting.HistoryEntry.Type.DOMAIN_CREATE;
+import static google.registry.testing.DatabaseHelper.createTld;
+import static google.registry.testing.DatabaseHelper.newDomainBase;
+import static google.registry.testing.DatabaseHelper.persistPremiumList;
+import static google.registry.testing.DatabaseHelper.persistResource;
+import static google.registry.util.DateTimeUtils.END_OF_TIME;
+import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.joda.money.CurrencyUnit.USD;
+import static org.joda.time.DateTimeZone.UTC;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedMap;
+import google.registry.flows.EppException;
+import google.registry.flows.FlowMetadata;
+import google.registry.flows.HttpSessionMetadata;
+import google.registry.flows.SessionMetadata;
+import google.registry.flows.custom.DomainPricingCustomLogic;
+import google.registry.model.billing.BillingEvent;
+import google.registry.model.billing.BillingEvent.Reason;
+import google.registry.model.billing.BillingEvent.Recurring;
+import google.registry.model.billing.BillingEvent.RenewalPriceBehavior;
+import google.registry.model.domain.DomainBase;
+import google.registry.model.domain.DomainHistory;
+import google.registry.model.domain.fee.Fee;
+import google.registry.model.eppinput.EppInput;
+import google.registry.model.tld.Registry;
+import google.registry.testing.AppEngineExtension;
+import google.registry.testing.DualDatabaseTest;
+import google.registry.testing.FakeClock;
+import google.registry.testing.FakeHttpSession;
+import google.registry.testing.TestOfyAndSql;
+import google.registry.util.Clock;
+import java.util.Optional;
+import javax.inject.Inject;
+import org.joda.money.Money;
+import org.joda.time.DateTime;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.mockito.Mock;
+
+/** Unit tests for {@link DomainPricingLogic}. */
+@DualDatabaseTest
+public class DomainPricingLogicTest {
+  DomainPricingLogic domainPricingLogic = new DomainPricingLogic();
+
+  @RegisterExtension
+  public final AppEngineExtension appEngine =
+      AppEngineExtension.builder().withDatastoreAndCloudSql().build();
+
+  @Inject Clock clock = new FakeClock(DateTime.now(UTC));
+  @Mock EppInput eppInput;
+  SessionMetadata sessionMetadata;
+  @Mock FlowMetadata flowMetadata;
+  Registry registry;
+  DomainBase domain;
+
+  @BeforeEach
+  void beforeEach() throws Exception {
+    createTld("example");
+    sessionMetadata = new HttpSessionMetadata(new FakeHttpSession());
+    domainPricingLogic.customLogic =
+        new DomainPricingCustomLogic(eppInput, sessionMetadata, flowMetadata);
+    registry =
+        persistResource(
+            Registry.get("example")
+                .asBuilder()
+                .setRenewBillingCostTransitions(
+                    ImmutableSortedMap.of(
+                        START_OF_TIME, Money.of(USD, 1), clock.nowUtc(), Money.of(USD, 10)))
+                .setPremiumList(persistPremiumList("tld2", USD, "premium,USD 100"))
+                .build());
+  }
+
+  /** helps to set up the domain info and returns a recurring billing event for testing */
+  private Recurring persistDomainAndSetRecurringBillingEvent(
+      String domainName, RenewalPriceBehavior renewalPriceBehavior, Optional<Money> renewalPrice) {
+    domain =
+        persistResource(
+            newDomainBase(domainName)
+                .asBuilder()
+                .setCreationTimeForTest(DateTime.parse("1999-01-05T00:00:00Z"))
+                .build());
+    DomainHistory historyEntry =
+        persistResource(
+            new DomainHistory.Builder()
+                .setRegistrarId(domain.getCreationRegistrarId())
+                .setType(DOMAIN_CREATE)
+                .setModificationTime(DateTime.parse("1999-01-05T00:00:00Z"))
+                .setDomain(domain)
+                .build());
+    Recurring recurring =
+        persistResource(
+            new BillingEvent.Recurring.Builder()
+                .setParent(historyEntry)
+                .setRegistrarId(domain.getCreationRegistrarId())
+                .setEventTime((DateTime.parse("1999-01-05T00:00:00Z")))
+                .setFlags(ImmutableSet.of(AUTO_RENEW))
+                .setId(2L)
+                .setReason(Reason.RENEW)
+                .setRenewalPriceBehavior(renewalPriceBehavior)
+                .setRenewalPrice(renewalPrice.isPresent() ? renewalPrice.get() : null)
+                .setRecurrenceEndTime(END_OF_TIME)
+                .setTargetId(domain.getDomainName())
+                .build());
+    persistResource(domain.asBuilder().setAutorenewBillingEvent(recurring.createVKey()).build());
+    return recurring;
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_oneYear_standardDomain_noBilling_isStandardPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(registry, "standard.example", clock.nowUtc(), 1, null))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 10).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_multiYear_standardDomain_noBilling_isStandardPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(registry, "standard.example", clock.nowUtc(), 5, null))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 50).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_oneYear_premiumDomain_noBilling_isPremiumPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(registry, "premium.example", clock.nowUtc(), 1, null))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 100).getAmount(), RENEW, true))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_multiYear_premiumDomain_noBilling_isPremiumPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(registry, "premium.example", clock.nowUtc(), 5, null))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 500).getAmount(), RENEW, true))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_oneYear_premiumDomain_default_isPremiumPrice() throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "premium.example",
+                clock.nowUtc(),
+                1,
+                persistDomainAndSetRecurringBillingEvent(
+                    "premium.example", DEFAULT, Optional.empty())))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 100).getAmount(), RENEW, true))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_multiYear_premiumDomain_default_isPremiumCost() throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "premium.example",
+                clock.nowUtc(),
+                5,
+                persistDomainAndSetRecurringBillingEvent(
+                    "premium.example", DEFAULT, Optional.empty())))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 500).getAmount(), RENEW, true))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_oneYear_standardDomain_default_isNonPremiumPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "standard.example",
+                clock.nowUtc(),
+                1,
+                persistDomainAndSetRecurringBillingEvent(
+                    "premium.example", DEFAULT, Optional.empty())))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 10).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_multiYear_standardDomain_default_isNonPremiumCost()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "standard.example",
+                clock.nowUtc(),
+                5,
+                persistDomainAndSetRecurringBillingEvent(
+                    "standard.example", DEFAULT, Optional.empty())))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 50).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_oneYear_premiumDomain_anchorTenant_isNonPremiumPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "premium.example",
+                clock.nowUtc(),
+                1,
+                persistDomainAndSetRecurringBillingEvent(
+                    "premium.example", NONPREMIUM, Optional.empty())))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 10).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_multiYear_premiumDomain_anchorTenant_isNonPremiumCost()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "premium.example",
+                clock.nowUtc(),
+                5,
+                persistDomainAndSetRecurringBillingEvent(
+                    "premium.example", NONPREMIUM, Optional.empty())))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 50).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_oneYear_standardDomain_anchorTenant_isNonPremiumPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "standard.example",
+                clock.nowUtc(),
+                1,
+                persistDomainAndSetRecurringBillingEvent(
+                    "standard.example", NONPREMIUM, Optional.empty())))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 10).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_multiYear_standardDomain_anchorTenant_isNonPremiumCost()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "standard.example",
+                clock.nowUtc(),
+                5,
+                persistDomainAndSetRecurringBillingEvent(
+                    "standard.example", NONPREMIUM, Optional.empty())))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 50).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_oneYear_standardDomain_internalRegistration_isSpecifiedPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "standard.example",
+                clock.nowUtc(),
+                1,
+                persistDomainAndSetRecurringBillingEvent(
+                    "standard.example", SPECIFIED, Optional.of(Money.of(USD, 1)))))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 1).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_multiYear_standardDomain_internalRegistration_isSpecifiedPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "standard.example",
+                clock.nowUtc(),
+                5,
+                persistDomainAndSetRecurringBillingEvent(
+                    "standard.example", SPECIFIED, Optional.of(Money.of(USD, 1)))))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 5).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_oneYear_premiumDomain_internalRegistration_isSpecifiedPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "premium.example",
+                clock.nowUtc(),
+                1,
+                persistDomainAndSetRecurringBillingEvent(
+                    "premium.example", SPECIFIED, Optional.of(Money.of(USD, 17)))))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 17).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_multiYear_premiumDomain_internalRegistration_isSpecifiedPrice()
+      throws EppException {
+    assertThat(
+            domainPricingLogic.getRenewPrice(
+                registry,
+                "premium.example",
+                clock.nowUtc(),
+                5,
+                persistDomainAndSetRecurringBillingEvent(
+                    "premium.example", SPECIFIED, Optional.of(Money.of(USD, 17)))))
+        .isEqualTo(
+            new FeesAndCredits.Builder()
+                .setCurrency(USD)
+                .addFeeOrCredit(Fee.create(Money.of(USD, 85).getAmount(), RENEW, false))
+                .build());
+  }
+
+  @TestOfyAndSql
+  void testGetDomainRenewPrice_negativeYear_throwsException() throws EppException {
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                domainPricingLogic.getRenewPrice(
+                    registry, "standard.example", clock.nowUtc(), -1, null));
+    assertThat(thrown).hasMessageThat().isEqualTo("Number of years must be positive");
+  }
+}

core/src/test/java/google/registry/model/registrar/RegistrarTest.java

@@ -121,7 +121,6 @@ class RegistrarTest extends EntityTestCase {
                 .setIcannReferralEmail("foo@example.com")
                 .setDriveFolderId("drive folder id")
                 .setIanaIdentifier(8L)
-                .setBillingIdentifier(5325L)
                 .setBillingAccountMap(
                     ImmutableMap.of(CurrencyUnit.USD, "abc123", CurrencyUnit.JPY, "789xyz"))
                 .setPhonePasscode("01234")
@@ -261,12 +260,11 @@ class RegistrarTest extends EntityTestCase {
   }
 
   @TestOfyAndSql
-  void testSuccess_clearingIanaAndBillingIds() {
+  void testSuccess_clearingIanaId() {
     registrar
         .asBuilder()
         .setType(Type.TEST)
         .setIanaIdentifier(null)
-        .setBillingIdentifier(null)
         .build();
   }
 

core/src/test/java/google/registry/rdap/RdapJsonFormatterTest.java

@@ -15,6 +15,7 @@
 package google.registry.rdap;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.rdap.RdapDataStructures.EventAction.TRANSFER;
 import static google.registry.rdap.RdapTestHelper.assertThat;
 import static google.registry.testing.DatabaseHelper.createTld;
 import static google.registry.testing.DatabaseHelper.persistResource;
@@ -28,7 +29,9 @@ import static google.registry.testing.TestDataHelper.loadFile;
 import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
 import com.google.gson.Gson;
 import com.google.gson.JsonObject;
 import google.registry.model.contact.ContactResource;
@@ -51,6 +54,7 @@ import google.registry.testing.DualDatabaseTest;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectExtension;
 import google.registry.testing.TestOfyAndSql;
+import google.registry.testing.TestSqlOnly;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -482,6 +486,17 @@ class RdapJsonFormatterTest {
         .isEqualTo(loadJson("rdapjson_domain_summary.json"));
   }
 
+  @TestSqlOnly
+  void testGetLastHistoryEntryByType() {
+    // Expected data are from "rdapjson_domain_summary.json"
+    assertThat(
+            Maps.transformValues(
+                rdapJsonFormatter.getLastHistoryEntryByType(domainBaseFull),
+                HistoryEntry::getModificationTime))
+        .containsExactlyEntriesIn(
+            ImmutableMap.of(TRANSFER, DateTime.parse("1999-12-01T00:00:00.000Z")));
+  }
+
   @TestOfyAndSql
   void testDomain_logged_out() {
     rdapJsonFormatter.rdapAuthorization = RdapAuthorization.PUBLIC_AUTHORIZATION;

core/src/test/java/google/registry/server/RegistryTestServerMain.java

@@ -147,8 +147,8 @@ public final class RegistryTestServerMain {
                     : UserInfo.create(loginEmail, loginUserId))
             .build();
     appEngine.setUp();
-    AppEngineExtension.loadInitialData();
     new JpaTestExtensions.Builder().buildIntegrationTestExtension().beforeEach(null);
+    AppEngineExtension.loadInitialData();
     System.out.printf("%sLoading Datastore fixtures...%s\n", BLUE, RESET);
     for (Fixture fixture : fixtures) {
       fixture.load();

core/src/test/java/google/registry/tools/CreateRegistrarCommandTest.java

@@ -545,28 +545,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
     assertThat(registrar.get().getIanaIdentifier()).isEqualTo(12345);
   }
 
-  @TestOfyAndSql
-  void testSuccess_billingId() throws Exception {
-    runCommandForced(
-        "--name=blobio",
-        "--password=some_password",
-        "--registrar_type=REAL",
-        "--iana_id=8",
-        "--billing_id=12345",
-        "--passcode=01234",
-        "--icann_referral_email=foo@bar.test",
-        "--street=\"123 Fake St\"",
-        "--city Fakington",
-        "--state MA",
-        "--zip 00351",
-        "--cc US",
-        "clientz");
-
-    Optional<Registrar> registrar = Registrar.loadByRegistrarId("clientz");
-    assertThat(registrar).isPresent();
-    assertThat(registrar.get().getBillingIdentifier()).isEqualTo(12345);
-  }
-
   @TestOfyAndSql
   void testSuccess_poNumber() throws Exception {
     runCommandForced(
@@ -805,7 +783,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
         "--registrar_type=TEST",
         "--icann_referral_email=foo@bar.test",
         "--iana_id=null",
-        "--billing_id=null",
         "--phone=null",
         "--fax=null",
         "--url=null",
@@ -821,7 +798,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
     assertThat(registrarOptional).isPresent();
     Registrar registrar = registrarOptional.get();
     assertThat(registrar.getIanaIdentifier()).isNull();
-    assertThat(registrar.getBillingIdentifier()).isNull();
     assertThat(registrar.getPhoneNumber()).isNull();
     assertThat(registrar.getFaxNumber()).isNull();
     assertThat(registrar.getUrl()).isNull();
@@ -835,7 +811,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
         "--password=some_password",
         "--registrar_type=TEST",
         "--iana_id=",
-        "--billing_id=",
         "--phone=",
         "--fax=",
         "--url=",
@@ -852,7 +827,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
     assertThat(registrarOptional).isPresent();
     Registrar registrar = registrarOptional.get();
     assertThat(registrar.getIanaIdentifier()).isNull();
-    assertThat(registrar.getBillingIdentifier()).isNull();
     assertThat(registrar.getPhoneNumber()).isNull();
     assertThat(registrar.getFaxNumber()).isNull();
     assertThat(registrar.getUrl()).isNull();
@@ -1514,48 +1488,6 @@ class CreateRegistrarCommandTest extends CommandTestCase<CreateRegistrarCommand>
                 "clientz"));
   }
 
-  @TestOfyAndSql
-  void testFailure_negativeBillingId() {
-    assertThrows(
-        IllegalArgumentException.class,
-        () ->
-            runCommandForced(
-                "--name=blobio",
-                "--password=some_password",
-                "--registrar_type=REAL",
-                "--iana_id=8",
-                "--billing_id=-1",
-                "--passcode=01234",
-                "--icann_referral_email=foo@bar.test",
-                "--street=\"123 Fake St\"",
-                "--city Fakington",
-                "--state MA",
-                "--zip 00351",
-                "--cc US",
-                "clientz"));
-  }
-
-  @TestOfyAndSql
-  void testFailure_nonIntegerBillingId() {
-    assertThrows(
-        ParameterException.class,
-        () ->
-            runCommandForced(
-                "--name=blobio",
-                "--password=some_password",
-                "--registrar_type=REAL",
-                "--iana_id=8",
-                "--billing_id=ABC12345",
-                "--passcode=01234",
-                "--icann_referral_email=foo@bar.test",
-                "--street=\"123 Fake St\"",
-                "--city Fakington",
-                "--state MA",
-                "--zip 00351",
-                "--cc US",
-                "clientz"));
-  }
-
   @TestOfyAndSql
   void testFailure_missingPhonePasscode() {
     assertThrows(

core/src/test/java/google/registry/tools/MutatingCommandTest.java

@@ -33,6 +33,7 @@ import google.registry.model.registrar.Registrar;
 import google.registry.persistence.VKey;
 import google.registry.testing.AppEngineExtension;
 import java.util.Arrays;
+import java.util.Optional;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -58,7 +59,7 @@ public class MutatingCommandTest {
   void beforeEach() {
     registrar1 = persistNewRegistrar("Registrar1", "Registrar1", Registrar.Type.REAL, 1L);
     registrar2 = persistNewRegistrar("Registrar2", "Registrar2", Registrar.Type.REAL, 2L);
-    newRegistrar1 = registrar1.asBuilder().setBillingIdentifier(42L).build();
+    newRegistrar1 = registrar1.asBuilder().setPoNumber(Optional.of("23")).build();
     newRegistrar2 = registrar2.asBuilder().setBlockPremiumNames(true).build();
 
     createTld("tld");
@@ -94,18 +95,19 @@ public class MutatingCommandTest {
     };
     command.init();
     String changes = command.prompt();
-    assertThat(changes).isEqualTo(
-        "Update HostResource@2-ROID\n"
-            + "lastEppUpdateTime: null -> 2014-09-09T09:09:09.000Z\n"
-            + "\n"
-            + "Update HostResource@3-ROID\n"
-            + "currentSponsorClientId: TheRegistrar -> Registrar2\n"
-            + "\n"
-            + "Update Registrar@Registrar1\n"
-            + "billingIdentifier: null -> 42\n"
-            + "\n"
-            + "Update Registrar@Registrar2\n"
-            + "blockPremiumNames: false -> true\n");
+    assertThat(changes)
+        .isEqualTo(
+            "Update HostResource@2-ROID\n"
+                + "lastEppUpdateTime: null -> 2014-09-09T09:09:09.000Z\n"
+                + "\n"
+                + "Update HostResource@3-ROID\n"
+                + "currentSponsorClientId: TheRegistrar -> Registrar2\n"
+                + "\n"
+                + "Update Registrar@Registrar1\n"
+                + "poNumber: null -> 23\n"
+                + "\n"
+                + "Update Registrar@Registrar2\n"
+                + "blockPremiumNames: false -> true\n");
     String results = command.execute();
     assertThat(results).isEqualTo("Updated 4 entities.\n");
     assertThat(loadByEntity(host1)).isEqualTo(newHost1);

core/src/test/java/google/registry/tools/UpdateRegistrarCommandTest.java

@@ -25,6 +25,7 @@ import static google.registry.testing.DatabaseHelper.newRegistry;
 import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.joda.money.CurrencyUnit.JPY;
+import static org.joda.money.CurrencyUnit.USD;
 import static org.joda.time.DateTimeZone.UTC;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -42,7 +43,6 @@ import google.registry.testing.AppEngineExtension;
 import google.registry.util.CidrAddressBlock;
 import java.math.BigDecimal;
 import java.util.Optional;
-import org.joda.money.CurrencyUnit;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
@@ -347,13 +347,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
     assertThat(loadRegistrar("NewRegistrar").getIanaIdentifier()).isEqualTo(12345);
   }
 
-  @Test
-  void testSuccess_billingId() throws Exception {
-    assertThat(loadRegistrar("NewRegistrar").getBillingIdentifier()).isNull();
-    runCommand("--billing_id=12345", "--force", "NewRegistrar");
-    assertThat(loadRegistrar("NewRegistrar").getBillingIdentifier()).isEqualTo(12345);
-  }
-
   @Test
   void testSuccess_poNumber() throws Exception {
     assertThat(loadRegistrar("NewRegistrar").getPoNumber()).isEmpty();
@@ -368,7 +361,18 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
     runCommand("--billing_account_map=USD=abc123,JPY=789xyz", "--force", "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())
-        .containsExactly(CurrencyUnit.USD, "abc123", CurrencyUnit.JPY, "789xyz");
+        .containsExactly(USD, "abc123", JPY, "789xyz");
+  }
+
+  @Test
+  void testSuccess_billingAccountMap_nullify() throws Exception {
+    persistResource(
+        loadRegistrar("NewRegistrar")
+            .asBuilder()
+            .setBillingAccountMap(ImmutableMap.of(USD, "abc123", JPY, "789xyz"))
+            .build());
+    runCommand("--billing_account_map=\"\"", "--force", "NewRegistrar");
+    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
   }
 
   @Test
@@ -415,8 +419,7 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
         loadRegistrar("NewRegistrar").asBuilder().setBillingAccountMap(ImmutableMap.of()).build());
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).isEmpty();
     runCommand("--billing_account_map=JPY=789xyz", "--allowed_tlds=foo", "--force", "NewRegistrar");
-    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())
-        .containsExactly(CurrencyUnit.JPY, "789xyz");
+    assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap()).containsExactly(JPY, "789xyz");
   }
 
   @Test
@@ -425,12 +428,11 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
     persistResource(
         loadRegistrar("NewRegistrar")
             .asBuilder()
-            .setBillingAccountMap(
-                ImmutableMap.of(CurrencyUnit.USD, "abc123", CurrencyUnit.JPY, "789xyz"))
+            .setBillingAccountMap(ImmutableMap.of(USD, "abc123", JPY, "789xyz"))
             .build());
     runCommand("--billing_account_map=JPY=123xyz", "--allowed_tlds=foo", "--force", "NewRegistrar");
     assertThat(loadRegistrar("NewRegistrar").getBillingAccountMap())
-        .containsExactly(CurrencyUnit.JPY, "123xyz", CurrencyUnit.USD, "abc123");
+        .containsExactly(JPY, "123xyz", USD, "abc123");
   }
 
   @Test
@@ -495,7 +497,7 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
             .setContactsRequireSyncing(true)
             .build());
     // Make some unrelated change where we don't specify the flags for the booleans.
-    runCommandForced("--billing_id=12345", "NewRegistrar");
+    runCommandForced("NewRegistrar");
     // Make sure that the boolean fields didn't get reset back to false.
     Registrar reloadedRegistrar = loadRegistrar("NewRegistrar");
     assertThat(reloadedRegistrar.getBlockPremiumNames()).isTrue();
@@ -520,7 +522,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
                 .asBuilder()
                 .setType(Type.PDT) // for non-null IANA ID
                 .setIanaIdentifier(9995L)
-                .setBillingIdentifier(1L)
                 .setPhoneNumber("+1.2125555555")
                 .setFaxNumber("+1.2125555556")
                 .setUrl("http://www.example.tld")
@@ -528,7 +529,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
                 .build());
 
     assertThat(registrar.getIanaIdentifier()).isNotNull();
-    assertThat(registrar.getBillingIdentifier()).isNotNull();
     assertThat(registrar.getPhoneNumber()).isNotNull();
     assertThat(registrar.getFaxNumber()).isNotNull();
     assertThat(registrar.getUrl()).isNotNull();
@@ -537,7 +537,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
     runCommand(
         "--registrar_type=TEST", // necessary for null IANA ID
         "--iana_id=null",
-        "--billing_id=null",
         "--phone=null",
         "--fax=null",
         "--url=null",
@@ -547,7 +546,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
 
     registrar = loadRegistrar("NewRegistrar");
     assertThat(registrar.getIanaIdentifier()).isNull();
-    assertThat(registrar.getBillingIdentifier()).isNull();
     assertThat(registrar.getPhoneNumber()).isNull();
     assertThat(registrar.getFaxNumber()).isNull();
     assertThat(registrar.getUrl()).isNull();
@@ -563,7 +561,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
                 .asBuilder()
                 .setType(Type.PDT) // for non-null IANA ID
                 .setIanaIdentifier(9995L)
-                .setBillingIdentifier(1L)
                 .setPhoneNumber("+1.2125555555")
                 .setFaxNumber("+1.2125555556")
                 .setUrl("http://www.example.tld")
@@ -571,7 +568,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
                 .build());
 
     assertThat(registrar.getIanaIdentifier()).isNotNull();
-    assertThat(registrar.getBillingIdentifier()).isNotNull();
     assertThat(registrar.getPhoneNumber()).isNotNull();
     assertThat(registrar.getFaxNumber()).isNotNull();
     assertThat(registrar.getUrl()).isNotNull();
@@ -580,7 +576,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
     runCommand(
         "--registrar_type=TEST", // necessary for null IANA ID
         "--iana_id=",
-        "--billing_id=",
         "--phone=",
         "--fax=",
         "--url=",
@@ -590,7 +585,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
 
     registrar = loadRegistrar("NewRegistrar");
     assertThat(registrar.getIanaIdentifier()).isNull();
-    assertThat(registrar.getBillingIdentifier()).isNull();
     assertThat(registrar.getPhoneNumber()).isNull();
     assertThat(registrar.getFaxNumber()).isNull();
     assertThat(registrar.getUrl()).isNull();
@@ -654,20 +648,6 @@ class UpdateRegistrarCommandTest extends CommandTestCase<UpdateRegistrarCommand>
         ParameterException.class, () -> runCommand("--iana_id=ABC123", "--force", "NewRegistrar"));
   }
 
-  @Test
-  void testFailure_negativeBillingId() {
-    assertThrows(
-        IllegalArgumentException.class,
-        () -> runCommand("--billing_id=-1", "--force", "NewRegistrar"));
-  }
-
-  @Test
-  void testFailure_nonIntegerBillingId() {
-    assertThrows(
-        ParameterException.class,
-        () -> runCommand("--billing_id=ABC123", "--force", "NewRegistrar"));
-  }
-
   @Test
   void testFailure_passcodeTooShort() {
     assertThrows(

core/src/test/resources/google/registry/model/schema.txt

@@ -568,7 +568,6 @@ class google.registry.model.registrar.Registrar {
   google.registry.model.registrar.Registrar$Type type;
   google.registry.model.registrar.RegistrarAddress internationalizedAddress;
   google.registry.model.registrar.RegistrarAddress localizedAddress;
-  java.lang.Long billingIdentifier;
   java.lang.Long ianaIdentifier;
   java.lang.String clientCertificate;
   java.lang.String clientCertificateHash;

docs/configuration.md

@@ -52,6 +52,13 @@ you will need to make any modifications beyond simple changes to
 likely you'll need to add cronjobs, URL paths, Datastore indexes, and task
 queues, and thus edit those associated XML files.
 
+The existing codebase is configured for running a full-scale registry with
+multiple TLDs.  In order to deploy to App Engine, you will either need to
+[increase your
+quota](https://cloud.google.com/compute/quotas#requesting_additional_quota) to
+allow for at least 100 running instances or reduce `max-instances` in the
+backend `appengine-web.xml` files to 25 or less.
+
 ## Global configuration
 
 Global configuration is managed through YAML files that are built with and