Smarter schema that handles dotted aliases and resolver methods

[SmileyChris]

Fixes #291 (even though it was closed) and fixes #250 and adds in new functionality to allow calculated fields via resolve_ static methods on the response schema class.

Includes tests and docs.

[SmileyChris]

I bumped the pydantic version to 1.9 to support the fancier DjangoGetter method being used. The alternative is to backport the pydantic fix to a method on the ninja Schema class:

    @classmethod
    def _decompose_class(cls, obj: Any) -> GetterDict:
        if isinstance(obj, GetterDict):
            return obj
        return super()._decompose_class(obj)

[vitalik]

I bumped the pydantic version to 1.9
well this cannot be merged - there ALOT projects that depends on it
I would keep minimum 1.6 pydantic while it still not breakable

[SmileyChris]

well this cannot be merged - there ALOT projects that depends on it
I would keep minimum 1.6 pydantic while it still not breakable

Sure, I backported that little bit of logic from Pydantic into the Schema._decompose_class method and reverted the changes bumping the minimum version. Then I squashed everything to keep things tidier :)

[vitalik]

Hi @SmileyChris

I have one concern about this PR

Basically it's the performance penalty here:

    def __getitem__(self, key: str) -> Any:
        resolve_func = getattr(self._cls, f"resolve_{key}", None) if self._cls else None
        if resolve_func and callable(resolve_func):
            item = resolve_func(self._obj)
        else:
            try:
                item = attrgetter(key)(self._obj)
            except AttributeError as e:
                raise KeyError(key) from e
        return self.format_result(item)

Every attribute will call for getattr, callable check, attrgetter, etc

but in 99% of the cases schemas are just generally pulling existing atrs...

maybe solution here would be first try to get attr default way and if attr is not there try to resolve it with attrgetter/resolve_func:

try:
    return getattr(self._obj)
except AttributeError:
    if resolve_func and callable(resolve_func):
           return resolve_func(self._obj) 
    else:
        return attrgetter(key)(self._obj)

[SmileyChris]

At first glance, that looked like a fine solution. But it'll get in the way allowing for resolve_<attr> functions if it's shadowing an existing attribute.

Doing some basic %timeit tests, it looks like it only adds about 400ns to each attr call, so 0.0000004s.

# Old class
In [8]: %timeit old_django_getter['name']
455 ns ± 2.44 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

# New class
In [8]: %timeit new_django_getter['name']
875 ns ± 15.7 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

The repr of setting that up, showing an example of using a resolver over an existing attribute:

In [1]: from django.conf import settings

In [2]: settings.configure()

In [3]: from ninja.schema import DjangoGetter, Schema

In [4]: class PersonSchema(Schema):
   ...:     name: str
   ...:     age: int
   ...:     @staticmethod
   ...:     def resolve_name(obj):
   ...:         return obj.name.title()
   ...: 

In [5]: class Person:
   ...:     name = 'chris beaven'
   ...:     age = 99
   ...: 

In [6]: new_django_getter = DjangoGetter(Person(), PersonSchema)

In [7]: new_django_getter['name']
Out[7]: 'Chris Beaven'

The only extra check for the 99% will be a check for the resolve method, then the use of attrgetter rather than getattr which looks like it's about 70ns slower. I'll optimize it to try a standard getattr first.

[SmileyChris]

Ok, now the only overhead for the 99% case is a getattr for a resolve attr first. Python logic means it won't check if it's callable unless the resolve attr is found.

[vitalik]

@SmileyChris great, looks promising

I guess the only small design issue is this part:

    @staticmethod
    def resolve_owner(obj):

that you have to to mark method as static... I guess generally people will forget it and will use self as first argument (maybe need some validation on top to force it never forgotten)

or maybe just pass self somehow anyway ?

def resolve_owner(self, obj):

also would be nice to have access to self dict to cover long awaited feature #935 from pydantic:

class Rectangle(BaseModel):
    width: int
    length: int
    area: int

    def resolve_area(self) -> int:
        return self.width * self.length

[SmileyChris]

Technically it doesn't need to be a static method, that's just me being a bit pedantic in the documentation. Since it's not being called as a bound method on an instance of the class, the first argument will be passed in, it just won't be an instance of the class itself, which is slightly confusing... But the method is being called from the class object, and expects one parameter, so a normal method like `def resolve_area(self) -> int:` already also works fine.

…

On Sun, 23 Jan 2022 at 02:27, Vitaliy Kucheryaviy ***@***.***> wrote: @SmileyChris <https://github.com/SmileyChris> great, looks promising I guess the only small design issue is this part: @staticmethod def resolve_owner(obj): that you have to to mark method as static... I guess generally people will forget it and will use self as first argument (maybe need some validation on top to force it never forgotten) or maybe just pass self somehow anyway ? def resolve_owner(self, obj): also would be nice to have access to self dict to cover long awaited feature #935 <pydantic/pydantic#935> from pydantic: class Rectangle(BaseModel): width: int length: int area: int def resolve_area(self) -> int: return self.width * self.length — Reply to this email directly, view it on GitHub <#317 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2275ASYUUAYQ5YBT7FFTUXKWE7ANCNFSM5LRLRVVA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[SmileyChris]

Ok, now resolvers can be standard methods, giving you access to self like you want (it has to be a bit magic, because the real schema instance is only created after all the fields are resolved fully).

[vitalik]

Hi @SmileyChris

I've been running some test - so far looks good except the perfomance for from_orm

example - 50k executions for from_orm takes 10 seconds on my machine (while it's only 1 second before)

So I basically moved the function to find resolve_* attributes to one time call and cache on class level

see - da28fa1

It still in progress - there are few coverage missing lines, but so far looks promising

[SmileyChris]

That's nice caching speedup. How about this metaclass approach to keep resolver data on their own classes rather than one mega dictionary?

[vitalik]

@SmileyChris
Thank you! Works perfect