Fix optional access for VKey nested keys (#539)

* Fix optional access for VKey nested keys

We should have used ofNullable() instead of of() for key creation.  Also add a
unit test.

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

@@ -41,11 +41,6 @@ public class VKey<T> extends ImmutableObject {
     this.primaryKey = primaryKey;
   }
 
-  public static <T> VKey<T> create(
-      Class<? extends T> kind, com.googlecode.objectify.Key<T> ofyKey, Object primaryKey) {
-    return new VKey(kind, ofyKey, primaryKey);
-  }
-
   public static <T> VKey<T> createSql(Class<? extends T> kind, Object primaryKey) {
     return new VKey(kind, null, primaryKey);
   }
@@ -72,7 +67,7 @@ public class VKey<T> extends ImmutableObject {
 
   /** Returns the SQL primary key if it exists. */
   public Optional<Object> maybeGetSqlKey() {
-    return Optional.of(this.primaryKey);
+    return Optional.ofNullable(this.primaryKey);
   }
 
   /** Returns the objectify key. */
@@ -83,6 +78,6 @@ public class VKey<T> extends ImmutableObject {
 
   /** Returns the objectify key if it exists. */
   public Optional<com.googlecode.objectify.Key<T>> maybeGetOfyKey() {
-    return Optional.of(this.ofyKey);
+    return Optional.ofNullable(this.ofyKey);
   }
 }

core/src/test/java/google/registry/persistence/VKeyTest.java

@@ -0,0 +1,45 @@
+// Copyright 2020 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.persistence;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.googlecode.objectify.Key;
+import google.registry.testing.AppEngineRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import google.registry.testing.TestObject;
+
+@RunWith(JUnit4.class)
+public class VKeyTest {
+
+  @Rule
+  public final AppEngineRule appEngineRule =
+      AppEngineRule.builder().withDatastoreAndCloudSql().build();
+
+  public VKeyTest() {}
+
+  @Test
+  public void testOptionalAccessors() {
+    VKey<TestObject> key = VKey.create(TestObject.class, null, null);
+    assertThat(key.maybeGetSqlKey().isPresent()).isFalse();
+    assertThat(key.maybeGetOfyKey().isPresent()).isFalse();
+
+    Key<TestObject> ofyKey = Key.create(TestObject.create("foo"));
+    assertThat(VKey.createOfy(TestObject.class, ofyKey).maybeGetOfyKey().get()).isEqualTo(ofyKey);
+    assertThat(VKey.createSql(TestObject.class, "foo").maybeGetSqlKey().get()).isEqualTo("foo");
+  }
+}