Ensure VKey is actually serializable (#1235)

* Ensure VKey is actually serializable

Tighten field type so that non-serializable object cannot be set as
sqlKey.

This would make it easier to make EppResource entities Serializable in
the future.

core/src/main/java/google/registry/persistence/BillingVKey.java

@@ -21,6 +21,7 @@ import google.registry.model.billing.BillingEvent.OneTime;
 import google.registry.model.billing.BillingEvent.Recurring;
 import google.registry.model.domain.DomainBase;
 import google.registry.model.reporting.HistoryEntry;
+import java.io.Serializable;
 import javax.annotation.Nullable;
 import javax.persistence.AttributeOverride;
 import javax.persistence.AttributeOverrides;
@@ -46,7 +47,7 @@ public abstract class BillingVKey<K> extends EppHistoryVKey<K, DomainBase> {
   }
 
   @Override
-  public Object createSqlKey() {
+  public Serializable createSqlKey() {
     return billingId;
   }
 

core/src/main/java/google/registry/persistence/DomainHistoryVKey.java

@@ -21,6 +21,7 @@ import google.registry.model.domain.DomainBase;
 import google.registry.model.domain.DomainHistory;
 import google.registry.model.domain.DomainHistory.DomainHistoryId;
 import google.registry.model.reporting.HistoryEntry;
+import java.io.Serializable;
 import javax.persistence.Embeddable;
 
 /** {@link VKey} for {@link HistoryEntry} which parent is {@link DomainBase}. */
@@ -35,7 +36,7 @@ public class DomainHistoryVKey extends EppHistoryVKey<HistoryEntry, DomainBase>
   }
 
   @Override
-  public Object createSqlKey() {
+  public Serializable createSqlKey() {
     return new DomainHistoryId(repoId, historyRevisionId);
   }
 

core/src/main/java/google/registry/persistence/EppHistoryVKey.java

@@ -101,7 +101,7 @@ public abstract class EppHistoryVKey<K, E extends EppResource> extends Immutable
     return VKey.create(vKeyType, createSqlKey(), createOfyKey());
   }
 
-  public abstract Object createSqlKey();
+  public abstract Serializable createSqlKey();
 
   public abstract Key<K> createOfyKey();
 }

core/src/main/java/google/registry/persistence/VKey.java

@@ -37,7 +37,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
   private static final long serialVersionUID = -5291472863840231240L;
 
   // The SQL key for the referenced entity.
-  Object sqlKey;
+  Serializable sqlKey;
 
   // The objectify key for the referenced entity.
   Key<T> ofyKey;
@@ -46,7 +46,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
 
   VKey() {}
 
-  VKey(Class<? extends T> kind, Key<T> ofyKey, Object sqlKey) {
+  VKey(Class<? extends T> kind, Key<T> ofyKey, Serializable sqlKey) {
     this.kind = kind;
     this.ofyKey = ofyKey;
     this.sqlKey = sqlKey;
@@ -57,7 +57,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
    *
    * <p>Deprecated. Create symmetric keys with create() instead.
    */
-  public static <T> VKey<T> createSql(Class<T> kind, Object sqlKey) {
+  public static <T> VKey<T> createSql(Class<T> kind, Serializable sqlKey) {
     checkArgumentNotNull(kind, "kind must not be null");
     checkArgumentNotNull(sqlKey, "sqlKey must not be null");
     return new VKey<T>(kind, null, sqlKey);
@@ -71,7 +71,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
   }
 
   /** Creates a {@link VKey} which only contains both sql and ofy primary key. */
-  public static <T> VKey<T> create(Class<T> kind, Object sqlKey, Key<T> ofyKey) {
+  public static <T> VKey<T> create(Class<T> kind, Serializable sqlKey, Key<T> ofyKey) {
     checkArgumentNotNull(kind, "kind must not be null");
     checkArgumentNotNull(sqlKey, "sqlKey must not be null");
     checkArgumentNotNull(ofyKey, "ofyKey must not be null");
@@ -84,8 +84,8 @@ public class VKey<T> extends ImmutableObject implements Serializable {
    * <p>IMPORTANT USAGE NOTE: Datastore entities that are not roots of entity groups (i.e. those
    * that do not have a null parent in their Objectify keys) require the full entity group
    * inheritance chain to be specified and thus cannot use this create method. You need to use
-   * {@link #create(Class, Object, Key)} instead and pass in the full, valid parent field in the
-   * Datastore key.
+   * {@link #create(Class, Serializable, Key)} instead and pass in the full, valid parent field in
+   * the Datastore key.
    */
   public static <T> VKey<T> create(Class<T> kind, long id) {
     checkArgument(
@@ -102,8 +102,8 @@ public class VKey<T> extends ImmutableObject implements Serializable {
    * <p>IMPORTANT USAGE NOTE: Datastore entities that are not roots of entity groups (i.e. those
    * that do not have a null parent in their Objectify keys) require the full entity group
    * inheritance chain to be specified and thus cannot use this create method. You need to use
-   * {@link #create(Class, Object, Key)} instead and pass in the full, valid parent field in the
-   * Datastore key.
+   * {@link #create(Class, Serializable, Key)} instead and pass in the full, valid parent field in
+   * the Datastore key.
    */
   public static <T> VKey<T> create(Class<T> kind, String name) {
     checkArgument(
@@ -172,7 +172,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
       throw new IllegalArgumentException("Missing value for last key of type " + lastClass);
     }
 
-    Object sqlKey = getSqlKey();
+    Serializable sqlKey = getSqlKey();
     Key<T> ofyKey =
         sqlKey instanceof Long
             ? Key.create(lastKey, getKind(), (Long) sqlKey)
@@ -197,7 +197,7 @@ public class VKey<T> extends ImmutableObject implements Serializable {
   }
 
   /** Returns the SQL primary key. */
-  public Object getSqlKey() {
+  public Serializable getSqlKey() {
     checkState(sqlKey != null, "Attempting obtain a null SQL key.");
     return this.sqlKey;
   }

core/src/main/java/google/registry/persistence/converter/VKeyConverter.java

@@ -16,11 +16,13 @@ package google.registry.persistence.converter;
 
 import com.googlecode.objectify.Key;
 import google.registry.persistence.VKey;
+import java.io.Serializable;
 import javax.annotation.Nullable;
 import javax.persistence.AttributeConverter;
 
 /** Converts VKey to a string column. */
-public abstract class VKeyConverter<T, C> implements AttributeConverter<VKey<? extends T>, C> {
+public abstract class VKeyConverter<T, C extends Serializable>
+    implements AttributeConverter<VKey<? extends T>, C> {
   @Override
   @Nullable
   public C convertToDatabaseColumn(@Nullable VKey<? extends T> attribute) {

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java

@@ -46,6 +46,7 @@ import google.registry.schema.replay.SqlOnlyEntity;
 import google.registry.util.Clock;
 import google.registry.util.Retrier;
 import google.registry.util.SystemSleeper;
+import java.io.Serializable;
 import java.lang.reflect.Array;
 import java.lang.reflect.Field;
 import java.util.Calendar;
@@ -489,7 +490,8 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
             loadByKey(
                 VKey.createSql(
                     possibleChild.getClass(),
-                    emf.getPersistenceUnitUtil().getIdentifier(possibleChild)));
+                    // Casting to Serializable is safe according to JPA (JSR 338 sec. 2.4).
+                    (Serializable) emf.getPersistenceUnitUtil().getIdentifier(possibleChild)));
     return returnValue;
   }