Full Playlists API Endpoint (#869)

* Full playlists endpoint

* Fix bugs with playlist endpoint

* Add second redis cache in playlist query for unpopulated playlists

* Fix lint errors

* Fix unsafe indexing into args

* Address PR comments

discovery-provider/src/api/v1/api.py

@@ -2,7 +2,7 @@
 from flask.helpers import url_for
 from flask_restx import Resource, Api
 from src.api.v1.users import ns as users_ns
-from src.api.v1.playlists import ns as playlists_ns
+from src.api.v1.playlists import ns as playlists_ns, full_ns as full_playlists_ns
 from src.api.v1.tracks import ns as tracks_ns, full_ns as full_tracks_ns
 from src.api.v1.metrics import ns as metrics_ns
 from src.api.v1.models.users import ns as models_ns
@@ -31,3 +31,4 @@ def specs_url(self):
 bp_full = Blueprint('api_v1_full', __name__, url_prefix='/v1/full')
 api_v1_full = ApiWithHTTPS(bp_full, version='1.0')
 api_v1_full.add_namespace(full_tracks_ns)
+api_v1_full.add_namespace(full_playlists_ns)

discovery-provider/src/api/v1/helpers.py

@@ -1,10 +1,13 @@
+import logging
 from src import api_helpers
 from src.utils.config import shared_config
 from hashids import Hashids
 from flask_restx import fields, reqparse
 from src.queries.search_queries import SearchKind
 from datetime import datetime
 
+logger = logging.getLogger(__name__)
+
 HASH_MIN_LENGTH = 5
 HASH_SALT = "azowernasdfoia"
 
@@ -158,6 +161,10 @@ def extend_playlist(playlist):
     if ("user" in playlist):
         playlist["user"] = extend_user(playlist["user"])
     playlist = add_playlist_artwork(playlist)
+
+    playlist["followee_reposts"] = list(map(extend_repost, playlist["followee_reposts"]))
+    playlist["followee_saves"] = list(map(extend_favorite, playlist["followee_saves"]))
+
     playlist["favorite_count"] = playlist["save_count"]
     return playlist
 

discovery-provider/src/api/v1/models/common.py

@@ -9,28 +9,12 @@
     "user_id": fields.String(required=True)
 })
 
-repost_full = ns.clone("repost_full", repost, {
-    "blockhash": fields.String(required=True),
-    "blocknumber": fields.Integer(required=True),
-    "created_at": fields.String(required=True),
-    "is_current": fields.Boolean(required=True),
-    "is_delete": fields.Boolean(required=True),
-})
-
 favorite = ns.model('favorite', {
     "favorite_item_id": fields.String(required=True),
     "favorite_type": fields.String(required=True),
     "user_id": fields.String(required=True)
 })
 
-favorite_full = ns.clone("favorite_full", favorite, {
-    "blockhash": fields.String(required=True),
-    "blocknumber": fields.Integer(required=True),
-    "created_at": fields.String(required=True),
-    "is_current": fields.Boolean(required=True),
-    "is_delete": fields.Boolean(required=True),
-})
-
 version_metadata = ns.model("version_metadata", {
     "service": fields.String(required=True),
     "version": fields.String(required=True)

discovery-provider/src/api/v1/models/playlists.py

@@ -1,6 +1,6 @@
-from src.api.v1.helpers import make_response
 from flask_restx import fields
-from src.api.v1.models.users import user_model
+from src.api.v1.models.users import user_model, user_model_full
+from src.api.v1.models.tracks import track_full
 from .common import favorite, ns, repost
 
 playlist_artwork = ns.model('playlist_artwork', {
@@ -30,19 +30,16 @@
     "user": fields.Nested(user_model, required=True),
 })
 
-playlist_full = ns.model('playlist_full', playlist_model, {
-    "blockhash": fields.String(required=True),
-    "blocknumber": fields.Integer(required=True),
+full_playlist_model = ns.clone('playlist_full', playlist_model, {
     "created_at": fields.String,
     "followee_reposts": fields.List(fields.Nested(repost)),
     "followee_saves": fields.List(fields.Nested(favorite)),
     "has_current_user_reposted": fields.Boolean(required=True),
-    "has_current_user_saved	true": fields.Boolean(required=True),
-    "is_current": fields.Boolean(required=True),
+    "has_current_user_saved": fields.Boolean(required=True),
     "is_delete": fields.Boolean(required=True),
     "is_private": fields.Boolean(required=True),
-    "upc": fields.String,
     "updated_at": fields.String,
     "playlist_contents": fields.Nested(playlist_contents, required=True),
-    "user_id": fields.String(required=True),
+    "user": fields.Nested(user_model_full, required=True),
+    "tracks": fields.List(fields.Nested(track_full), required=True)
 })

discovery-provider/src/api/v1/playlists.py

@@ -1,11 +1,11 @@
 import logging
 from src.queries.get_top_playlists import get_top_playlists # pylint: disable=C0302
-from src.api.v1.models.playlists import playlist_model
+from src.api.v1.models.playlists import playlist_model, full_playlist_model
 from src.queries.get_playlists import get_playlists
 from flask_restx import Resource, Namespace, fields, reqparse
 from src.queries.get_playlist_tracks import get_playlist_tracks
 from src.api.v1.helpers import abort_not_found, decode_with_abort, extend_playlist, extend_track,\
-    make_response, success_response, search_parser, abort_bad_request_param
+    make_response, success_response, search_parser, abort_bad_request_param, decode_string_id
 from .models.tracks import track
 from src.queries.search_queries import SearchKind, search
 from src.utils.redis_cache import cache
@@ -14,10 +14,33 @@
 logger = logging.getLogger(__name__)
 
 ns = Namespace('playlists', description='Playlist related operations')
+full_ns = Namespace('playlists', description='Full playlist related operations')
 
 playlists_response = make_response("playlist_response", ns, fields.List(fields.Nested(playlist_model)))
+full_playlists_response = make_response("full_playlist_response", full_ns, fields.List(fields.Nested(full_playlist_model)))
 
-@ns.route("/<string:playlist_id>")
+def get_playlist(playlist_id, current_user_id):
+    """Returns a single playlist, or None"""
+    args = {
+        "playlist_id": [playlist_id],
+        "with_users": True,
+        "current_user_id": current_user_id
+    }
+    playlists = get_playlists(args)
+    if playlists:
+        return extend_playlist(playlists[0])
+    return None
+
+def get_tracks_for_playlist(playlist_id, current_user_id=None):
+    args = {"playlist_id": playlist_id, "with_users": True, "current_user_id": current_user_id}
+    playlist_tracks = get_playlist_tracks(args)
+    if not playlist_tracks:
+        abort_not_found(playlist_id, ns)
+    tracks = list(map(extend_track, playlist_tracks))
+    return tracks
+
+PLAYLIST_ROUTE = "/<string:playlist_id>"
+@ns.route(PLAYLIST_ROUTE)
 class Playlist(Resource):
     @record_metrics
     @ns.doc(
@@ -34,14 +57,33 @@ class Playlist(Resource):
     def get(self, playlist_id):
         """Fetch a playlist."""
         playlist_id = decode_with_abort(playlist_id, ns)
-        args = {"playlist_id": [playlist_id], "with_users": True}
-        playlists = get_playlists(args)
-        playlists = list(map(extend_playlist, playlists))
-        response = success_response(playlists)
+        playlist = get_playlist(playlist_id, None)
+        response = success_response([playlist] if playlist else [])
         return response
 
 playlist_tracks_response = make_response("playlist_tracks_response", ns, fields.List(fields.Nested(track)))
 
+full_playlist_parser = reqparse.RequestParser()
+full_playlist_parser.add_argument('user_id', required=False)
+@full_ns.route(PLAYLIST_ROUTE)
+class FullPlaylist(Resource):
+    @ns.marshal_with(full_playlists_response)
+    @cache(ttl_sec=5)
+    def get(self, playlist_id):
+        """Fetch a playlist."""
+        playlist_id = decode_with_abort(playlist_id, full_ns)
+        args = full_playlist_parser.parse_args()
+        current_user_id = None
+        if args.get("user_id"):
+            current_user_id = decode_string_id(args["user_id"])
+
+        playlist = get_playlist(playlist_id, current_user_id)
+        if playlist:
+            tracks = get_tracks_for_playlist(playlist_id, current_user_id)
+            playlist["tracks"] = tracks
+        response = success_response([playlist] if playlist else [])
+        return response
+
 @ns.route("/<string:playlist_id>/tracks")
 class PlaylistTracks(Resource):
     @record_metrics
@@ -59,11 +101,7 @@ class PlaylistTracks(Resource):
     def get(self, playlist_id):
         """Fetch tracks within a playlist."""
         decoded_id = decode_with_abort(playlist_id, ns)
-        args = {"playlist_id": decoded_id, "with_users": True}
-        playlist_tracks = get_playlist_tracks(args)
-        if not playlist_tracks:
-            abort_not_found(playlist_id, ns)
-        tracks = list(map(extend_track, playlist_tracks))
+        tracks = get_tracks_for_playlist(decoded_id)
         return success_response(tracks)
 
 playlist_search_result = make_response("playlist_search_result", ns, fields.List(fields.Nested(playlist_model)))

discovery-provider/src/api/v1/tracks.py

@@ -232,6 +232,7 @@ def get_cache_key(self):
         key = extract_key(request.path, request_items.items())
         return key
 
+    @record_metrics
     @full_ns.marshal_with(full_tracks_response)
     def get(self):
         args = full_trending_parser.parse_args()

discovery-provider/src/queries/get_playlists.py

@@ -2,23 +2,42 @@
 import sqlalchemy
 from sqlalchemy import desc
 
+from flask.globals import request
 from src import exceptions
 from src.models import Playlist, RepostType, SaveType
 from src.utils import helpers
 from src.utils.db_session import get_db_read_replica
-from src.queries.query_helpers import get_current_user_id, paginate_query, \
+from src.queries.query_helpers import paginate_query, \
   populate_playlist_metadata, get_users_ids, get_users_by_id
+from src.utils.redis_cache import extract_key, use_redis_cache
 
 logger = logging.getLogger(__name__)
 
+UNPOPULATED_PLAYLIST_CACHE_DURATION_SEC = 10
+
+def make_cache_key(args):
+    cache_keys = {
+        "user_id": args.get("user_id"),
+        "with_users": args.get("with_users")
+    }
+
+    if args.get("playlist_id"):
+        ids = args.get("playlist_id")
+        ids = map(str, ids)
+        ids = ",".join(ids)
+        cache_keys["playlist_id"] = ids
+
+    key = extract_key(f"unpopulated-playlist:{request.path}", cache_keys.items())
+    return key
+
 def get_playlists(args):
     playlists = []
-    current_user_id = get_current_user_id(required=False)
-    filter_out_private_playlists = True
+    current_user_id = args.get("current_user_id")
 
     db = get_db_read_replica()
     with db.scoped_session() as session:
-        try:
+        def get_unpopulated_playlists():
+            filter_out_private_playlists = True
             playlist_query = (
                 session.query(Playlist)
                 .filter(Playlist.is_current == True)
@@ -63,7 +82,17 @@ def get_playlists(args):
             # retrieve playlist ids list
             playlist_ids = list(map(lambda playlist: playlist["playlist_id"], playlists))
 
-            current_user_id = get_current_user_id(required=False)
+            return (playlists, playlist_ids)
+
+        try:
+            # Get unpopulated playlists, either via
+            # redis cache or via get_unpopulated_playlists
+            key = make_cache_key(args)
+
+            (playlists, playlist_ids) = use_redis_cache(
+                key,
+                UNPOPULATED_PLAYLIST_CACHE_DURATION_SEC,
+                get_unpopulated_playlists)
 
             # bundle peripheral info into playlist results
             playlists = populate_playlist_metadata(

discovery-provider/src/queries/queries.py

@@ -123,6 +123,8 @@ def get_playlists_route():
         args["user_id"] = request.args.get("user_id", type=int)
     if "with_users" in request.args:
         args["with_users"] = parse_bool_param(request.args.get("with_users"))
+    current_user_id = get_current_user_id(required=False)
+    args["current_user_id"] = current_user_id
     playlists = get_playlists(args)
     return api_helpers.success_response(playlists)
 

discovery-provider/src/utils/query_params.py

@@ -7,4 +7,4 @@
 def stringify_query_params(req_arg_items):
     req_arg_items = filter(lambda x: x[0] not in exclude_param_set, req_arg_items)
     req_arg_items = sorted(req_arg_items)
-    return f"&".join(["{}={}".format(x[0].lower(), x[1].lower()) for x in req_arg_items])
+    return f"&".join(["{}={}".format(x[0].lower(), str(x[1]).lower()) for x in req_arg_items])

discovery-provider/src/utils/redis_cache_test.py

@@ -1,8 +1,8 @@
 import json
 from time import sleep
 from unittest.mock import patch
-from src.utils.redis_cache import cache
 import flask
+from src.utils.redis_cache import cache
 
 def test_cache(redis_mock):
     """Test that the redis cache decorator works"""