From a7e363a8352efc70c8d160ef9526dc4572733a1e Mon Sep 17 00:00:00 2001 From: idchlife Date: Thu, 12 Sep 2019 18:05:11 +0300 Subject: [PATCH] Fix ambiguity error with Polymorphism candidates (isinstance() problem) (#691) * Fix for better Polymorphism support Right now Polymorphism fails when using something like this (models in sqlalchemy): models: ```python class Product(BaseModel): discr = Column(String) __mapper_args__ = { 'polymorphic_on': discr, 'polymorphic_identity': "product" } class VegetableProduct(Product): name = Column(String) __mapper_args__ = { 'polymorphic_identity': "vegetable" } class DairyProduct(VegetableProduct): __mapper_args__ = { 'polymorphic_identity': "dairy" } ``` mapper: ```python resource_product = api.model("Product", { 'discr': fields.String }) product_mapper = { VegetableProduct: api.inherit("Vegetable", resource_product, { 'name': fields.String }), DairyProduct: api.inherit("Dairy", resource_product, { 'name': fields.String }), } resource_result = api.model("ResourceResult", { 'products': fields.List(fields.Polymorphism(product_mapper)) }) ``` This sparkles error here: https://github.com/noirbizarre/flask-restplus/blob/master/flask_restplus/fields.py#L682 Because candidates will be: VegetableProduct and DairyProduct, since isinstance() will return True for both classes (I personally surprised it works like this) Checking by __class__.__name__ removes this error and it works as intended. I hope my quick fix did not break anything, I'm eager to update it if I don't know, more elegant solution is needed. * adjusted ambiguity test for new feature * stricter check for type * changed to simple class check * type() check for Polymorphism, adjusted tests * type() check for Polymorphism, adjusted tests * removed local dev files * updated test name, also PR in docblock --- flask_restplus/fields.py | 2 +- tests/test_fields.py | 8 +++++--- tests/test_swagger.py | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/flask_restplus/fields.py b/flask_restplus/fields.py index 7e75700a..ad2e00fe 100644 --- a/flask_restplus/fields.py +++ b/flask_restplus/fields.py @@ -674,7 +674,7 @@ def output(self, key, obj, ordered=False, **kwargs): if not hasattr(value, '__class__'): raise ValueError('Polymorph field only accept class instances') - candidates = [fields for cls, fields in iteritems(self.mapping) if isinstance(value, cls)] + candidates = [fields for cls, fields in iteritems(self.mapping) if type(value) == cls] if len(candidates) <= 0: raise ValueError('Unknown class: ' + value.__class__.__name__) diff --git a/tests/test_fields.py b/tests/test_fields.py index d0117353..fbc156f0 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -1177,7 +1177,10 @@ class Child2(object): with pytest.raises(ValueError): api.marshal({'owner': object()}, thing) - def test_polymorph_field_ambiguous_mapping(self, api): + def test_polymorph_field_does_not_have_ambiguous_mappings(self, api): + """ + Regression test for https://github.com/noirbizarre/flask-restplus/pull/691 + """ parent = api.model('Parent', { 'name': fields.String, }) @@ -1201,8 +1204,7 @@ class Child(Parent): 'owner': fields.Polymorph(mapping), }) - with pytest.raises(ValueError): - api.marshal({'owner': Child()}, thing) + api.marshal({'owner': Child()}, thing) def test_polymorph_field_required_default(self, api): parent = api.model('Person', { diff --git a/tests/test_swagger.py b/tests/test_swagger.py index 91bccfa5..9e583f55 100644 --- a/tests/test_swagger.py +++ b/tests/test_swagger.py @@ -2279,11 +2279,11 @@ def get(self): assert path['get']['responses']['200']['schema']['$ref'] == '#/definitions/Output' def test_polymorph_inherit_list(self, api, client): - class Child1: + class Child1(object): name = 'Child1' extra1 = 'extra1' - class Child2: + class Child2(object): name = 'Child2' extra2 = 'extra2'