fix: fix review changes

googleapis · Aug 21, 2020 · a4e8e47 · a4e8e47
1 parent f3b224c
commit a4e8e47
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 3 deletions.
diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CustomClassMapper.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CustomClassMapper.java
@@ -266,7 +266,7 @@ private static <T> T deserializeToParameterizedType(
       Type genericType = type.getActualTypeArguments()[0];
       if (o instanceof List) {
         List<Object> list = (List<Object>) o;
-        List<Object> result = null;
+        List<Object> result;
         try {
           result =
               (rawType == List.class)
@@ -276,7 +276,11 @@ private static <T> T deserializeToParameterizedType(
             | IllegalAccessException
             | NoSuchMethodException
             | InvocationTargetException e) {
-          throw new RuntimeException(e);
+          throw deserializeError(
+              context.errorPath,
+              String.format(
+                  "Unable to deserialize to the %s type: %s",
+                  rawType.getSimpleName(), e.toString()));
         }
         for (int i = 0; i < list.size(); i++) {
           result.add(
@@ -298,7 +302,21 @@ private static <T> T deserializeToParameterizedType(
             "Only Maps with string keys are supported, but found Map with key type " + keyType);
       }
       Map<String, Object> map = expectMap(o, context);
-      HashMap<String, Object> result = new HashMap<>();
+      HashMap<String, Object> result;
+      try {
+        result =
+            (rawType == Map.class)
+                ? new HashMap<String, Object>()
+                : (HashMap<String, Object>) rawType.getDeclaredConstructor().newInstance();
+      } catch (InstantiationException
+          | IllegalAccessException
+          | NoSuchMethodException
+          | InvocationTargetException e) {
+        throw deserializeError(
+            context.errorPath,
+            String.format(
+                "Unable to deserialize to the %s type: %s", rawType.getSimpleName(), e.toString()));
+      }
       for (Map.Entry<String, Object> entry : map.entrySet()) {
         result.put(
             entry.getKey(),

diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java
@@ -25,6 +25,7 @@
 import static com.google.cloud.firestore.LocalFirestoreHelper.DOCUMENT_PATH;
 import static com.google.cloud.firestore.LocalFirestoreHelper.FIELD_TRANSFORM_COMMIT_RESPONSE;
 import static com.google.cloud.firestore.LocalFirestoreHelper.FOO_LIST;
+import static com.google.cloud.firestore.LocalFirestoreHelper.FOO_MAP;
 import static com.google.cloud.firestore.LocalFirestoreHelper.GEO_POINT;
 import static com.google.cloud.firestore.LocalFirestoreHelper.NESTED_CLASS_OBJECT;
 import static com.google.cloud.firestore.LocalFirestoreHelper.SERVER_TIMESTAMP_PROTO;
@@ -1105,5 +1106,38 @@ public void deserializeCustomList() throws ExecutionException, InterruptedExcept
         snapshot.toObject(LocalFirestoreHelper.CustomList.class);
 
     assertEquals(FOO_LIST, customList.fooList);
+    assertEquals(SINGLE_FIELD_OBJECT, customList.fooList.get(0));
+  }
+
+  @Test
+  public void deserializeCustomMap() throws ExecutionException, InterruptedException {
+    ImmutableMap CUSTOM_MAP_PROTO =
+        ImmutableMap.<String, Value>builder()
+            .put(
+                "fooMap",
+                Value.newBuilder()
+                    .setMapValue(
+                        MapValue.newBuilder()
+                            .putFields(
+                                "customMap",
+                                Value.newBuilder()
+                                    .setMapValue(
+                                        MapValue.newBuilder().putAllFields(SINGLE_FIELD_PROTO))
+                                    .build())
+                            .build())
+                    .build())
+            .build();
+    doAnswer(getAllResponse(CUSTOM_MAP_PROTO))
+        .when(firestoreMock)
+        .streamRequest(
+            getAllCapture.capture(),
+            streamObserverCapture.capture(),
+            Matchers.<ServerStreamingCallable>any());
+    DocumentSnapshot snapshot = documentReference.get().get();
+    LocalFirestoreHelper.CustomMap customMap =
+        snapshot.toObject(LocalFirestoreHelper.CustomMap.class);
+
+    assertEquals(FOO_MAP, customMap.fooMap);
+    assertEquals(SINGLE_FIELD_OBJECT, customMap.fooMap.get("customMap"));
   }
 }
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java
@@ -144,6 +144,7 @@ public final class LocalFirestoreHelper {
   public static final GeoPoint GEO_POINT;
   public static final Blob BLOB;
   public static final FooList<SingleField> FOO_LIST = new FooList<>();
+  public static final FooMap<String, SingleField> FOO_MAP = new FooMap<>();
 
   public static final Precondition UPDATE_PRECONDITION;
 
@@ -178,6 +179,18 @@ public CustomList() {}
     public FooList<SingleField> fooList;
   }
 
+  public static class FooMap<K, V> extends HashMap<K, V> {
+    public FooMap() {
+      super();
+    }
+  }
+
+  public static class CustomMap {
+    public CustomMap() {}
+
+    public FooMap<String, SingleField> fooMap;
+  }
+
   public static class NestedClass {
     public SingleField first = new SingleField();
     public AllSupportedTypes second = new AllSupportedTypes();
@@ -787,6 +800,7 @@ public boolean equals(Object o) {
     SINGLE_FILED_MAP_WITH_DOT = map("c.d", (Object) "bar");
     SINGLE_FIELD_OBJECT = new SingleField();
     FOO_LIST.add(SINGLE_FIELD_OBJECT);
+    FOO_MAP.put("customMap", SINGLE_FIELD_OBJECT);
     SINGLE_FIELD_PROTO = map("foo", Value.newBuilder().setStringValue("bar").build());
     UPDATED_POJO_PROTO =
         map(

diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java
@@ -17,6 +17,7 @@
 package com.google.cloud.firestore.it;
 
 import static com.google.cloud.firestore.LocalFirestoreHelper.FOO_LIST;
+import static com.google.cloud.firestore.LocalFirestoreHelper.FOO_MAP;
 import static com.google.cloud.firestore.LocalFirestoreHelper.UPDATE_SINGLE_FIELD_OBJECT;
 import static com.google.cloud.firestore.LocalFirestoreHelper.map;
 import static com.google.common.truth.Truth.assertThat;
@@ -1412,7 +1413,23 @@ public void deserializeCustomList() throws Exception {
     DocumentSnapshot documentSnapshots = documentReference.get().get();
     LocalFirestoreHelper.CustomList targetCustomList =
         documentSnapshots.toObject(LocalFirestoreHelper.CustomList.class);
+
     assertEquals(FOO_LIST, targetCustomList.fooList);
+    assertEquals(SINGLE_FIELD_OBJECT, targetCustomList.fooList.get(0));
+  }
+
+  @Test
+  public void deserializeCustomMap() throws Exception {
+    LocalFirestoreHelper.CustomMap customMap = new LocalFirestoreHelper.CustomMap();
+    customMap.fooMap = FOO_MAP;
+    DocumentReference documentReference = randomColl.document("doc1");
+    documentReference.set(customMap).get();
+    DocumentSnapshot documentSnapshots = documentReference.get().get();
+    LocalFirestoreHelper.CustomMap targetCustomMap =
+        documentSnapshots.toObject(LocalFirestoreHelper.CustomMap.class);
+
+    assertEquals(FOO_MAP, targetCustomMap.fooMap);
+    assertEquals(SINGLE_FIELD_OBJECT, targetCustomMap.fooMap.get("customMap"));
   }
 
   /** Wrapper around ApiStreamObserver that returns the results in a list. */