Skip to content

Firestore: support updating an individual field with pojo as a value from all update methods #126

@menulis

Description

@menulis

The fix googleapis/google-cloud-java#6711 introduced an ability to update an individual field with pojo as a value. However, this does not work with all update method signatures.

UpdateBuilder has two private polymorphic performUpdate methods: the first one (1) that takes 5 arguments and internally calls CustomClassMapper.convertToPlainJavaTypes followed by the call to the second performUpdate method (2) that takes three arguments:

private T performUpdate(
      @Nonnull DocumentReference documentReference,
      @Nonnull Precondition options,
      @Nonnull FieldPath fieldPath,
      @Nullable Object value,
      Object[] moreFieldsAndValues
) {
    Object data = CustomClassMapper.convertToPlainJavaTypes(value);
    // ...
    return performUpdate(documentReference, fields, options);
}
private T performUpdate(
      @Nonnull DocumentReference documentReference,
      @Nonnull final Map<FieldPath, Object> fields,
      @Nonnull Precondition precondition
) {
    ///...
}

UpdateBuilder has six public polymorphic update methods, but only four of them (the ones that take value argument of type Object) call (1), the other two are calling (2). The same is true for the six update methods in DocumentReference.

Not only this is a bit confusing, I also believe it is quite common to update multiple fields with a pojo as a value in a batch or transaction. Therefore it should be assumed that one or more values of the Map<String, Object> fields parameter can be pojos. This way the two versions of performUpdate could be merged into one or the only method that calls (2) could be (1).

To sum up, depending on which update signature is used, attempting to update a field with pojo as a value may still result in an exception (valid at least for google-cloud-firestore 1.32.4):

com.google.cloud.firestore.FirestoreException: Cannot convert DummyPojo to Firestore Value
    at com.google.cloud.firestore.FirestoreException.invalidState (FirestoreException.java:55)
    at com.google.cloud.firestore.UserDataConverter.encodeValue (UserDataConverter.java:180)
    at com.google.cloud.firestore.UserDataConverter.encodeValue (UserDataConverter.java:163)
    at com.google.cloud.firestore.DocumentSnapshot.fromObject (DocumentSnapshot.java:89)
    at com.google.cloud.firestore.UpdateBuilder.performUpdate (UpdateBuilder.java:514)
    at com.google.cloud.firestore.UpdateBuilder.update (UpdateBuilder.java:347)

Metadata

Metadata

Assignees

Labels

api: firestoreIssues related to the googleapis/java-firestore API.priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions