Skip to content

Commit

Permalink
Join relationships using attributes, not strings
Browse files Browse the repository at this point in the history
This commit fixes the warning:

RemovedIn20Warning: Using strings to indicate column or relationship paths in loader options is deprecated and will be removed in SQLAlchemy 2.0.  Please use the class-bound attribute directly. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)

For rationale, see section: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#orm-query-joining-loading-on-relationships-uses-attributes-not-strings
  • Loading branch information
amCap1712 committed Sep 30, 2022
1 parent 4e27f06 commit 448d4f6
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 112 deletions.
2 changes: 1 addition & 1 deletion brainzutils/musicbrainz_db/artist.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def fetch_multiple_artists(mbids, includes=None):
check_includes('artist', includes)

with mb_session() as db:
query = db.query(models.Artist).options(joinedload('type'))
query = db.query(models.Artist).options(joinedload(models.Artist.type))

artists = get_entities_by_gids(
query=query,
Expand Down
2 changes: 1 addition & 1 deletion brainzutils/musicbrainz_db/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def fetch_multiple_events(mbids, includes=None):
includes_data = defaultdict(dict)
check_includes('event', includes)
with mb_session() as db:
query = db.query(models.Event).options(joinedload('type'))
query = db.query(models.Event).options(joinedload(models.Event.type))
events = get_entities_by_gids(
query=query,
entity_type='event',
Expand Down
10 changes: 6 additions & 4 deletions brainzutils/musicbrainz_db/helpers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from mbdata.utils.models import get_link_model
from mbdata.models import Tag
from mbdata.models import Tag, Link
from sqlalchemy.orm import joinedload
from sqlalchemy import func
from brainzutils.musicbrainz_db.models import ENTITY_MODELS
Expand All @@ -23,8 +23,10 @@ def get_relationship_info(db, target_type, source_type, source_entity_ids, inclu
relation = get_link_model(source_model, target_model)

query = db.query(relation).\
options(joinedload("link", innerjoin=True)).\
options(joinedload("link.link_type", innerjoin=True))
options(
joinedload(relation.link, innerjoin=True).
joinedload(Link.link_type, innerjoin=True)
)
if relation.entity0.property.mapper.class_ == relation.entity1.property.mapper.class_:
_relationship_link_helper(relation, query, "entity0", "entity1", target_type, source_entity_ids, includes_data)
_relationship_link_helper(relation, query, "entity1", "entity0", target_type, source_entity_ids, includes_data)
Expand Down Expand Up @@ -52,7 +54,7 @@ def _relationship_link_helper(relation, query, source_attr, target_attr, target_
"""
source_id_attr = source_attr + "_id"
query = query.filter(getattr(relation, source_id_attr).in_(source_entity_ids))
query = query.options(joinedload(target_attr, innerjoin=True))
query = query.options(joinedload(getattr(relation, target_attr), innerjoin=True))
relation_type = target_type + "-rels"
for link in query:
includes_data[getattr(link, source_id_attr)].setdefault('relationship_objs', {}).\
Expand Down
4 changes: 2 additions & 2 deletions brainzutils/musicbrainz_db/label.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ def fetch_multiple_labels(mbids, includes=None):
check_includes('label', includes)
with mb_session() as db:
query = db.query(models.Label).\
options(joinedload("type")).\
options(joinedload("area"))
options(joinedload(models.Label.type)).\
options(joinedload(models.Label.area))
labels = get_entities_by_gids(
query=query,
entity_type='label',
Expand Down
4 changes: 2 additions & 2 deletions brainzutils/musicbrainz_db/place.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ def fetch_multiple_places(mbids, includes=None):
check_includes('place', includes)
with mb_session() as db:
query = db.query(models.Place).\
options(joinedload("area")).\
options(joinedload("type"))
options(joinedload(models.Place.area)).\
options(joinedload(models.Place.type))
places = get_entities_by_gids(
query=query,
entity_type='place',
Expand Down
14 changes: 8 additions & 6 deletions brainzutils/musicbrainz_db/recording.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from brainzutils.musicbrainz_db.serialize import serialize_recording
from brainzutils.musicbrainz_db.utils import get_entities_by_gids
from collections import defaultdict
from mbdata.models import Recording
from mbdata.models import Recording, ArtistCredit, ArtistCreditName
from sqlalchemy.orm import joinedload, subqueryload


Expand Down Expand Up @@ -72,13 +72,15 @@ def fetch_multiple_recordings(mbids, includes=None):
with mb_session() as db:
query = db.query(Recording)

if 'artist' in includes or 'artists' in includes:
query = query.options(joinedload("artist_credit", innerjoin=True))
if 'artist' in includes:
query = query.options(joinedload(Recording.artist_credit, innerjoin=True))

if 'artists' in includes:
query = query.\
options(subqueryload("artist_credit.artists")).\
options(joinedload("artist_credit.artists.artist", innerjoin=True))
query = query.options(
joinedload(Recording.artist_credit, innerjoin=True).
joinedload(ArtistCredit.artists).
joinedload(ArtistCreditName.artist)
)

recordings = get_entities_by_gids(
query=query,
Expand Down
49 changes: 27 additions & 22 deletions brainzutils/musicbrainz_db/release.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from collections import defaultdict
from mbdata import models
from mbdata.models import Release, ReleaseGroup, Medium, Track, Recording, ArtistCredit, ArtistCreditName
from sqlalchemy.orm import joinedload
from brainzutils.musicbrainz_db import exceptions as mb_exceptions
from brainzutils.musicbrainz_db import mb_session
Expand Down Expand Up @@ -44,23 +44,28 @@ def fetch_multiple_releases(mbids, includes=None):
includes_data = defaultdict(dict)
check_includes('release', includes)
with mb_session() as db:
query = db.query(models.Release)
query = db.query(Release)
if 'release-groups' in includes:
query = query.options(joinedload('release_group'))
query = query.options(joinedload(Release.release_group))
if 'artists' in includes:
query = query.\
options(joinedload("artist_credit")).\
options(joinedload("artist_credit.artists")).\
options(joinedload("artist_credit.artists.artist"))
query = query.options(
joinedload(Recording.artist_credit).
joinedload(ArtistCredit.artists).
joinedload(ArtistCreditName.artist)
)
if 'media' in includes:
# Fetch media with tracks
query = query.options(joinedload('mediums')).\
options(joinedload('mediums.tracks')).\
options(joinedload('mediums.format')).\
options(joinedload('mediums.tracks.recording')).\
options(joinedload('mediums.tracks.recording.artist_credit')).\
options(joinedload('mediums.tracks.recording.artist_credit.artists')).\
options(joinedload('mediums.tracks.recording.artist_credit.artists.artist'))
query = query\
.options(
joinedload(Release.mediums)
.options(
joinedload(Medium.format),
joinedload(Medium.tracks).
joinedload(Track.recording).
joinedload(Recording.artist_credit).
joinedload(ArtistCredit.artists).
joinedload(ArtistCreditName.artist))
)
releases = get_entities_by_gids(
query=query,
entity_type='release',
Expand Down Expand Up @@ -103,9 +108,9 @@ def browse_releases(release_group_id, includes=None):
if includes is None:
includes = []
with mb_session() as db:
release_ids = db.query(models.Release.gid).\
join(models.ReleaseGroup).\
filter(models.ReleaseGroup.gid == release_group_id).all()
release_ids = db.query(Release.gid).\
join(ReleaseGroup).\
filter(ReleaseGroup.gid == release_group_id).all()
release_ids = [release_id[0] for release_id in release_ids]
releases = fetch_multiple_releases(release_ids, includes=includes)
return releases
Expand Down Expand Up @@ -142,11 +147,11 @@ def get_releases_using_recording_mbid(recording_mbid):
recording_redirect = recording.get_recording_by_mbid(recording_mbid)
recording_mbid = recording_redirect['mbid']
with mb_session() as db:
releases = db.query(models.Release).\
join(models.Medium).\
join(models.Track).\
join(models.Recording).\
filter(models.Recording.gid == recording_mbid).all()
releases = db.query(Release).\
join(Medium).\
join(Track).\
join(Recording).\
filter(Recording.gid == recording_mbid).all()

serial_releases = [serialize_releases(release) for release in releases]
if not serial_releases:
Expand Down
18 changes: 10 additions & 8 deletions brainzutils/musicbrainz_db/release_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def get_mapped_release_types(release_types):
Returns:
List of mapped release types.
"""

release_types = [release_type.lower() for release_type in release_types]
mapped_release_types = []
with mb_session() as db:
Expand All @@ -30,7 +30,7 @@ def get_mapped_release_types(release_types):
raise mb_exceptions.InvalidTypeError("Bad release_types: {rtype} is not supported".format(rtype = release_type))
else:
mapped_release_types.append(release_type_mapping[release_type])

return mapped_release_types


Expand Down Expand Up @@ -67,14 +67,16 @@ def fetch_multiple_release_groups(mbids, includes=None):
check_includes('release_group', includes)
with mb_session() as db:
# Join table meta which contains release date for a release group
query = db.query(models.ReleaseGroup).options(joinedload("meta")).\
options(joinedload("type"))
query = db.query(models.ReleaseGroup).options(joinedload(models.ReleaseGroup.meta)).\
options(joinedload(models.ReleaseGroup.type))

if 'artists' in includes:
query = query.\
options(joinedload("artist_credit")).\
options(joinedload("artist_credit.artists")).\
options(joinedload("artist_credit.artists.artist"))
options(
joinedload(models.ReleaseGroup.artist_credit).
joinedload(models.ArtistCredit.artists).
joinedload(models.ArtistCreditName.artist)
)

release_groups = get_entities_by_gids(
query=query,
Expand Down Expand Up @@ -182,7 +184,7 @@ def get_release_groups_for_artist(artist_id, release_types=None, limit=None, off

def _get_release_groups_for_artist_query(db, artist_id, release_types):
return db.query(models.ReleaseGroup).\
options(joinedload('meta')).\
options(joinedload(models.ReleaseGroup.meta)).\
join(models.ReleaseGroupPrimaryType).join(models.ReleaseGroupMeta).\
join(models.ArtistCreditName, models.ArtistCreditName.artist_credit_id == models.ReleaseGroup.artist_credit_id).\
join(models.Artist, models.Artist.id == models.ArtistCreditName.artist_id).\
Expand Down
129 changes: 64 additions & 65 deletions brainzutils/musicbrainz_db/tests/test_helper.py
Original file line number Diff line number Diff line change
@@ -1,80 +1,79 @@
from collections import defaultdict
from unittest import TestCase
from unittest.mock import MagicMock

import pytest
from mbdata import models

from brainzutils.musicbrainz_db.serialize import serialize_relationships
from brainzutils.musicbrainz_db.helpers import get_relationship_info
import brainzutils.musicbrainz_db as mb
from brainzutils.musicbrainz_db.helpers import get_tags
from brainzutils.musicbrainz_db.test_data import linkplaceurl_1, linkplaceurl_2, place_suisto
from brainzutils.musicbrainz_db.utils import get_entities_by_gids


class HelpersTestCase(TestCase):
@pytest.mark.database
class TestHelpers:

def setUp(self):
mb.mb_session = MagicMock()
self.mock_db = mb.mb_session.return_value.__enter__.return_value
self.tags_query = self.mock_db.query.return_value.join.return_value.\
join.return_value.filter.return_value.group_by.return_value.all
self.relationships_query = self.mock_db.query.return_value.options.return_value.\
options.return_value.filter.return_value.options

def test_get_tags(self):
def test_get_tags(self, engine):
data = defaultdict(dict)
self.tags_query.return_value = [(1820974, ['hip hop', 'hip-hop/rap'])]
release_group_tags = get_tags(
db=self.mock_db,
entity_model=models.ReleaseGroup,
tag_model=models.ReleaseGroupTag,
foreign_tag_id=models.ReleaseGroupTag.release_group_id,
entity_ids=['1820974'],
)
for release_group_id, tags in release_group_tags:
data[release_group_id]['tags'] = tags
expected_data = {
1820974: {
'tags': ['hip hop', 'hip-hop/rap']
with mb.mb_session() as db:
release_group_tags = get_tags(
db=db,
entity_model=models.ReleaseGroup,
tag_model=models.ReleaseGroupTag,
foreign_tag_id=models.ReleaseGroupTag.release_group_id,
entity_ids=[253487],
)
for release_group_id, tags in release_group_tags:
data[release_group_id]['tags'] = tags
expected_data = {
253487: {
'tags': ['classical', 'ballet']
}
}
}
data = dict(data)
self.assertDictEqual(data, expected_data)
assert dict(data) == expected_data

def test_get_relationship_info(self):
def test_get_relationship_info(self, engine):
data = {}
self.relationships_query.return_value = [linkplaceurl_1, linkplaceurl_2]
includes_data = defaultdict(dict)
get_relationship_info(
db=self.mock_db,
target_type='url',
source_type='place',
source_entity_ids=['955'],
includes_data=includes_data,
)
serialize_relationships(data, place_suisto, includes_data[place_suisto.id]['relationship_objs'])
expected_data = {
'url-rels': [
{
'type': 'official homepage',
'type-id': '696b79da-7e45-40e6-a9d4-b31438eb7e5d',
'begin-year': None,
'end-year': None,
'direction': 'forward',
'url': {
'mbid': '7462ea62-7439-47f7-93bc-a425d1d989e8',
'url': 'http://www.suisto.fi/'
with mb.mb_session() as db:
gid = "3185e028-9a08-448b-83e3-873dfda40476"
place = get_entities_by_gids(
query=db.query(models.Place),
entity_type='place',
mbids=[gid],
)[gid]
get_relationship_info(
db=db,
target_type='url',
source_type='place',
source_entity_ids=[place.id],
includes_data=includes_data,
)
serialize_relationships(data, place, includes_data[place.id]['relationship_objs'])
expected_data = {
'url-rels': [
{
'type': 'wikidata',
'type-id': 'e6826618-b410-4b8d-b3b5-52e29eac5e1f',
'begin-year': None,
'end-year': None,
'direction': 'forward',
'url': {
'mbid': '86d64bb6-bcee-4cda-b1f8-050394664671',
'url': 'https://www.wikidata.org/wiki/Q2489904'
}
},
{
'type': 'discogs',
'type-id': '1c140ac8-8dc2-449e-92cb-52c90d525640',
'begin-year': None,
'end-year': None,
'direction': 'forward',
'url': {
'mbid': '06332787-5aac-4e4c-95b9-75cf729ae308',
'url': 'https://www.discogs.com/label/266610'
}
}
},
{
'type': 'social network',
'type-id': '040de4d5-ace5-4cfb-8a45-95c5c73bce01',
'begin-year': None,
'end-year': None,
'direction': 'forward',
'url': {
'mbid': '8de22e00-c8e8-475f-814e-160ef761da63',
'url': 'https://twitter.com/Suisto'
}
}
]
}
self.assertDictEqual(data, expected_data)
]
}
assert data == expected_data
2 changes: 1 addition & 1 deletion brainzutils/musicbrainz_db/work.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def fetch_multiple_works(mbids, includes=None):
includes_data = defaultdict(dict)
check_includes('work', includes)
with mb_session() as db:
query = db.query(models.Work).options(joinedload("type"))
query = db.query(models.Work).options(joinedload(models.Work.type))

works = get_entities_by_gids(
query=query,
Expand Down

0 comments on commit 448d4f6

Please sign in to comment.