lovasoa / marshmallow_dataclass

Automatic generation of marshmallow schemas from dataclasses.
https://lovasoa.github.io/marshmallow_dataclass/html/marshmallow_dataclass.html
MIT License
456 stars 78 forks source link

Fix recursive references when using class_schema() #189

Closed noirbee closed 2 years ago

noirbee commented 2 years ago

Fixes https://github.com/lovasoa/marshmallow_dataclass/issues/188

The @marshmallow_dataclass.dataclass decorator relies on a lazily computed .Schema attribute/descriptor on the decorated class to solve this issue. For class_schema() this would probably not be a valid approach as the expectation is that its "clazz" argument would not be modified.

Not my best work, but the @lru_cache on _internal_class_schema() made it hard to do otherwise: passing e.g. a stack of already processed classes breaks other tests which rely on the behaviour, since calls on different classes would have different stacks.

I'm also open to suggestions for a better name than seen_classes for this

The # noqa: F821 lines in the tests are due to https://github.com/PyCQA/pyflakes/issues/567

lovasoa commented 2 years ago

thanks

lovasoa commented 2 years ago

@noirbee : would you like to be added to this repo as a collaborator ? You have been making useful pull requests, and I currently do not have a lot of time to allocate to it.

davidecotten commented 2 years ago

Recent changes seem to break inheritance. Is that intended?

...
from marshmallow_dataclass import dataclass

@dataclass
class ThingA:
    Schema: ClassVar[Type[Schema]]
    name: str

@dataclass
class ThingB(ThingA):
    Schema: ClassVar[Type[Schema]]
    short_name: str
>> ThingA.Schema
'ThingA'
>> ThingB.Schema
<class 'marshmallow.schema.ThingB'>
noirbee commented 2 years ago

Recent changes seem to break inheritance. Is that intended?

Hello @davidecotten , I think this is due to interaction between the lazy_attribute_class introduced in fac0b6dc12b11c77094c4f7904ec4aadfb4d554f and explicitly declaring a Schema ClassVar on the dataclass.

AFAICT Python's own @dataclass decorator does some introspection which triggers the descriptor's __get__() when ThingB is declared (i.e. before running ThingA.Schema), which sets its called attribute, which means it then returns a string to avoid recursion.

As a workaround, removing the ClassVar declaration fixes the problem for the base class.

I'll try to take a better look at it in favour of maybe only using the other mechanic I've introduced in this PR.

davidecotten commented 2 years ago

@noirbee Thank you. That makes sense and I've confirmed the workaround.