From 84f602ef8be67e5748b77e549d46ea53d0c74335 Mon Sep 17 00:00:00 2001 From: Suraj Dhamecha <48670070+suraj-qlogic@users.noreply.github.com> Date: Fri, 17 Jul 2020 00:37:08 +0530 Subject: [PATCH] fix: add support for deleting nested fields that contain periods (#295) --- .../com/google/cloud/firestore/BasePath.java | 16 +++++++++++++++- .../cloud/firestore/UserDataConverter.java | 4 +++- .../cloud/firestore/DocumentReferenceTest.java | 15 +++++++++++++++ .../cloud/firestore/LocalFirestoreHelper.java | 2 ++ .../google/cloud/firestore/it/ITSystemTest.java | 15 +++++++++++++++ 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BasePath.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BasePath.java index 964f95003..be48f521d 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BasePath.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/BasePath.java @@ -54,11 +54,25 @@ B getParent() { * @param path A relative path */ B append(String path) { + return append(path, true); + } + + /** + * Returns a new path pointing to a child of this Path. + * + * @param path A relative path + * @param splitPath Whether or not the path should be split + */ + B append(String path, boolean splitPath) { Preconditions.checkArgument( path != null && !path.isEmpty(), "'path' must be a non-empty String"); ImmutableList.Builder components = ImmutableList.builder(); components.addAll(this.getSegments()); - components.add(splitChildPath(path)); + if (splitPath) { + components.add(splitChildPath(path)); + } else { + components.add(path); + } return createPathWithSegments(components.build()); } diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java index 2b08a12c7..14ee9553a 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UserDataConverter.java @@ -160,7 +160,9 @@ static Value encodeValue( Map map = (Map) sanitizedObject; for (Map.Entry entry : map.entrySet()) { - Value encodedValue = encodeValue(path.append(entry.getKey()), entry.getValue(), options); + Value encodedValue = + encodeValue( + path.append(entry.getKey(), /* splitPath= */ false), entry.getValue(), options); if (encodedValue != null) { res.putFields(entry.getKey(), encodedValue); } 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 030f53de2..6a908d56f 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 @@ -1069,4 +1069,19 @@ public void updateIndividualPojo() throws ExecutionException, InterruptedExcepti assertCommitEquals(expectedCommit, request); } } + + @Test + public void deleteNestedFieldUsingFieldPath() throws Exception { + doReturn(SINGLE_WRITE_COMMIT_RESPONSE) + .when(firestoreMock) + .sendRequest( + commitCapture.capture(), Matchers.>any()); + FieldPath path = FieldPath.of("a.b", "c.d"); + documentReference.update(path, FieldValue.delete()).get(); + CommitRequest expectedCommit = + commit( + update( + Collections.emptyMap(), Collections.singletonList("`a.b`.`c.d`"))); + assertEquals(expectedCommit, commitCapture.getValue()); + } } 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 541af7034..a57f36c05 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 @@ -93,6 +93,7 @@ public final class LocalFirestoreHelper { public static final Map EMPTY_MAP_PROTO; public static final Map SINGLE_FIELD_MAP; + public static final Map SINGLE_FILED_MAP_WITH_DOT; public static final SingleField SINGLE_FIELD_OBJECT; public static final Map SINGLE_FIELD_PROTO; public static final DocumentSnapshot SINGLE_FIELD_SNAPSHOT; @@ -723,6 +724,7 @@ public boolean equals(Object o) { map("inner", Value.newBuilder().setMapValue(MapValue.getDefaultInstance()).build()); SINGLE_FIELD_MAP = map("foo", (Object) "bar"); + SINGLE_FILED_MAP_WITH_DOT = map("c.d", (Object) "bar"); SINGLE_FIELD_OBJECT = new SingleField(); SINGLE_FIELD_PROTO = map("foo", Value.newBuilder().setStringValue("bar").build()); UPDATED_POJO_PROTO = 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 6ac8d3d1d..04f57b6a8 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 @@ -87,6 +87,8 @@ public class ITSystemTest { private final SingleField SINGLE_FIELD_OBJECT = LocalFirestoreHelper.SINGLE_FIELD_OBJECT; private final AllSupportedTypes ALL_SUPPORTED_TYPES_OBJECT = LocalFirestoreHelper.ALL_SUPPORTED_TYPES_OBJECT; + private final Map SINGLE_FILED_MAP_WITH_DOT = + LocalFirestoreHelper.SINGLE_FILED_MAP_WITH_DOT; @Rule public TestName testName = new TestName(); @@ -1269,4 +1271,17 @@ public void testInstanceReturnedByGetServiceCanBeUsedToDeserializeAQuery() throw fs.close(); Query.fromProto(fs, proto); } + + @Test + public void deleteNestedFieldUsingFieldPath() throws Exception { + DocumentReference documentReference = randomColl.document("doc1"); + documentReference.set(map("a.b", (Object) SINGLE_FILED_MAP_WITH_DOT)).get(); + DocumentSnapshot documentSnapshots = documentReference.get().get(); + assertEquals(SINGLE_FILED_MAP_WITH_DOT, documentSnapshots.getData().get("a.b")); + + FieldPath path = FieldPath.of("a.b", "c.d"); + documentReference.update(path, FieldValue.delete()).get(); + documentSnapshots = documentReference.get().get(); + assertNull(documentSnapshots.getData().get("c.d")); + } }