Replace command.applyTo() with inlined builder operations

These are much easier to understand.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=137446273

javatests/google/registry/flows/contact/ContactUpdateFlowTest.java

@@ -24,14 +24,14 @@ import static google.registry.testing.DatastoreHelper.persistResource;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import google.registry.flows.ResourceFlowTestCase;
+import google.registry.flows.ResourceFlowUtils.AddRemoveSameValueException;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
 import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
+import google.registry.flows.ResourceFlowUtils.StatusNotClientSettableException;
 import google.registry.flows.contact.ContactFlowUtils.BadInternationalizedPostalInfoException;
 import google.registry.flows.contact.ContactFlowUtils.DeclineContactDisclosureFieldDisallowedPolicyException;
-import google.registry.flows.exceptions.AddRemoveSameValueEppException;
 import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException;
 import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
-import google.registry.flows.exceptions.StatusNotClientSettableException;
 import google.registry.model.contact.ContactAddress;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.contact.PostalInfo;
@@ -84,7 +84,7 @@ public class ContactUpdateFlowTest
   }
 
   @Test
-  public void testSuccess_updatingOnePostalInfoDeletesTheOther() throws Exception {
+  public void testSuccess_updatingInternationalizedPostalInfoDeletesLocalized() throws Exception {
     ContactResource contact =
         persistResource(
             newContactResource(getUniqueIdFromCommand()).asBuilder()
@@ -104,12 +104,60 @@ public class ContactUpdateFlowTest
     // the localized one since they are treated as a pair for update purposes.
     assertAboutContacts().that(contact)
         .hasNonNullLocalizedPostalInfo().and()
-        .hasInternationalizedPostalInfo(null);
+        .hasNullInternationalizedPostalInfo();
 
     runFlowAssertResponse(readFile("contact_update_response.xml"));
     assertAboutContacts().that(reloadResourceByForeignKey())
-        .hasLocalizedPostalInfo(null).and()
-        .hasNonNullInternationalizedPostalInfo();
+        .hasNullLocalizedPostalInfo().and()
+        .hasInternationalizedPostalInfo(new PostalInfo.Builder()
+            .setType(Type.INTERNATIONALIZED)
+            .setAddress(new ContactAddress.Builder()
+                .setStreet(ImmutableList.of("124 Example Dr.", "Suite 200"))
+                .setCity("Dulles")
+                .setState("VA")
+                .setZip("20166-6503")
+                .setCountryCode("US")
+                .build())
+            .build());
+  }
+
+  @Test
+  public void testSuccess_updatingLocalizedPostalInfoDeletesInternationalized() throws Exception {
+    setEppInput("contact_update_localized.xml");
+    ContactResource contact =
+        persistResource(
+            newContactResource(getUniqueIdFromCommand()).asBuilder()
+                .setInternationalizedPostalInfo(new PostalInfo.Builder()
+                    .setType(Type.INTERNATIONALIZED)
+                    .setAddress(new ContactAddress.Builder()
+                        .setStreet(ImmutableList.of("111 8th Ave", "4th Floor"))
+                        .setCity("New York")
+                        .setState("NY")
+                        .setZip("10011")
+                        .setCountryCode("US")
+                        .build())
+                    .build())
+                .build());
+    clock.advanceOneMilli();
+    // The test xml updates the localized postal info and should therefore implicitly delete
+    // the internationalized one since they are treated as a pair for update purposes.
+    assertAboutContacts().that(contact)
+        .hasNonNullInternationalizedPostalInfo().and()
+        .hasNullLocalizedPostalInfo();
+
+    runFlowAssertResponse(readFile("contact_update_response.xml"));
+    assertAboutContacts().that(reloadResourceByForeignKey())
+        .hasNullInternationalizedPostalInfo().and()
+        .hasLocalizedPostalInfo(new PostalInfo.Builder()
+            .setType(Type.LOCALIZED)
+            .setAddress(new ContactAddress.Builder()
+                .setStreet(ImmutableList.of("124 Example Dr.", "Suite 200"))
+                .setCity("Dulles")
+                .setState("VA")
+                .setZip("20166-6503")
+                .setCountryCode("US")
+                .build())
+            .build());
   }
 
   @Test
@@ -148,6 +196,73 @@ public class ContactUpdateFlowTest
             .build());
   }
 
+
+  @Test
+  public void testSuccess_updateOnePostalInfo_touchOtherPostalInfoPreservesIt() throws Exception {
+    setEppInput("contact_update_partial_postalinfo_preserve_int.xml");
+    persistResource(
+        newContactResource(getUniqueIdFromCommand()).asBuilder()
+        .setLocalizedPostalInfo(new PostalInfo.Builder()
+            .setType(Type.LOCALIZED)
+            .setName("A. Person")
+            .setOrg("Company Inc.")
+            .setAddress(new ContactAddress.Builder()
+                .setStreet(ImmutableList.of("123 4th st", "5th Floor"))
+                .setCity("City")
+                .setState("AB")
+                .setZip("12345")
+                .setCountryCode("US")
+                .build())
+            .build())
+        .setInternationalizedPostalInfo(new PostalInfo.Builder()
+            .setType(Type.INTERNATIONALIZED)
+            .setName("B. Person")
+            .setOrg("Company Co.")
+            .setAddress(new ContactAddress.Builder()
+                .setStreet(ImmutableList.of("100 200th Dr.", "6th Floor"))
+                .setCity("Town")
+                .setState("CD")
+                .setZip("67890")
+                .setCountryCode("US")
+                .build())
+            .build())
+        .build());
+    clock.advanceOneMilli();
+    // The test xml updates the address of the localized postal info. It also sets the name of the
+    // internationalized postal info to the same value it previously had, which causes it to be
+    // preserved. If the xml had not mentioned the internationalized one at all it would have been
+    // deleted.
+    runFlowAssertResponse(readFile("contact_update_response.xml"));
+    assertAboutContacts().that(reloadResourceByForeignKey())
+        .hasLocalizedPostalInfo(
+            new PostalInfo.Builder()
+                .setType(Type.LOCALIZED)
+                .setName("A. Person")
+                .setOrg("Company Inc.")
+                .setAddress(new ContactAddress.Builder()
+                    .setStreet(ImmutableList.of("456 5th st"))
+                    .setCity("Place")
+                    .setState("CD")
+                    .setZip("54321")
+                    .setCountryCode("US")
+                    .build())
+                .build())
+        .and()
+        .hasInternationalizedPostalInfo(
+            new PostalInfo.Builder()
+                .setType(Type.INTERNATIONALIZED)
+                .setName("B. Person")
+                .setOrg("Company Co.")
+                .setAddress(new ContactAddress.Builder()
+                    .setStreet(ImmutableList.of("100 200th Dr.", "6th Floor"))
+                    .setCity("Town")
+                    .setState("CD")
+                    .setZip("67890")
+                    .setCountryCode("US")
+                    .build())
+                .build());
+  }
+
   @Test
   public void testFailure_neverExisted() throws Exception {
     thrown.expect(
@@ -264,7 +379,7 @@ public class ContactUpdateFlowTest
   public void testFailure_addRemoveSameValue() throws Exception {
     setEppInput("contact_update_add_remove_same.xml");
     persistActiveContact(getUniqueIdFromCommand());
-    thrown.expect(AddRemoveSameValueEppException.class);
+    thrown.expect(AddRemoveSameValueException.class);
     runFlow();
   }
 }

javatests/google/registry/flows/contact/testdata/contact_update_localized.xml

@@ -0,0 +1,36 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <update>
+      <contact:update
+       xmlns:contact="urn:ietf:params:xml:ns:contact-1.0">
+        <contact:id>sh8013</contact:id>
+        <contact:add>
+          <contact:status s="clientDeleteProhibited"/>
+        </contact:add>
+        <contact:chg>
+          <contact:postalInfo type="loc">
+            <contact:org/>
+            <contact:addr>
+              <contact:street>124 Example Dr.</contact:street>
+              <contact:street>Suite 200</contact:street>
+              <contact:city>Dulles</contact:city>
+              <contact:sp>VA</contact:sp>
+              <contact:pc>20166-6503</contact:pc>
+              <contact:cc>US</contact:cc>
+            </contact:addr>
+          </contact:postalInfo>
+          <contact:voice>+1.7034444444</contact:voice>
+          <contact:fax/>
+          <contact:authInfo>
+            <contact:pw>2fooBAR</contact:pw>
+          </contact:authInfo>
+          <contact:disclose flag="1">
+            <contact:voice/>
+            <contact:email/>
+          </contact:disclose>
+        </contact:chg>
+      </contact:update>
+    </update>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>

javatests/google/registry/flows/contact/testdata/contact_update_partial_postalinfo_preserve_int.xml

@@ -0,0 +1,25 @@
+<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
+  <command>
+    <update>
+      <contact:update
+       xmlns:contact="urn:ietf:params:xml:ns:contact-1.0">
+        <contact:id>sh8013</contact:id>
+        <contact:chg>
+          <contact:postalInfo type="loc">
+            <contact:addr>
+              <contact:street>456 5th st</contact:street>
+              <contact:city>Place</contact:city>
+              <contact:sp>CD</contact:sp>
+              <contact:pc>54321</contact:pc>
+              <contact:cc>US</contact:cc>
+            </contact:addr>
+          </contact:postalInfo>
+          <contact:postalInfo type="int">
+            <contact:org>Company Co.</contact:org>
+          </contact:postalInfo>
+        </contact:chg>
+      </contact:update>
+    </update>
+    <clTRID>ABC-12345</clTRID>
+  </command>
+</epp>

javatests/google/registry/flows/domain/DomainApplicationUpdateFlowTest.java

@@ -32,8 +32,10 @@ import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
 import google.registry.flows.EppException.UnimplementedExtensionException;
 import google.registry.flows.ResourceFlowTestCase;
+import google.registry.flows.ResourceFlowUtils.AddRemoveSameValueException;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
 import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
+import google.registry.flows.ResourceFlowUtils.StatusNotClientSettableException;
 import google.registry.flows.domain.DomainApplicationUpdateFlow.ApplicationStatusProhibitsUpdateException;
 import google.registry.flows.domain.DomainFlowUtils.ApplicationDomainNameMismatchException;
 import google.registry.flows.domain.DomainFlowUtils.DuplicateContactForRoleException;
@@ -50,10 +52,8 @@ import google.registry.flows.domain.DomainFlowUtils.SecDnsAllUsageException;
 import google.registry.flows.domain.DomainFlowUtils.TooManyDsRecordsException;
 import google.registry.flows.domain.DomainFlowUtils.TooManyNameserversException;
 import google.registry.flows.domain.DomainFlowUtils.UrgentAttributeNotSupportedException;
-import google.registry.flows.exceptions.AddRemoveSameValueEppException;
 import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException;
 import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
-import google.registry.flows.exceptions.StatusNotClientSettableException;
 import google.registry.model.contact.ContactResource;
 import google.registry.model.domain.DesignatedContact;
 import google.registry.model.domain.DesignatedContact.Type;
@@ -587,7 +587,7 @@ public class DomainApplicationUpdateFlowTest
         .setNameservers(ImmutableSet.of(Key.create(
             loadByForeignKey(HostResource.class, "ns1.example.tld", clock.nowUtc()))))
         .build());
-    thrown.expect(AddRemoveSameValueEppException.class);
+    thrown.expect(AddRemoveSameValueException.class);
     runFlow();
   }
 
@@ -601,7 +601,7 @@ public class DomainApplicationUpdateFlowTest
             Key.create(
                 loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc())))))
         .build());
-    thrown.expect(AddRemoveSameValueEppException.class);
+    thrown.expect(AddRemoveSameValueException.class);
     runFlow();
   }
 

javatests/google/registry/flows/domain/DomainUpdateFlowTest.java

@@ -44,8 +44,10 @@ import com.googlecode.objectify.Key;
 import google.registry.flows.EppException.UnimplementedExtensionException;
 import google.registry.flows.EppRequestSource;
 import google.registry.flows.ResourceFlowTestCase;
+import google.registry.flows.ResourceFlowUtils.AddRemoveSameValueException;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
 import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
+import google.registry.flows.ResourceFlowUtils.StatusNotClientSettableException;
 import google.registry.flows.domain.DomainFlowUtils.DuplicateContactForRoleException;
 import google.registry.flows.domain.DomainFlowUtils.EmptySecDnsUpdateException;
 import google.registry.flows.domain.DomainFlowUtils.FeesMismatchException;
@@ -64,11 +66,9 @@ import google.registry.flows.domain.DomainFlowUtils.SecDnsAllUsageException;
 import google.registry.flows.domain.DomainFlowUtils.TooManyDsRecordsException;
 import google.registry.flows.domain.DomainFlowUtils.TooManyNameserversException;
 import google.registry.flows.domain.DomainFlowUtils.UrgentAttributeNotSupportedException;
-import google.registry.flows.exceptions.AddRemoveSameValueEppException;
 import google.registry.flows.exceptions.OnlyToolCanPassMetadataException;
 import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException;
 import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
-import google.registry.flows.exceptions.StatusNotClientSettableException;
 import google.registry.model.billing.BillingEvent;
 import google.registry.model.billing.BillingEvent.Reason;
 import google.registry.model.contact.ContactResource;
@@ -1003,7 +1003,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
             .setNameservers(ImmutableSet.of(Key.create(
                 loadByForeignKey(HostResource.class, "ns1.example.foo", clock.nowUtc()))))
             .build());
-    thrown.expect(AddRemoveSameValueEppException.class);
+    thrown.expect(AddRemoveSameValueException.class);
     runFlow();
   }
 
@@ -1018,7 +1018,7 @@ public class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow,
                 Key.create(
                     loadByForeignKey(ContactResource.class, "sh8013", clock.nowUtc())))))
             .build());
-    thrown.expect(AddRemoveSameValueEppException.class);
+    thrown.expect(AddRemoveSameValueException.class);
     runFlow();
   }
 

javatests/google/registry/flows/host/HostUpdateFlowTest.java

@@ -14,6 +14,7 @@
 
 package google.registry.flows.host;
 
+import static com.google.common.base.Strings.nullToEmpty;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.flows.async.RefreshDnsOnHostRenameAction.QUEUE_ASYNC_HOST_RENAME;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
@@ -41,11 +42,12 @@ import com.google.common.net.InetAddresses;
 import com.googlecode.objectify.Key;
 import google.registry.flows.EppRequestSource;
 import google.registry.flows.ResourceFlowTestCase;
+import google.registry.flows.ResourceFlowUtils.AddRemoveSameValueException;
 import google.registry.flows.ResourceFlowUtils.ResourceDoesNotExistException;
 import google.registry.flows.ResourceFlowUtils.ResourceNotOwnedException;
+import google.registry.flows.ResourceFlowUtils.StatusNotClientSettableException;
 import google.registry.flows.exceptions.ResourceHasClientUpdateProhibitedException;
 import google.registry.flows.exceptions.ResourceStatusProhibitsOperationException;
-import google.registry.flows.exceptions.StatusNotClientSettableException;
 import google.registry.flows.host.HostFlowUtils.HostNameTooShallowException;
 import google.registry.flows.host.HostFlowUtils.InvalidHostNameException;
 import google.registry.flows.host.HostFlowUtils.SuperordinateDomainDoesNotExistException;
@@ -69,14 +71,14 @@ import org.junit.Test;
 public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, HostResource> {
 
   private void setEppHostUpdateInput(
-      String oldHostName, String newHostName, String addHostAddrs, String remHostAddrs) {
+      String oldHostName, String newHostName, String ipOrStatusToAdd, String ipOrStatusToRem) {
     setEppInput(
         "host_update.xml",
         ImmutableMap.of(
             "OLD-HOSTNAME", oldHostName,
             "NEW-HOSTNAME", newHostName,
-            "ADD-HOSTADDRS", (addHostAddrs == null) ? "" : addHostAddrs,
-            "REM-HOSTADDRS", (remHostAddrs == null) ? "" : remHostAddrs));
+            "ADD-HOSTADDRSORSTATUS", nullToEmpty(ipOrStatusToAdd),
+            "REM-HOSTADDRSORSTATUS", nullToEmpty(ipOrStatusToRem)));
   }
 
   public HostUpdateFlowTest() {
@@ -793,6 +795,34 @@ public class HostUpdateFlowTest extends ResourceFlowTestCase<HostUpdateFlow, Hos
     runFlow();
   }
 
+  @Test
+  public void testFailure_addRemoveSameStatusValues() throws Exception {
+    createTld("tld");
+    persistActiveDomain("example.tld");
+    setEppHostUpdateInput(
+        "ns1.example.tld",
+        "ns2.example.tld",
+        "<host:status s=\"clientUpdateProhibited\"/>",
+        "<host:status s=\"clientUpdateProhibited\"/>");
+    persistActiveHost(oldHostName());
+    thrown.expect(AddRemoveSameValueException.class);
+    runFlow();
+  }
+
+  @Test
+  public void testFailure_addRemoveSameInetAddresses() throws Exception {
+    createTld("tld");
+    persistActiveDomain("example.tld");
+    setEppHostUpdateInput(
+        "ns1.example.tld",
+        "ns2.example.tld",
+        "<host:addr ip=\"v4\">192.0.2.22</host:addr>",
+        "<host:addr ip=\"v4\">192.0.2.22</host:addr>");
+    persistActiveHost(oldHostName());
+    thrown.expect(AddRemoveSameValueException.class);
+    runFlow();
+  }
+
   @Test
   public void testFailure_clientProhibitedStatusValue() throws Exception {
     createTld("tld");

javatests/google/registry/flows/host/testdata/host_update.xml

@@ -5,11 +5,11 @@
        xmlns:host="urn:ietf:params:xml:ns:host-1.0">
         <host:name>%OLD-HOSTNAME%</host:name>
         <host:add>
-          %ADD-HOSTADDRS%
+          %ADD-HOSTADDRSORSTATUS%
           <host:status s="clientUpdateProhibited"/>
         </host:add>
         <host:rem>
-          %REM-HOSTADDRS%
+          %REM-HOSTADDRSORSTATUS%
         </host:rem>
         <host:chg>
           <host:name>%NEW-HOSTNAME%</host:name>

javatests/google/registry/model/contact/ContactCommandTest.java

@@ -16,14 +16,10 @@ package google.registry.model.contact;
 
 import static google.registry.flows.EppXmlTransformer.marshalInput;
 import static google.registry.flows.EppXmlTransformer.validateInput;
-import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
-import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.xml.ValidationMode.LENIENT;
 import static google.registry.xml.XmlTestUtils.assertXmlEquals;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import com.google.common.collect.ImmutableList;
-import google.registry.model.contact.PostalInfo.Type;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.EppLoader;
 import org.junit.Rule;
@@ -103,75 +99,4 @@ public class ContactCommandTest {
   public void testTransferRequest() throws Exception {
     doXmlRoundtripTest("contact_transfer_request.xml");
   }
-
-  @Test
-  public void testPostalInfoOverlay() {
-    createTld("foo");
-
-    ContactResource contact = new ContactResource.Builder()
-        .setLocalizedPostalInfo(new PostalInfo.Builder()
-            .setType(Type.LOCALIZED)
-            .setName("loc name")
-            .build())
-        .setInternationalizedPostalInfo(new PostalInfo.Builder()
-            .setType(Type.INTERNATIONALIZED)
-            .setName("int name")
-            .build())
-        .build();
-    ContactCommand.Update.Change change = new ContactCommand.Update.Change();
-
-    // Updating one field of the loc should delete the int and leave the loc otherwise untouched.
-    change.postalInfo = ImmutableList.of(new PostalInfo.Builder()
-        .setType(Type.LOCALIZED)
-        .setOrg("org")
-        .build());
-    ContactResource.Builder builder = contact.asBuilder();
-    change.applyTo(builder);
-    ContactResource changed = builder.build();
-    assertAboutContacts().that(changed)
-        .hasNullInternationalizedPostalInfo().and()
-        .hasLocalizedPostalInfo(new PostalInfo.Builder()
-            .setType(Type.LOCALIZED)
-            .setName("loc name")
-            .setOrg("org")
-            .build());
-
-    // Updating one field of the int should delete the loc and leave the int otherwise untouched.
-    change.postalInfo = ImmutableList.of(new PostalInfo.Builder()
-        .setType(Type.INTERNATIONALIZED)
-        .setOrg("org")
-        .build());
-    builder = contact.asBuilder();
-    change.applyTo(builder);
-    changed = builder.build();
-    assertAboutContacts().that(changed)
-        .hasNullLocalizedPostalInfo().and()
-        .hasInternationalizedPostalInfo(new PostalInfo.Builder()
-            .setType(Type.INTERNATIONALIZED)
-            .setName("int name")
-            .setOrg("org")
-            .build());
-
-    // Updating one field of the int and touching the loc with no changes should preserve both.
-    change.postalInfo = ImmutableList.of(
-        new PostalInfo.Builder()
-            .setType(Type.INTERNATIONALIZED)
-            .setName("new int name")
-            .build(),
-        new PostalInfo.Builder()
-            .setType(Type.LOCALIZED)
-            .build());
-    builder = contact.asBuilder();
-    change.applyTo(builder);
-    changed = builder.build();
-    assertAboutContacts().that(changed)
-        .hasLocalizedPostalInfo(new PostalInfo.Builder()
-            .setType(Type.LOCALIZED)
-            .setName("loc name")
-            .build()).and()
-        .hasInternationalizedPostalInfo(new PostalInfo.Builder()
-            .setType(Type.INTERNATIONALIZED)
-            .setName("new int name")
-            .build());
-  }
 }