🐛 Fix jsonable_encoder using include and exclude parameters for non-Pydantic objects

[xaviml]

This fixes #2594. I added more few cases with include and exclude parameters, which were missing. Now the parameters are consistent with the way that it works with Pydantic.

When I first added the new tests in test_jsonable_encoder.py (without changing the codebase), these were the errors I got:

================================= test session starts =================================
platform linux -- Python 3.8.5, pytest-5.4.3, py-1.10.0, pluggy-0.13.1
rootdir: /home/xaviml/work/fastapi
plugins: cov-2.10.0, asyncio-0.14.0
collected 15 items                                                                    

tests/test_jsonable_encoder.py FFF............                                  [100%]

====================================== FAILURES =======================================
__________________________________ test_encode_dict ___________________________________

    def test_encode_dict():
        pet = {"name": "Firulais", "owner": {"name": "Foo"}}
        assert jsonable_encoder(pet) == {"name": "Firulais", "owner": {"name": "Foo"}}
>       assert jsonable_encoder(pet, include={"name"}) == {"name": "Firulais"}
E       AssertionError: assert {'name': 'Fir...name': 'Foo'}} == {'name': 'Firulais'}
E         Omitting 1 identical items, use -vv to show
E         Left contains 1 more item:
E         {'owner': {'name': 'Foo'}}
E         Use -v to get the full diff

tests/test_jsonable_encoder.py:99: AssertionError
__________________________________ test_encode_class __________________________________

    def test_encode_class():
        person = Person(name="Foo")
        pet = Pet(owner=person, name="Firulais")
        assert jsonable_encoder(pet) == {"name": "Firulais", "owner": {"name": "Foo"}}
>       assert jsonable_encoder(pet, include={"name"}) == {"name": "Firulais"}
E       AssertionError: assert {'name': 'Fir...name': 'Foo'}} == {'name': 'Firulais'}
E         Omitting 1 identical items, use -vv to show
E         Left contains 1 more item:
E         {'owner': {'name': 'Foo'}}
E         Use -v to get the full diff

tests/test_jsonable_encoder.py:112: AssertionError
________________________________ test_encode_dictable _________________________________

    def test_encode_dictable():
        person = DictablePerson(name="Foo")
        pet = DictablePet(owner=person, name="Firulais")
        assert jsonable_encoder(pet) == {"name": "Firulais", "owner": {"name": "Foo"}}
>       assert jsonable_encoder(pet, include={"name"}) == {"name": "Firulais"}
E       AssertionError: assert {'name': 'Fir...name': 'Foo'}} == {'name': 'Firulais'}
E         Omitting 1 identical items, use -vv to show
E         Left contains 1 more item:
E         {'owner': {'name': 'Foo'}}
E         Use -v to get the full diff

tests/test_jsonable_encoder.py:125: AssertionError
================================== warnings summary ===================================
env/lib/python3.8/site-packages/aiofiles/os.py:10
env/lib/python3.8/site-packages/aiofiles/os.py:10
env/lib/python3.8/site-packages/aiofiles/os.py:10
env/lib/python3.8/site-packages/aiofiles/os.py:10
env/lib/python3.8/site-packages/aiofiles/os.py:10
env/lib/python3.8/site-packages/aiofiles/os.py:10
  /home/xaviml/work/fastapi/env/lib/python3.8/site-packages/aiofiles/os.py:10: DeprecationWarning: "@coroutine" decorator is deprecated since Python 3.8, use "async def" instead
    def run(*args, loop=None, executor=None, **kwargs):

-- Docs: https://docs.pytest.org/en/latest/warnings.html
=============================== short test summary info ===============================
FAILED tests/test_jsonable_encoder.py::test_encode_dict - AssertionError: assert {'n...
FAILED tests/test_jsonable_encoder.py::test_encode_class - AssertionError: assert {'...
FAILED tests/test_jsonable_encoder.py::test_encode_dictable - AssertionError: assert...
====================== 3 failed, 12 passed, 6 warnings in 0.51s =======================

You can see that even I added the same tests to test_encode_model_with_default, they all passed. This is because the include and exclude done by Pydantic was done correctly. Therefore, what this PR adds is:

• More test cases to include and exclude parameters from the jsonable_encoder function
• Consistent functionality with different types of objects (BaseModel, dict, and Object)

[codecov]

Codecov Report

Merging #2606 (af87159) into master (8a9a117) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2606   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          533       533           
  Lines        13701     13725   +24     
=========================================
+ Hits         13701     13725   +24     

Impacted Files	Coverage Δ
fastapi/encoders.py	100.00% <100.00%> (ø)
tests/test_jsonable_encoder.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

📝 Docs preview for commit 0e5f1cf at: https://5ff45eba9a7a280778e1db88--fastapi.netlify.app

[falkben]

I haven't had a chance to check this out thoroughly, but how does this handle nested allowed/exclude keys? My understanding is that include/exclude with pydantic models is not only at the top level but can be specified even in a nested or inherited model.

Or, worded another way, if you exclude a key at the top level, will it also exclude keys inside the dict at lower levels of the same name? Is that intended or desired? Because this is a recursive function, it's a bit hard for me to reason about without testing it out.

[xaviml]

Hi @falkben,

Thank you for your reply. You are indeed correct it is a recursive function, however, when passing to children, the include and exclude is not passed (this was like this already). Not sure what is desired or not, but I am seeking consistency, so I went ahead and try how it works when passing a BaseModel object:

class Model1(BaseModel):
    foo: str
    bar: str
    blabla: str


class Model2(BaseModel):
    foo: str
    bar: str
    model1: Model1

def test_jsonable_encoder_nested():
    model1 = Model1(foo="foo1", bar="bar1", blabla="blabla1")
    model2 = Model2(foo="foo1", bar="bar1", model1=model1)
    jsonable_encoder(model2, include={"foo"})
    # returns: {'foo': 'foo1'}
    jsonable_encoder(model2, exclude={"foo"})
    # {'bar': 'bar1', 'model1': {'bar': 'bar1', 'blabla': 'blabla1', 'foo': 'foo1'}}

Then if we try with a dict:

def test_jsonable_encoder_nested_dict():
    model = {'bar': 'bar1', 'foo': 'foo1', 'model1': {'bar': 'bar1', 'blabla': 'blabla1', 'foo': 'foo1'}}
    jsonable_encoder(model, include={"foo"})
    # {'foo': 'foo1'}
    jsonable_encoder(model, exclude={"foo"})
    # {'bar': 'bar1', 'model1': {'bar': 'bar1', 'blabla': 'blabla1', 'foo': 'foo1'}}

You can see that it behaves the same way as Pydantic, which is not including and excluding for children objects.

I guess this will need to be added to the test suite, so we can define how the function is expected to work for future reference. Do you agree?

Regards,
Xavi M.

[xaviml]

Sorry I did not answer your question about nested include and exclude. This PR does not add the functionality that Pydantic has with nested includes and excludes as defined in here. In my opinion, this would be part of a new feature if we are interested in adding it, so should be treated in another PR. This PR fixes a bug that the jsonable_encoder function has.

[xaviml]

Hi there,

Just for the awareness, this bug impacts the code from full-stack-fastapi-postgresql, which is a widely used template project for FastAPI. The problem comes when there is a circular dependency in the ORM (e.g. SQLAlchemy), then the jsonable_encoder would fall into a RecursionError: maximum recursion depth exceeded in comparison error. For this reason, I changed this line to:

obj_data = jsonable_encoder(db_obj, include=set(obj_in.__fields__.keys()))

Finally, after this change in my code is when I realized this bug, which made me realized that includes does not work when passing an ORM object instead of a Pydantic one.

I am not worried because I checked out the jsonable_encoder function to my private repository, but I am just concerned that someone else might be facing the same problem.

Regards,
Xavi M.

[github-actions]

📝 Docs preview for commit 11876ca at: https://625d1e4b94cca815fb062aeb--fastapi.netlify.app

[xaviml]

PR was rebased and fixed to be compatible with include and exclude dictionaries, which was added in this PR and merged: #2016

[github-actions]

📝 Docs preview for commit 939a0ef at: https://62718bd8565c130056849df4--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit 9658135 at: https://627825599f18150be582ef45--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit 5dac57a at: https://628a83330c8e941472fda48e--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit 03c5043 at: https://628a842683a2300f24171db8--fastapi.netlify.app

[github-actions]

📝 Docs preview for commit af87159 at: https://62fea44bffb39a04d745aa03--fastapi.netlify.app

[tiangolo]

Great, thank you @xaviml! 🚀

Thanks for the detailed explanation and tests. And thanks @falkben for the input! 🍰