Remove support for @Mapify (#1691)

We no longer need to support Objectify's @Mapify logic. This substantially simplified how we store maps in the database.
2026-09-01 21:57:02 +00:00 · 2022-07-19 11:13:23 -04:00
parent 47ce568964
commit 49b1b2d058
22 changed files with 240 additions and 659 deletions
@@ -15,16 +15,12 @@
 package google.registry.model.common;

 import static com.google.common.truth.Truth.assertThat;
-import static google.registry.model.common.TimedTransitionProperty.forMapify;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.joda.time.DateTimeZone.UTC;
 import static org.junit.jupiter.api.Assertions.assertThrows;

-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableSortedMap;
-import java.util.Map;
-import java.util.Set;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -37,35 +33,17 @@ class TimedTransitionPropertyTest {
  private static final DateTime DATE_2 = DateTime.parse("2002-01-01T00:00:00.0Z");
  private static final DateTime DATE_3 = DateTime.parse("2003-01-01T00:00:00.0Z");

-  // Simple implementation of TimedTransition for testing.  Public so it can be instantiated via
-  // reflection.
-  @VisibleForTesting
-  public static class StringTimedTransition
-      extends TimedTransitionProperty.TimedTransition<String> {
-    private String value;
-
-    @Override
-    public String getValue() {
-      return value;
-    }
-
-    @Override
-    public void setValue(String value) {
-      this.value = value;
-    }
-  }
-
  private static final ImmutableSortedMap<DateTime, String> values = ImmutableSortedMap.of(
      START_OF_TIME, "0",
      DATE_1, "1",
      DATE_2, "2",
      DATE_3, "3");

-  private TimedTransitionProperty<String, StringTimedTransition> timedString;
+  private TimedTransitionProperty<String> timedString;

  @BeforeEach
  void init() {
-    timedString = TimedTransitionProperty.fromValueMap(values, StringTimedTransition.class);
+    timedString = TimedTransitionProperty.fromValueMap(values);
  }

  @Test
@@ -73,8 +51,7 @@ class TimedTransitionPropertyTest {
    assertThat(timedString.toValueMap()).isEqualTo(values);
  }

-  private static void testGetValueAtTime(
-      TimedTransitionProperty<String, StringTimedTransition> timedString) {
+  private static void testGetValueAtTime(TimedTransitionProperty<String> timedString) {
    assertThat(timedString.getValueAtTime(A_LONG_TIME_AGO)).isEqualTo("0");
    assertThat(timedString.getValueAtTime(START_OF_TIME.minusMillis(1))).isEqualTo("0");
    assertThat(timedString.getValueAtTime(START_OF_TIME)).isEqualTo("0");
@@ -106,74 +83,35 @@ class TimedTransitionPropertyTest {
    assertThat(timedString.getNextTransitionAfter(DATE_3)).isNull();
  }

-  @Test
-  void testSuccess_simulatedLoad() {
-    // Just for testing, don't extract transitions from a TimedTransitionProperty in real code.
-    Set<Map.Entry<DateTime, StringTimedTransition>> transitions = timedString.entrySet();
-    timedString = forMapify("0", StringTimedTransition.class);
-    // Simulate a load from Datastore by clearing and then re-inserting the original transitions.
-    timedString.clear();
-    for (Map.Entry<DateTime, StringTimedTransition> transition : transitions) {
-      timedString.put(transition.getKey(), transition.getValue());
-    }
-    timedString.checkValidity();
-    testGetValueAtTime(timedString);
-  }
-
  @Test
  void testFailure_valueMapNotChronologicallyOrdered() {
    assertThrows(
        IllegalArgumentException.class,
        () ->
            TimedTransitionProperty.fromValueMap(
-                ImmutableSortedMap.<DateTime, String>reverseOrder().put(START_OF_TIME, "0").build(),
-                StringTimedTransition.class));
+                ImmutableSortedMap.<DateTime, String>reverseOrder()
+                    .put(START_OF_TIME, "0")
+                    .build()));
  }

  @Test
  void testFailure_transitionTimeBeforeStartOfTime() {
    assertThrows(
        IllegalArgumentException.class,
-        () ->
-            TimedTransitionProperty.fromValueMap(
-                ImmutableSortedMap.of(A_LONG_TIME_AGO, "?"), StringTimedTransition.class));
+        () -> TimedTransitionProperty.fromValueMap(ImmutableSortedMap.of(A_LONG_TIME_AGO, "?")));
  }

  @Test
  void testFailure_noValues() {
    assertThrows(
        IllegalArgumentException.class,
-        () ->
-            TimedTransitionProperty.fromValueMap(
-                ImmutableSortedMap.of(), StringTimedTransition.class));
+        () -> TimedTransitionProperty.fromValueMap(ImmutableSortedMap.of()));
  }

  @Test
  void testFailure_noValueAtStartOfTime() {
    assertThrows(
        IllegalArgumentException.class,
-        () ->
-            TimedTransitionProperty.fromValueMap(
-                ImmutableSortedMap.of(DATE_1, "1"), StringTimedTransition.class));
-  }
-
-  @Test
-  void testFailure_noValuesAfterSimulatedEmptyLoad() {
-    timedString = forMapify("0", StringTimedTransition.class);
-    // Simulate a load from Datastore by clearing, but don't insert any transitions.
-    timedString.clear();
-    assertThrows(IllegalStateException.class, timedString::checkValidity);
-  }
-
-  @Test
-  void testFailure_noValueAtStartOfTimeAfterSimulatedLoad() {
-    // Just for testing, don't extract transitions from a TimedTransitionProperty in real code.
-    StringTimedTransition transition1 = timedString.get(DATE_1);
-    timedString = forMapify("0", StringTimedTransition.class);
-    // Simulate a load from Datastore by clearing and inserting transitions, but deliberately
-    // omit a transition corresponding to START_OF_TIME.
-    timedString.clear();
-    timedString.put(DATE_1, transition1);
-    assertThrows(IllegalStateException.class, timedString::checkValidity);
+        () -> TimedTransitionProperty.fromValueMap(ImmutableSortedMap.of(DATE_1, "1")));
  }
 }
@@ -26,7 +26,6 @@ import com.google.common.collect.ImmutableSortedMap;
 import google.registry.model.ImmutableObject;
 import google.registry.model.common.TimedTransitionProperty;
 import google.registry.model.domain.token.AllocationToken.TokenStatus;
-import google.registry.model.domain.token.AllocationToken.TokenStatusTransition;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
 import javax.persistence.Entity;
@@ -55,8 +54,8 @@ public class AllocationTokenStatusTransitionConverterTest {

  @Test
  void roundTripConversion_returnsSameTimedTransitionProperty() {
-    TimedTransitionProperty<TokenStatus, TokenStatusTransition> timedTransitionProperty =
-        TimedTransitionProperty.fromValueMap(values, TokenStatusTransition.class);
+    TimedTransitionProperty<TokenStatus> timedTransitionProperty =
+        TimedTransitionProperty.fromValueMap(values);
    AllocationTokenStatusTransitionConverterTestEntity testEntity =
        new AllocationTokenStatusTransitionConverterTestEntity(timedTransitionProperty);
    insertInDb(testEntity);
@@ -75,12 +74,12 @@ public class AllocationTokenStatusTransitionConverterTest {

    @Id String name = "id";

-    TimedTransitionProperty<TokenStatus, TokenStatusTransition> timedTransitionProperty;
+    TimedTransitionProperty<TokenStatus> timedTransitionProperty;

    private AllocationTokenStatusTransitionConverterTestEntity() {}

    private AllocationTokenStatusTransitionConverterTestEntity(
-        TimedTransitionProperty<TokenStatus, TokenStatusTransition> timedTransitionProperty) {
+        TimedTransitionProperty<TokenStatus> timedTransitionProperty) {
      this.timedTransitionProperty = timedTransitionProperty;
    }
  }
@@ -23,7 +23,6 @@ import static org.joda.money.CurrencyUnit.USD;
 import com.google.common.collect.ImmutableSortedMap;
 import google.registry.model.ImmutableObject;
 import google.registry.model.common.TimedTransitionProperty;
-import google.registry.model.tld.Registry.BillingCostTransition;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
 import javax.persistence.Entity;
@@ -49,8 +48,8 @@ public class BillingCostTransitionConverterTest {

  @Test
  void roundTripConversion_returnsSameTimedTransitionProperty() {
-    TimedTransitionProperty<Money, BillingCostTransition> timedTransitionProperty =
-        TimedTransitionProperty.fromValueMap(values, BillingCostTransition.class);
+    TimedTransitionProperty<Money> timedTransitionProperty =
+        TimedTransitionProperty.fromValueMap(values);
    TestEntity testEntity = new TestEntity(timedTransitionProperty);
    insertInDb(testEntity);
    TestEntity persisted =
@@ -63,12 +62,11 @@ public class BillingCostTransitionConverterTest {

    @Id String name = "id";

-    TimedTransitionProperty<Money, BillingCostTransition> timedTransitionProperty;
+    TimedTransitionProperty<Money> timedTransitionProperty;

    private TestEntity() {}

-    private TestEntity(
-        TimedTransitionProperty<Money, BillingCostTransition> timedTransitionProperty) {
+    private TestEntity(TimedTransitionProperty<Money> timedTransitionProperty) {
      this.timedTransitionProperty = timedTransitionProperty;
    }
  }
@@ -20,7 +20,6 @@ import static google.registry.testing.DatabaseHelper.insertInDb;

 import com.google.common.collect.ImmutableMap;
 import google.registry.model.ImmutableObject;
-import google.registry.model.registrar.Registrar.BillingAccountEntry;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
 import java.util.Map;
@@ -39,12 +38,10 @@ public class CurrencyToBillingConverterTest {

  @Test
  void roundTripConversion_returnsSameCurrencyToBillingMap() {
-    ImmutableMap<CurrencyUnit, BillingAccountEntry> currencyToBilling =
+    ImmutableMap<CurrencyUnit, String> currencyToBilling =
        ImmutableMap.of(
-            CurrencyUnit.of("USD"),
-            new BillingAccountEntry(CurrencyUnit.of("USD"), "accountId1"),
-            CurrencyUnit.of("CNY"),
-            new BillingAccountEntry(CurrencyUnit.of("CNY"), "accountId2"));
+            CurrencyUnit.of("USD"), "accountId1",
+            CurrencyUnit.of("CNY"), "accountId2");
    TestEntity testEntity = new TestEntity(currencyToBilling);
    insertInDb(testEntity);
    TestEntity persisted =
@@ -57,11 +54,11 @@ public class CurrencyToBillingConverterTest {

    @Id String name = "id";

-    Map<CurrencyUnit, BillingAccountEntry> currencyToBilling;
+    Map<CurrencyUnit, String> currencyToBilling;

    private TestEntity() {}

-    private TestEntity(Map<CurrencyUnit, BillingAccountEntry> currencyToBilling) {
+    private TestEntity(Map<CurrencyUnit, String> currencyToBilling) {
      this.currencyToBilling = currencyToBilling;
    }
  }
@@ -22,7 +22,6 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import com.google.common.collect.ImmutableSortedMap;
 import google.registry.model.ImmutableObject;
 import google.registry.model.common.DatabaseMigrationStateSchedule.MigrationState;
-import google.registry.model.common.DatabaseMigrationStateSchedule.MigrationStateTransition;
 import google.registry.model.common.TimedTransitionProperty;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
@@ -58,8 +57,8 @@ public class DatabaseMigrationScheduleTransitionConverterTest {

  @Test
  void roundTripConversion_returnsSameTimedTransitionProperty() {
-    TimedTransitionProperty<MigrationState, MigrationStateTransition> timedTransitionProperty =
-        TimedTransitionProperty.fromValueMap(values, MigrationStateTransition.class);
+    TimedTransitionProperty<MigrationState> timedTransitionProperty =
+        TimedTransitionProperty.fromValueMap(values);
    DatabaseMigrationScheduleTransitionConverterTestEntity testEntity =
        new DatabaseMigrationScheduleTransitionConverterTestEntity(timedTransitionProperty);
    insertInDb(testEntity);
@@ -79,12 +78,12 @@ public class DatabaseMigrationScheduleTransitionConverterTest {

    @Id String name = "id";

-    TimedTransitionProperty<MigrationState, MigrationStateTransition> timedTransitionProperty;
+    TimedTransitionProperty<MigrationState> timedTransitionProperty;

    private DatabaseMigrationScheduleTransitionConverterTestEntity() {}

    private DatabaseMigrationScheduleTransitionConverterTestEntity(
-        TimedTransitionProperty<MigrationState, MigrationStateTransition> timedTransitionProperty) {
+        TimedTransitionProperty<MigrationState> timedTransitionProperty) {
      this.timedTransitionProperty = timedTransitionProperty;
    }
  }
@@ -20,7 +20,6 @@ import static google.registry.testing.DatabaseHelper.insertInDb;
 import static org.junit.jupiter.api.Assertions.assertThrows;

 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
 import google.registry.model.ImmutableObject;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
@@ -128,7 +127,7 @@ public class StringMapConverterBaseTest {
  }

  private static class Key extends ImmutableObject {
-    private String key;
+    private final String key;

    private Key(String key) {
      this.key = key;
@@ -136,7 +135,7 @@ public class StringMapConverterBaseTest {
  }

  private static class Value extends ImmutableObject {
-    private String value;
+    private final String value;

    private Value(String value) {
      this.value = value;
@@ -144,16 +143,32 @@ public class StringMapConverterBaseTest {
  }

  @Converter(autoApply = true)
-  private static class TestStringMapConverter extends StringMapConverterBase<Key, Value> {
+  private static class TestStringMapConverter
+      extends StringMapConverterBase<Key, Value, Map<Key, Value>> {

    @Override
-    Map.Entry<String, String> convertToDatabaseMapEntry(Map.Entry<Key, Value> entry) {
-      return Maps.immutableEntry(entry.getKey().key, entry.getValue().value);
+    protected String convertKeyToString(Key key) {
+      return key.key;
    }

    @Override
-    Map.Entry<Key, Value> convertToEntityMapEntry(Map.Entry<String, String> entry) {
-      return Maps.immutableEntry(new Key(entry.getKey()), new Value(entry.getValue()));
+    protected String convertValueToString(Value value) {
+      return value.value;
+    }
+
+    @Override
+    protected Key convertStringToKey(String string) {
+      return new Key(string);
+    }
+
+    @Override
+    protected Value convertStringToValue(String string) {
+      return new Value(string);
+    }
+
+    @Override
+    protected Map<Key, Value> convertMapToDerivedType(Map<Key, Value> map) {
+      return map;
    }
  }

@@ -21,13 +21,10 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.junit.jupiter.api.Assertions.assertThrows;

 import com.google.common.collect.ImmutableSortedMap;
-import com.google.common.collect.Maps;
 import google.registry.model.ImmutableObject;
 import google.registry.model.common.TimedTransitionProperty;
-import google.registry.model.common.TimedTransitionProperty.TimedTransition;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
-import java.util.Map;
 import javax.persistence.Converter;
 import javax.persistence.Entity;
 import javax.persistence.Id;
@@ -54,8 +51,8 @@ class TimedTransitionPropertyConverterBaseTest {
          DATE_1, "val2",
          DATE_2, "val3");

-  private static final TimedTransitionProperty<String, TestTransition> TIMED_TRANSITION_PROPERTY =
-      TimedTransitionProperty.fromValueMap(VALUES, TestTransition.class);
+  private static final TimedTransitionProperty<String> TIMED_TRANSITION_PROPERTY =
+      TimedTransitionProperty.fromValueMap(VALUES);

  @Test
  void roundTripConversion_returnsSameTimedTransitionProperty() {
@@ -74,11 +71,11 @@ class TimedTransitionPropertyConverterBaseTest {
        jpaTm().transact(() -> jpaTm().getEntityManager().find(TestEntity.class, "id"));
    assertThat(persisted.property).containsExactlyEntriesIn(TIMED_TRANSITION_PROPERTY);
    ImmutableSortedMap<DateTime, String> newValues = ImmutableSortedMap.of(START_OF_TIME, "val4");
-    persisted.property = TimedTransitionProperty.fromValueMap(newValues, TestTransition.class);
+    persisted.property = TimedTransitionProperty.fromValueMap(newValues);
    jpaTm().transact(() -> jpaTm().getEntityManager().merge(persisted));
    TestEntity updated =
        jpaTm().transact(() -> jpaTm().getEntityManager().find(TestEntity.class, "id"));
-    assertThat(updated.property.toValueMap()).isEqualTo(newValues);
+    assertThat(updated.property).isEqualTo(newValues);
  }

  @Test
@@ -131,37 +128,18 @@ class TimedTransitionPropertyConverterBaseTest {
    jpaTm().transact(() -> jpaTm().getEntityManager().createNativeQuery(sql).executeUpdate());
  }

-  public static class TestTransition extends TimedTransition<String> {
-    private String transition;
-
-    @Override
-    public String getValue() {
-      return transition;
-    }
-
-    @Override
-    protected void setValue(String transition) {
-      this.transition = transition;
-    }
-  }
-
  @Converter(autoApply = true)
  private static class TestTimedTransitionPropertyConverter
-      extends TimedTransitionPropertyConverterBase<String, TestTransition> {
+      extends TimedTransitionPropertyConverterBase<String> {

    @Override
-    Map.Entry<DateTime, String> convertToEntityMapEntry(Map.Entry<String, String> entry) {
-      return Maps.immutableEntry(DateTime.parse(entry.getKey()), entry.getValue());
+    protected String convertValueToString(String value) {
+      return value;
    }

    @Override
-    Class<TestTransition> getTimedTransitionSubclass() {
-      return TestTransition.class;
-    }
-
-    @Override
-    Map.Entry<String, String> convertToDatabaseMapEntry(Map.Entry<DateTime, TestTransition> entry) {
-      return Maps.immutableEntry(entry.getKey().toString(), entry.getValue().getValue());
+    protected String convertStringToValue(String string) {
+      return string;
    }
  }

@@ -170,12 +148,12 @@ class TimedTransitionPropertyConverterBaseTest {

    @Id String name = "id";

-    TimedTransitionProperty<String, TestTransition> property;
+    TimedTransitionProperty<String> property;

    private TestEntity() {}

-    private TestEntity(TimedTransitionProperty<String, TestTransition> timedTransitionProperty) {
-      this.property = timedTransitionProperty;
+    private TestEntity(TimedTransitionProperty<String> timedTransitionProperty) {
+      property = timedTransitionProperty;
    }
  }
 }
@@ -23,7 +23,6 @@ import com.google.common.collect.ImmutableSortedMap;
 import google.registry.model.ImmutableObject;
 import google.registry.model.common.TimedTransitionProperty;
 import google.registry.model.tld.Registry.TldState;
-import google.registry.model.tld.Registry.TldStateTransition;
 import google.registry.persistence.transaction.JpaTestExtensions;
 import google.registry.persistence.transaction.JpaTestExtensions.JpaUnitTestExtension;
 import javax.persistence.Entity;
@@ -52,8 +51,8 @@ class TldStateTransitionConverterTest {

  @Test
  void roundTripConversion_returnsSameTimedTransitionProperty() {
-    TimedTransitionProperty<TldState, TldStateTransition> timedTransitionProperty =
-        TimedTransitionProperty.fromValueMap(values, TldStateTransition.class);
+    TimedTransitionProperty<TldState> timedTransitionProperty =
+        TimedTransitionProperty.fromValueMap(values);
    TestEntity testEntity = new TestEntity(timedTransitionProperty);
    insertInDb(testEntity);
    TestEntity persisted =
@@ -66,12 +65,11 @@ class TldStateTransitionConverterTest {

    @Id String name = "id";

-    TimedTransitionProperty<TldState, TldStateTransition> timedTransitionProperty;
+    TimedTransitionProperty<TldState> timedTransitionProperty;

    private TestEntity() {}

-    private TestEntity(
-        TimedTransitionProperty<TldState, TldStateTransition> timedTransitionProperty) {
+    private TestEntity(TimedTransitionProperty<TldState> timedTransitionProperty) {
      this.timedTransitionProperty = timedTransitionProperty;
    }
  }
@@ -337,7 +337,7 @@ class google.registry.model.domain.token.AllocationToken {
  google.registry.model.CreateAutoTimestamp creationTime;
  google.registry.model.UpdateAutoTimestamp updateTimestamp;
  google.registry.model.billing.BillingEvent$RenewalPriceBehavior renewalPriceBehavior;
-  google.registry.model.common.TimedTransitionProperty<google.registry.model.domain.token.AllocationToken$TokenStatus, google.registry.model.domain.token.AllocationToken$TokenStatusTransition> tokenStatusTransitions;
+  google.registry.model.common.TimedTransitionProperty<google.registry.model.domain.token.AllocationToken$TokenStatus> tokenStatusTransitions;
  google.registry.model.domain.token.AllocationToken$TokenType tokenType;
  google.registry.persistence.DomainHistoryVKey redemptionHistoryEntry;
  int discountYears;
@@ -351,10 +351,6 @@ enum google.registry.model.domain.token.AllocationToken$TokenStatus {
  NOT_STARTED;
  VALID;
 }
-class google.registry.model.domain.token.AllocationToken$TokenStatusTransition {
-  google.registry.model.domain.token.AllocationToken$TokenStatus tokenStatus;
-  org.joda.time.DateTime transitionTime;
-}
 enum google.registry.model.domain.token.AllocationToken$TokenType {
  SINGLE_USE;
  UNLIMITED_USE;
@@ -520,17 +516,13 @@ class google.registry.model.registrar.Registrar {
  java.lang.String url;
  java.lang.String whoisServer;
  java.util.List<google.registry.util.CidrAddressBlock> ipAddressWhitelist;
-  java.util.Map<org.joda.money.CurrencyUnit, google.registry.model.registrar.Registrar$BillingAccountEntry> billingAccountMap;
+  java.util.Map<org.joda.money.CurrencyUnit, java.lang.String> billingAccountMap;
  java.util.Set<java.lang.String> allowedTlds;
  java.util.Set<java.lang.String> rdapBaseUrls;
  org.joda.time.DateTime lastCertificateUpdateTime;
  org.joda.time.DateTime lastExpiringCertNotificationSentDate;
  org.joda.time.DateTime lastExpiringFailoverCertNotificationSentDate;
 }
-class google.registry.model.registrar.Registrar$BillingAccountEntry {
-  java.lang.String accountId;
-  org.joda.money.CurrencyUnit currency;
-}
 enum google.registry.model.registrar.Registrar$State {
  ACTIVE;
  DISABLED;