Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SERVER_TIMESTAMP should survive deep copies (dataclasses.asdict) #820

Closed
mgraczyk opened this issue Dec 12, 2023 · 0 comments · Fixed by #821
Closed

SERVER_TIMESTAMP should survive deep copies (dataclasses.asdict) #820

mgraczyk opened this issue Dec 12, 2023 · 0 comments · Fixed by #821
Labels
api: firestore Issues related to the googleapis/python-firestore API.

Comments

@mgraczyk
Copy link
Contributor

Problem

Currently SERVER_TIMESTAMP does not work if it has been copied via copy.deepcopy.
This is especially important for dataclasses.

Here is an example script to demonstrate the problem

from google.cloud.firestore_v1.transforms import Sentinel
from google.cloud.firestore_v1.transforms import SERVER_TIMESTAMP
import dataclasses

@dataclasses.dataclass
class Example:
  timestamp: type[SERVER_TIMESTAMP] = SERVER_TIMESTAMP

example = Example()


assert example.timestamp is SERVER_TIMESTAMP

asdict = dataclasses.asdict(example)
assert asdict['timestamp'] is SERVER_TIMESTAMP

Currently the second assertion fails because the Sentinel has been deep copied.

Traceback (most recent call last):
  File "/Users/michael/dev/rfp-tool/test.py", line 15, in <module>
    assert asdict['timestamp'] is SERVER_TIMESTAMP
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

Solution

The Sentinel class should define dunder __copy__ and __deepcopy__ methods.

diff --git a/google/cloud/firestore_v1/transforms.py b/google/cloud/firestore_v1/transforms.py
index f1361c9..ef0def8 100644
--- a/google/cloud/firestore_v1/transforms.py
+++ b/google/cloud/firestore_v1/transforms.py
@@ -26,6 +26,15 @@ class Sentinel(object):
     def __repr__(self):
         return "Sentinel: {}".format(self.description)

+    def __copy__(self):
+        # Sentinel identity should be preserved across copies.
+        return self
+
+    def __deepcopy__(self):
+        # Sentinel identity should be preserved across deep copies.
+        return self
+
+

 DELETE_FIELD = Sentinel("Value used to delete a field in a document.")

I will send a PR attached to this issue.

@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/python-firestore API. label Dec 12, 2023
mgraczyk added a commit to Quilt-AI/python-firestore that referenced this issue Dec 12, 2023
mgraczyk added a commit to Quilt-AI/python-firestore that referenced this issue Dec 12, 2023
mgraczyk added a commit to Quilt-AI/python-firestore that referenced this issue Dec 12, 2023
mgraczyk added a commit to Quilt-AI/python-firestore that referenced this issue Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/python-firestore API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant