From 0e20e52214e1085a7a116de7ff0912b2495a4e4d Mon Sep 17 00:00:00 2001 From: suraj dhamecha Date: Fri, 14 Aug 2020 20:04:29 +0530 Subject: [PATCH 1/3] fix: add support for deserialize a class extending ArrayList --- .../cloud/firestore/CustomClassMapper.java | 13 +++++++- .../firestore/DocumentReferenceTest.java | 33 +++++++++++++++++++ .../cloud/firestore/LocalFirestoreHelper.java | 14 ++++++++ .../cloud/firestore/it/ITSystemTest.java | 13 ++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) 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 index 6c685767e..f50852c2b 100644 --- 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,18 @@ private static T deserializeToParameterizedType( Type genericType = type.getActualTypeArguments()[0]; if (o instanceof List) { List list = (List) o; - List result = new ArrayList<>(list.size()); + List result = null; + try { + result = + (rawType == List.class) + ? new ArrayList<>(list.size()) + : (List) rawType.getDeclaredConstructor().newInstance(); + } catch (InstantiationException + | IllegalAccessException + | NoSuchMethodException + | InvocationTargetException e) { + throw new RuntimeException(e); + } for (int i = 0; i < list.size(); i++) { result.add( deserializeToType( 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 index 9464c4c22..6196499eb 100644 --- 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 @@ -24,6 +24,7 @@ import static com.google.cloud.firestore.LocalFirestoreHelper.DOCUMENT_NAME; 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.GEO_POINT; import static com.google.cloud.firestore.LocalFirestoreHelper.NESTED_CLASS_OBJECT; import static com.google.cloud.firestore.LocalFirestoreHelper.SERVER_TIMESTAMP_PROTO; @@ -69,10 +70,13 @@ import com.google.cloud.firestore.LocalFirestoreHelper.InvalidPOJO; import com.google.cloud.firestore.spi.v1.FirestoreRpc; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.firestore.v1.ArrayValue; import com.google.firestore.v1.BatchGetDocumentsRequest; import com.google.firestore.v1.BatchGetDocumentsResponse; import com.google.firestore.v1.CommitRequest; import com.google.firestore.v1.CommitResponse; +import com.google.firestore.v1.MapValue; import com.google.firestore.v1.Value; import java.math.BigInteger; import java.util.ArrayList; @@ -1073,4 +1077,33 @@ public void deleteNestedFieldUsingFieldPath() throws Exception { Collections.emptyMap(), Collections.singletonList("`a.b`.`c.d`"))); assertEquals(expectedCommit, commitCapture.getValue()); } + + @Test + public void deserializeCustomList() throws ExecutionException, InterruptedException { + ImmutableMap CUSTOM_LIST_PROTO = + ImmutableMap.builder() + .put( + "fooList", + Value.newBuilder() + .setArrayValue( + ArrayValue.newBuilder() + .addValues( + Value.newBuilder() + .setMapValue( + MapValue.newBuilder().putAllFields(SINGLE_FIELD_PROTO)) + .build())) + .build()) + .build(); + doAnswer(getAllResponse(CUSTOM_LIST_PROTO)) + .when(firestoreMock) + .streamRequest( + getAllCapture.capture(), + streamObserverCapture.capture(), + Matchers.any()); + DocumentSnapshot snapshot = documentReference.get().get(); + LocalFirestoreHelper.CustomList customList = + snapshot.toObject(LocalFirestoreHelper.CustomList.class); + + assertEquals(FOO_LIST, customList.fooList); + } } 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 index db022ba95..7048f4c7e 100644 --- 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 @@ -143,6 +143,7 @@ public final class LocalFirestoreHelper { public static final Timestamp TIMESTAMP; public static final GeoPoint GEO_POINT; public static final Blob BLOB; + public static final FooList FOO_LIST = new FooList<>(); public static final Precondition UPDATE_PRECONDITION; @@ -165,6 +166,18 @@ public boolean equals(Object o) { } } + public static class FooList extends ArrayList { + public FooList() { + super(); + } + } + + public static class CustomList { + public CustomList() {} + + public FooList fooList; + } + public static class NestedClass { public SingleField first = new SingleField(); public AllSupportedTypes second = new AllSupportedTypes(); @@ -773,6 +786,7 @@ public boolean equals(Object o) { SINGLE_FIELD_MAP = map("foo", (Object) "bar"); SINGLE_FILED_MAP_WITH_DOT = map("c.d", (Object) "bar"); SINGLE_FIELD_OBJECT = new SingleField(); + FOO_LIST.add(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 index 512e88464..2da7cd218 100644 --- 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 @@ -16,6 +16,7 @@ package com.google.cloud.firestore.it; +import static com.google.cloud.firestore.LocalFirestoreHelper.FOO_LIST; 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; @@ -1402,6 +1403,18 @@ public Void updateCallback(Transaction transaction) throws Exception { } } + @Test + public void deserializeCustomList() throws Exception { + LocalFirestoreHelper.CustomList customList = new LocalFirestoreHelper.CustomList(); + customList.fooList = FOO_LIST; + DocumentReference documentReference = randomColl.document("doc1"); + documentReference.set(customList).get(); + DocumentSnapshot documentSnapshots = documentReference.get().get(); + LocalFirestoreHelper.CustomList targetCustomList = + documentSnapshots.toObject(LocalFirestoreHelper.CustomList.class); + assertEquals(FOO_LIST, targetCustomList.fooList); + } + /** Wrapper around ApiStreamObserver that returns the results in a list. */ private static class StreamConsumer implements ApiStreamObserver { SettableApiFuture> done = SettableApiFuture.create(); From 24e3ae97c9b8704542f328291dc493ea3c6a8826 Mon Sep 17 00:00:00 2001 From: suraj dhamecha Date: Thu, 20 Aug 2020 22:53:15 +0530 Subject: [PATCH 2/3] fix: fix review changes --- .../cloud/firestore/CustomClassMapper.java | 24 +++++++++++-- .../firestore/DocumentReferenceTest.java | 34 +++++++++++++++++++ .../cloud/firestore/LocalFirestoreHelper.java | 14 ++++++++ .../cloud/firestore/it/ITSystemTest.java | 17 ++++++++++ 4 files changed, 86 insertions(+), 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 index f50852c2b..9b3678567 100644 --- 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 deserializeToParameterizedType( Type genericType = type.getActualTypeArguments()[0]; if (o instanceof List) { List list = (List) o; - List result = null; + List result; try { result = (rawType == List.class) @@ -276,7 +276,11 @@ private static 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 deserializeToParameterizedType( "Only Maps with string keys are supported, but found Map with key type " + keyType); } Map map = expectMap(o, context); - HashMap result = new HashMap<>(); + HashMap result; + try { + result = + (rawType == Map.class) + ? new HashMap() + : (HashMap) 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 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 index 6196499eb..0cce72d07 100644 --- 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.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.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 index 7048f4c7e..47d63e376 100644 --- 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 FOO_LIST = new FooList<>(); + public static final FooMap FOO_MAP = new FooMap<>(); public static final Precondition UPDATE_PRECONDITION; @@ -178,6 +179,18 @@ public CustomList() {} public FooList fooList; } + public static class FooMap extends HashMap { + public FooMap() { + super(); + } + } + + public static class CustomMap { + public CustomMap() {} + + public FooMap 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 index 2da7cd218..8036bc71b 100644 --- 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. */ From 8843b5a96bb28c4f26617b2beb85308ab5c35fba Mon Sep 17 00:00:00 2001 From: suraj dhamecha Date: Thu, 27 Aug 2020 16:05:58 +0530 Subject: [PATCH 3/3] fix: fix review changes --- .../java/com/google/cloud/firestore/CustomClassMapper.java | 5 ++--- 1 file changed, 2 insertions(+), 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 index 9b3678567..96689ff6f 100644 --- 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 @@ -279,8 +279,7 @@ private static T deserializeToParameterizedType( throw deserializeError( context.errorPath, String.format( - "Unable to deserialize to the %s type: %s", - rawType.getSimpleName(), e.toString())); + "Unable to deserialize to %s: %s", rawType.getSimpleName(), e.toString())); } for (int i = 0; i < list.size(); i++) { result.add( @@ -315,7 +314,7 @@ private static T deserializeToParameterizedType( throw deserializeError( context.errorPath, String.format( - "Unable to deserialize to the %s type: %s", rawType.getSimpleName(), e.toString())); + "Unable to deserialize to %s: %s", rawType.getSimpleName(), e.toString())); } for (Map.Entry entry : map.entrySet()) { result.put(