FireStore: Saving items that have both empty object values and SERVER_TIMESTAMP results in missing attributes.

[hishnash]

When saving data to firestore with an attribute that is an empty {}:

if the date fields are firestore.SERVER_TIMESTAMP then the field with an empty {} is not persisted to the db but if the date fields are Datetime instances then the empty object fields are persisted.

OS type and version

1. MacOS 10.13.6 (17G2307)
2. DOCKER container: Linux version 4.9.93-linuxkit-aufs (root@856d34d1168e) (gcc version 6.4.0 (Alpin
   e 6.4.0) ) Add support for namespaces #1 SMP Wed Jun 6 16:55:56 UTC 2018

Python version and virtual environment information python --version

Python 3.6.6

google-cloud-python versions

google-api-core==1.3.0
google-auth==1.5.1
google-cloud-core==0.28.1
google-cloud-firestore==0.29.0
google-cloud-storage==1.11.0
google-resumable-media==0.3.1

Stacktrace if available

NA

Steps to reproduce

1. save to the db:

data = {
     'date_created': firestore.SERVER_TIMESTAMP,
     'date_modified': firestore.SERVER_TIMESTAMP,
     'resources': {}
}

transaction.set(
    reference,
    data,
    option=db.write_option(exists=False),
)

2. look in firestore and notice resources attribute is not present.

save with date_created and date_modified = datetime.datetime.utcnow() and resources attributes is set with an empty object value.

[tseaver]

I can verify that the error is present empty dict value is omitted in google.cloud.firestore_v1._helpers.pbs_for_set (i.e, we are not copying the empty dict before we send it on the wire to the back-end).

[tseaver]

@lukesneeringer added an explicit skip of empty dicts in _helpers.process_server_timestamp in 8d03ef8.

@jba Can you confirm one way or another whether skipping empty dict values is correct?

[jba]

@schmidt-sebastian

[schmidt-sebastian]

You can implement this in a couple different ways, but in order to get the conformance tests to pass, you need to send the absolute minimum requests that produces the desired outcome:

• For set(empty:{}), you need to send an write request that sets 'empty' to an empty map.
• For set(nonEmpty:{time:ServerTimestamp}), you only need to send a server transform, since the backend will implicitly create or convert nonEmpty to a map.

This cases are covered well by the conformance tests, and I am a little surprised that this is broken. Let me know if I am not understanding this correctly.

[tseaver]

@schmidt-sebastian The reported case is a combination of the two. The code in Python which generates the server transform ends up munging the data (to comb out the server timestamp fields) and ends up dropping empty maps.

[tseaver]

I think the intent of the code in place is to avoid sending an empty map if the original map contained only server transforms: it overlooks the case that the map was originally empty.

[jba]

@jadekler to add this case to the conformance tests.

[tseaver]

PR #6050 implements a fix based on the understanding I noted above.