From 3d5ab8d068103b8d3983d19be481593d85cb9b3a Mon Sep 17 00:00:00 2001
From: mountford <mountford@google.com>
Date: Fri, 30 Sep 2016 15:05:11 -0700
Subject: [PATCH] Add success exception for TestExtraLogicManager

TestExtraLogicManager is pretty kludgy, and should be replaced with injection, mocking, etc. But in the meantime, using a dedicated error to signal its success, rather than IllegalArgumentException as was done before, at least makes things a little easier to follow.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=134832315
---
 .../flows/domain/DomainCreateFlowTest.java       |  3 ++-
 .../flows/domain/DomainDeleteFlowTest.java       |  3 ++-
 .../flows/domain/DomainRenewFlowTest.java        |  3 ++-
 .../domain/DomainRestoreRequestFlowTest.java     |  3 ++-
 .../domain/DomainTransferRequestFlowTest.java    |  4 +++-
 .../flows/domain/DomainUpdateFlowTest.java       | 16 +++++++++++-----
 .../model/domain/TestExtraLogicManager.java      | 15 ++++++++++++++-
 7 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java
index 50a52a772..a33814c01 100644
--- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java
+++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java
@@ -106,6 +106,7 @@ import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.LrpToken;
 import google.registry.model.domain.TestExtraLogicManager;
+import google.registry.model.domain.TestExtraLogicManager.TestExtraLogicManagerSuccessException;
 import google.registry.model.domain.launch.ApplicationStatus;
 import google.registry.model.domain.launch.LaunchNotice;
 import google.registry.model.domain.rgp.GracePeriodStatus;
@@ -1616,7 +1617,7 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow,
   public void testSuccess_flags() throws Exception {
     persistContactsAndHosts();
     setEppInput("domain_create_flags.xml", ImmutableMap.of("FEE", "42"));
-    thrown.expect(IllegalArgumentException.class, "flag1,flag2");
+    thrown.expect(TestExtraLogicManagerSuccessException.class, "flag1,flag2");
     runFlow();
   }
 }
diff --git a/javatests/google/registry/flows/domain/DomainDeleteFlowTest.java b/javatests/google/registry/flows/domain/DomainDeleteFlowTest.java
index 96b2619bc..63753e678 100644
--- a/javatests/google/registry/flows/domain/DomainDeleteFlowTest.java
+++ b/javatests/google/registry/flows/domain/DomainDeleteFlowTest.java
@@ -57,6 +57,7 @@ import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.TestExtraLogicManager;
+import google.registry.model.domain.TestExtraLogicManager.TestExtraLogicManagerSuccessException;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.eppcommon.ProtocolDefinition.ServiceExtension;
 import google.registry.model.eppcommon.StatusValue;
@@ -707,7 +708,7 @@ public class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow,
   public void testSuccess_flags() throws Exception {
     setEppInput("domain_delete_flags.xml");
     setupSuccessfulTest();
-    thrown.expect(IllegalArgumentException.class, "deleted");
+    thrown.expect(TestExtraLogicManagerSuccessException.class, "deleted");
     runFlow();
   }
 }
diff --git a/javatests/google/registry/flows/domain/DomainRenewFlowTest.java b/javatests/google/registry/flows/domain/DomainRenewFlowTest.java
index 3b374357b..f62a1b979 100644
--- a/javatests/google/registry/flows/domain/DomainRenewFlowTest.java
+++ b/javatests/google/registry/flows/domain/DomainRenewFlowTest.java
@@ -55,6 +55,7 @@ import google.registry.model.billing.BillingEvent.Reason;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.TestExtraLogicManager;
+import google.registry.model.domain.TestExtraLogicManager.TestExtraLogicManagerSuccessException;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.poll.PollMessage;
@@ -616,7 +617,7 @@ public class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, D
   public void testSuccess_flags() throws Exception {
     setEppInput("domain_renew_flags.xml");
     persistDomain();
-    thrown.expect(IllegalArgumentException.class, "renewed");
+    thrown.expect(TestExtraLogicManagerSuccessException.class, "renewed");
     runFlow();
   }
 }
diff --git a/javatests/google/registry/flows/domain/DomainRestoreRequestFlowTest.java b/javatests/google/registry/flows/domain/DomainRestoreRequestFlowTest.java
index 913f144ae..6d181fabb 100644
--- a/javatests/google/registry/flows/domain/DomainRestoreRequestFlowTest.java
+++ b/javatests/google/registry/flows/domain/DomainRestoreRequestFlowTest.java
@@ -57,6 +57,7 @@ import google.registry.model.billing.BillingEvent.Reason;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.TestExtraLogicManager;
+import google.registry.model.domain.TestExtraLogicManager.TestExtraLogicManagerSuccessException;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.poll.PollMessage;
@@ -556,7 +557,7 @@ public class DomainRestoreRequestFlowTest extends
   public void testSuccess_flags() throws Exception {
     setEppInput("domain_update_restore_request_flags.xml");
     persistPendingDeleteDomain();
-    thrown.expect(IllegalArgumentException.class, "restored");
+    thrown.expect(TestExtraLogicManagerSuccessException.class, "restored");
     runFlow();
   }
 }
diff --git a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java
index 832414734..bc76a706b 100644
--- a/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java
+++ b/javatests/google/registry/flows/domain/DomainTransferRequestFlowTest.java
@@ -63,6 +63,7 @@ import google.registry.model.domain.DomainAuthInfo;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.TestExtraLogicManager;
+import google.registry.model.domain.TestExtraLogicManager.TestExtraLogicManagerSuccessException;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
 import google.registry.model.eppcommon.StatusValue;
@@ -769,7 +770,8 @@ public class DomainTransferRequestFlowTest
     setEppInput("domain_transfer_request_flags.xml");
     setupDomain("example", "flags");
     eppLoader.replaceAll("JD1234-REP", contact.getRepoId());
-    thrown.expect(IllegalArgumentException.class, "add:flag1,flag2;remove:flag3,flag4");
+    thrown.expect(
+        TestExtraLogicManagerSuccessException.class, "add:flag1,flag2;remove:flag3,flag4");
     runFlow();
   }
 }
diff --git a/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java b/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java
index 1a871729c..db93774a3 100644
--- a/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java
+++ b/javatests/google/registry/flows/domain/DomainUpdateFlowTest.java
@@ -77,6 +77,7 @@ import google.registry.model.domain.DesignatedContact.Type;
 import google.registry.model.domain.DomainResource;
 import google.registry.model.domain.GracePeriod;
 import google.registry.model.domain.TestExtraLogicManager;
+import google.registry.model.domain.TestExtraLogicManager.TestExtraLogicManagerSuccessException;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.domain.secdns.DelegationSignerData;
 import google.registry.model.eppcommon.StatusValue;
@@ -1197,7 +1198,8 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
     setEppInput("domain_update_addremove_flags.xml", ImmutableMap.of("DOMAIN", "update-0"));
     persistReferencedEntities();
     persistDomain();
-    thrown.expect(IllegalArgumentException.class, "add:flag1,flag2;remove:flag3,flag4");
+    thrown.expect(
+        TestExtraLogicManagerSuccessException.class, "add:flag1,flag2;remove:flag3,flag4");
     runFlow();
   }
 
@@ -1219,7 +1221,8 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
         ImmutableMap.of("DOMAIN", "update-0", "FEE", "0.00"));
     persistReferencedEntities();
     persistDomain();
-    thrown.expect(IllegalArgumentException.class, "add:flag1,flag2;remove:flag3,flag4");
+    thrown.expect(
+        TestExtraLogicManagerSuccessException.class, "add:flag1,flag2;remove:flag3,flag4");
     runFlow();
   }
 
@@ -1229,7 +1232,8 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
     setEppInput("domain_update_addremove_flags.xml", ImmutableMap.of("DOMAIN", "renew-0"));
     persistReferencedEntities();
     persistDomain();
-    thrown.expect(IllegalArgumentException.class, "add:flag1,flag2;remove:flag3,flag4");
+    thrown.expect(
+        TestExtraLogicManagerSuccessException.class, "add:flag1,flag2;remove:flag3,flag4");
     runFlow();
   }
 
@@ -1251,7 +1255,8 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
         ImmutableMap.of("DOMAIN", "renew-0", "FEE", "0.00"));
     persistReferencedEntities();
     persistDomain();
-    thrown.expect(IllegalArgumentException.class, "add:flag1,flag2;remove:flag3,flag4");
+    thrown.expect(
+        TestExtraLogicManagerSuccessException.class, "add:flag1,flag2;remove:flag3,flag4");
     runFlow();
   }
 
@@ -1284,7 +1289,8 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
         ImmutableMap.of("DOMAIN", "update-13", "FEE", "13.00"));
     persistReferencedEntities();
     persistDomain();
-    thrown.expect(IllegalArgumentException.class, "add:flag1,flag2;remove:flag3,flag4");
+    thrown.expect(
+        TestExtraLogicManagerSuccessException.class, "add:flag1,flag2;remove:flag3,flag4");
     runFlow();
   }
 }
diff --git a/javatests/google/registry/model/domain/TestExtraLogicManager.java b/javatests/google/registry/model/domain/TestExtraLogicManager.java
index 7e5b6732e..a622fa1aa 100644
--- a/javatests/google/registry/model/domain/TestExtraLogicManager.java
+++ b/javatests/google/registry/model/domain/TestExtraLogicManager.java
@@ -43,6 +43,19 @@ public class TestExtraLogicManager implements RegistryExtraFlowLogic {
 
   private String messageToThrow = null;
 
+  /**
+   * Dummy exception used to signal success. This is thrown by the commitAdditionalLogicChanges
+   * method to indicate to the test that everything worked properly, because the
+   * TestExtraLogicManager instance used by the flow will have been created deep in the flow and is
+   * not accessible to the test code directly.  We should fix this when we make the extra flow logic
+   * injected.
+   */
+  public static class TestExtraLogicManagerSuccessException extends RuntimeException {
+    TestExtraLogicManagerSuccessException(String message) {
+      super(message);
+    }
+  }
+
   @Override
   public List<String> getExtensionFlags(
       DomainResource domainResource, String clientId, DateTime asOfDate) {
@@ -221,6 +234,6 @@ public class TestExtraLogicManager implements RegistryExtraFlowLogic {
   public void commitAdditionalLogicChanges() {
     checkNotNull(messageToThrow);
     // Throw a specific exception as a signal to the test code that we made it through to here.
-    throw new IllegalArgumentException(messageToThrow);
+    throw new TestExtraLogicManagerSuccessException(messageToThrow);
   }
 }