-
Notifications
You must be signed in to change notification settings - Fork 37
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
fix: Fix Timestamp, Duration and FieldMask marshaling in REST transport #334
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
# Copyright 2022 Google LLC | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# https://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from google.protobuf import field_mask_pb2 | ||
|
||
|
||
class FieldMaskRule: | ||
"""A marshal between FieldMask and strings. | ||
|
||
See https://github.com/googleapis/proto-plus-python/issues/333 | ||
and | ||
https://developers.google.com/protocol-buffers/docs/proto3#json | ||
for more details. | ||
""" | ||
|
||
def to_python(self, value, *, absent: bool = None): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not used - should be removed ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not used, but other rules still have it (i ti snot used in most places), so keeping it like this for consistency. |
||
return value | ||
|
||
def to_proto(self, value): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What are possible types for value ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this protoplus proto or protobuf proto ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Proto-plus |
||
if isinstance(value, str): | ||
field_mask_value = field_mask_pb2.FieldMask() | ||
field_mask_value.FromJsonString(value=value) | ||
return field_mask_value | ||
|
||
return value |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
# Copyright 2022 Google LLC | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# https://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from google.protobuf import field_mask_pb2 | ||
|
||
import proto | ||
from proto.marshal.marshal import BaseMarshal | ||
|
||
|
||
def test_field_mask_read(): | ||
class Foo(proto.Message): | ||
mask = proto.Field( | ||
proto.MESSAGE, | ||
number=1, | ||
message=field_mask_pb2.FieldMask, | ||
) | ||
|
||
foo = Foo(mask=field_mask_pb2.FieldMask(paths=["f.b.d", "f.c"])) | ||
|
||
assert isinstance(foo.mask, field_mask_pb2.FieldMask) | ||
assert foo.mask.paths == ["f.b.d", "f.c"] | ||
|
||
|
||
def test_field_mask_write_string(): | ||
class Foo(proto.Message): | ||
mask = proto.Field( | ||
proto.MESSAGE, | ||
number=1, | ||
message=field_mask_pb2.FieldMask, | ||
) | ||
|
||
foo = Foo() | ||
foo.mask = "f.b.d,f.c" | ||
|
||
assert isinstance(foo.mask, field_mask_pb2.FieldMask) | ||
assert foo.mask.paths == ["f.b.d", "f.c"] | ||
|
||
|
||
def test_field_mask_write_pb2(): | ||
class Foo(proto.Message): | ||
mask = proto.Field( | ||
proto.MESSAGE, | ||
number=1, | ||
message=field_mask_pb2.FieldMask, | ||
) | ||
|
||
foo = Foo() | ||
foo.mask = field_mask_pb2.FieldMask(paths=["f.b.d", "f.c"]) | ||
|
||
assert isinstance(foo.mask, field_mask_pb2.FieldMask) | ||
assert foo.mask.paths == ["f.b.d", "f.c"] | ||
|
||
|
||
def test_field_mask_absence(): | ||
class Foo(proto.Message): | ||
mask = proto.Field( | ||
proto.MESSAGE, | ||
number=1, | ||
message=field_mask_pb2.FieldMask, | ||
) | ||
|
||
foo = Foo() | ||
assert not foo.mask.paths | ||
|
||
|
||
def test_timestamp_del(): | ||
class Foo(proto.Message): | ||
mask = proto.Field( | ||
proto.MESSAGE, | ||
number=1, | ||
message=field_mask_pb2.FieldMask, | ||
) | ||
|
||
foo = Foo() | ||
foo.mask = field_mask_pb2.FieldMask(paths=["f.b.d", "f.c"]) | ||
|
||
del foo.mask | ||
assert not foo.mask.paths | ||
|
||
|
||
def test_timestamp_to_python_idempotent(): | ||
# This path can never run in the current configuration because proto | ||
# values are the only thing ever saved, and `to_python` is a read method. | ||
# | ||
# However, we test idempotency for consistency with `to_proto` and | ||
# general resiliency. | ||
marshal = BaseMarshal() | ||
py_value = "f.b.d,f.c" | ||
assert marshal.to_python(field_mask_pb2.FieldMask, py_value) is py_value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it expected to be a string ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what specifically? This just follows a similar convention of other Rules (they do exaclty the same thing as I have here).