Change commit log bucket counts in tests

I'm setting it to three buckets across all tests, because the default one bucket
wasn't realistic enough, and allowed some tests to pass that shouldn't have,
essentially by accident.

This also changes RegistryConfig from being an interface to being an abstract
base class. The medium term goal here is to have it be a static class so that it
can provide fields from the YAML-derived POJO in situations where Dagger
injection isn't feasible.

The expected end state is as follows:

default-config.yaml -- The master config file that provides defaults for all
values.

nomulus-config.yaml -- A per-environment config file that overrides the defaults
from the previous file.

YamlConfig.java -- The POJO that the aforementioned YAML files are deserialized
into.

RegistryConfig.java -- Contains a static, memoized instance of YamlConfig and
provides static methods for getting some of those values.

ConfigModule -- Will become a static inner class of RegistryConfig, using Dagger
to provide most of the fields from the memoized YamlConfig instance. This way,
all configuration will be coming from a single place: RegistryConfig.java.

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

javatests/google/registry/backup/CommitLogCheckpointActionTest.java

@@ -62,8 +62,10 @@ public class CommitLogCheckpointActionTest {
     task.clock = new FakeClock(now);
     task.strategy = strategy;
     task.taskEnqueuer = new TaskEnqueuer(new Retrier(null, 1));
-    when(strategy.computeCheckpoint()).thenReturn(
-        CommitLogCheckpoint.create(now, ImmutableMap.of(1, START_OF_TIME)));
+    when(strategy.computeCheckpoint())
+        .thenReturn(
+            CommitLogCheckpoint.create(
+                now, ImmutableMap.of(1, START_OF_TIME, 2, START_OF_TIME, 3, START_OF_TIME)));
   }
 
   @Test

javatests/google/registry/backup/CommitLogCheckpointStrategyTest.java

@@ -24,7 +24,6 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableMap;
 import com.googlecode.objectify.VoidWork;
-import google.registry.config.TestRegistryConfig;
 import google.registry.model.common.Cursor;
 import google.registry.model.common.Cursor.CursorType;
 import google.registry.model.ofy.CommitLogBucket;
@@ -93,13 +92,6 @@ public class CommitLogCheckpointStrategyTest {
     strategy.clock = clock;
     strategy.ofy = ofy;
 
-    // Use three commit log buckets for easier but sufficiently complex testing.
-    configRule.override(new TestRegistryConfig() {
-      @Override
-      public int getCommitLogBucketCount() {
-        return 3;
-      }});
-
     // Need to inject clock into Ofy so that createTld() below will get the right time.
     inject.setStaticField(Ofy.class, "clock", clock);
     // Inject a fake bucket ID supplier so we can dole out specific bucket IDs to commit logs.

javatests/google/registry/backup/DeleteOldCommitLogsActionTest.java

@@ -15,10 +15,11 @@
 package google.registry.backup;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.config.RegistryConfig.getCommitLogBucketCount;
+import static google.registry.model.ofy.ObjectifyService.ofy;
 import static org.joda.time.Duration.millis;
 
 import com.googlecode.objectify.VoidWork;
-import google.registry.config.TestRegistryConfig;
 import google.registry.model.ofy.CommitLogManifest;
 import google.registry.model.ofy.CommitLogMutation;
 import google.registry.model.ofy.Ofy;
@@ -26,10 +27,8 @@ import google.registry.model.registrar.Registrar;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
 import google.registry.testing.FakeClock;
-import google.registry.testing.RegistryConfigRule;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -44,29 +43,28 @@ public class DeleteOldCommitLogsActionTest {
       .withDatastore()
       .build();
 
-  @Rule
-  public final RegistryConfigRule configRule = new RegistryConfigRule();
-
   @Rule
   public final ExceptionRule thrown = new ExceptionRule();
 
   private final FakeClock clock = new FakeClock(DateTime.parse("2000-01-01TZ"));
   private final Ofy ofy = new Ofy(clock);
-  private final DeleteOldCommitLogsAction task = new DeleteOldCommitLogsAction();
 
-  @Before
-  public void before() throws Exception {
-    task.bucketNum = 1;
-    task.clock = clock;
-    task.maxAge = Duration.millis(2);
-    task.maxDeletes = 4;
-    task.ofy = ofy;
+  private void runInAllBuckets(int maxDeletes) {
+    for (int bucketNum = 1; bucketNum <= getCommitLogBucketCount(); bucketNum++) {
+      DeleteOldCommitLogsAction task = new DeleteOldCommitLogsAction();
+      task.bucketNum = bucketNum;
+      task.clock = clock;
+      task.maxAge = Duration.millis(2);
+      task.maxDeletes = maxDeletes;
+      task.ofy = ofy;
+      task.run();
+    }
   }
 
   @Test
   public void testRun_noCommitLogs_doesNothing() throws Exception {
     assertManifestAndMutationCounts(0, 0);
-    task.run();
+    runInAllBuckets(4);
     assertManifestAndMutationCounts(0, 0);
   }
 
@@ -75,7 +73,7 @@ public class DeleteOldCommitLogsActionTest {
     createCommitLog();
     clock.advanceOneMilli();
     assertManifestAndMutationCounts(1, 2);
-    task.run();
+    runInAllBuckets(4);
     assertManifestAndMutationCounts(1, 2);
   }
 
@@ -83,7 +81,7 @@ public class DeleteOldCommitLogsActionTest {
   public void testRun_commitLogEqualToThreshold_doesntGetDeleted() throws Exception {
     createCommitLog();
     clock.advanceBy(millis(2));
-    task.run();
+    runInAllBuckets(4);
     assertManifestAndMutationCounts(1, 2);
   }
 
@@ -91,7 +89,7 @@ public class DeleteOldCommitLogsActionTest {
   public void testRun_commitLogOlderThanThreshold_getsDeleted() throws Exception {
     createCommitLog();
     clock.advanceBy(millis(3));
-    task.run();
+    runInAllBuckets(4);
     assertManifestAndMutationCounts(0, 0);
   }
 
@@ -101,34 +99,31 @@ public class DeleteOldCommitLogsActionTest {
     clock.advanceBy(millis(3));
     createCommitLog();
     assertManifestAndMutationCounts(2, 4);
-    task.run();
+    runInAllBuckets(4);
     assertManifestAndMutationCounts(1, 2);
   }
 
   @Test
   public void testRun_twoOlderThanThreshold_bothGetDeletedInSameTransaction() throws Exception {
-    task.maxDeletes = 2;
     createCommitLog();
     clock.advanceOneMilli();
     createCommitLog();
     clock.advanceBy(millis(3));
     assertManifestAndMutationCounts(2, 4);
-    task.run();
+    runInAllBuckets(2);
     assertManifestAndMutationCounts(0, 0);
   }
 
   @Test
   public void testRun_twoOlderThanThreshold_bothGetDeletedInTwoTransactions() throws Exception {
-    task.maxDeletes = 1;
     createCommitLog();
     clock.advanceOneMilli();
     createCommitLog();
     clock.advanceBy(millis(3));
     createCommitLog();
     assertManifestAndMutationCounts(3, 6);
-    task.run();
-    assertManifestAndMutationCounts(2, 4);
-    task.run();
+    runInAllBuckets(1);
+    runInAllBuckets(1);
     assertManifestAndMutationCounts(1, 2);
   }
 
@@ -136,20 +131,22 @@ public class DeleteOldCommitLogsActionTest {
   public void testRun_commitLogOlderButInADifferentBucket_doesntGetDeleted() throws Exception {
     createCommitLog();
     clock.advanceBy(millis(31337));
-    configRule.override(new TestRegistryConfig() {
-      @Override public int getCommitLogBucketCount() { return 2; }
-    });
-    task.bucketNum = 2;
+    int usedBucketNum = ofy().load().type(CommitLogManifest.class).list().get(0).getBucketId();
+    DeleteOldCommitLogsAction task = new DeleteOldCommitLogsAction();
+    task.bucketNum = (usedBucketNum % getCommitLogBucketCount()) + 1;
+    task.clock = clock;
+    task.maxAge = Duration.millis(2);
+    task.maxDeletes = 20;
+    task.ofy = ofy;
     task.run();
     assertManifestAndMutationCounts(1, 2);
   }
 
   @Test
   public void testRun_lessThanATenthOfOldData_doesntGetDeleted() throws Exception {
-    task.maxDeletes = 20;
     createCommitLog();
     clock.advanceBy(millis(2));
-    task.run();
+    runInAllBuckets(20);
     assertManifestAndMutationCounts(1, 2);
   }
 

javatests/google/registry/backup/ExportCommitLogDiffActionTest.java

@@ -30,7 +30,6 @@ import com.google.appengine.tools.cloudstorage.GcsServiceFactory;
 import com.google.common.collect.ImmutableMap;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.ObjectifyService;
-import google.registry.config.TestRegistryConfig;
 import google.registry.model.ImmutableObject;
 import google.registry.model.ofy.CommitLogBucket;
 import google.registry.model.ofy.CommitLogCheckpoint;
@@ -38,7 +37,6 @@ import google.registry.model.ofy.CommitLogManifest;
 import google.registry.model.ofy.CommitLogMutation;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.GcsTestingUtils;
-import google.registry.testing.RegistryConfigRule;
 import google.registry.testing.TestObject;
 import java.util.List;
 import org.joda.time.DateTime;
@@ -52,20 +50,11 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class ExportCommitLogDiffActionTest {
 
-  private static final int NUM_BUCKETS = 3;
-
   @Rule
   public final AppEngineRule appEngine = AppEngineRule.builder()
       .withDatastore()
       .build();
 
-  @Rule
-  public final RegistryConfigRule configRule = new RegistryConfigRule(
-      new TestRegistryConfig() {
-        @Override public int getCommitLogBucketCount() {
-          return NUM_BUCKETS;
-        }});
-
   /** Local GCS service available for testing. */
   private final GcsService gcsService = GcsServiceFactory.createGcsService();
 

javatests/google/registry/backup/RestoreCommitLogsActionTest.java

@@ -40,7 +40,6 @@ import com.google.common.collect.Lists;
 import com.google.common.primitives.Longs;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.ObjectifyService;
-import google.registry.config.TestRegistryConfig;
 import google.registry.model.ImmutableObject;
 import google.registry.model.ofy.CommitLogBucket;
 import google.registry.model.ofy.CommitLogCheckpoint;
@@ -96,11 +95,6 @@ public class RestoreCommitLogsActionTest {
     action.diffLister.gcsService = gcsService;
     action.diffLister.gcsBucket = GCS_BUCKET;
     action.diffLister.executor = newDirectExecutorService();
-    configRule.override(new TestRegistryConfig() {
-      @Override
-      public int getCommitLogBucketCount() {
-        return 3;
-      }});
   }
 
   @Test

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

@@ -53,7 +53,7 @@ public class ContactResourceTest extends EntityTestCase {
   public void setUp() throws Exception {
     createTld("foobar");
     // Set up a new persisted ContactResource entity.
-    contactResource = cloneAndSetAutoTimestamps(
+    contactResource = persistResource(cloneAndSetAutoTimestamps(
         new ContactResource.Builder()
             .setContactId("contact_id")
             .setRepoId("1-FOOBAR")
@@ -113,8 +113,7 @@ public class ContactResourceTest extends EntityTestCase {
                 .setTransferStatus(TransferStatus.SERVER_APPROVED)
                 .setTransferRequestTrid(Trid.create("client trid"))
                 .build())
-            .build());
-    persistResource(contactResource);
+            .build()));
   }
 
   @Test

javatests/google/registry/model/domain/DomainApplicationTest.java

@@ -65,7 +65,7 @@ public class DomainApplicationTest extends EntityTestCase {
   public void setUp() throws Exception {
     createTld("com");
     // Set up a new persisted domain application entity.
-    domainApplication = cloneAndSetAutoTimestamps(
+    domainApplication = persistResource(cloneAndSetAutoTimestamps(
         new DomainApplication.Builder()
             .setFullyQualifiedDomainName("example.com")
             .setRepoId("1-COM")
@@ -96,8 +96,7 @@ public class DomainApplicationTest extends EntityTestCase {
             .setEncodedSignedMarks(ImmutableList.of(EncodedSignedMark.create("base64", "abcdefg=")))
             .setApplicationStatus(ApplicationStatus.ALLOCATED)
             .setAuctionPrice(Money.of(USD, 11))
-            .build());
-    persistResource(domainApplication);
+            .build()));
   }
 
   @Test

javatests/google/registry/model/domain/DomainResourceTest.java

@@ -107,7 +107,7 @@ public class DomainResourceTest extends EntityTestCase {
     Key<PollMessage.OneTime> onetimePollKey =
         Key.create(historyEntryKey, PollMessage.OneTime.class, 1);
     // Set up a new persisted domain entity.
-    domain = cloneAndSetAutoTimestamps(
+    domain = persistResource(cloneAndSetAutoTimestamps(
         new DomainResource.Builder()
             .setFullyQualifiedDomainName("example.com")
             .setRepoId("4-COM")
@@ -158,8 +158,7 @@ public class DomainResourceTest extends EntityTestCase {
             .setApplication(Key.create(DomainApplication.class, 1))
             .addGracePeriod(GracePeriod.create(
                 GracePeriodStatus.ADD, clock.nowUtc().plusDays(1), "registrar", null))
-            .build());
-    persistResource(domain);
+            .build()));
   }
 
   @Test

javatests/google/registry/model/host/HostResourceTest.java

@@ -72,21 +72,21 @@ public class HostResourceTest extends EntityTestCase {
                 .build())
             .build());
     hostResource =
-        cloneAndSetAutoTimestamps(
-            new HostResource.Builder()
-                .setRepoId("DEADBEEF-COM")
-                .setFullyQualifiedHostName("ns1.example.com")
-                .setCreationClientId("a registrar")
-                .setLastEppUpdateTime(clock.nowUtc())
-                .setLastEppUpdateClientId("another registrar")
-                .setLastTransferTime(clock.nowUtc())
-                .setInetAddresses(ImmutableSet.of(InetAddresses.forString("127.0.0.1")))
-                .setStatusValues(ImmutableSet.of(StatusValue.OK))
-                .setSuperordinateDomain(
-                    Key.create(
-                        loadByForeignKey(DomainResource.class, "example.com", clock.nowUtc())))
-                .build());
-    persistResource(hostResource);
+        persistResource(
+            cloneAndSetAutoTimestamps(
+                new HostResource.Builder()
+                    .setRepoId("DEADBEEF-COM")
+                    .setFullyQualifiedHostName("ns1.example.com")
+                    .setCreationClientId("a registrar")
+                    .setLastEppUpdateTime(clock.nowUtc())
+                    .setLastEppUpdateClientId("another registrar")
+                    .setLastTransferTime(clock.nowUtc())
+                    .setInetAddresses(ImmutableSet.of(InetAddresses.forString("127.0.0.1")))
+                    .setStatusValues(ImmutableSet.of(StatusValue.OK))
+                    .setSuperordinateDomain(
+                        Key.create(
+                            loadByForeignKey(DomainResource.class, "example.com", clock.nowUtc())))
+                    .build()));
   }
 
   @Test

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

@@ -24,11 +24,9 @@ import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.annotation.Cache;
-import google.registry.config.TestRegistryConfig;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
 import google.registry.testing.InjectRule;
-import google.registry.testing.RegistryConfigRule;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -44,9 +42,6 @@ public class CommitLogBucketTest {
       .withDatastore()
       .build();
 
-  @Rule
-  public final RegistryConfigRule configRule = new RegistryConfigRule();
-
   @Rule
   public final InjectRule inject = new InjectRule();
 
@@ -57,12 +52,6 @@ public class CommitLogBucketTest {
 
   @Before
   public void before() {
-    // Use 10 buckets to make the tests below more realistic.
-    configRule.override(new TestRegistryConfig() {
-      @Override
-      public int getCommitLogBucketCount() {
-        return 10;
-      }});
     // Save the bucket with some non-default properties set so that we can distinguish a correct
     // load from one that returns a newly created bucket instance.
     bucket = persistResource(
@@ -108,19 +97,17 @@ public class CommitLogBucketTest {
 
   @Test
   public void test_loadBucket_forNonexistentBucket_returnsNewBucket() {
-    assertThat(loadBucket(getBucketKey(10))).isEqualTo(
-        new CommitLogBucket.Builder().setBucketNum(10).build());
+    assertThat(loadBucket(getBucketKey(3))).isEqualTo(
+        new CommitLogBucket.Builder().setBucketNum(3).build());
   }
 
   @Test
   public void test_loadAllBuckets_loadsExistingBuckets_orNewOnesIfNonexistent() {
     ImmutableSet<CommitLogBucket> buckets = loadAllBuckets();
-    assertThat(buckets).hasSize(10);
+    assertThat(buckets).hasSize(3);
     assertThat(buckets).contains(bucket);
-    for (int i = 2; i <= 10; ++i) {
-      assertThat(buckets).contains(
-          new CommitLogBucket.Builder().setBucketNum(i).build());
-    }
+    assertThat(buckets).contains(new CommitLogBucket.Builder().setBucketNum(2).build());
+    assertThat(buckets).contains(new CommitLogBucket.Builder().setBucketNum(3).build());
   }
 
   @Test

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

@@ -19,12 +19,10 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.common.collect.ImmutableMap;
-import google.registry.config.TestRegistryConfig;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
 import google.registry.testing.RegistryConfigRule;
 import org.joda.time.DateTime;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -49,16 +47,6 @@ public class CommitLogCheckpointTest {
   private static final DateTime T2 = START_OF_TIME.plusMillis(1);
   private static final DateTime T3 = START_OF_TIME.plusMillis(2);
 
-  @Before
-  public void before() {
-    // Use 3 buckets to make the tests below more realistic.
-    configRule.override(new TestRegistryConfig() {
-      @Override
-      public int getCommitLogBucketCount() {
-        return 3;
-      }});
-  }
-
   @Test
   public void test_getCheckpointTime() {
     DateTime now = DateTime.now(UTC);
@@ -74,18 +62,6 @@ public class CommitLogCheckpointTest {
     assertThat(checkpoint.getBucketTimestamps()).containsExactly(1, T1, 2, T2, 3, T3);
   }
 
-  @Test
-  public void test_getBucketTimestamps_whenOnlyOneBucket_stillWorks() {
-    configRule.override(new TestRegistryConfig() {
-      @Override
-      public int getCommitLogBucketCount() {
-        return 1;
-      }});
-    CommitLogCheckpoint checkpoint =
-        CommitLogCheckpoint.create(DateTime.now(UTC), ImmutableMap.of(1, T1));
-    assertThat(checkpoint.getBucketTimestamps()).containsExactly(1, T1);
-  }
-
   @Test
   public void test_create_notEnoughBucketTimestamps_throws() {
     thrown.expect(IllegalArgumentException.class, "Bucket ids are incorrect");

javatests/google/registry/tools/server/KillAllCommitLogsActionTest.java

@@ -40,6 +40,7 @@ import google.registry.model.ofy.CommitLogMutation;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.mapreduce.MapreduceTestCase;
 import java.util.List;
+import org.joda.time.DateTime;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -72,10 +73,11 @@ public class KillAllCommitLogsActionTest extends MapreduceTestCase<KillAllCommit
           newContactResource(String.format("abc%d", nextContactId++)));
     }
     persistResource(CommitLogCheckpointRoot.create(START_OF_TIME.plusDays(1)));
+    DateTime bucketTime = START_OF_TIME.plusDays(2);
     persistResource(
         CommitLogCheckpoint.create(
             START_OF_TIME.plusDays(1),
-            ImmutableMap.of(1, START_OF_TIME.plusDays(2))));
+            ImmutableMap.of(1, bucketTime, 2, bucketTime, 3, bucketTime)));
     for (Class<?> clazz : AFFECTED_TYPES) {
       assertThat(ofy().load().type(clazz)).named("entities of type " + clazz).isNotEmpty();
     }