From 2871176f334e0200bb038dc40c0180d97d90d294 Mon Sep 17 00:00:00 2001 From: Daniel Erenrich Date: Fri, 1 Feb 2019 04:59:54 -0800 Subject: [PATCH] [improvement] add support for hashing / settype + more tests (#39) ## Before this PR - Comparison of Union types was broken - No tests for enum or union types - No way to represent Sets separate from Lists ## After this PR This PR does a few things: - fixes and tests https://github.com/palantir/conjure-python-client/issues/17 - adds tests for the union and enum types which had bugs before - adds a SetType for the eventual native support for frozenset to resolve https://github.com/palantir/conjure-python/issues/27 later --- conjure_python_client/__init__.py | 1 + conjure_python_client/_lib/__init__.py | 1 + conjure_python_client/_lib/types.py | 32 +++++++- conjure_python_client/_serde/decoder.py | 41 +++++++++-- conjure_python_client/_serde/encoder.py | 2 +- test/serde/test_decode_enum.py | 47 ++++++++++++ test/serde/test_decode_object.py | 7 +- test/serde/test_decode_set.py | 98 +++++++++++++++++++++++++ test/serde/test_decode_union.py | 93 +++++++++++++++++++++++ 9 files changed, 309 insertions(+), 13 deletions(-) create mode 100644 test/serde/test_decode_enum.py create mode 100644 test/serde/test_decode_set.py create mode 100644 test/serde/test_decode_union.py diff --git a/conjure_python_client/__init__.py b/conjure_python_client/__init__.py index b58edc37..6ea1e6f2 100644 --- a/conjure_python_client/__init__.py +++ b/conjure_python_client/__init__.py @@ -33,5 +33,6 @@ 'RequestsClient', 'Service', 'ServiceConfiguration', + 'SetType', 'SslConfiguration', ] diff --git a/conjure_python_client/_lib/__init__.py b/conjure_python_client/_lib/__init__.py index 342b1257..9b2fe4eb 100644 --- a/conjure_python_client/_lib/__init__.py +++ b/conjure_python_client/_lib/__init__.py @@ -27,4 +27,5 @@ 'DictType', 'ListType', 'OptionalType', + 'SetType' ] diff --git a/conjure_python_client/_lib/types.py b/conjure_python_client/_lib/types.py index b000206d..72dad6db 100644 --- a/conjure_python_client/_lib/types.py +++ b/conjure_python_client/_lib/types.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import List, Dict, Type, Any, Union +from typing import List, Dict, Type, Any, Union, FrozenSet from enum import Enum from .case import to_snake_case @@ -24,7 +24,8 @@ class ConjureType(object): DecodableType = Union[ - int, float, bool, str, ConjureType, List[Any], Dict[Any, Any] + int, float, bool, str, ConjureType, + List[Any], Dict[Any, Any], FrozenSet[Any] ] @@ -36,6 +37,18 @@ def __init__(self, item_type): self.item_type = item_type +class SetType(ConjureType): + _item_type = None # type: Type[DecodableType] + + def __init__(self, item_type): + # type: (Type[DecodableType]) -> None + self._item_type = item_type + + @property + def item_type(self): + return self._item_type + + class DictType(ConjureType): key_type = None # type: Type[DecodableType] value_type = None # type: Type[DecodableType] @@ -78,6 +91,11 @@ def _fields(cls): name to the field definition""" return {} + def __hash__(self): + values_tuple = tuple(self._fields().values()) + keys_tuple = tuple(self._fields().keys()) + return hash((values_tuple, keys_tuple)) + def __eq__(self, other): # type: (Any) -> bool if not isinstance(other, self.__class__): @@ -121,6 +139,12 @@ def _options(cls): to the field definition for that type""" return {} + def __hash__(self): + values = tuple([getattr(self, attr) for + attr, field_def in + self._options().items()]) + return hash(values) + def __eq__(self, other): # type: (Any) -> bool if not isinstance(other, self.__class__): @@ -129,9 +153,9 @@ def __eq__(self, other): assert isinstance(other, ConjureUnionType) pythonic_sanitized_identifier = \ - sanitize_identifier(to_snake_case(self.type)) + sanitize_identifier(to_snake_case(self._type)) - return other.type == self.type and \ + return other._type == self._type and \ getattr(self, pythonic_sanitized_identifier) == \ getattr(other, pythonic_sanitized_identifier) diff --git a/conjure_python_client/_serde/decoder.py b/conjure_python_client/_serde/decoder.py index 260f759f..4c64b0a3 100644 --- a/conjure_python_client/_serde/decoder.py +++ b/conjure_python_client/_serde/decoder.py @@ -20,9 +20,10 @@ DictType, ListType, OptionalType, + SetType, BinaryType) from typing import Optional -from typing import Dict, Any, List +from typing import Dict, Any, List, FrozenSet import inspect import json @@ -62,12 +63,14 @@ def check_null_field( cls, obj, deserialized, python_arg_name, field_definition): if isinstance(field_definition.field_type, ListType): deserialized[python_arg_name] = [] + elif isinstance(field_definition.field_type, SetType): + deserialized[python_arg_name] = frozenset() elif isinstance(field_definition.field_type, DictType): deserialized[python_arg_name] = {} elif isinstance(field_definition.field_type, OptionalType): deserialized[python_arg_name] = None else: - raise Exception( + raise ValueError( "field {} not found in object {}".format( field_definition.identifier, obj ) @@ -99,7 +102,10 @@ def decode_conjure_union_type(cls, obj, conjure_type): deserialized = {} # type: Dict[str, Any] if type_of_union not in obj or obj[type_of_union] is None: - cls.check_null_field(obj, deserialized, conjure_field_definition) + cls.check_null_field(obj, + deserialized, + attr, + conjure_field_definition) else: value = obj[type_of_union] field_type = conjure_field_definition.field_type @@ -118,7 +124,7 @@ def decode_conjure_enum_type(cls, obj, conjure_type): An instance of enum of type conjure_type. """ if not (isinstance(obj, str) or str(type(obj)) == ""): - raise Exception( + raise TypeError( 'Expected to find str type but found {} instead'.format( type(obj))) @@ -149,7 +155,7 @@ def decode_dict( and the values are of type value_type. """ if not isinstance(obj, dict): - raise Exception("expected a python dict") + raise TypeError("expected a python dict") if key_type == str or isinstance(key_type, BinaryType) \ or (inspect.isclass(key_type) and issubclass(key_type, ConjureEnumType)): @@ -176,10 +182,28 @@ def decode_list(cls, obj, element_type): element_type. """ if not isinstance(obj, list): - raise Exception("expected a python list") + raise TypeError("expected a python list") return list(map(lambda x: cls.do_decode(x, element_type), obj)) + @classmethod + def decode_set(cls, obj, element_type): + # type: (List[Any], ConjureTypeType) -> FrozenSet[Any] + """Decodes json into a frozenset, handling conversion of the elements. + + Args: + obj: the json object to decode + element_type: a class object which is the conjure type of + the elements in this list. + Returns: + A python frozenset where the elements are instances of type + element_type. + """ + if not isinstance(obj, (list, set, frozenset)): + raise TypeError("expected a python list, set or frozenset") + + return frozenset(map(lambda x: cls.do_decode(x, element_type), obj)) + @classmethod def decode_optional(cls, obj, object_type): # type: (Optional[Any], ConjureTypeType) -> Optional[Any] @@ -201,7 +225,7 @@ def decode_optional(cls, obj, object_type): @classmethod def decode_primitive(cls, obj, object_type): def raise_mismatch(): - raise Exception( + raise TypeError( 'Expected to find {} type but found {} instead'.format( object_type, type(obj))) @@ -250,6 +274,9 @@ def do_decode(cls, obj, obj_type): elif isinstance(obj_type, OptionalType): return cls.decode_optional(obj, obj_type.item_type) + elif isinstance(obj_type, SetType): + return cls.decode_set(obj, obj_type.item_type) + return cls.decode_primitive(obj, obj_type) def decode(self, obj, obj_type): diff --git a/conjure_python_client/_serde/encoder.py b/conjure_python_client/_serde/encoder.py index 18d5599b..cd8d5742 100644 --- a/conjure_python_client/_serde/encoder.py +++ b/conjure_python_client/_serde/encoder.py @@ -75,7 +75,7 @@ def do_encode(cls, obj): elif isinstance(obj, ConjureEnumType): return obj.value - elif isinstance(obj, list): + elif isinstance(obj, (set, frozenset, list)): return list(map(cls.do_encode, obj)) elif isinstance(obj, dict): diff --git a/test/serde/test_decode_enum.py b/test/serde/test_decode_enum.py new file mode 100644 index 00000000..15c03a05 --- /dev/null +++ b/test/serde/test_decode_enum.py @@ -0,0 +1,47 @@ + +# (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. +# +# 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 +# +# http://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. + +import pytest +from conjure_python_client import ConjureDecoder, ConjureEnumType + + +class TestEnum(ConjureEnumType): + + A = 'A' + '''A''' + B = 'B' + '''B''' + C = 'C' + '''C''' + UNKNOWN = 'UNKNOWN' + '''UNKNOWN''' + + def __reduce_ex__(self, proto): + return self.__class__, (self.name,) + + +def test_enum_decode(): + decoded_A = ConjureDecoder().read_from_string("\"A\"", TestEnum) + decoded_B = ConjureDecoder().read_from_string("\"B\"", TestEnum) + decoded_A2 = ConjureDecoder().read_from_string("\"A\"", TestEnum) + assert decoded_A != decoded_B + assert decoded_A == decoded_A2 + + decoded_unk = ConjureDecoder().read_from_string("\"G\"", TestEnum) + assert repr(decoded_unk) == "TestEnum.UNKNOWN" + + with pytest.raises(TypeError): + decoded_integer = ConjureDecoder().read_from_string("5", TestEnum) + diff --git a/test/serde/test_decode_object.py b/test/serde/test_decode_object.py index 28567b29..d8e841c0 100644 --- a/test/serde/test_decode_object.py +++ b/test/serde/test_decode_object.py @@ -24,6 +24,7 @@ def test_object_decodes_when_exact_fields_are_present(): """{"fileSystemId": "foo", "path": "bar"}""", CreateDatasetRequest ) assert decoded == CreateDatasetRequest("foo", "bar") + assert hash(decoded) is not None def test_object_with_extra_fields_should_only_keep_expected_fields(): @@ -32,27 +33,31 @@ def test_object_with_extra_fields_should_only_keep_expected_fields(): CreateDatasetRequest, ) assert decoded == CreateDatasetRequest("foo", "bar") + assert hash(decoded) is not None def test_object_with_list_field_decodes(): decoded = ConjureDecoder().read_from_string('{"value": []}', ListExample) assert decoded == ListExample([]) + assert hash(decoded) is not None def test_object_with_omitted_list_field_decodes(): decoded = ConjureDecoder().read_from_string('{}', ListExample) assert decoded == ListExample([]) + assert hash(decoded) is not None def test_object_with_map_field_decodes(): decoded = ConjureDecoder().read_from_string('{"value": {}}', MapExample) assert decoded == MapExample({}) + assert hash(decoded) is not None def test_object_with_omitted_map_field_decodes(): decoded = ConjureDecoder().read_from_string('{}', MapExample) assert decoded == MapExample({}) - + assert hash(decoded) is not None def test_object_with_missing_field_should_throw_helpful_exception(): with pytest.raises(Exception) as excinfo: diff --git a/test/serde/test_decode_set.py b/test/serde/test_decode_set.py new file mode 100644 index 00000000..2731a42d --- /dev/null +++ b/test/serde/test_decode_set.py @@ -0,0 +1,98 @@ +# (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. +# +# 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 +# +# http://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. + +import pytest +from conjure_python_client import ConjureDecoder, ConjureEncoder, SetType, ConjureUnionType, ConjureFieldDefinition + +class TestUnion(ConjureUnionType): + + _field_c = None + _field_b = None + _field_a = None + + @classmethod + def _options(cls): + # type: () -> Dict[str, ConjureFieldDefinition] + return { + 'field_c': ConjureFieldDefinition('fieldC', int), + 'field_b': ConjureFieldDefinition('fieldB', str), + 'field_a': ConjureFieldDefinition('fieldA', SetType(int)) + } + + def __init__(self, field_c=None, field_b=None, field_a=None): + if (field_c is not None) + (field_b is not None) + (field_a is not None) != 1: + raise ValueError('a union must contain a single member') + + if field_c is not None: + self._field_c = field_c + self._type = 'fieldC' + if field_b is not None: + self._field_b = field_b + self._type = 'fieldB' + if field_a is not None: + self._field_a = field_a + self._type = 'fieldA' + + @property + def field_c(self): + return self._field_c + + @property + def field_b(self): + return self._field_b + + @property + def field_a(self): + return self._field_a + + +def test_set_with_well_typed_items_decodes(): + decoded = ConjureDecoder().read_from_string("[1,2,3]", SetType(int)) + assert type(decoded) is frozenset + assert type(list(decoded)[0]) is int + +def test_set_in_enum_decode(): + decoded = ConjureDecoder().read_from_string('{"type": "fieldA"}', TestUnion) + assert type(decoded.field_a) is frozenset + assert len(decoded.field_a) == 0 + +def test_set_with_one_badly_typed_item_fails(): + with pytest.raises(Exception): + ConjureDecoder().read_from_string("""[1,"two",3]""", SetType(int)) + + +def test_set_with_no_items_decodes(): + decoded = ConjureDecoder().read_from_string("[]", SetType(int)) + assert type(decoded) is frozenset + + +def test_set_from_json_object_fails(): + with pytest.raises(Exception): + ConjureDecoder().read_from_string("{}", SetType(int)) + + +def test_set_does_not_decode_from_json_null(): + with pytest.raises(Exception): + ConjureDecoder().read_from_string("null", SetType(int)) + + +def test_set_does_not_decode_from_json_string(): + with pytest.raises(Exception): + ConjureDecoder().read_from_string("\"hello\"", SetType(int)) + +def test_set_encoder(): + encoded = ConjureEncoder.do_encode(frozenset([5,6,7])) + assert type(encoded) is list + encoded = ConjureEncoder.do_encode(set([5,6,7])) + assert type(encoded) is list diff --git a/test/serde/test_decode_union.py b/test/serde/test_decode_union.py new file mode 100644 index 00000000..d26be280 --- /dev/null +++ b/test/serde/test_decode_union.py @@ -0,0 +1,93 @@ +# (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. +# +# 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 +# +# http://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. + +import pytest +from conjure_python_client import ConjureDecoder, ListType, ConjureUnionType, ConjureFieldDefinition + +class TestUnion(ConjureUnionType): + + _field_c = None # type: int + _field_b = None # type: str + _field_a = None # type: List[int] + + @classmethod + def _options(cls): + # type: () -> Dict[str, ConjureFieldDefinition] + return { + 'field_c': ConjureFieldDefinition('fieldC', int), + 'field_b': ConjureFieldDefinition('fieldB', str), + 'field_a': ConjureFieldDefinition('fieldA', ListType(int)) + } + + def __init__(self, field_c=None, field_b=None, field_a=None): + if (field_c is not None) + (field_b is not None) + (field_a is not None) != 1: + raise ValueError('a union must contain a single member') + + if field_c is not None: + self._field_c = field_c + self._type = 'fieldC' + if field_b is not None: + self._field_b = field_b + self._type = 'fieldB' + if field_a is not None: + self._field_a = field_a + self._type = 'fieldA' + + @property + def field_c(self): + # type: () -> int + return self._field_c + + @property + def field_b(self): + # type: () -> str + return self._field_b + + @property + def field_a(self): + # type: () -> List[int] + return self._field_a + + def accept(self, visitor): + # type: (TestUnionVisitor) -> Any + if not isinstance(visitor, TestUnionVisitor): + raise ValueError('{} is not an instance of TestUnionVisitor'.format(visitor.__class__.__name__)) + if self.type == 'fieldC': + return visitor._field_c(self.field_c) + if self.type == 'fieldB': + return visitor._field_b(self.field_b) + if self.type == 'fieldA': + return visitor._field_a(self.field_a) + + +def test_union_decoder(): + decoded_A = ConjureDecoder().read_from_string('{"type":"fieldB", "fieldB": "foo"}', TestUnion) + decoded_B = ConjureDecoder().read_from_string('{"type":"fieldC", "fieldC": 5}', TestUnion) + decoded_A2 = ConjureDecoder().read_from_string('{"type":"fieldB", "fieldB": "bar"}', TestUnion) + decoded_A3 = ConjureDecoder().read_from_string('{"type":"fieldB", "fieldB": "bar"}', TestUnion) + decoded_C = ConjureDecoder().read_from_string('{"type":"fieldA"}', TestUnion) + assert type(decoded_A) is TestUnion + assert type(decoded_B) is TestUnion + assert type(decoded_C) is TestUnion + assert decoded_A != decoded_B + assert decoded_A != decoded_A2 + assert decoded_C != decoded_A + assert decoded_A3 == decoded_A2 + assert not decoded_C.field_a + +def test_invalid_decode(): + with pytest.raises(ValueError): + decoded_invalid = ConjureDecoder().read_from_string('{"type":"fieldC", "fieldB": "bar"}', TestUnion) + +