From 180f5a965ca2ea8b22338d0cc186b3d8d3bb997e Mon Sep 17 00:00:00 2001 From: Praful Makani Date: Tue, 26 Nov 2019 01:03:11 +0530 Subject: [PATCH] feat(firestore): allow passing POJOs as field values throughout API reference (#6843) Ported from https://github.com/firebase/firebase-android-sdk/pull/76 --- .../cloud/firestore/CollectionReference.java | 22 +----- .../cloud/firestore/DocumentReference.java | 54 ++------------ .../cloud/firestore/DocumentSnapshot.java | 27 +++++++ .../google/cloud/firestore/UpdateBuilder.java | 72 ++++--------------- .../cloud/firestore/LocalFirestoreHelper.java | 15 +++- .../cloud/firestore/WriteBatchTest.java | 21 ++++++ 6 files changed, 85 insertions(+), 126 deletions(-) diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionReference.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionReference.java index 3939c2aa7..b5a684696 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionReference.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/CollectionReference.java @@ -28,7 +28,6 @@ import com.google.firestore.v1.DocumentMask; import com.google.firestore.v1.ListDocumentsRequest; import java.util.Iterator; -import java.util.Map; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -171,13 +170,13 @@ public void remove() { * Adds a new document to this collection with the specified data, assigning it a document ID * automatically. * - * @param fields A Map containing the data for the new document. + * @param fields The Map or POJO containing the data for the new document. * @return An ApiFuture that will be resolved with the DocumentReference of the newly created * document. * @see #document() */ @Nonnull - public ApiFuture add(@Nonnull final Map fields) { + public ApiFuture add(@Nonnull final Object fields) { final DocumentReference documentReference = document(); ApiFuture createFuture = documentReference.create(fields); @@ -192,23 +191,6 @@ public DocumentReference apply(WriteResult writeResult) { MoreExecutors.directExecutor()); } - /** - * 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 An ApiFuture that will be resolved with the DocumentReference of the newly created - * document. - * @see #document() - */ - public ApiFuture add(Object pojo) { - Object converted = CustomClassMapper.convertToPlainJavaTypes(pojo); - if (!(converted instanceof Map)) { - FirestoreException.invalidState("Can't set a document's data to an array or primitive"); - } - return add((Map) converted); - } - /** Returns a resource path pointing to this collection. */ ResourcePath getResourcePath() { return options.getParentPath().append(options.getCollectionId()); diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentReference.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentReference.java index 3a6e95b8a..48c3b634a 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentReference.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentReference.java @@ -135,24 +135,11 @@ public T apply(List results) { MoreExecutors.directExecutor()); } - /** - * Creates a new Document at the DocumentReference's Location. It fails the write if the document - * exists. - * - * @param fields A map of the fields and values for the document. - * @return An ApiFuture that will be resolved when the write finishes. - */ - @Nonnull - public ApiFuture create(@Nonnull Map fields) { - WriteBatch writeBatch = firestore.batch(); - return extractFirst(writeBatch.create(this, fields).commit()); - } - /** * Creates a new Document at the DocumentReference location. It fails the write if the document * exists. * - * @param pojo A map of the fields and values for the document. + * @param pojo The Map or POJO that will be used to populate the document contents. * @return An ApiFuture that will be resolved when the write finishes. */ @Nonnull @@ -165,11 +152,12 @@ public ApiFuture create(@Nonnull Object pojo) { * Overwrites the document referred to by this DocumentReference. If no document exists yet, it * will be created. If a document already exists, it will be overwritten. * - * @param fields A map of the fields and values for the document. + * @param fields The fields to write to the document (e.g. a Map or a POJO containing the desired + * document contents). * @return An ApiFuture that will be resolved when the write finishes. */ @Nonnull - public ApiFuture set(@Nonnull Map fields) { + public ApiFuture set(@Nonnull Object fields) { WriteBatch writeBatch = firestore.batch(); return extractFirst(writeBatch.set(this, fields).commit()); } @@ -179,45 +167,17 @@ public ApiFuture set(@Nonnull Map fields) { * exist, it will be created. If you pass {@link SetOptions}, the provided data can be merged into * an existing document. * - * @param fields A map of the fields and values for the document. + * @param fields The fields to write on the document (e.g. a Map or a POJO containing the desired + * document contents). * @param options An object to configure the set behavior. * @return An ApiFuture that will be resolved when the write finishes. */ @Nonnull - public ApiFuture set( - @Nonnull Map fields, @Nonnull SetOptions options) { + public ApiFuture set(@Nonnull Object fields, @Nonnull SetOptions options) { WriteBatch writeBatch = firestore.batch(); return extractFirst(writeBatch.set(this, fields, options).commit()); } - /** - * Overwrites the document referred to by this DocumentReference. If no document exists yet, 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 An ApiFuture that will be resolved when the write finishes. - */ - @Nonnull - public ApiFuture set(@Nonnull Object pojo) { - WriteBatch writeBatch = firestore.batch(); - return extractFirst(writeBatch.set(this, pojo).commit()); - } - - /** - * 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 An ApiFuture that will be resolved when the write finishes. - */ - @Nonnull - public ApiFuture set(@Nonnull Object pojo, @Nonnull SetOptions options) { - WriteBatch writeBatch = firestore.batch(); - return extractFirst(writeBatch.set(this, pojo, options).commit()); - } - /** * Updates fields in the document referred to by this DocumentReference. If the document doesn't * exist yet, the update will fail. diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentSnapshot.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentSnapshot.java index 325d7ee3b..957759936 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentSnapshot.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/DocumentSnapshot.java @@ -262,6 +262,19 @@ public Object get(@Nonnull String field) { return get(FieldPath.fromDotSeparatedString(field)); } + /** + * 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.fromDotSeparatedString(field), valueType); + } + /** * Returns the value at the field or null if the field doesn't exist. * @@ -280,6 +293,20 @@ public Object get(@Nonnull FieldPath fieldPath) { return convertToDateIfNecessary(decodedValue); } + /** + * 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, Class valueType) { + Object data = get(fieldPath); + return data == null ? null : CustomClassMapper.convertToCustomClass(data, valueType, docRef); + } + private Object convertToDateIfNecessary(Object decodedValue) { if (decodedValue instanceof Timestamp) { if (!this.firestore.areTimestampsInSnapshotsEnabled()) { diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java index f342ad5c6..efd0b67dc 100644 --- a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java +++ b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java @@ -105,13 +105,16 @@ private static Map expandObject(Map data) { * exists. * * @param documentReference The DocumentReference to create. - * @param fields A map of the fields and values for the document. + * @param fields The Map or POJO that will be used to populate the document contents. * @return The instance for chaining. */ @Nonnull - public T create( - @Nonnull DocumentReference documentReference, @Nonnull Map fields) { - return performCreate(documentReference, fields); + public T create(@Nonnull DocumentReference documentReference, @Nonnull Object fields) { + Object data = CustomClassMapper.convertToPlainJavaTypes(fields); + if (!(data instanceof Map)) { + FirestoreException.invalidState("Can't set a document's data to an array or primitive"); + } + return performCreate(documentReference, (Map) data); } private T performCreate( @@ -146,33 +149,16 @@ private Mutation addMutation() { return mutation; } - /** - * Creates a new Document at the DocumentReference location. It fails the write if the document - * exists. - * - * @param documentReference The DocumentReference to create. - * @param pojo A map of the fields and values for the document. - * @return The instance for chaining. - */ - @Nonnull - public T create(@Nonnull DocumentReference documentReference, @Nonnull Object pojo) { - Object data = CustomClassMapper.convertToPlainJavaTypes(pojo); - if (!(data instanceof Map)) { - FirestoreException.invalidState("Can't set a document's data to an array or primitive"); - } - return performCreate(documentReference, (Map) data); - } - /** * Overwrites the document referred to by this DocumentReference. If the document doesn't exist * yet, it will be created. If a document already exists, it will be overwritten. * * @param documentReference The DocumentReference to overwrite. - * @param fields A map of the field paths and values for the document. + * @param fields The Map or POJO that will be used to populate the document contents. * @return The instance for chaining. */ @Nonnull - public T set(@Nonnull DocumentReference documentReference, @Nonnull Map fields) { + public T set(@Nonnull DocumentReference documentReference, @Nonnull Object fields) { return set(documentReference, fields, SetOptions.OVERWRITE); } @@ -182,47 +168,16 @@ public T set(@Nonnull DocumentReference documentReference, @Nonnull Map fields, - @Nonnull SetOptions options) { - return performSet(documentReference, fields, options); - } - - /** - * Overwrites the document referred to by this DocumentReference. If the document doesn't exist - * yet, it will be created. If a document already exists, it will be overwritten. - * - * @param documentReference The DocumentReference to overwrite. - * @param pojo The POJO that will be used to populate the document contents. - * @return The instance for chaining. - */ - @Nonnull - public T set(@Nonnull DocumentReference documentReference, @Nonnull Object pojo) { - return set(documentReference, pojo, SetOptions.OVERWRITE); - } - - /** - * Overwrites the document referred to by this DocumentReference. If the document doesn't exist - * yet, it will be created. If you pass {@link SetOptions}, the provided data can be merged into - * an existing document. - * - * @param documentReference The DocumentReference to overwrite. - * @param pojo The POJO that will be used to populate the document contents. + * @param fields The Map or POJO that will be used to populate the document contents. * @param options An object to configure the set behavior. * @return The instance for chaining. */ @Nonnull public T set( @Nonnull DocumentReference documentReference, - @Nonnull Object pojo, + @Nonnull Object fields, @Nonnull SetOptions options) { - Object data = CustomClassMapper.convertToPlainJavaTypes(pojo); + Object data = CustomClassMapper.convertToPlainJavaTypes(fields); if (!(data instanceof Map)) { throw new IllegalArgumentException("Can't set a document's data to an array or primitive"); } @@ -471,8 +426,9 @@ private T performUpdate( @Nonnull FieldPath fieldPath, @Nullable Object value, Object[] moreFieldsAndValues) { + Object data = CustomClassMapper.convertToPlainJavaTypes(value); Map fields = new HashMap<>(); - fields.put(fieldPath, value); + fields.put(fieldPath, data); Preconditions.checkArgument( moreFieldsAndValues.length % 2 == 0, "moreFieldsAndValues must be key-value pairs."); 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 151070d85..0aff92980 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 @@ -89,8 +89,10 @@ public final class LocalFirestoreHelper { public static final Map SINGLE_FIELD_PROTO; public static final DocumentSnapshot SINGLE_FIELD_SNAPSHOT; public static final Value SINGLE_FIELD_VALUE; + public static final SingleField UPDATE_SINGLE_FIELD_OBJECT; public static final Map UPDATED_FIELD_MAP; public static final Map UPDATED_FIELD_PROTO; + public static final Map UPDATED_SINGLE_FIELD_PROTO; public static final Map SINGLE_FLOAT_MAP; public static final Map SINGLE_FLOAT_PROTO; @@ -731,10 +733,21 @@ public boolean equals(Object o) { Value.Builder singleFieldValueBuilder = Value.newBuilder(); singleFieldValueBuilder.getMapValueBuilder().putAllFields(SINGLE_FIELD_PROTO); SINGLE_FIELD_VALUE = singleFieldValueBuilder.build(); + UPDATE_SINGLE_FIELD_OBJECT = new SingleField(); + UPDATE_SINGLE_FIELD_OBJECT.foo = "foobar"; UPDATED_FIELD_MAP = map("foo", (Object) "foobar"); UPDATED_FIELD_PROTO = map("foo", Value.newBuilder().setStringValue("foobar").build()); - + UPDATED_SINGLE_FIELD_PROTO = + ImmutableMap.builder() + .put( + "foo", + Value.newBuilder() + .setMapValue( + MapValue.newBuilder() + .putFields("foo", Value.newBuilder().setStringValue("foobar").build())) + .build()) + .build(); SERVER_TIMESTAMP_MAP = new HashMap<>(); SERVER_TIMESTAMP_MAP.put("foo", FieldValue.serverTimestamp()); SERVER_TIMESTAMP_MAP.put("inner", new HashMap()); diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java index 2f1f32fbc..ac992db35 100644 --- a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java +++ b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/WriteBatchTest.java @@ -16,6 +16,8 @@ package com.google.cloud.firestore; +import static com.google.cloud.firestore.LocalFirestoreHelper.UPDATED_SINGLE_FIELD_PROTO; +import static com.google.cloud.firestore.LocalFirestoreHelper.UPDATE_SINGLE_FIELD_OBJECT; import static com.google.cloud.firestore.LocalFirestoreHelper.commit; import static com.google.cloud.firestore.LocalFirestoreHelper.commitResponse; import static com.google.cloud.firestore.LocalFirestoreHelper.create; @@ -109,6 +111,25 @@ public void updateDocument() throws Exception { assertEquals(commit(writes.toArray(new Write[] {})), commitRequest); } + @Test + public void updateDocumentWithPOJO() throws Exception { + doReturn(commitResponse(1, 0)) + .when(firestoreMock) + .sendRequest( + commitCapture.capture(), Matchers.>any()); + + batch.update(documentReference, "foo", UPDATE_SINGLE_FIELD_OBJECT); + assertEquals(1, batch.getMutationsSize()); + + List writeResults = batch.commit().get(); + assertEquals(1, writeResults.size()); + + CommitRequest actual = commitCapture.getValue(); + CommitRequest expected = + commit(update(UPDATED_SINGLE_FIELD_PROTO, Collections.singletonList("foo"))); + assertEquals(expected, actual); + } + @Test public void setDocument() throws Exception { doReturn(commitResponse(4, 0))