Skip to content

Commit b6a8346

Browse files
alastairalastair
committed
Always return rating if it exists, only do 1 query for ratings
Previously we would do 1 query to get an entity and then another to get the rating (and then only 1 query to get an entity and its rating if we joined to the redirects table!). This simplifies the queries to always join to the meta table if it exists. Because we always join to this table anyway, remove the rating includes (it was spelt wrong anyway and never returned data) and always return it
1 parent bf71cf3 commit b6a8346

14 files changed

Lines changed: 48 additions & 51 deletions

‎brainzutils/musicbrainz_db/artist.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,6 @@ def fetch_multiple_artists(mbids, includes=None):
7474
includes_data=includes_data,
7575
)
7676

77-
if 'rating' in includes:
78-
for artist in artists.values():
79-
includes_data[artist.id]['rating'] = artist.rating
80-
8177
if 'type' in includes:
8278
for artist in artists.values():
8379
includes_data[artist.id]['type'] = artist.type

‎brainzutils/musicbrainz_db/event.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,4 @@ def fetch_multiple_events(mbids, includes=None):
9191
includes_data=includes_data,
9292
)
9393

94-
if 'rating' in includes:
95-
for event in events.values():
96-
includes_data[event.id]['rating'] = event.rating
97-
9894
return {str(mbid): serialize_events(event, includes_data[event.id]) for mbid, event in events.items()}

‎brainzutils/musicbrainz_db/includes.py‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,19 @@
1818

1919
RELATION_INCLUDES = [entity + '-rels' for entity in RELATABLE_TYPES]
2020

21-
TAG_INCLUDES = ["tags", "user-tags"]
22-
23-
RATING_INCLUDES = ["ratings", "user-ratings"]
21+
TAG_INCLUDES = ["tags"]
2422

2523
VALID_INCLUDES = {
2624
'place': ["aliases", "annotation"] + RELATION_INCLUDES + TAG_INCLUDES,
27-
'event': ["aliases"] + RELATION_INCLUDES + TAG_INCLUDES + RATING_INCLUDES,
28-
'recording': ["artist", "artists", "isrc"] + TAG_INCLUDES + RELATION_INCLUDES + RATING_INCLUDES,
29-
'release_group': ["artists", "media", "releases"] + TAG_INCLUDES + RELATION_INCLUDES + RATING_INCLUDES,
25+
'event': ["aliases"] + RELATION_INCLUDES + TAG_INCLUDES,
26+
'recording': ["artist", "artists", "isrc"] + TAG_INCLUDES + RELATION_INCLUDES,
27+
'release_group': ["artists", "media", "releases"] + TAG_INCLUDES + RELATION_INCLUDES,
3028
'release': [
3129
"artists", "labels", "recordings", "release-groups", "media", "annotation", "aliases"
3230
] + TAG_INCLUDES + RELATION_INCLUDES,
33-
'artist': ["recordings", "releases", "media", "aliases", "annotation", "type"] + RELATION_INCLUDES + TAG_INCLUDES + RATING_INCLUDES,
34-
'label': ["area", "aliases", "annotation"] + RELATION_INCLUDES + TAG_INCLUDES + RATING_INCLUDES,
35-
'work': ["artists", "recordings", "aliases", "annotation"] + RELATION_INCLUDES + TAG_INCLUDES + RATING_INCLUDES,
31+
'artist': ["recordings", "releases", "media", "aliases", "annotation", "type"] + RELATION_INCLUDES + TAG_INCLUDES,
32+
'label': ["area", "aliases", "annotation"] + RELATION_INCLUDES + TAG_INCLUDES,
33+
'work': ["artists", "recordings", "aliases", "annotation"] + RELATION_INCLUDES + TAG_INCLUDES,
3634
'editor': [], # TODO: List includes here (BU-18)
3735
}
3836

‎brainzutils/musicbrainz_db/label.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,4 @@ def fetch_multiple_labels(mbids, includes=None):
7474
includes_data[label.id]['type'] = label.type
7575
includes_data[label.id]['area'] = label.area
7676

77-
if 'rating' in includes:
78-
for label in labels.values():
79-
includes_data[label.id]['rating'] = label.rating
80-
8177
return {str(mbid): serialize_labels(label, includes_data[label.id]) for mbid, label in labels.items()}

‎brainzutils/musicbrainz_db/recording.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,6 @@ def fetch_multiple_recordings(mbids, includes=None):
8888

8989
recording_ids = [recording.id for recording in recordings.values()]
9090

91-
if 'rating' in includes:
92-
for recording in recordings.values():
93-
includes_data[recording.id]['rating'] = recording.rating
94-
9591
if 'artist' in includes:
9692
for recording in recordings.values():
9793
includes_data[recording.id]['artist'] = recording.artist_credit

‎brainzutils/musicbrainz_db/release_group.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,6 @@ def fetch_multiple_release_groups(mbids, includes=None):
107107
for release_group_id, tags in release_group_tags:
108108
includes_data[release_group_id]['tags'] = tags
109109

110-
if 'rating' in includes:
111-
for release_group in release_groups.values():
112-
includes_data[release_group.id]['rating'] = release_group.rating
113-
114110
for release_group in release_groups.values():
115111
includes_data[release_group.id]['meta'] = release_group.meta
116112
includes_data[release_group.id]['type'] = release_group.type

‎brainzutils/musicbrainz_db/serialize.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ def serialize_recording(recording, includes=None):
118118
if recording.video:
119119
data['video'] = True
120120

121-
if 'rating' in includes and includes['rating']:
121+
if getattr(recording, 'rating', None):
122122
data['rating'] = recording.rating
123123

124124
if 'artist' in includes:
@@ -187,7 +187,7 @@ def serialize_labels(label, includes=None):
187187
if 'area' in includes and includes['area']:
188188
data['area'] = includes['area'].name
189189

190-
if 'rating' in includes and includes['rating']:
190+
if getattr(label, 'rating', None):
191191
data['rating'] = label.rating
192192

193193
if 'relationship_objs' in includes:
@@ -215,7 +215,7 @@ def serialize_artists(artist, includes=None):
215215
if 'type' in includes:
216216
data['type'] = artist.type.name
217217

218-
if 'rating' in includes and includes['rating']:
218+
if getattr(artist, 'rating', None):
219219
data['rating'] = artist.rating
220220

221221
if 'relationship_objs' in includes:
@@ -249,7 +249,7 @@ def serialize_release_groups(release_group, includes=None):
249249
if 'type' in includes and includes['type']:
250250
data['type'] = includes['type'].name
251251

252-
if 'rating' in includes and includes['rating']:
252+
if getattr(release_group, 'rating', None):
253253
data['rating'] = release_group.rating
254254

255255
if 'artist-credit-phrase' in includes:
@@ -354,7 +354,7 @@ def serialize_events(event, includes=None):
354354
if 'type' in includes and includes['type']:
355355
data['type'] = includes['type'].name
356356

357-
if 'rating' in includes and includes['rating']:
357+
if getattr(event, 'rating', None):
358358
data['rating'] = event.rating
359359

360360
if 'relationship_objs' in includes:
@@ -389,7 +389,7 @@ def serialize_works(work, includes=None):
389389
if 'type' in includes and includes['type']:
390390
data['type'] = includes['type'].name
391391

392-
if 'rating' in includes and includes['rating']:
392+
if getattr(work, 'rating', None):
393393
data['rating'] = work.rating
394394

395395
if 'relationship_objs' in includes:

‎brainzutils/musicbrainz_db/tests/test_artist.py‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ def test_get_artist_by_mbid(self, engine):
1313
"name": "Linkin Park",
1414
"sort_name": "Linkin Park",
1515
"comment": "American rock band",
16-
"life-span": {"begin": "1999"}
16+
"life-span": {"begin": "1999"},
17+
"rating": 86,
1718
}
1819

1920
def test_get_artist_by_mbid_redirect(self, engine):
@@ -25,6 +26,7 @@ def test_get_artist_by_mbid_redirect(self, engine):
2526
"sort_name": "Madonna",
2627
"comment": "American singer-songwriter, actress, businesswoman, “Queen of Pop”",
2728
"life-span": {"begin": "1958-08-16"},
29+
"rating": 88,
2830
}
2931

3032
def test_fetch_multiple_artists(self, engine):
@@ -38,15 +40,17 @@ def test_fetch_multiple_artists(self, engine):
3840
"sort_name": "JAY‐Z",
3941
"type": "Person",
4042
"comment": "US rapper",
41-
"life-span": {"begin": "1969-12-04"}
43+
"life-span": {"begin": "1969-12-04"},
44+
"rating": 71,
4245
}
4346
assert artists["f59c5520-5f46-4d2c-b2c4-822eabf53419"] == {
4447
"mbid": "f59c5520-5f46-4d2c-b2c4-822eabf53419",
4548
"name": "Linkin Park",
4649
"sort_name": "Linkin Park",
4750
"type": "Group",
4851
"comment": "American rock band",
49-
"life-span": {"begin": "1999"}
52+
"life-span": {"begin": "1999"},
53+
"rating": 86,
5054
}
5155

5256
def test_fetch_multiple_artists_redirect(self, engine):
@@ -59,6 +63,7 @@ def test_fetch_multiple_artists_redirect(self, engine):
5963
"sort_name": "Linkin Park",
6064
"comment": "American rock band",
6165
"life-span": {"begin": "1999"},
66+
"rating": 86
6267
}
6368

6469
def test_fetch_multiple_artists_missing(self, engine):

‎brainzutils/musicbrainz_db/tests/test_event.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ def test_get_event_by_mbid_redirect(self, engine):
2121
'mbid': '499559c8-b84b-422e-8ad7-b746d48c21aa',
2222
'name': '1995-10-11: Riverport Amphitheatre, Maryland Heights, Missouri',
2323
'life-span': {'begin': '1995-10-11', 'end': '1995-10-11'},
24+
'rating': 100,
2425
}
2526

2627
def test_fetch_multiple_events(self, engine):
@@ -39,6 +40,7 @@ def test_fetch_multiple_events_redirect(self, engine):
3940
'mbid': '499559c8-b84b-422e-8ad7-b746d48c21aa',
4041
'name': '1995-10-11: Riverport Amphitheatre, Maryland Heights, Missouri',
4142
'life-span': {'begin': '1995-10-11', 'end': '1995-10-11'},
43+
'rating': 100,
4244
}}
4345

4446
def test_fetch_multiple_events_empty(self, engine):

‎brainzutils/musicbrainz_db/tests/test_label.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ def test_get_label_by_mbid(self, engine):
1414
"type": "Original Production",
1515
"area": "United States",
1616
"life-span": {"begin": "1996"},
17+
"rating": 100,
1718
}
1819

1920
def test_get_label_by_mbid_redirect(self, engine):
@@ -25,6 +26,7 @@ def test_get_label_by_mbid_redirect(self, engine):
2526
"area": "United States",
2627
"comment": "Warner Music imprint",
2728
"life-span": {"begin": "1947"},
29+
"rating": 100,
2830
}
2931

3032
def test_fetch_multiple_labels(self, engine):
@@ -47,6 +49,7 @@ def test_fetch_multiple_labels(self, engine):
4749
"type": "Original Production",
4850
"area": "United States",
4951
"life-span": {"begin": "1996"},
52+
"rating": 100
5053
}
5154

5255
def test_fetch_multiple_labels_redirect(self, engine):
@@ -61,6 +64,7 @@ def test_fetch_multiple_labels_redirect(self, engine):
6164
"area": "United States",
6265
"comment": "Warner Music imprint",
6366
"life-span": {"begin": "1947"},
67+
"rating": 100,
6468
}
6569

6670
def test_fetch_multiple_labels_missing(self, engine):

0 commit comments

Comments
 (0)