From ee69e4743115f7f2ad122528209cf140a9904e36 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Mon, 25 Jul 2022 22:29:42 +0100 Subject: [PATCH 1/2] feat: fetch resource if accessing missing field on simplified instance --- src/deezer/resources/resource.py | 25 ++++++++ ...urce.test_access_field_shallow_object.yaml | 57 +++++++++++++++++++ tests/resources/test_resource.py | 26 +++++++++ 3 files changed, 108 insertions(+) create mode 100644 tests/resources/cassettes/TestResource.test_access_field_shallow_object.yaml diff --git a/src/deezer/resources/resource.py b/src/deezer/resources/resource.py index 70fa60a6b..75a0de710 100644 --- a/src/deezer/resources/resource.py +++ b/src/deezer/resources/resource.py @@ -18,7 +18,9 @@ class Resource: id: int type: str + _fields: tuple[str, ...] _fields_parsers = {} + _fetched: bool def __init__(self, client, json): self.client = client @@ -79,3 +81,26 @@ def get_paginated_list( parent=self, **kwargs, ) + + def __getattr__(self, item: str) -> Any: + """ + Called when the default attribute access fails with an AttributeError. + + This is a fallback method, not need to call the parent implementation. + If the attribute is found through the normal mechanism, this is NOT called. + """ + class_annotations = self.__class__.__annotations__ + if item in class_annotations and not getattr(self, "_fetched", False): + full_resource = self.get() + missing_fields = set(full_resource._fields) - set(self._fields) + for field_name in missing_fields: + setattr(self, field_name, getattr(full_resource, field_name)) + return getattr(self, item) + raise AttributeError( + f"'{self.__class__.__name__}' object has no attribute '{item}'" + ) + + def get(self): + """Get the resource from the API.""" + self._fetched = True + return self.client.request("GET", f"{self.type}/{self.id}") diff --git a/tests/resources/cassettes/TestResource.test_access_field_shallow_object.yaml b/tests/resources/cassettes/TestResource.test_access_field_shallow_object.yaml new file mode 100644 index 000000000..dbbab4cb9 --- /dev/null +++ b/tests/resources/cassettes/TestResource.test_access_field_shallow_object.yaml @@ -0,0 +1,57 @@ +interactions: + - request: + body: null + headers: + Accept: + - "*/*" + Accept-Encoding: + - identity + Connection: + - keep-alive + User-Agent: + - python-requests/2.27.1 + method: GET + uri: https://api.deezer.com/episode/343457312 + response: + body: + string: + '{"id":343457312,"title":"Stuart Hogg and the GOAT","description":"There + are big questions to ponder on this week''s pod; who hacked Chris'' Twitter + account? Was England''s victory over South Africa the best of Eddie Jones'' + reign? And has New Zealand''s aura been vaporised? Danny, Ugo and Chris also + chat to Scotland captain Stuart Hogg about his new teeth, breaking the try + record and the importance of Chris Harris both on and off the pitch. They + also look to the Six Nations with the northern hemisphere teams ending the + autumn on a high.","available":true,"release_date":"2021-11-22 23:42:00","duration":3254,"link":"https:\/\/www.deezer.com\/episode\/343457312","share":"https:\/\/www.deezer.com\/episode\/343457312?utm_source=deezer&utm_content=episode-343457312&utm_term=0_1658789177&utm_medium=web","picture":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/180x180-000000-80-0-0.jpg","picture_small":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/56x56-000000-80-0-0.jpg","picture_medium":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/250x250-000000-80-0-0.jpg","picture_big":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/500x500-000000-80-0-0.jpg","picture_xl":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/1000x1000-000000-80-0-0.jpg","podcast":{"id":700072,"title":"Rugby + Union Weekly","link":"https:\/\/www.deezer.com\/show\/700072","picture":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/180x180-000000-80-0-0.jpg","picture_small":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/56x56-000000-80-0-0.jpg","picture_medium":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/250x250-000000-80-0-0.jpg","picture_big":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/500x500-000000-80-0-0.jpg","picture_xl":"https:\/\/e-cdns-images.dzcdn.net\/images\/talk\/f2a011a6e87bd793c8e77240ab36dfaa\/1000x1000-000000-80-0-0.jpg","type":"podcast"},"type":"episode"}' + headers: + Access-Control-Allow-Credentials: + - "true" + Access-Control-Allow-Headers: + - X-Requested-With, Content-Type, Authorization, Origin, Accept, Accept-Encoding + Access-Control-Allow-Methods: + - POST, GET, OPTIONS, DELETE, PUT + Access-Control-Expose-Headers: + - Location + Access-Control-Max-Age: + - "86400" + Connection: + - keep-alive + Content-Length: + - "2188" + Content-Type: + - application/json; charset=utf-8 + Server: + - Apache + Vary: + - Accept-Encoding + X-Content-Type-Options: + - nosniff + X-Host: + - blm-web-94 + x-org: + - FR + status: + code: 200 + message: OK +version: 1 diff --git a/tests/resources/test_resource.py b/tests/resources/test_resource.py index 020c8c5bd..6c4153aad 100644 --- a/tests/resources/test_resource.py +++ b/tests/resources/test_resource.py @@ -2,6 +2,8 @@ import pytest +import deezer + pytestmark = pytest.mark.vcr @@ -11,3 +13,27 @@ def test_resource_relation(self, client): album = client.get_album(302127) tracks = album.get_tracks() assert tracks[0].album is album + + def test_access_field_shallow_object(self, client): + """Accessing a field of shallow object fetches the full object.""" + episode = deezer.Episode( + client, + json={ + "id": 343457312, + "type": "episode", + }, + ) + assert episode.link == "https://www.deezer.com/episode/343457312" + + def test_field_not_found(self, client): + """When field is missing an attribute error is raised without API calls.""" + episode = deezer.Episode( + client, + json={ + "id": 343457312, + "type": "episode", + }, + ) + with pytest.raises(AttributeError) as exc_info: + episode.something + assert str(exc_info.value) == "'Episode' object has no attribute 'something'" From b69a84d3ec97838d812ff772c3aa5a17cb30bf9b Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Tue, 26 Jul 2022 00:08:28 +0100 Subject: [PATCH 2/2] docs: add a paragraph about the n+1 API calls --- docs/source/usage.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/source/usage.md b/docs/source/usage.md index 66746eea1..0bf34ed4c 100644 --- a/docs/source/usage.md +++ b/docs/source/usage.md @@ -117,6 +117,22 @@ As you can see, it doesn't look like we're making API requests, but under the ho You can tell the difference, though: attributes access are using the data from the resource which was already fetched, while calling a method on the resource does extra API requests. +#### N+1 API calls + +When traversing relations, the Deezer API returns a simplified version of the objects. For example, the [album tracks](https://developers.deezer.com/api/album/tracks) have fewer fields returned that when you get [a track](https://developers.deezer.com/api/track) directly and this applies to lot of related resources: podcast episodes, artist albums, ... + +To mitigate this problem and make sure the resources being returned have all the documented attributes, the client might make additional API calls to get the full version of the resources. This happens lazily, when you access an attribute of the resource which wasn't returned yet. If you try to access an attribute that doesn't exist on the resource, you'll get a `AttributeError` without extra API calls. + +This might cause N+1 API calls if you're doing this in a loop: + +```python +>>> podcast = client.get_podcast(699612) +... for episode in podcast.get_episodes(): +... print(episode.link) # This will make an API call for each episode +``` + +This is because the `link` field isn't returned when listing episodes of a podcast. + ### Getting the raw data At some point, you might want to get the resources exported as Python dictionaries to store them somewhere else or transform them further.