Skip to content

Commit

Permalink
Avoid converting for unknown return types
Browse files Browse the repository at this point in the history
In v0.9.3, the `StandardCoverter.create_response_body_converter` was
changed to a pass-through converter: i.e., the method would simply
return the given type as the response converter. This can cause
unintended behavior in the case when the consumer method has a return
annotation that is not handled by any registered converters. In this
case, the converter chain will invoke the standard coverter's behavior,
and try to convert the runtime response by invoking the annotated
type's constructor. This can cause failures that are difficult to debug
for end users, such as talkpython/100daysofcode-with-python-course#53.

This regression was introduced in 89c43d1 (#204).
  • Loading branch information
prkumar committed Feb 7, 2021
1 parent dff35c8 commit 3653a67
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 11 deletions.
21 changes: 16 additions & 5 deletions tests/unit/test_converters.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,24 @@ def test_convert(self):


class TestStandardConverter(object):
def test_create_response_body_converter(self, converter_mock):
def test_create_response_body_converter_with_converter(
self, converter_mock
):
# Setup
factory = standard.StandardConverter()

# Run & Verify: Pass-through converters
converter = factory.create_response_body_converter(converter_mock)
assert converter is converter_mock

def test_create_response_body_converter_with_unknown_type(self):
# Setup
factory = standard.StandardConverter()

# Run & Verify: does not know how to convert any types
converter = factory.create_response_body_converter(dict)
assert converter is None


class TestConverterFactoryRegistry(object):
backend = converters.ConverterFactoryRegistry._converter_factory_registry
Expand Down Expand Up @@ -487,19 +497,20 @@ def test_create_request_body_converter(self, pydantic_model_mock):
def test_convert_complex_model(self):
from json import loads
from datetime import datetime
from typing import List

class ComplexModel(pydantic.BaseModel):
when = datetime.utcnow() # type: datetime
where = 'http://example.com' # type: pydantic.AnyUrl
some = [1] # type: List[int]
where = "http://example.com" # type: pydantic.AnyUrl
some = [1] # type: typing.List[int]

model = ComplexModel()
request_body = {}
expected_result = loads(model.json())

converter = converters.PydanticConverter()
request_converter = converter.create_request_body_converter(ComplexModel)
request_converter = converter.create_request_body_converter(
ComplexModel
)

result = request_converter.convert(request_body)

Expand Down
4 changes: 4 additions & 0 deletions uplink/clients/io/transitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ def fail(exc_type, exc_val, exc_tb):
"""
Transitions the execution to fail with a specific error.
This will prompt the execution of any RequestTemplate.after_exception hooks.
Args:
exc_type: The exception class.
exc_val: The exception object.
Expand All @@ -63,6 +65,8 @@ def prepare(request):
"""
Transitions the execution to prepare the given request.
This will prompt the execution of any RequestTemplate.before_request.
Args:
request: The intended request data to be sent.
"""
Expand Down
11 changes: 5 additions & 6 deletions uplink/converters/standard.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@ class StandardConverter(interfaces.Factory):
converters could handle a particular type.
"""

@staticmethod
def pass_through_converter(type_, *args, **kwargs):
return type_
def create_request_body_converter(self, cls, request_definition):
return cls

create_response_body_converter = (
create_request_body_converter
) = pass_through_converter
def create_response_body_converter(self, cls, *args, **kwargs):
if isinstance(cls, interfaces.Converter):
return cls

def create_string_converter(self, type_, *args, **kwargs):
return Cast(type_, StringConverter()) # pragma: no cover

0 comments on commit 3653a67

Please sign in to comment.