diff --git a/firebase-firestore/CHANGELOG.md b/firebase-firestore/CHANGELOG.md index 3b5bb4adf73..f8222997291 100644 --- a/firebase-firestore/CHANGELOG.md +++ b/firebase-firestore/CHANGELOG.md @@ -1,4 +1,10 @@ # Unreleased +- [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. +- [feature] DocumentSnapshot.get() now supports retrieving fields as + custom objects (POJOs) by passing a Class instance, e.g. + `snapshot.get("field", CustomType.class)`. # 17.1.2 - [changed] Changed the internal handling for locally updated documents that diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/POJOTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/POJOTest.java index 5e8c941f89f..bb60af5f7c3 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/POJOTest.java +++ b/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,55 @@ 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> 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 testDocumentSnapshotGetWithPOJOs() { + DocumentReference ref = testDocument(); + + // Go offline so that we can verify server timestamp behavior overload. + ref.getFirestore().disableNetwork(); + + POJO pojo = new POJO(1.0, "a", ref); + ref.set(map("field", pojo)); + + DocumentSnapshot snap = waitFor(ref.get()); + + assertEquals(pojo, snap.get("field", POJO.class)); + assertEquals(pojo, snap.get(FieldPath.of("field"), POJO.class)); + assertEquals( + pojo, snap.get("field", POJO.class, DocumentSnapshot.ServerTimestampBehavior.DEFAULT)); + assertEquals( + pojo, + snap.get( + FieldPath.of("field"), POJO.class, DocumentSnapshot.ServerTimestampBehavior.DEFAULT)); + } + @Test public void setFieldMaskMustHaveCorrespondingValue() { CollectionReference collection = testCollection(); diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ServerTimestampTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ServerTimestampTest.java index 9b71486e3bb..0f9e0652969 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ServerTimestampTest.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ServerTimestampTest.java @@ -248,6 +248,28 @@ public void testServerTimestampsUsesPreviousValueFromLocalMutation() { assertThat(remoteSnapshot.get("a")).isInstanceOf(Timestamp.class); } + @Test + public void testServerTimestampBehaviorOverloadsOfDocumentSnapshotGet() { + writeInitialData(); + waitFor(docRef.update(updateData)); + DocumentSnapshot snap = accumulator.awaitLocalEvent(); + + // Default behavior should return null timestamp (via any overload). + assertNull(snap.get("when")); + assertNull(snap.get(FieldPath.of("when"))); + assertNull(snap.get("when", Timestamp.class)); + assertNull(snap.get(FieldPath.of("when"), Timestamp.class)); + + // Estimate should return a Timestamp object (via any overload). + assertThat(snap.get("when", ServerTimestampBehavior.ESTIMATE)).isInstanceOf(Timestamp.class); + assertThat(snap.get(FieldPath.of("when"), ServerTimestampBehavior.ESTIMATE)) + .isInstanceOf(Timestamp.class); + assertThat(snap.get("when", Timestamp.class, ServerTimestampBehavior.ESTIMATE)) + .isInstanceOf(Timestamp.class); + assertThat(snap.get(FieldPath.of("when"), Timestamp.class, ServerTimestampBehavior.ESTIMATE)) + .isInstanceOf(Timestamp.class); + } + @Test public void testServerTimestampsWorkViaTransactionSet() { waitFor( diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java index 7bdc168da09..dd9ae258d67 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java @@ -257,22 +257,16 @@ public void writesMustNotContainReservedFieldNames() { @Test public void setsMustNotContainFieldValueDelete() { - // PORTING NOTE: We avoid using expectSetError(), since it hits the POJO overload which - // can't handle FieldValue.delete(). - DocumentReference ref = testDocument(); - expectError( - () -> ref.set(map("foo", FieldValue.delete())), + expectSetError( + map("foo", FieldValue.delete()), "Invalid data. FieldValue.delete() can only be used with update() and set() with " + "SetOptions.merge() (found in field foo)"); } @Test public void updatesMustNotContainNestedFieldValueDeletes() { - // PORTING NOTE: We avoid using expectSetError(), since it hits the POJO overload which - // can't handle FieldValue.delete(). - DocumentReference ref = testDocument(); - expectError( - () -> ref.update(map("foo", map("bar", FieldValue.delete()))), + expectUpdateError( + map("foo", map("bar", FieldValue.delete())), "Invalid data. FieldValue.delete() can only appear at the top level of your update data " + "(found in field foo.bar)"); } @@ -376,8 +370,8 @@ public void arrayTransformsFailInQueries() { @Test public void arrayTransformsRejectInvalidElements() { DocumentReference doc = testDocument(); - String reason = "Invalid data. Unsupported type: com.google.firebase.firestore.ValidationTest"; - // TODO: If we get more permissive with POJOs, perhaps we should make this work. + String reason = + "No properties to serialize found on class com.google.firebase.firestore.ValidationTest"; expectError(() -> doc.set(map("x", FieldValue.arrayUnion(1, this))), reason); expectError(() -> doc.set(map("x", FieldValue.arrayRemove(1, this))), reason); } @@ -558,15 +552,8 @@ private static void expectWriteError( DocumentReference ref = testDocument(); if (includeSets) { - if (data instanceof Map) { - @SuppressWarnings("unchecked") - Map setMap = (Map) 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 +580,7 @@ private static void expectWriteError( (Function) transaction -> { if (includeSets) { - if (data instanceof Map) { - @SuppressWarnings("unchecked") - Map setMap = (Map) 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); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/CollectionReference.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/CollectionReference.java index db3cf10a113..c8364f7a76f 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/CollectionReference.java +++ b/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 add(@NonNull Map data) { + public Task 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 add(@NonNull Map 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 add(@NonNull Object pojo) { - return add(firestore.getDataConverter().convertPOJO(pojo)); - } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentReference.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentReference.java index 4fe7323b62f..cde2b8a3e3c 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentReference.java +++ b/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 set(@NonNull Map data) { + public Task set(@NonNull Object data) { return set(data, SetOptions.OVERWRITE); } @@ -157,13 +158,14 @@ public Task set(@NonNull Map 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 set(@NonNull Map data, @NonNull SetOptions options) { + public Task 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 set(@NonNull Map 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 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 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. diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentSnapshot.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentSnapshot.java index e3444ab2e5e..4a13eefd90a 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentSnapshot.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentSnapshot.java @@ -289,6 +289,69 @@ public Object get( firestore.getFirestoreSettings().areTimestampsInSnapshotsEnabled())); } + /** + * Returns the value at the field, converted to a POJO, or null if the field or document doesn't + * exist. + * + * @param field The path to the field + * @param valueType The Java class to convert the field value to. + * @return The value at the given field or null. + */ + @Nullable + public T get(@NonNull String field, @NonNull Class valueType) { + return get(FieldPath.fromDotSeparatedPath(field), valueType, ServerTimestampBehavior.DEFAULT); + } + + /** + * Returns the value at the field, converted to a POJO, or null if the field or document doesn't + * exist. + * + * @param field The path to the field + * @param valueType The Java class to convert the field value to. + * @param serverTimestampBehavior Configures the behavior for server timestamps that have not yet + * been set to their final value. + * @return The value at the given field or null. + */ + @Nullable + public T get( + @NonNull String field, + @NonNull Class valueType, + @NonNull ServerTimestampBehavior serverTimestampBehavior) { + return get(FieldPath.fromDotSeparatedPath(field), valueType, serverTimestampBehavior); + } + + /** + * Returns the value at the field, converted to a POJO, or null if the field or document doesn't + * exist. + * + * @param fieldPath The path to the field + * @param valueType The Java class to convert the field value to. + * @return The value at the given field or null. + */ + @Nullable + public T get(@NonNull FieldPath fieldPath, @NonNull Class valueType) { + return get(fieldPath, valueType, ServerTimestampBehavior.DEFAULT); + } + + /** + * Returns the value at the field, converted to a POJO, or null if the field or document doesn't + * exist. + * + * @param fieldPath The path to the field + * @param valueType The Java class to convert the field value to. + * @param serverTimestampBehavior Configures the behavior for server timestamps that have not yet + * been set to their final value. + * @return The value at the given field or null. + */ + @Nullable + public T get( + @NonNull FieldPath fieldPath, + @NonNull Class valueType, + @NonNull ServerTimestampBehavior serverTimestampBehavior) { + Object data = get(fieldPath, serverTimestampBehavior); + return data == null ? null : CustomClassMapper.convertToCustomClass(data, valueType); + } + /** * Returns the value of the field as a boolean. If the value is not a boolean this will throw a * runtime exception. diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/Transaction.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/Transaction.java index 11f7601d49c..8951d3c4b08 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/Transaction.java +++ b/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 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 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. diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java index d02ae4eaac6..898a189fa3a 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java @@ -70,17 +70,16 @@ public UserDataConverter(DatabaseId databaseId) { } /** Parse document data from a non-merge set() call. */ - public ParsedSetData parseSetData(Map input) { + public ParsedSetData parseSetData(Object input) { ParseAccumulator accumulator = new ParseAccumulator(UserData.Source.Set); - FieldValue updateData = parseData(input, accumulator.rootContext()); - - return accumulator.toSetData((ObjectValue) updateData); + ObjectValue updateData = convertAndParseDocumentData(input, accumulator.rootContext()); + return accumulator.toSetData(updateData); } /** Parse document data from a set() call with SetOptions.merge() set. */ - public ParsedSetData parseMergeData(Map input, @Nullable FieldMask fieldMask) { + public ParsedSetData parseMergeData(Object input, @Nullable FieldMask fieldMask) { ParseAccumulator accumulator = new ParseAccumulator(UserData.Source.MergeSet); - ObjectValue updateData = (ObjectValue) parseData(input, accumulator.rootContext()); + ObjectValue updateData = convertAndParseDocumentData(input, accumulator.rootContext()); if (fieldMask != null) { // Verify that all elements specified in the field mask are part of the parsed context. @@ -94,7 +93,6 @@ public ParsedSetData parseMergeData(Map input, @Nullable FieldMa } return accumulator.toMergeData(updateData, fieldMask); - } else { return accumulator.toMergeData(updateData); } @@ -118,7 +116,9 @@ public ParsedUpdateData parseUpdateData(Map data) { // Add it to the field mask, but don't add anything to updateData. context.addToFieldMask(fieldPath); } else { - @Nullable FieldValue parsedValue = parseData(fieldValue, context.childContext(fieldPath)); + @Nullable + FieldValue parsedValue = + convertAndParseFieldData(fieldValue, context.childContext(fieldPath)); if (parsedValue != null) { context.addToFieldMask(fieldPath); updateData = updateData.set(fieldPath, parsedValue); @@ -168,7 +168,8 @@ public ParsedUpdateData parseUpdateData(List fieldsAndValues) { // Add it to the field mask, but don't add anything to updateData. context.addToFieldMask(parsedField); } else { - FieldValue parsedValue = parseData(fieldValue, context.childContext(parsedField)); + FieldValue parsedValue = + convertAndParseFieldData(fieldValue, context.childContext(parsedField)); if (parsedValue != null) { context.addToFieldMask(parsedField); updateData = updateData.set(parsedField, parsedValue); @@ -183,7 +184,7 @@ public ParsedUpdateData parseUpdateData(List fieldsAndValues) { public FieldValue parseQueryValue(Object input) { ParseAccumulator accumulator = new ParseAccumulator(UserData.Source.Argument); - @Nullable FieldValue parsed = parseData(input, accumulator.rootContext()); + @Nullable FieldValue parsed = convertAndParseFieldData(input, accumulator.rootContext()); hardAssert(parsed != null, "Parsed data should not be null."); hardAssert( accumulator.getFieldTransforms().isEmpty(), @@ -191,32 +192,38 @@ public FieldValue parseQueryValue(Object input) { return parsed; } + /** Converts a POJO to native types and then parses it into model types. */ + private FieldValue convertAndParseFieldData(Object input, ParseContext context) { + Object converted = CustomClassMapper.convertToPlainJavaTypes(input); + return parseData(converted, context); + } + /** - * Converts a POJO into a Map, throwing appropriate errors if it wasn't actually a proper POJO. + * Converts a POJO to native types and then parses it into model types. It expects the input to + * conform to document data (i.e. it must parse into an ObjectValue model type) and will throw an + * appropriate error otherwise. */ - public Map convertPOJO(Object pojo) { - checkNotNull(pojo, "Provided data must not be null."); - String reason = + private ObjectValue convertAndParseDocumentData(Object input, ParseContext context) { + String badDocReason = "Invalid data. Data must be a Map or a suitable POJO object, but it was "; // Check Array before calling CustomClassMapper since it'll give you a confusing message - // to use List instead, which also won't work. - if (pojo.getClass().isArray()) { - throw new IllegalArgumentException(reason + "an array"); + // to use List instead, which also won't work in a set(). + if (input.getClass().isArray()) { + throw new IllegalArgumentException(badDocReason + "an array"); } - Object converted = CustomClassMapper.convertToPlainJavaTypes(pojo); - if (!(converted instanceof Map)) { - throw new IllegalArgumentException(reason + "of type: " + Util.typeName(pojo)); - } + Object converted = CustomClassMapper.convertToPlainJavaTypes(input); + FieldValue value = parseData(converted, context); - @SuppressWarnings("unchecked") // CustomClassMapper promises to map keys to Strings. - Map map = (Map) converted; - return map; + if (!(value instanceof ObjectValue)) { + throw new IllegalArgumentException(badDocReason + "of type: " + Util.typeName(input)); + } + return (ObjectValue) value; } /** - * Internal helper for parsing user data. + * Recursive helper for parsing user data. * * @param input Data to be parsed. * @param context A context object representing the current path being parsed, the source of the @@ -416,7 +423,7 @@ private List parseArrayTransformElements(List elements) { // being unioned or removed are not considered writes since they cannot // contain any FieldValue sentinels, etc. ParseContext context = accumulator.rootContext(); - result.add(parseData(element, context.childContext(i))); + result.add(convertAndParseFieldData(element, context.childContext(i))); } return result; } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/WriteBatch.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/WriteBatch.java index 6590b9254f0..ec0fdd23c76 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/WriteBatch.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/WriteBatch.java @@ -59,12 +59,13 @@ public class WriteBatch { * 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 WriteBatch instance. Used for chaining method calls. */ @NonNull @PublicApi - public WriteBatch set(@NonNull DocumentReference documentRef, @NonNull Map data) { + public WriteBatch set(@NonNull DocumentReference documentRef, @NonNull Object data) { return set(documentRef, data, SetOptions.OVERWRITE); } @@ -74,18 +75,18 @@ public WriteBatch set(@NonNull DocumentReference documentRef, @NonNull Map 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."); verifyNotCommitted(); ParsedSetData parsed = options.isMerge() @@ -95,37 +96,6 @@ public WriteBatch 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 WriteBatch instance. Used for chaining method calls. - */ - @NonNull - @PublicApi - public WriteBatch 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 WriteBatch instance. Used for chaining method calls. - */ - @NonNull - @PublicApi - public WriteBatch 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. diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java index 2fc95af3d74..a82b1c83427 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/util/CustomClassMapper.java @@ -161,7 +161,8 @@ private static Object serialize(T o, ErrorPath path) { || o instanceof Timestamp || o instanceof GeoPoint || o instanceof Blob - || o instanceof DocumentReference) { + || o instanceof DocumentReference + || o instanceof FieldValue) { return o; } else { Class clazz = (Class) o.getClass(); @@ -508,12 +509,12 @@ private static T convertBean(Object o, Class clazz, ErrorPath path) { } } - private static RuntimeException serializeError(ErrorPath path, String reason) { + private static IllegalArgumentException serializeError(ErrorPath path, String reason) { reason = "Could not serialize object. " + reason; if (path.getLength() > 0) { reason = reason + " (found in field '" + path.toString() + "')"; } - return new RuntimeException(reason); + return new IllegalArgumentException(reason); } private static RuntimeException deserializeError(ErrorPath path, String reason) { diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java index 1e1cafe69fc..7855d4bea65 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/FieldValueTest.java @@ -411,7 +411,7 @@ public void testArraysFail() { wrap(array); fail("wrap should have failed"); } catch (IllegalArgumentException e) { - assertNotEquals(-1, e.getMessage().indexOf("use a List instead")); + assertNotEquals(-1, e.getMessage().indexOf("use Lists instead")); } }