Google-cloud-python: Firestore: Out of expect when document update SERVER_TIMESTAMP and an arbitrary value by field path

[fishballLin]

Environment details

1. OS type and version:
   Both happened on Windows 7 and Ubuntu 18.04
2. Python version:
   python 3.6
3. google-cloud- version:
   firebase-admin 2.14.0
   google-api-core 1.7.0
   google-auth 1.6.2
   google-cloud-core 0.29.1
   google-cloud-firestore 0.31.0
   google-cloud-storage 1.13.2
   google-resumable-media 0.3.2
   googleapis-common-protos 1.5.5

Reference

The following case is modified by the firestore document update section example.

Steps to reproduce

1. To illustrate this, consider a document with

>>> snapshot = document.get()
>>> snapshot.to_dict()
{
    'foo': {
        'bar': 'baz',
    },
    'other': True,
}

stored on the server.

1. To set a field to the current time and another field to an arbitrary value(ex: "test"). We expect sending

>>> field_updates = {
...     'foo.now': firestore.SERVER_TIMESTAMP,
...     'foo.test': "test",
... }
>>> document.update(field_updates)

would update the value on the server to:

>>> snapshot = document.get()
>>> snapshot.to_dict()
{
    'foo': {
        'bar': 'baz',
        'now': datetime.datetime(2019, ...),
        'test': 'test'
    },
    'other': True,
}

1. However, in the real case, the value on the server are:
   ```
   >>> snapshot = document.get()
   >>> snapshot.to_dict()
   {
   'foo': {
   'now': datetime.datetime(2019, ...),
   'test': 'test'
   },
   'other': True,
   }

4. Apparently, the previous field 'foo.bar' disappear, but it doesn't match our expect.

#### Code example

##### Step 1: prepare the data
```python
import firebase_admin
from firebase_admin import credentials, firestore

cred = credentials.Certificate('your-api-key.json')
firebase_admin.initialize_app(cred)

db = firestore.client()
document = db.collection('test').document()
document.set({
    "foo":{
        "bar": 'baz'
    },
    'other': True
})
snapshot = document.get()
print(snapshot.to_dict())

console:

{
    'foo': {
        'bar': 'baz',
    },
    'other': True,
}

Step 2: update the data

field_updates = {
    'foo.test': 'test',
    'foo.now': firestore.SERVER_TIMESTAMP,
}
document.update(field_updates)

snapshot = document.get()
print(snapshot.to_dict())

console:

{
    'foo': {
        'now': datetime.datetime(2019, ...),
        'test': 'test'
    },
    'other': True,
}

Expected output

{
    'foo': {
        'bar': 'baz',
        'now': datetime.datetime(2019, ...),
        'test': 'test'
    },
    'other': True,
}

Thank you.

[tseaver]

Working to reproduce with this gist:

$ python3.6 -m venv /tmp/gcp/7215
$ /tmp/gcp/7215/bin/pip install --upgrade setuptools pip
...
Successfully installed pip-19.0.1 setuptools-40.7.1
$ /tmp/gcp/7215/bin/pip install google-cloud-firestore
...
Successfully installed cachetools-3.0.0 certifi-2018.11.29 chardet-3.0.4 google-api-core-1.7.0 google-auth-1.6.2 google-cloud-core-0.29.1 google-cloud-firestore-0.31.0 googleapis-common-protos-1.5.6 grpcio-1.18.0 idna-2.8 protobuf-3.6.1 pyasn1-0.4.5 pyasn1-modules-0.2.4 pytz-2018.9 requests-2.21.0 rsa-4.0 six-1.12.0 urllib3-1.24.1
$ /tmp/gcp/7215/bin/python /tmp/gcp/7215/reproduce_7215.py

Prepared document: o8IQPjUXF4YgLMdjoaae
{'foo': {'bar': 'baz'}, 'other': True}

Updated document: o8IQPjUXF4YgLMdjoaae
{'foo': {'now': DatetimeWithNanoseconds(2019, 1, 29, 17, 18, 10, 477000, tzinfo=<UTC>),
         'test': 'test'},
 'other': True}

Deleted document: o8IQPjUXF4YgLMdjoaae

[tseaver]

@fishballLin Thanks for the detailed report!

[fishballLin]

@tseaver If you need any help or further information, please contact with me any time.
Thank you.

[tseaver]

OK, the Write protobufs being sent with the CommitRequest look like:

[update {
  name: "projects/tseaver-firestore/databases/(default)/documents/test/hCpyH9RfMqgbSK1TjuNe"
  fields {
    key: "foo"
    value {
      map_value {
        fields {
          key: "test"
          value {
            string_value: "test"
          }
        }
      }
    }
  }
}
update_mask {
  field_paths: "foo"
  field_paths: "foo.test"
}
current_document {
  exists: true
}
,
 transform {
  document: "projects/tseaver-firestore/databases/(default)/documents/test/hCpyH9RfMqgbSK1TjuNe"
  field_transforms {
    field_path: "foo.now"
    set_to_server_value: REQUEST_TIME
  }
}

Which is why foo['bar'] is getting deleted: having foo in the update_mask.field_paths means that foo is supposed to be replaced with the contents of the foo key in the update message.

We are generating it that way on account of a requirement that we include the "parent" key in the update mask of any nested transform.

@jadekler I think this may be a hole in our conformance testing: can you verify that the foo.bar field should not be erased when setting foo.now to the server timestamp in an update request?

[tseaver]

@jadekler I've created a textproto testcase which passes on the current Python implementation, but exposes this (unexpected) behavior.

[jadekler]

@tseaver https://github.com/googleapis/google-cloud-common/pull/275