From 706f3b44edd79fe02f5d83e7076b7655f1ff261c Mon Sep 17 00:00:00 2001
From: nickfelt <nickfelt@google.com>
Date: Tue, 19 Apr 2016 08:29:27 -0700
Subject: [PATCH] Enforce currency consistency for server status billing cost

Also adds tests for currency consistency for other costs, and cleans
up some of the testing for negative currencies (since I was there).
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=120231198
---
 .../registry/model/registry/Registry.java     | 16 +++---
 .../DomainApplicationCreateFlowTest.java      |  1 +
 .../flows/domain/DomainCreateFlowTest.java    |  1 +
 .../flows/domain/DomainRenewFlowTest.java     |  1 +
 .../domain/DomainRestoreRequestFlowTest.java  |  1 +
 .../domain/DomainTransferRequestFlowTest.java |  1 +
 .../registry/model/registry/RegistryTest.java | 50 +++++++++++++++----
 .../registry/tools/CreateTldCommandTest.java  |  1 +
 .../registry/tools/UpdateTldCommandTest.java  |  2 +
 9 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/java/com/google/domain/registry/model/registry/Registry.java b/java/com/google/domain/registry/model/registry/Registry.java
index 148b8ee11..d8d77a812 100644
--- a/java/com/google/domain/registry/model/registry/Registry.java
+++ b/java/com/google/domain/registry/model/registry/Registry.java
@@ -657,7 +657,7 @@ public class Registry extends BackupGroupRoot implements Buildable {
     }
 
     public Builder setCreateBillingCost(Money amount) {
-      checkArgument(amount.isPositiveOrZero(), "billing costs must be non-negative");
+      checkArgument(amount.isPositiveOrZero(), "create billing cost cannot be negative");
       getInstance().createBillingCost = amount;
       return this;
     }
@@ -698,7 +698,7 @@ public class Registry extends BackupGroupRoot implements Buildable {
     }
 
     public Builder setRestoreBillingCost(Money amount) {
-      checkArgument(amount.isPositiveOrZero(), "billing costs must be non-negative");
+      checkArgument(amount.isPositiveOrZero(), "restore billing cost cannot be negative");
       getInstance().restoreBillingCost = amount;
       return this;
     }
@@ -713,7 +713,7 @@ public class Registry extends BackupGroupRoot implements Buildable {
      */
     public Builder setRenewBillingCostTransitions(
         ImmutableSortedMap<DateTime, Money> renewCostsMap) {
-      checkNotNull(renewCostsMap, "Renew billing costs map cannot be null");
+      checkNotNull(renewCostsMap, "renew billing costs map cannot be null");
       checkArgument(Iterables.all(
           renewCostsMap.values(),
           new Predicate<Money>() {
@@ -721,7 +721,7 @@ public class Registry extends BackupGroupRoot implements Buildable {
             public boolean apply(Money amount) {
               return amount.isPositiveOrZero();
             }}),
-          "Renew billing costs cannot be negative");
+          "renew billing cost cannot be negative");
       getInstance().renewBillingCostTransitions =
           TimedTransitionProperty.fromValueMap(renewCostsMap, BillingCostTransition.class);
       return this;
@@ -733,7 +733,8 @@ public class Registry extends BackupGroupRoot implements Buildable {
     }
 
     public Builder setServerStatusChangeBillingCost(Money amount) {
-      checkArgument(amount.isPositiveOrZero(), "billing costs must be non-negative");
+      checkArgument(
+          amount.isPositiveOrZero(), "server status change billing cost cannot be negative");
       getInstance().serverStatusChangeBillingCost = amount;
       return this;
     }
@@ -785,6 +786,9 @@ public class Registry extends BackupGroupRoot implements Buildable {
       checkArgument(
           instance.getStandardRestoreCost().getCurrencyUnit().equals(instance.currency),
           "Restore cost must be in the registry's currency");
+      checkArgument(
+          instance.getServerStatusChangeCost().getCurrencyUnit().equals(instance.currency),
+          "Server status change cost must be in the registry's currency");
       checkArgument(
           Iterables.all(
               instance.getRenewBillingCostTransitions().values(),
@@ -793,7 +797,7 @@ public class Registry extends BackupGroupRoot implements Buildable {
                 public boolean apply(Money money) {
                   return money.getCurrencyUnit().equals(instance.currency);
                 }}),
-          "Renew costs must be in the registry's currency");
+          "Renew cost must be in the registry's currency");
       instance.tldStrId = tldName;
       instance.tldUnicode = Idn.toUnicode(tldName);
       return super.build();
diff --git a/javatests/com/google/domain/registry/flows/domain/DomainApplicationCreateFlowTest.java b/javatests/com/google/domain/registry/flows/domain/DomainApplicationCreateFlowTest.java
index ef4d8871b..0ac370dda 100644
--- a/javatests/com/google/domain/registry/flows/domain/DomainApplicationCreateFlowTest.java
+++ b/javatests/com/google/domain/registry/flows/domain/DomainApplicationCreateFlowTest.java
@@ -1139,6 +1139,7 @@ public class DomainApplicationCreateFlowTest
         .setCreateBillingCost(Money.of(EUR, 13))
         .setRestoreBillingCost(Money.of(EUR, 11))
         .setRenewBillingCostTransitions(ImmutableSortedMap.of(START_OF_TIME, Money.of(EUR, 7)))
+        .setServerStatusChangeBillingCost(Money.of(EUR, 19))
         .build());
     persistContactsAndHosts();
     clock.advanceOneMilli();
diff --git a/javatests/com/google/domain/registry/flows/domain/DomainCreateFlowTest.java b/javatests/com/google/domain/registry/flows/domain/DomainCreateFlowTest.java
index 86d4ded84..5b0e97852 100644
--- a/javatests/com/google/domain/registry/flows/domain/DomainCreateFlowTest.java
+++ b/javatests/com/google/domain/registry/flows/domain/DomainCreateFlowTest.java
@@ -558,6 +558,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
         .setCreateBillingCost(Money.of(EUR, 13))
         .setRestoreBillingCost(Money.of(EUR, 11))
         .setRenewBillingCostTransitions(ImmutableSortedMap.of(START_OF_TIME, Money.of(EUR, 7)))
+        .setServerStatusChangeBillingCost(Money.of(EUR, 19))
         .build());
     persistContactsAndHosts();
     runFlow();
diff --git a/javatests/com/google/domain/registry/flows/domain/DomainRenewFlowTest.java b/javatests/com/google/domain/registry/flows/domain/DomainRenewFlowTest.java
index f0dfaae34..b6d68dd53 100644
--- a/javatests/com/google/domain/registry/flows/domain/DomainRenewFlowTest.java
+++ b/javatests/com/google/domain/registry/flows/domain/DomainRenewFlowTest.java
@@ -340,6 +340,7 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
             .setCreateBillingCost(Money.of(EUR, 13))
             .setRestoreBillingCost(Money.of(EUR, 11))
             .setRenewBillingCostTransitions(ImmutableSortedMap.of(START_OF_TIME, Money.of(EUR, 7)))
+            .setServerStatusChangeBillingCost(Money.of(EUR, 19))
             .build());
     persistDomain();
     runFlow();
diff --git a/javatests/com/google/domain/registry/flows/domain/DomainRestoreRequestFlowTest.java b/javatests/com/google/domain/registry/flows/domain/DomainRestoreRequestFlowTest.java
index d5b2adce2..3045b847b 100644
--- a/javatests/com/google/domain/registry/flows/domain/DomainRestoreRequestFlowTest.java
+++ b/javatests/com/google/domain/registry/flows/domain/DomainRestoreRequestFlowTest.java
@@ -284,6 +284,7 @@ public class DomainRestoreRequestFlowTest extends
             .setCreateBillingCost(Money.of(EUR, 13))
             .setRestoreBillingCost(Money.of(EUR, 11))
             .setRenewBillingCostTransitions(ImmutableSortedMap.of(START_OF_TIME, Money.of(EUR, 7)))
+            .setServerStatusChangeBillingCost(Money.of(EUR, 19))
             .build());
     runFlow();
   }
diff --git a/javatests/com/google/domain/registry/flows/domain/DomainTransferRequestFlowTest.java b/javatests/com/google/domain/registry/flows/domain/DomainTransferRequestFlowTest.java
index 988010116..792d36195 100644
--- a/javatests/com/google/domain/registry/flows/domain/DomainTransferRequestFlowTest.java
+++ b/javatests/com/google/domain/registry/flows/domain/DomainTransferRequestFlowTest.java
@@ -443,6 +443,7 @@ public class DomainTransferRequestFlowTest
             .setCreateBillingCost(Money.of(EUR, 13))
             .setRestoreBillingCost(Money.of(EUR, 11))
             .setRenewBillingCostTransitions(ImmutableSortedMap.of(START_OF_TIME, Money.of(EUR, 7)))
+            .setServerStatusChangeBillingCost(Money.of(EUR, 19))
             .build());
     doFailingTest("domain_transfer_request_fee.xml");
   }
diff --git a/javatests/com/google/domain/registry/model/registry/RegistryTest.java b/javatests/com/google/domain/registry/model/registry/RegistryTest.java
index 3aee0a352..27ca41ee4 100644
--- a/javatests/com/google/domain/registry/model/registry/RegistryTest.java
+++ b/javatests/com/google/domain/registry/model/registry/RegistryTest.java
@@ -25,6 +25,7 @@ import static com.google.domain.registry.testing.DatastoreHelper.persistPremiumL
 import static com.google.domain.registry.testing.DatastoreHelper.persistReservedList;
 import static com.google.domain.registry.util.DateTimeUtils.END_OF_TIME;
 import static com.google.domain.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.joda.money.CurrencyUnit.EUR;
 import static org.joda.money.CurrencyUnit.USD;
 
 import com.google.common.collect.ImmutableSet;
@@ -399,23 +400,54 @@ public class RegistryTest extends EntityTestCase {
   }
 
   @Test
-  public void testFailure_badRenewBillingCostTransitionValue() {
-    thrown.expect(IllegalArgumentException.class);
+  public void testFailure_negativeRenewBillingCostTransitionValue() {
+    thrown.expect(IllegalArgumentException.class, "billing cost cannot be negative");
     Registry.get("tld").asBuilder()
-        .setRenewBillingCostTransitions(ImmutableSortedMap.of(clock.nowUtc(), Money.of(USD, -42)))
+        .setRenewBillingCostTransitions(ImmutableSortedMap.of(START_OF_TIME, Money.of(USD, -42)));
+  }
+
+  @Test
+  public void testFailure_negativeCreateBillingCost() {
+    thrown.expect(IllegalArgumentException.class, "billing cost cannot be negative");
+    Registry.get("tld").asBuilder().setCreateBillingCost(Money.of(USD, -42));
+  }
+
+  @Test
+  public void testFailure_negativeRestoreBillingCost() {
+    thrown.expect(IllegalArgumentException.class, "billing cost cannot be negative");
+    Registry.get("tld").asBuilder().setRestoreBillingCost(Money.of(USD, -42));
+  }
+
+  @Test
+  public void testFailure_negativeServerStatusChangeBillingCost() {
+    thrown.expect(IllegalArgumentException.class, "billing cost cannot be negative");
+    Registry.get("tld").asBuilder().setServerStatusChangeBillingCost(Money.of(USD, -42));
+  }
+
+  @Test
+  public void testFailure_renewBillingCostTransitionValue_wrongCurrency() {
+    thrown.expect(IllegalArgumentException.class, "cost must be in the registry's currency");
+    Registry.get("tld").asBuilder()
+        .setRenewBillingCostTransitions(ImmutableSortedMap.of(START_OF_TIME, Money.of(EUR, 42)))
         .build();
   }
 
   @Test
-  public void testFailure_badCreateBillingCost() {
-    thrown.expect(IllegalArgumentException.class);
-    Registry.get("tld").asBuilder().setCreateBillingCost(Money.of(USD, -42)).build();
+  public void testFailure_createBillingCost_wrongCurrency() {
+    thrown.expect(IllegalArgumentException.class, "cost must be in the registry's currency");
+    Registry.get("tld").asBuilder().setCreateBillingCost(Money.of(EUR, 42)).build();
   }
 
   @Test
-  public void testFailure_badRestoreBillingCost() {
-    thrown.expect(IllegalArgumentException.class);
-    Registry.get("tld").asBuilder().setRestoreBillingCost(Money.of(USD, -42)).build();
+  public void testFailure_restoreBillingCost_wrongCurrency() {
+    thrown.expect(IllegalArgumentException.class, "cost must be in the registry's currency");
+    Registry.get("tld").asBuilder().setRestoreBillingCost(Money.of(EUR, 42)).build();
+  }
+
+  @Test
+  public void testFailure_serverStatusChangeBillingCost_wrongCurrency() {
+    thrown.expect(IllegalArgumentException.class, "cost must be in the registry's currency");
+    Registry.get("tld").asBuilder().setServerStatusChangeBillingCost(Money.of(EUR, 42)).build();
   }
 
   @Test
diff --git a/javatests/com/google/domain/registry/tools/CreateTldCommandTest.java b/javatests/com/google/domain/registry/tools/CreateTldCommandTest.java
index 1d347b3ce..c9e79e635 100644
--- a/javatests/com/google/domain/registry/tools/CreateTldCommandTest.java
+++ b/javatests/com/google/domain/registry/tools/CreateTldCommandTest.java
@@ -165,6 +165,7 @@ public class CreateTldCommandTest extends CommandTestCase<CreateTldCommand> {
         "--create_billing_cost=\"JPY 12345\"",
         "--restore_billing_cost=\"JPY 67890\"",
         "--initial_renew_billing_cost=\"JPY 101112\"",
+        "--server_status_change_cost=\"JPY 97865\"",
         "--roid_suffix=Q9JYB4C",
         "xn--q9jyb4c");
     Registry registry = Registry.get("xn--q9jyb4c");
diff --git a/javatests/com/google/domain/registry/tools/UpdateTldCommandTest.java b/javatests/com/google/domain/registry/tools/UpdateTldCommandTest.java
index b532ab6e5..ebe5b6fa5 100644
--- a/javatests/com/google/domain/registry/tools/UpdateTldCommandTest.java
+++ b/javatests/com/google/domain/registry/tools/UpdateTldCommandTest.java
@@ -215,11 +215,13 @@ public class UpdateTldCommandTest extends CommandTestCase<UpdateTldCommand> {
             .setRestoreBillingCost(Money.ofMajor(JPY, 1))
             .setRenewBillingCostTransitions(
                 ImmutableSortedMap.of(START_OF_TIME, Money.ofMajor(JPY, 1)))
+            .setServerStatusChangeBillingCost(Money.ofMajor(JPY, 1))
             .build());
     runCommandForced(
         "--create_billing_cost=\"JPY 12345\"",
         "--restore_billing_cost=\"JPY 67890\"",
         "--renew_billing_cost_transitions=\"0=JPY 101112\"",
+        "--server_status_change_cost=\"JPY 97865\"",
         "xn--q9jyb4c");
     assertThat(Registry.get("xn--q9jyb4c").getStandardCreateCost())
         .isEqualTo(Money.ofMajor(JPY, 12345));