Remove the old RegistryConfig paradigm entirely

We are now ready to begin configuration using YAML, mediated by ConfigModule.

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

docs/configuration.md

@@ -56,41 +56,15 @@ queues, and thus edit those associated XML files.
 
 ## Global configuration
 
-There are two different mechanisms by which global configuration is managed:
+Global configuration is managed through **`ConfigModule`**, which is a Dagger
-`RegistryConfig` (the old way) and `ConfigModule` (the new way). Ideally there
+module that provides injectable configuration options. Some configuration
-would just be one, but the required code cleanup that hasn't been completed yet.
+options that can be changed in this class include timeout lengths and buffer
-If you are adding new options, prefer adding them to `ConfigModule`.
+sizes for various tasks, email addresses and URLs to use for various services,
-
+more Cloud Storage bucket names, and WHOIS disclaimer text. Currently, in order
-**`RegistryConfig`** is an interface, of which you write an implementing class
+to configure custom configuration, you need to copy `ConfigModule`, make changes
-containing the configuration values. `RegistryConfigLoader` is the class that
+to it, and include your new version instead of the default one in all Dagger
-provides the instance of `RegistryConfig`, and defaults to returning
+components. In the near future, configuration via YAML files will be supported,
-`ProductionRegistryConfigExample`.
+after which point changes to `ConfigModule` will not be required at all.
-
-In order to create a configuration specific to your registry, we recommend
-copying the `ProductionRegistryConfigExample` class to a new class that will not
-be shared publicly, setting the `google.registry.config` system property in the
-`appengine-web.xml` files to the fully qualified class name of that new class so
-that `RegistryConfigLoader` will load it instead, and then editing said new
-class to add your specific configuration options. There is one
-`appengine-web.xml` file per service (so three per environment). The same
-configuration class must be used for each service, but different ones can be
-used for different environments.
-
-The `RegistryConfig` class has documentation on all of the methods that should
-be sufficient to explain what each option is, and
-`ProductionRegistryConfigExample` provides an example value for each one. Some
-example configuration options in this interface include the App Engine project
-ID, the number of days to retain commit logs, the names of various Cloud Storage
-bucket names, and URLs for some required services both external and internal.
-
-**`ConfigModule`** is a Dagger module that provides injectable configuration
-options (some of which come from `RegistryConfig` above, but most of which do
-not). This is preferred over `RegistryConfig` for new configuration options
-because being able to inject configuration options is a nicer pattern that makes
-for cleaner code. Some configuration options that can be changed in this class
-include timeout lengths and buffer sizes for various tasks, email addresses and
-URLs to use for various services, more Cloud Storage bucket names, and WHOIS
-disclaimer text.
 
 ## Sensitive global configuration
 

java/google/registry/config/ConfigModule.java

@@ -52,10 +52,6 @@ import org.joda.time.Duration;
  * in the user's repository. For this to work, other files need to be copied too, such as the
  * {@code @Component} instances under {@code google.registry.module}.  This allows modules to be
  * substituted at the {@code @Component} level.
- *
- * <p>There's also a deprecated configuration class that needs to be overridden and supplied via a
- * system property. See the instructions in {@link ProductionRegistryConfigExample} and
- * {@link RegistryConfigLoader}.
  */
 @Module
 public final class ConfigModule {

java/google/registry/config/ProductionRegistryConfigExample.java

@@ -1,43 +0,0 @@
-// Copyright 2016 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.config;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-
-import com.google.common.base.Optional;
-import javax.annotation.concurrent.Immutable;
-
-/**
- * Default production configuration for global constants that can't be injected.
- *
- * <p><b>Warning:</b> Editing this file in a forked repository is not recommended. The recommended
- * approach is to copy this file, give it a different name, and then change the system property
- * described in the {@link RegistryConfigLoader} documentation.
- */
-@Immutable
-public final class ProductionRegistryConfigExample extends RegistryConfig {
-
-  @SuppressWarnings("unused")
-  private final RegistryEnvironment environment;
-
-  public ProductionRegistryConfigExample(RegistryEnvironment environment) {
-    this.environment = checkNotNull(environment);
-  }
-
-  @Override
-  public Optional<String> getECatcherAddress() {
-    throw new UnsupportedOperationException();  // n/a
-  }
-}

java/google/registry/config/RegistryConfig.java

@@ -17,18 +17,14 @@ package google.registry.config;
 import static google.registry.config.ConfigUtils.makeUrl;
 
 import com.google.common.base.Ascii;
-import com.google.common.base.Optional;
 import com.google.common.net.HostAndPort;
 import java.net.URL;
 import org.joda.time.Duration;
 
 /**
  * Registry configuration for global constants that can't be injected.
- *
- * <p>The goal of this custom configuration system is to have our project environments configured
- * in type-safe Java code that can be refactored, rather than XML files and system properties.
  */
-public abstract class RegistryConfig {
+public final class RegistryConfig {
 
   /**
    * Returns the App Engine project ID, which is based off the environment name.
@@ -104,8 +100,6 @@ public abstract class RegistryConfig {
     }
   }
 
-  public abstract Optional<String> getECatcherAddress();
-
   /**
    * Returns the address of the Nomulus app HTTP server.
    *
@@ -211,5 +205,5 @@ public abstract class RegistryConfig {
     }
   }
 
-  // XXX: Please consider using ConfigModule instead of adding new methods to this file.
+  private RegistryConfig() {}
 }

java/google/registry/config/RegistryConfigLoader.java

@@ -1,76 +0,0 @@
-// Copyright 2016 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.config;
-
-import java.lang.reflect.Constructor;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Modifier;
-import javax.annotation.concurrent.ThreadSafe;
-
-/**
- * System property loader of {@link RegistryConfig} instance.
- *
- * <p>This class reflectively loads the Java class defined by the system property
- * {@value #REGISTRY_CONFIG_PROPERTY} whose default value is {@value #REGISTRY_CONFIG_DEFAULT} and
- * can be set in {@code appengine-web.xml}. Once the class is loaded, its constructor is called,
- * passing the {@link RegistryEnvironment} as a single parameter.
- */
-@ThreadSafe
-@Deprecated // will be replaced by YAML config; see b/33386530 for details
-public final class RegistryConfigLoader {
-
-  public static final String REGISTRY_CONFIG_PROPERTY = "google.registry.config";
-  public static final String REGISTRY_CONFIG_DEFAULT =
-      "google.registry.config.ProductionRegistryConfigExample";
-
-  static RegistryConfig load(RegistryEnvironment environment) {
-    String className = System.getProperty(REGISTRY_CONFIG_PROPERTY, REGISTRY_CONFIG_DEFAULT);
-    Class<?> clazz;
-    try {
-      clazz = Class.forName(className);
-    } catch (ClassNotFoundException e) {
-      throw new RuntimeException(String.format(
-          "Failed to load '%s' as specified by system property '%s'",
-          className, REGISTRY_CONFIG_PROPERTY), e);
-    }
-    if (!RegistryConfig.class.isAssignableFrom(clazz)) {
-      throw new RuntimeException(String.format(
-          "%s does not implement %s",
-          clazz.getSimpleName(), RegistryConfig.class.getSimpleName()));
-    }
-    @SuppressWarnings("unchecked")
-    Class<? extends RegistryConfig> clazzy = (Class<? extends RegistryConfig>) clazz;
-    if (!Modifier.isPublic(clazzy.getModifiers())) {
-      throw new RuntimeException(String.format(
-          "Must be a public class: %s", clazzy.getCanonicalName()));
-    }
-    Constructor<? extends RegistryConfig> constructor;
-    try {
-      constructor = clazzy.getConstructor(RegistryEnvironment.class);
-    } catch (NoSuchMethodException | SecurityException e) {
-      throw new RuntimeException(String.format(
-          "Must have a public constructor(RegistryEnvironment): %s", clazzy.getCanonicalName()), e);
-    }
-    try {
-      return constructor.newInstance(environment);
-    } catch (InvocationTargetException e) {
-      throw new RuntimeException(
-          String.format("%s constructor threw an exception", clazzy.getSimpleName()), e);
-    } catch (InstantiationException | IllegalAccessException | IllegalArgumentException e) {
-      throw new RuntimeException(
-          String.format("Failed to instantiate: %s", clazz.getCanonicalName()), e);
-    }
-  }
-}

java/google/registry/config/RegistryEnvironment.java

@@ -14,9 +14,7 @@
 
 package google.registry.config;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Ascii;
-import javax.annotation.Nullable;
 
 /** Registry environments. */
 public enum RegistryEnvironment {
@@ -54,39 +52,6 @@ public enum RegistryEnvironment {
     return valueOf(Ascii.toUpperCase(System.getProperty(PROPERTY, UNITTEST.name())));
   }
 
-  /**
-   * Returns configuration for this registry environment.
-   *
-   * <p><b>WARNING:</b> Do not store this value to a static field, otherwise you won't be able to
-   * override it for testing. You should instead store the environment object to a static field.
-   */
-  public RegistryConfig config() {
-    if (configOverride != null) {
-      return configOverride;
-    } else if (this == UNITTEST) {
-      return testingConfig;
-    } else {
-      return config;
-    }
-  }
-
-  /** Globally override registry configuration from within a unit test. */
-  @VisibleForTesting
-  @Deprecated
-  public static void overrideConfigurationForTesting(@Nullable RegistryConfig newConfig) {
-    configOverride = newConfig;
-  }
-
-  @Nullable
-  @Deprecated
-  private static RegistryConfig configOverride;
-
-  @Deprecated
-  private static final RegistryConfig testingConfig = new TestRegistryConfig();
-
-  @Deprecated
-  private final RegistryConfig config = RegistryConfigLoader.load(this);
-
   /** System property for configuring which environment we should use. */
   public static final String PROPERTY = "google.registry.environment";
 }

java/google/registry/config/TestRegistryConfig.java

@@ -1,30 +0,0 @@
-// Copyright 2016 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.config;
-
-import com.google.common.base.Optional;
-
-/**
- * An implementation of RegistryConfig for unit testing that contains suitable testing data.
- */
-public class TestRegistryConfig extends RegistryConfig {
-
-  public TestRegistryConfig() {}
-
-  @Override
-  public Optional<String> getECatcherAddress() {
-    throw new UnsupportedOperationException();
-  }
-}

java/google/registry/model/ofy/Ofy.java

@@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Predicates.notNull;
 import static com.google.common.collect.Maps.uniqueIndex;
 import static com.googlecode.objectify.ObjectifyService.ofy;
+import static google.registry.config.RegistryConfig.getBaseOfyRetryDuration;
 import static google.registry.util.CollectionUtils.union;
 import static google.registry.util.ObjectifyUtils.OBJECTS_TO_KEYS;
 
@@ -37,7 +38,6 @@ import com.googlecode.objectify.Work;
 import com.googlecode.objectify.cmd.Deleter;
 import com.googlecode.objectify.cmd.Loader;
 import com.googlecode.objectify.cmd.Saver;
-import google.registry.config.RegistryEnvironment;
 import google.registry.model.annotations.NotBackedUp;
 import google.registry.model.annotations.VirtualEntity;
 import google.registry.model.ofy.ReadOnlyWork.KillTransactionException;
@@ -212,7 +212,7 @@ public class Ofy {
    */
   @VisibleForTesting
   <R> R transactCommitLoggedWork(CommitLoggedWork<R> work) {
-    long baseRetryMillis = RegistryEnvironment.get().config().getBaseOfyRetryDuration().getMillis();
+    long baseRetryMillis = getBaseOfyRetryDuration().getMillis();
     for (long attempt = 0, sleepMillis = baseRetryMillis;
         true;
         attempt++, sleepMillis *= 2) {

java/google/registry/model/registrar/Registrar.java

@@ -24,6 +24,7 @@ import static com.google.common.base.Strings.emptyToNull;
 import static com.google.common.base.Strings.nullToEmpty;
 import static com.google.common.collect.Sets.immutableEnumSet;
 import static com.google.common.io.BaseEncoding.base64;
+import static google.registry.config.RegistryConfig.getRegistrarDefaultReferralUrl;
 import static google.registry.config.RegistryConfig.getRegistrarDefaultWhoisServer;
 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
 import static google.registry.model.ofy.ObjectifyService.ofy;
@@ -53,7 +54,6 @@ import com.googlecode.objectify.annotation.IgnoreSave;
 import com.googlecode.objectify.annotation.Index;
 import com.googlecode.objectify.annotation.Parent;
 import com.googlecode.objectify.condition.IfNull;
-import google.registry.config.RegistryEnvironment;
 import google.registry.model.Buildable;
 import google.registry.model.CreateAutoTimestamp;
 import google.registry.model.ImmutableObject;
@@ -162,8 +162,6 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable
     BRAINTREE;
   }
 
-  private static final RegistryEnvironment ENVIRONMENT = RegistryEnvironment.get();
-
   /** Regex for E.164 phone number format specified by {@code contact.xsd}. */
   private static final Pattern E164_PATTERN = Pattern.compile("\\+[0-9]{1,3}\\.[0-9]{1,14}");
 
@@ -463,10 +461,7 @@ public class Registrar extends ImmutableObject implements Buildable, Jsonifiable
   }
 
   public String getReferralUrl() {
-    if (referralUrl == null) {
+    return firstNonNull(referralUrl, getRegistrarDefaultReferralUrl().toString());
-      return ENVIRONMENT.config().getRegistrarDefaultReferralUrl().toString();
-    }
-    return referralUrl;
   }
 
   public String getIcannReferralEmail() {

java/google/registry/tools/AppEngineConnectionFlags.java

@@ -17,7 +17,7 @@ package google.registry.tools;
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
 import com.google.common.net.HostAndPort;
-import google.registry.config.RegistryEnvironment;
+import google.registry.config.RegistryConfig;
 
 /**
  * Class to contain the configuration flags for AppEngineConnection.
@@ -29,7 +29,7 @@ import google.registry.config.RegistryEnvironment;
 class AppEngineConnectionFlags {
 
   @Parameter(names = "--server", description = "HOST[:PORT] to which remote commands are sent.")
-  private static HostAndPort server = RegistryEnvironment.get().config().getServer();
+  private static HostAndPort server = RegistryConfig.getServer();
 
   HostAndPort getServer() {
     return server;

javatests/google/registry/backup/CommitLogCheckpointStrategyTest.java

@@ -33,7 +33,6 @@ import google.registry.model.registry.Registry;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectRule;
-import google.registry.testing.RegistryConfigRule;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 import org.junit.Before;
@@ -54,9 +53,6 @@ public class CommitLogCheckpointStrategyTest {
   @Rule
   public final InjectRule inject = new InjectRule();
 
-  @Rule
-  public final RegistryConfigRule configRule = new RegistryConfigRule();
-
   final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01TZ"));
   final Ofy ofy = new Ofy(clock);
   final CommitLogCheckpointStrategy strategy = new CommitLogCheckpointStrategy();

javatests/google/registry/backup/RestoreCommitLogsActionTest.java

@@ -49,7 +49,6 @@ import google.registry.model.ofy.CommitLogMutation;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeSleeper;
-import google.registry.testing.RegistryConfigRule;
 import google.registry.testing.TestObject;
 import google.registry.util.Retrier;
 import java.io.ByteArrayOutputStream;
@@ -80,9 +79,6 @@ public class RestoreCommitLogsActionTest {
       .withDatastore()
       .build();
 
-  @Rule
-  public final RegistryConfigRule configRule = new RegistryConfigRule();
-
   @Before
   public void init() {
     ObjectifyService.register(TestObject.class);

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

@@ -32,7 +32,6 @@ import static org.mockito.Mockito.verify;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import google.registry.config.RegistryEnvironment;
 import google.registry.model.common.Cursor;
 import google.registry.model.ofy.Ofy;
 import google.registry.model.registrar.Registrar;
@@ -55,8 +54,6 @@ import org.mockito.runners.MockitoJUnitRunner;
 @RunWith(MockitoJUnitRunner.class)
 public class SyncRegistrarsSheetTest {
 
-  private static final RegistryEnvironment ENVIRONMENT = RegistryEnvironment.get();
-
   @Rule
   public final AppEngineRule appEngine = AppEngineRule.builder()
       .withDatastore()
@@ -345,8 +342,7 @@ public class SyncRegistrarsSheetTest {
     assertThat(row).containsEntry("blockPremiumNames", "false");
     assertThat(row).containsEntry("ipAddressWhitelist", "");
     assertThat(row).containsEntry("url", "");
-    assertThat(row).containsEntry("referralUrl",
+    assertThat(row).containsEntry("referralUrl", getRegistrarDefaultReferralUrl().toString());
-        ENVIRONMENT.config().getRegistrarDefaultReferralUrl().toString());
     assertThat(row).containsEntry("icannReferralEmail", "");
   }
 }

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

@@ -21,7 +21,6 @@ import static org.joda.time.DateTimeZone.UTC;
 import com.google.common.collect.ImmutableMap;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
-import google.registry.testing.RegistryConfigRule;
 import org.joda.time.DateTime;
 import org.junit.Rule;
 import org.junit.Test;
@@ -37,9 +36,6 @@ public class CommitLogCheckpointTest {
       .withDatastore()
       .build();
 
-  @Rule
-  public final RegistryConfigRule configRule = new RegistryConfigRule();
-
   @Rule
   public final ExceptionRule thrown = new ExceptionRule();
 

javatests/google/registry/testing/RegistryConfigRule.java

@@ -1,56 +0,0 @@
-// Copyright 2016 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.testing;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-
-import com.google.common.base.Optional;
-import google.registry.config.RegistryConfig;
-import google.registry.config.RegistryEnvironment;
-import org.junit.rules.ExternalResource;
-
-/** JUnit Rule for overriding Nomulus configuration values. */
-@Deprecated // is obsoleted by YAML config; see b/33386530 for details
-public final class RegistryConfigRule extends ExternalResource {
-
-  private final Optional<RegistryConfig> override;
-
-  /** Creates a new instance where {@link #override(RegistryConfig)} will be called as needed. */
-  public RegistryConfigRule() {
-    this.override = Optional.absent();
-  }
-
-  /** Creates a new instance where {@code override} will be used for each test method. */
-  public RegistryConfigRule(RegistryConfig override) {
-    this.override = Optional.of(override);
-  }
-
-  /** Override registry configuration from inside a test method. */
-  public void override(RegistryConfig override) {
-    RegistryEnvironment.overrideConfigurationForTesting(checkNotNull(override));
-  }
-
-  @Override
-  protected void before() throws Exception {
-    if (override.isPresent()) {
-      RegistryEnvironment.overrideConfigurationForTesting(override.get());
-    }
-  }
-
-  @Override
-  protected void after() {
-    RegistryEnvironment.overrideConfigurationForTesting(null);
-  }
-}

javatests/google/registry/tmch/TmchActionTestCase.java

@@ -26,7 +26,6 @@ import google.registry.testing.BouncyCastleProviderRule;
 import google.registry.testing.ExceptionRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectRule;
-import google.registry.testing.RegistryConfigRule;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.runner.RunWith;
@@ -51,9 +50,6 @@ public class TmchActionTestCase {
   @Rule
   public final BouncyCastleProviderRule bouncy = new BouncyCastleProviderRule();
 
-  @Rule
-  public final RegistryConfigRule configRule = new RegistryConfigRule();
-
   @Rule
   public final ExceptionRule thrown = new ExceptionRule();
 

javatests/google/registry/tmch/TmchXmlSignatureTest.java

@@ -20,7 +20,6 @@ import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.InjectRule;
-import google.registry.testing.RegistryConfigRule;
 import java.security.SignatureException;
 import java.security.cert.CertificateExpiredException;
 import java.security.cert.CertificateNotYetValidException;
@@ -52,9 +51,6 @@ public class TmchXmlSignatureTest {
   @Rule
   public final InjectRule inject = new InjectRule();
 
-  @Rule
-  public final RegistryConfigRule configRule = new RegistryConfigRule();
-
   private final FakeClock clock = new FakeClock(DateTime.parse("2013-11-24T23:15:37.4Z"));
   private byte[] smdData;
   private TmchXmlSignature tmchXmlSignature;