From 7dc864774eacd701c467ceecb8403155ba1eb98b Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Fri, 12 Aug 2022 20:46:13 +0200 Subject: filter visible tracks in SQL While this requires some logic duplication, and it does mean that we have to generate complex SQL queries, it is also the preferred way of doing this, as we do not have to load all the tracks just to filter them out in the application. --- fietsboek/models/user.py | 88 ++++++++++++++++++++++++++++++++++++++++------ fietsboek/views/browse.py | 13 +++---- fietsboek/views/default.py | 2 +- 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/fietsboek/models/user.py b/fietsboek/models/user.py index dfddd78..a972f8e 100644 --- a/fietsboek/models/user.py +++ b/fietsboek/models/user.py @@ -168,8 +168,7 @@ class User(Base): principals.append('group:admins') return principals - @property - def all_tracks(self): + def all_tracks_query(self): """Returns a query that selects all the user's tracks. This includes the user's own tracks, as well as any tracks the user has @@ -182,11 +181,11 @@ class User(Base): >>> from sqlalchemy import select >>> from sqlalchemy.orm import aliased >>> user = retrieve_user() - >>> query = user.all_tracks + >>> query = user.all_tracks_query() >>> query = query.filter(query.c.type == TrackType.ORGANIC) >>> query = select(aliased(Track, query)) - :rtype: sqlalchemy.sql.Selectable + :rtype: sqlalchemy.sql.expression.Selectable """ # Late import to avoid cycles # pylint: disable=import-outside-toplevel @@ -196,22 +195,89 @@ class User(Base): # Create a fresh select so we can apply filter operations return union(own, friends).subquery() - def get_friends(self): - """Returns all friends of the user. + @classmethod + def visible_tracks_query(cls, user=None): + """Returns a query that selects all tracks visible to the given user. - This is not a simple SQLAlchemy property because the friendship - relation is complex. + The user might be ``None``, in which case tracks for public users are + returned. - :return: All friends of this user. - :rtype: list[User] + The returned query can be further modified, however, you need to use + :func:`sqlalchemy.orm.aliased` to access the correct objects (see also + :meth:`all_tracks_query`). + + :param user: The user for which the tracks should be searched. + :type user: User + :rtype: sqlalchemy.sql.expression.Selectable """ + # Late import to avoid cycles + # pylint: disable=import-outside-toplevel,protected-access + from .track import Track, Visibility, track_people_assoc + # We build the list of visible tracks in multiple steps, and then union + # them later. + queries = [] + # Step 1: Own tracks and tagged tracks + if user: + queries.append(select(user.all_tracks_query())) + # Step 2: Public tracks + queries.append(select(Track).where(Track.visibility == Visibility.PUBLIC)) + # Step 3: Logged in + if user: + queries.append(select(Track).where(Track.visibility == Visibility.LOGGED_IN)) + # Step 4: Friends' tracks + if user: + friend_query = user._friend_query() + friend_ids = select(friend_query.c.id) + queries.append( + select(Track) + # The owner also counts as a "tagged person", so we need to + # include FRIENDS_TAGGED here as well. + .where(Track.visibility.in_([Visibility.FRIENDS, Visibility.FRIENDS_TAGGED])) + .where(Track.owner_id.in_(friend_ids)) + ) + # Step 5: Am I a friend of a tagged person? + # We do this via a big join: + # Table -> Tagged people -> Friends (per column 1) + # Table -> Tagged people -> Friends (per column 2) + # With those, we have a product that contains a track and all friends + # that this track should be visible to. + if user: + queries.append( + select(Track) + .join(track_people_assoc) + .join(friends_assoc, track_people_assoc.c.user_id == friends_assoc.c.user_1_id) + .where(friends_assoc.c.user_2_id == user.id) + .where(Track.visibility == Visibility.FRIENDS_TAGGED) + ) + queries.append( + select(Track) + .join(track_people_assoc) + .join(friends_assoc, track_people_assoc.c.user_id == friends_assoc.c.user_2_id) + .where(friends_assoc.c.user_1_id == user.id) + .where(Track.visibility == Visibility.FRIENDS_TAGGED) + ) + + return union(*queries) + + def _friend_query(self): qry1 = (select(User) .filter(friends_assoc.c.user_1_id == self.id, friends_assoc.c.user_2_id == User.id)) qry2 = (select(User) .filter(friends_assoc.c.user_2_id == self.id, friends_assoc.c.user_1_id == User.id)) - query = select(User).from_statement(union(qry1, qry2)) + return union(qry1, qry2) + + def get_friends(self): + """Returns all friends of the user. + + This is not a simple SQLAlchemy property because the friendship + relation is complex. + + :return: All friends of this user. + :rtype: list[User] + """ + query = select(User).from_statement(self._friend_query()) yield from object_session(self).execute(query).scalars() def remove_friend(self, friend): diff --git a/fietsboek/views/browse.py b/fietsboek/views/browse.py index ffd3eae..03d9bf8 100644 --- a/fietsboek/views/browse.py +++ b/fietsboek/views/browse.py @@ -8,6 +8,7 @@ from pyramid.httpexceptions import HTTPForbidden, HTTPNotFound, HTTPBadRequest from pyramid.response import Response from sqlalchemy import select +from sqlalchemy.orm import aliased from .. import models, util @@ -153,14 +154,10 @@ def visible_tracks(dbsession, user): :return: The list of visible tracks. :rtype: ~collections.abc.Iterable[fietsboek.models.track.Track] """ - # This is fine for now, but we should try and do most of this work right in - # SQL in the future. - tracks = dbsession.execute(select(models.Track)).scalars() - return sorted( - (track for track in tracks if track.is_visible_to(user)), - key=lambda t: t.date, - reverse=True, - ) + temp_track = aliased(models.Track, models.User.visible_tracks_query(user).subquery()) + query = select(temp_track).order_by(temp_track.date_raw.desc()) + tracks = dbsession.execute(query).scalars() + return tracks @view_config(route_name="browse", renderer="fietsboek:templates/browse.jinja2", diff --git a/fietsboek/views/default.py b/fietsboek/views/default.py index 1df4942..de59f06 100644 --- a/fietsboek/views/default.py +++ b/fietsboek/views/default.py @@ -30,7 +30,7 @@ def home(request): 'home_content': content, } - query = request.identity.all_tracks + query = request.identity.all_tracks_query() query = select(aliased(models.Track, query)).where(query.c.type == TrackType.ORGANIC) summary = summaries.Summary() -- cgit v1.2.3