fix: add support for deleting nested fields that contain periods (#295)

googleapis · Jul 16, 2020 · 84f602e · 84f602e
1 parent b91c57c
commit 84f602e
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 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
@@ -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<String> 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
@@ -160,7 +160,9 @@ static Value encodeValue(
       Map<String, Object> map = (Map<String, Object>) sanitizedObject;
 
       for (Map.Entry<String, Object> 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
@@ -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.<UnaryCallable<CommitRequest, CommitResponse>>any());
+    FieldPath path = FieldPath.of("a.b", "c.d");
+    documentReference.update(path, FieldValue.delete()).get();
+    CommitRequest expectedCommit =
+        commit(
+            update(
+                Collections.<String, Value>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
@@ -93,6 +93,7 @@ public final class LocalFirestoreHelper {
   public static final Map<String, Value> EMPTY_MAP_PROTO;
 
   public static final Map<String, Object> SINGLE_FIELD_MAP;
+  public static final Map<String, Object> SINGLE_FILED_MAP_WITH_DOT;
   public static final SingleField SINGLE_FIELD_OBJECT;
   public static final Map<String, Value> 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
@@ -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<String, Object> 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"));
+  }
 }