Make name and address fields required on Registrar

The absence of these fields causes RDE failures, so they are in effect
required on any functioning registry system. We are currently
experiencing problems in sandbox caused by null values on these fields.

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

javatests/google/registry/export/sheet/SyncRegistrarsSheetTest.java

@@ -22,6 +22,7 @@ import static google.registry.model.common.Cursor.CursorType.SYNC_REGISTRAR_SHEE
 import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.deleteResource;
+import static google.registry.testing.DatastoreHelper.persistNewRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DatastoreHelper.persistSimpleResources;
 import static org.joda.time.DateTimeZone.UTC;
@@ -94,13 +95,7 @@ public class SyncRegistrarsSheetTest {
 
   @Test
   public void test_wereRegistrarsModified_atDifferentCursorTimes() throws Exception {
-    persistResource(new Registrar.Builder()
-        .setClientId("SomeRegistrar")
-        .setRegistrarName("Some Registrar Inc.")
-        .setType(Registrar.Type.REAL)
-        .setIanaIdentifier(8L)
-        .setState(Registrar.State.ACTIVE)
-        .build());
+    persistNewRegistrar("SomeRegistrar", "Some Registrar Inc.", Registrar.Type.REAL, 8L);
     persistResource(Cursor.createGlobal(SYNC_REGISTRAR_SHEET, clock.nowUtc().minusHours(1)));
     assertThat(newSyncRegistrarsSheet().wereRegistrarsModified()).isTrue();
     persistResource(Cursor.createGlobal(SYNC_REGISTRAR_SHEET, clock.nowUtc().plusHours(1)));
@@ -327,18 +322,14 @@ public class SyncRegistrarsSheetTest {
 
   @Test
   public void testRun_missingValues_stillWorks() throws Exception {
-    persistResource(new Registrar.Builder()
-        .setClientId("SomeRegistrar")
-        .setType(Registrar.Type.REAL)
-        .setIanaIdentifier(8L)
-        .build());
+    persistNewRegistrar("SomeRegistrar", "Some Registrar", Registrar.Type.REAL, 8L);
 
     newSyncRegistrarsSheet().run("foobar");
 
     verify(sheetSynchronizer).synchronize(eq("foobar"), rowsCaptor.capture());
     ImmutableMap<String, String> row = getOnlyElement(getOnlyElement(rowsCaptor.getAllValues()));
     assertThat(row).containsEntry("clientIdentifier", "SomeRegistrar");
-    assertThat(row).containsEntry("registrarName", "");
+    assertThat(row).containsEntry("registrarName", "Some Registrar");
     assertThat(row).containsEntry("state", "");
     assertThat(row).containsEntry("ianaIdentifier", "8");
     assertThat(row).containsEntry("billingIdentifier", "");
@@ -352,8 +343,8 @@ public class SyncRegistrarsSheetTest {
     assertThat(row).containsEntry("contactsMarkedAsWhoisAdmin", "");
     assertThat(row).containsEntry("contactsMarkedAsWhoisTech", "");
     assertThat(row).containsEntry("emailAddress", "");
-    assertThat(row).containsEntry("address.street", "UNKNOWN");
-    assertThat(row).containsEntry("address.city", "UNKNOWN");
+    assertThat(row).containsEntry("address.street", "123 Fake St");
+    assertThat(row).containsEntry("address.city", "Fakington");
     assertThat(row).containsEntry("address.state", "");
     assertThat(row).containsEntry("address.zip", "");
     assertThat(row).containsEntry("address.countryCode", "US");

javatests/google/registry/model/ofy/OfyFilterTest.java

@@ -16,6 +16,7 @@ package google.registry.model.ofy;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ofy.ObjectifyService.initOfy;
+import static google.registry.testing.DatastoreHelper.newContactResource;
 import static org.junit.Assert.fail;
 
 import com.google.appengine.tools.development.testing.LocalDatastoreServiceTestConfig;
@@ -26,8 +27,7 @@ import com.googlecode.objectify.ObjectifyFilter;
 import com.googlecode.objectify.ObjectifyService;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
-import google.registry.model.registrar.Registrar;
-import google.registry.model.registrar.Registrar.Type;
+import google.registry.model.contact.ContactResource;
 import google.registry.testing.ExceptionRule;
 import org.junit.After;
 import org.junit.Before;
@@ -93,9 +93,8 @@ public class OfyFilterTest {
   @Test
   public void testKeyCreateAfterFilter() throws Exception {
     new OfyFilter().init(null);
-    Registrar registrar =
-        new Registrar.Builder().setType(Type.TEST).setClientId("clientId").build();
-    Key.create(registrar);
+    ContactResource contact = newContactResource("contact1234");
+    Key.create(contact);
   }
 
   @Entity

javatests/google/registry/model/registrar/RegistrarTest.java

@@ -314,8 +314,26 @@ public class RegistrarTest extends EntityTestCase {
 
   @Test
   public void testFailure_missingRegistrarType() throws Exception {
-    thrown.expect(NullPointerException.class);
-    new Registrar.Builder().build();
+    thrown.expect(IllegalArgumentException.class, "Registrar type cannot be null");
+    new Registrar.Builder().setRegistrarName("blah").build();
+  }
+
+  @Test
+  public void testFailure_missingRegistrarName() throws Exception {
+    thrown.expect(IllegalArgumentException.class, "Registrar name cannot be null");
+    new Registrar.Builder().setClientId("blahid").setType(Registrar.Type.TEST).build();
+  }
+
+  @Test
+  public void testFailure_missingAddress() throws Exception {
+    thrown.expect(
+        IllegalArgumentException.class,
+        "Must specify at least one of localized or internationalized address");
+    new Registrar.Builder()
+        .setClientId("blahid")
+        .setType(Registrar.Type.TEST)
+        .setRegistrarName("Blah Co")
+        .build();
   }
 
   @Test

javatests/google/registry/rde/RdeMarshallerTest.java

@@ -20,8 +20,6 @@ import google.registry.model.registrar.Registrar;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ShardableTestCase;
 import google.registry.xml.XmlTestUtils;
-import org.joda.time.DateTime;
-import org.joda.time.format.ISODateTimeFormat;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -82,46 +80,10 @@ public class RdeMarshallerTest extends ShardableTestCase {
         "registrar.upDate");
   }
 
-  @Test
-  public void testMarshalRegistrar_breaksRdeXmlSchema_producesErrorMessage() throws Exception {
-    Registrar reg = Registrar.loadByClientId("TheRegistrar").asBuilder()
-        .setLocalizedAddress(null)
-        .setInternationalizedAddress(null)
-        .build();
-    DepositFragment fragment = new RdeMarshaller().marshalRegistrar(reg);
-    assertThat(fragment.type()).isEqualTo(RdeResourceType.REGISTRAR);
-    assertThat(fragment.xml()).isEmpty();
-    assertThat(fragment.error()).isEqualTo(""
-        + "RDE XML schema validation failed: "
-            + "Key<?>(EntityGroupRoot(\"cross-tld\")/Registrar(\"TheRegistrar\"))\n"
-        + "org.xml.sax.SAXParseException; lineNumber: 0; columnNumber: 0; cvc-complex-type.2.4.a: "
-            + "Invalid content was found starting with element 'rdeRegistrar:voice'. "
-            + "One of '{\"urn:ietf:params:xml:ns:rdeRegistrar-1.0\":postalInfo}' is expected.\n"
-        + "<rdeRegistrar:registrar>\n"
-        + "    <rdeRegistrar:id>TheRegistrar</rdeRegistrar:id>\n"
-        + "    <rdeRegistrar:name>The Registrar</rdeRegistrar:name>\n"
-        + "    <rdeRegistrar:gurid>1</rdeRegistrar:gurid>\n"
-        + "    <rdeRegistrar:status>ok</rdeRegistrar:status>\n"
-        + "    <rdeRegistrar:voice>+1.2223334444</rdeRegistrar:voice>\n"
-        + "    <rdeRegistrar:email>new.registrar@example.com</rdeRegistrar:email>\n"
-        + "    <rdeRegistrar:whoisInfo>\n"
-        + "        <rdeRegistrar:name>whois.nic.fakewhois.example</rdeRegistrar:name>\n"
-        + "    </rdeRegistrar:whoisInfo>\n"
-        + "    <rdeRegistrar:crDate>" + ft(reg.getCreationTime()) + "</rdeRegistrar:crDate>\n"
-        + "    <rdeRegistrar:upDate>" + ft(reg.getLastUpdateTime()) + "</rdeRegistrar:upDate>\n"
-        + "</rdeRegistrar:registrar>\n"
-        + "\n");
-  }
-
   @Test
   public void testMarshalRegistrar_unicodeCharacters_dontGetMangled() throws Exception {
     DepositFragment fragment =
         new RdeMarshaller().marshalRegistrar(Registrar.loadByClientId("TheRegistrar"));
     assertThat(fragment.xml()).contains("123 Example Bőulevard");
   }
-
-  /** Formats {@code timestamp} without milliseconds. */
-  private static String ft(DateTime timestamp) {
-    return ISODateTimeFormat.dateTimeNoMillis().withZoneUTC().print(timestamp);
-  }
 }

javatests/google/registry/rde/imports/RdeImportUtilsTest.java

@@ -47,6 +47,7 @@ import google.registry.model.host.HostResource;
 import google.registry.model.index.EppResourceIndex;
 import google.registry.model.index.EppResourceIndexBucket;
 import google.registry.model.index.ForeignKeyIndex;
+import google.registry.model.registrar.Registrar;
 import google.registry.model.registry.Registry.TldState;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
@@ -96,7 +97,7 @@ public class RdeImportUtilsTest extends ShardableTestCase {
     rdeImportUtils = new RdeImportUtils(ofy(), clock, "import-bucket", gcsUtils);
     createTld("test", TldState.PREDELEGATION);
     createTld("getld", TldState.GENERAL_AVAILABILITY);
-    persistNewRegistrar("RegistrarX", 1L);
+    persistNewRegistrar("RegistrarX", "RegistrarX", Registrar.Type.REAL, 1L);
   }
 
   @After
@@ -287,8 +288,8 @@ public class RdeImportUtilsTest extends ShardableTestCase {
         .setContacts(ImmutableSet.of(
             DesignatedContact.create(Type.ADMIN, Key.create(admin)),
             DesignatedContact.create(Type.TECH, Key.create(admin))))
-        .setPersistedCurrentSponsorClientId("RegistrarX")
-        .setCreationClientId("RegistrarX")
+        .setPersistedCurrentSponsorClientId("registrarx")
+        .setCreationClientId("registrarx")
         .setCreationTime(DateTime.parse("1999-04-03T22:00:00.0Z"))
         .setRegistrationExpirationTime(DateTime.parse("2015-04-03T22:00:00.0Z"))
         .build();

javatests/google/registry/testing/DatastoreHelper.java

@@ -83,6 +83,7 @@ import google.registry.model.ofy.ObjectifyService;
 import google.registry.model.poll.PollMessage;
 import google.registry.model.pricing.StaticPremiumListPricingEngine;
 import google.registry.model.registrar.Registrar;
+import google.registry.model.registrar.RegistrarAddress;
 import google.registry.model.registry.Registry;
 import google.registry.model.registry.Registry.TldState;
 import google.registry.model.registry.Registry.TldType;
@@ -611,14 +612,22 @@ public class DatastoreHelper {
         .build());
   }
 
-  /** Creates a stripped-down {@link Registrar} with the specified clientId and ianaIdentifier */
-  public static Registrar persistNewRegistrar(String clientId, long ianaIdentifier) {
+  /** Persists and returns a {@link Registrar} with the specified attributes. */
+  public static Registrar persistNewRegistrar(
+      String clientId, String registrarName, Registrar.Type type, long ianaIdentifier) {
     return persistSimpleResource(
         new Registrar.Builder()
-          .setClientId(clientId)
-          .setType(Registrar.Type.REAL)
-          .setIanaIdentifier(ianaIdentifier)
-          .build());
+            .setClientId(clientId)
+            .setRegistrarName(registrarName)
+            .setType(type)
+            .setIanaIdentifier(ianaIdentifier)
+            .setLocalizedAddress(
+                new RegistrarAddress.Builder()
+                    .setStreet(ImmutableList.of("123 Fake St"))
+                    .setCity("Fakington")
+                    .setCountryCode("US")
+                    .build())
+            .build());
   }
 
   private static Iterable<BillingEvent> getBillingEvents() {

javatests/google/registry/testing/FullFieldsTestEntityHelper.java

@@ -130,7 +130,8 @@ public final class FullFieldsTestEntityHelper {
     HostResource.Builder builder = new HostResource.Builder()
         .setRepoId(generateNewContactHostRoid())
         .setFullyQualifiedHostName(Idn.toASCII(fqhn))
-        .setCreationTimeForTest(DateTime.parse("2000-10-08T00:45:00Z"));
+        .setCreationTimeForTest(DateTime.parse("2000-10-08T00:45:00Z"))
+        .setPersistedCurrentSponsorClientId("TheRegistrar");
     if ((ip1 != null) || (ip2 != null)) {
       ImmutableSet.Builder<InetAddress> ipBuilder = new ImmutableSet.Builder<>();
       if (ip1 != null) {

javatests/google/registry/tools/LoadTestCommandTest.java

@@ -23,6 +23,7 @@ import static org.mockito.Mockito.verifyZeroInteractions;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.net.MediaType;
+import google.registry.model.registrar.Registrar;
 import google.registry.tools.ServerSideCommand.Connection;
 import org.junit.Before;
 import org.junit.Test;
@@ -37,7 +38,7 @@ public class LoadTestCommandTest extends CommandTestCase<LoadTestCommand> {
   public void setUp() throws Exception {
     command.setConnection(connection);
     createTld("example");
-    persistNewRegistrar("acme", 99);
+    persistNewRegistrar("acme", "ACME", Registrar.Type.REAL, 99L);
   }
 
   @Test

javatests/google/registry/tools/MutatingCommandTest.java

@@ -20,6 +20,7 @@ import static google.registry.model.ofy.ObjectifyService.ofy;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.deleteResource;
 import static google.registry.testing.DatastoreHelper.persistActiveHost;
+import static google.registry.testing.DatastoreHelper.persistNewRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static org.joda.time.DateTimeZone.UTC;
 
@@ -58,16 +59,8 @@ public class MutatingCommandTest {
 
   @Before
   public void init() {
-    registrar1 = persistResource(new Registrar.Builder()
-        .setType(Registrar.Type.REAL)
-        .setClientId("Registrar1")
-        .setIanaIdentifier(1L)
-        .build());
-    registrar2 = persistResource(new Registrar.Builder()
-        .setType(Registrar.Type.REAL)
-        .setClientId("Registrar2")
-        .setIanaIdentifier(2L)
-        .build());
+    registrar1 = persistNewRegistrar("Registrar1", "Registrar1", Registrar.Type.REAL, 1L);
+    registrar2 = persistNewRegistrar("Registrar2", "Registrar2", Registrar.Type.REAL, 2L);
     newRegistrar1 = registrar1.asBuilder().setBillingIdentifier(42L).build();
     newRegistrar2 = registrar2.asBuilder().setBlockPremiumNames(true).build();
 

javatests/google/registry/whois/NameserverWhoisResponseTest.java

@@ -16,7 +16,7 @@ package google.registry.whois;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.testing.DatastoreHelper.createTld;
-import static google.registry.testing.DatastoreHelper.persistResource;
+import static google.registry.testing.DatastoreHelper.persistNewRegistrar;
 import static google.registry.whois.WhoisHelper.loadWhoisTestFile;
 
 import com.google.common.collect.ImmutableList;
@@ -50,13 +50,7 @@ public class NameserverWhoisResponseTest {
 
   @Before
   public void setUp() {
-    persistResource(new Registrar.Builder()
-        .setClientId("example")
-        .setRegistrarName("Example Registrar, Inc.")
-        .setType(Registrar.Type.REAL)
-        .setIanaIdentifier(8L)
-        .build());
-
+    persistNewRegistrar("example", "Example Registrar, Inc.", Registrar.Type.REAL, 8L);
     createTld("tld");
 
     hostResource1 = new HostResource.Builder()

javatests/google/registry/whois/RegistrarWhoisResponseTest.java

@@ -15,6 +15,7 @@
 package google.registry.whois;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.DatastoreHelper.persistNewRegistrar;
 import static google.registry.testing.DatastoreHelper.persistResource;
 import static google.registry.testing.DatastoreHelper.persistSimpleResources;
 import static google.registry.whois.WhoisHelper.loadWhoisTestFile;
@@ -120,12 +121,8 @@ public class RegistrarWhoisResponseTest {
 
   @Test
   public void testSetOfFields() {
-    Registrar registrar = new Registrar.Builder()
-        .setClientId("exregistrar")
-        .setType(Registrar.Type.REAL)
-        .setIanaIdentifier(8L)
-        .setState(Registrar.State.ACTIVE)
-        .build();
+    Registrar registrar =
+        persistNewRegistrar("exregistrar", "Ex-Registrar", Registrar.Type.REAL, 8L);
 
     RegistrarWhoisResponse registrarWhoisResponse =
         new RegistrarWhoisResponse(registrar, clock.nowUtc());

javatests/google/registry/whois/testdata/whois_server_nameserver.txt

@@ -1,6 +1,6 @@
 Server Name: ns1.cat.lol
 IP Address: 1.2.3.4
-Registrar:
+Registrar: The Registrar
 Registrar WHOIS Server: whois.nic.fakewhois.example
 Registrar URL: http://www.referral.example/path
 >>> Last update of WHOIS database: 2009-06-29T20:13:00Z <<<