Allow passing POJOs as field values throughout API.

firebase-firestore/CHANGELOG.md

@@ -6,6 +6,9 @@
 - [changed] Eliminated superfluous update events for locally cached documents
   that are known to lag behind the server version. Instead, we buffer these
   events until the client has caught up with the server.
+- [feature] Custom objects (POJOs) can now be passed as a field value in
+  update(), within `Map<>` objects passed to set(), in array transform
+  operations, and in query filters.
 
 # 17.1.1
 - [fixed] Fixed an issue where the first `get()` call made after being offline

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/POJOTest.java

@@ -15,13 +15,18 @@
 package com.google.firebase.firestore;
 
 import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testCollection;
+import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testDocument;
 import static com.google.firebase.firestore.testutil.IntegrationTestUtil.waitFor;
 import static com.google.firebase.firestore.testutil.TestUtil.expectError;
+import static com.google.firebase.firestore.testutil.TestUtil.map;
 import static junit.framework.Assert.assertEquals;
 
 import android.support.test.runner.AndroidJUnit4;
+import com.google.android.gms.tasks.Task;
+import com.google.android.gms.tasks.Tasks;
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.testutil.IntegrationTestUtil;
+import java.util.ArrayList;
 import java.util.Date;
 import org.junit.After;
 import org.junit.Test;
@@ -175,7 +180,7 @@ public void testWriteAndRead() {
   }
 
   @Test
-  public void testUpdate() {
+  public void testSetMerge() {
     CollectionReference collection = testCollection();
     POJO data = new POJO(1.0, "a", collection.document());
     DocumentReference reference = waitFor(collection.add(data));
@@ -190,6 +195,33 @@ public void testUpdate() {
     assertEquals(expected, doc.toObject(POJO.class));
   }
 
+  // General smoke test that makes sure APIs accept POJOs.
+  @Test
+  public void testAPIsAcceptPOJOsForFields() {
+    DocumentReference ref = testDocument();
+    ArrayList<Task<?>> tasks = new ArrayList<>();
+
+    // as Map<> entries in a set() call.
+    POJO data = new POJO(1.0, "a", ref);
+    tasks.add(ref.set(map("a", data, "b", map("c", data))));
+
+    // as Map<> entries in an update() call.
+    tasks.add(ref.update(map("a", data)));
+
+    // as field values in an update() call.
+    tasks.add(ref.update("c", data));
+
+    // as values in arrayUnion() / arrayRemove().
+    tasks.add(ref.update("c", FieldValue.arrayUnion(data)));
+    tasks.add(ref.update("c", FieldValue.arrayRemove(data)));
+
+    // as Query parameters.
+    data.setBlob(null); // blobs are broken, see b/117680212
+    tasks.add(testCollection().whereEqualTo("field", data).get());
+
+    waitFor(Tasks.whenAll(tasks));
+  }
+
   @Test
   public void setFieldMaskMustHaveCorrespondingValue() {
     CollectionReference collection = testCollection();

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java

@@ -376,7 +376,8 @@ public void arrayTransformsFailInQueries() {
   @Test
   public void arrayTransformsRejectInvalidElements() {
     DocumentReference doc = testDocument();
-    String reason = "Invalid data. Unsupported type: com.google.firebase.firestore.ValidationTest";
+    String reason =
+        "No properties to serialize found on class com.google.firebase.firestore.ValidationTest";
     // TODO: If we get more permissive with POJOs, perhaps we should make this work.
     expectError(() -> doc.set(map("x", FieldValue.arrayUnion(1, this))), reason);
     expectError(() -> doc.set(map("x", FieldValue.arrayRemove(1, this))), reason);
@@ -558,15 +559,8 @@ private static void expectWriteError(
     DocumentReference ref = testDocument();
 
     if (includeSets) {
-      if (data instanceof Map) {
-        @SuppressWarnings("unchecked")
-        Map<String, Object> setMap = (Map<String, Object>) data;
-        expectError(() -> ref.set(setMap), reason);
-        expectError(() -> ref.getFirestore().batch().set(ref, setMap), reason);
-      } else {
-        expectError(() -> ref.set(data), reason);
-        expectError(() -> ref.getFirestore().batch().set(ref, data), reason);
-      }
+      expectError(() -> ref.set(data), reason);
+      expectError(() -> ref.getFirestore().batch().set(ref, data), reason);
     }
 
     if (includeUpdates) {
@@ -593,13 +587,7 @@ private static void expectWriteError(
                 (Function<Void>)
                     transaction -> {
                       if (includeSets) {
-                        if (data instanceof Map) {
-                          @SuppressWarnings("unchecked")
-                          Map<String, Object> setMap = (Map<String, Object>) data;
-                          expectError(() -> transaction.set(ref, setMap), reason);
-                        } else {
-                          expectError(() -> transaction.set(ref, data), reason);
-                        }
+                        expectError(() -> transaction.set(ref, data), reason);
                       }
                       if (includeUpdates) {
                         assertTrue("update() only support Maps.", data instanceof Map);

firebase-firestore/src/main/java/com/google/firebase/firestore/CollectionReference.java

@@ -23,7 +23,6 @@
 import com.google.firebase.firestore.model.ResourcePath;
 import com.google.firebase.firestore.util.Executors;
 import com.google.firebase.firestore.util.Util;
-import java.util.Map;
 import javax.annotation.Nullable;
 
 /**
@@ -117,12 +116,13 @@ public DocumentReference document(@NonNull String documentPath) {
    * Adds a new document to this collection with the specified data, assigning it a document ID
    * automatically.
    *
-   * @param data A Map containing the data for the new document.
+   * @param data The data to write to the document (e.g. a Map or a POJO containing the desired
+   *     document contents).
    * @return A Task that will be resolved with the DocumentReference of the newly created document.
    */
   @NonNull
   @PublicApi
-  public Task<DocumentReference> add(@NonNull Map<String, Object> data) {
+  public Task<DocumentReference> add(@NonNull Object data) {
     checkNotNull(data, "Provided data must not be null.");
     final DocumentReference ref = document();
     return ref.set(data)
@@ -134,17 +134,4 @@ public Task<DocumentReference> add(@NonNull Map<String, Object> data) {
               return ref;
             });
   }
-
-  /**
-   * Adds a new document to this collection with the specified POJO as contents, assigning it a
-   * document ID automatically.
-   *
-   * @param pojo The POJO that will be used to populate the contents of the document
-   * @return A Task that will be resolved with the DocumentReference of the newly created document.
-   */
-  @NonNull
-  @PublicApi
-  public Task<DocumentReference> add(@NonNull Object pojo) {
-    return add(firestore.getDataConverter().convertPOJO(pojo));
-  }
 }

firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentReference.java

@@ -143,12 +143,13 @@ public CollectionReference collection(@NonNull String collectionPath) {
    * Overwrites the document referred to by this DocumentReference. If the document does not yet
    * exist, it will be created. If a document already exists, it will be overwritten.
    *
-   * @param data A map of the fields and values for the document.
+   * @param data The data to write to the document (e.g. a Map or a POJO containing the desired
+   *     document contents).
    * @return A Task that will be resolved when the write finishes.
    */
   @NonNull
   @PublicApi
-  public Task<Void> set(@NonNull Map<String, Object> data) {
+  public Task<Void> set(@NonNull Object data) {
     return set(data, SetOptions.OVERWRITE);
   }
 
@@ -157,13 +158,14 @@ public Task<Void> set(@NonNull Map<String, Object> data) {
    * exist, it will be created. If you pass {@link SetOptions}, the provided data can be merged into
    * an existing document.
    *
-   * @param data A map of the fields and values for the document.
+   * @param data The data to write to the document (e.g. a Map or a POJO containing the desired
+   *     document contents).
    * @param options An object to configure the set behavior.
    * @return A Task that will be resolved when the write finishes.
    */
   @NonNull
   @PublicApi
-  public Task<Void> set(@NonNull Map<String, Object> data, @NonNull SetOptions options) {
+  public Task<Void> set(@NonNull Object data, @NonNull SetOptions options) {
     checkNotNull(data, "Provided data must not be null.");
     checkNotNull(options, "Provided options must not be null.");
     ParsedSetData parsed =
@@ -176,34 +178,6 @@ public Task<Void> set(@NonNull Map<String, Object> data, @NonNull SetOptions opt
         .continueWith(Executors.DIRECT_EXECUTOR, voidErrorTransformer());
   }
 
-  /**
-   * Overwrites the document referred to by this DocumentReference. If the document does not yet
-   * exist, it will be created. If a document already exists, it will be overwritten.
-   *
-   * @param pojo The POJO that will be used to populate the document contents
-   * @return A Task that will be resolved when the write finishes.
-   */
-  @NonNull
-  @PublicApi
-  public Task<Void> set(@NonNull Object pojo) {
-    return set(firestore.getDataConverter().convertPOJO(pojo), SetOptions.OVERWRITE);
-  }
-
-  /**
-   * Writes to the document referred to by this DocumentReference. If the document does not yet
-   * exist, it will be created. If you pass {@link SetOptions}, the provided data can be merged into
-   * an existing document.
-   *
-   * @param pojo The POJO that will be used to populate the document contents
-   * @param options An object to configure the set behavior.
-   * @return A Task that will be resolved when the write finishes.
-   */
-  @NonNull
-  @PublicApi
-  public Task<Void> set(@NonNull Object pojo, @NonNull SetOptions options) {
-    return set(firestore.getDataConverter().convertPOJO(pojo), options);
-  }
-
   /**
    * Updates fields in the document referred to by this DocumentReference. If no document exists
    * yet, the update will fail.

firebase-firestore/src/main/java/com/google/firebase/firestore/Transaction.java

@@ -60,13 +60,13 @@ public class Transaction {
    * yet exist, it will be created. If a document already exists, it will be overwritten.
    *
    * @param documentRef The DocumentReference to overwrite.
-   * @param data A map of the fields and values for the document.
+   * @param data The data to write to the document (e.g. a Map or a POJO containing the desired
+   *     document contents).
    * @return This Transaction instance. Used for chaining method calls.
    */
   @NonNull
   @PublicApi
-  public Transaction set(
-      @NonNull DocumentReference documentRef, @NonNull Map<String, Object> data) {
+  public Transaction set(@NonNull DocumentReference documentRef, @NonNull Object data) {
     return set(documentRef, data, SetOptions.OVERWRITE);
   }
 
@@ -76,16 +76,15 @@ public Transaction set(
    * into an existing document.
    *
    * @param documentRef The DocumentReference to overwrite.
-   * @param data A map of the fields and values for the document.
+   * @param data The data to write to the document (e.g. a Map or a POJO containing the desired
+   *     document contents).
    * @param options An object to configure the set behavior.
    * @return This Transaction instance. Used for chaining method calls.
    */
   @NonNull
   @PublicApi
   public Transaction set(
-      @NonNull DocumentReference documentRef,
-      @NonNull Map<String, Object> data,
-      @NonNull SetOptions options) {
+      @NonNull DocumentReference documentRef, @NonNull Object data, @NonNull SetOptions options) {
     firestore.validateReference(documentRef);
     checkNotNull(data, "Provided data must not be null.");
     checkNotNull(options, "Provided options must not be null.");
@@ -97,37 +96,6 @@ public Transaction set(
     return this;
   }
 
-  /**
-   * Overwrites the document referred to by the provided DocumentReference. If the document does not
-   * yet exist, it will be created. If a document already exists, it will be overwritten.
-   *
-   * @param documentRef The DocumentReference to overwrite.
-   * @param pojo The POJO that will be used to populate the document contents
-   * @return This Transaction instance. Used for chaining method calls.
-   */
-  @NonNull
-  @PublicApi
-  public Transaction set(@NonNull DocumentReference documentRef, @NonNull Object pojo) {
-    return set(documentRef, firestore.getDataConverter().convertPOJO(pojo), SetOptions.OVERWRITE);
-  }
-
-  /**
-   * Writes to the document referred to by the provided DocumentReference. If the document does not
-   * yet exist, it will be created. If you pass {@link SetOptions}, the provided data can be merged
-   * into an existing document.
-   *
-   * @param documentRef The DocumentReference to overwrite.
-   * @param pojo The POJO that will be used to populate the document contents
-   * @param options An object to configure the set behavior.
-   * @return This Transaction instance. Used for chaining method calls.
-   */
-  @NonNull
-  @PublicApi
-  public Transaction set(
-      @NonNull DocumentReference documentRef, @NonNull Object pojo, @NonNull SetOptions options) {
-    return set(documentRef, firestore.getDataConverter().convertPOJO(pojo), options);
-  }
-
   /**
    * Updates fields in the document referred to by the provided DocumentReference. If no document
    * exists yet, the update will fail.