diff --git a/packages/discovery-provider/ddl/migrations/0064_playlists_previously_containing_track.sql b/packages/discovery-provider/ddl/migrations/0064_playlists_previously_containing_track.sql new file mode 100644 index 00000000000..a2082d18dc2 --- /dev/null +++ b/packages/discovery-provider/ddl/migrations/0064_playlists_previously_containing_track.sql @@ -0,0 +1,14 @@ +BEGIN; + +-- disable triggers +ALTER TABLE tracks DISABLE trigger on_track; +ALTER TABLE tracks DISABLE trigger trg_tracks; + +ALTER TABLE tracks +ADD COLUMN IF NOT EXISTS playlists_previously_containing_track JSONB NOT NULL DEFAULT jsonb_build_object(); + +-- enable triggers +ALTER TABLE tracks ENABLE trigger on_track; +ALTER TABLE tracks ENABLE trigger trg_tracks; + +COMMIT; diff --git a/packages/discovery-provider/integration_tests/tasks/entity_manager/test_playlist_tracks.py b/packages/discovery-provider/integration_tests/tasks/entity_manager/test_playlist_tracks.py index 4450eaeae32..8ee71168b40 100644 --- a/packages/discovery-provider/integration_tests/tasks/entity_manager/test_playlist_tracks.py +++ b/packages/discovery-provider/integration_tests/tasks/entity_manager/test_playlist_tracks.py @@ -310,6 +310,13 @@ def test_remove_track_from_playlist(app, mocker): .playlists_containing_track == [] ) + assert ( + session.query(Track) + .filter(Track.track_id == 10) + .first() + .playlists_previously_containing_track + == {} + ) assert ( session.query(Track) .filter(Track.track_id == 20) @@ -317,9 +324,21 @@ def test_remove_track_from_playlist(app, mocker): .playlists_containing_track == [] ) + assert session.query(Track).filter( + Track.track_id == 20 + ).first().playlists_previously_containing_track == { + str(PLAYLIST_ID_OFFSET): {"time": 1585336422} + } assert session.query(Track).filter( Track.track_id == 30 ).first().playlists_containing_track == [PLAYLIST_ID_OFFSET] + assert ( + session.query(Track) + .filter(Track.track_id == 30) + .first() + .playlists_previously_containing_track + == {} + ) assert ( session.query(Track) .filter(Track.track_id == 40) @@ -327,9 +346,17 @@ def test_remove_track_from_playlist(app, mocker): .playlists_containing_track == [] ) + assert ( + session.query(Track) + .filter(Track.track_id == 40) + .first() + .playlists_previously_containing_track + == {} + ) -# Remove a track from a playlist and then restore it +# Remove a track from a playlist and then restore it to the same playlist, +# confirm there are no duplicates in playlists_containing_track restore_removed_track_to_playlist_tx_receipts = { "UpdatePlaylistTracklistUpdate": [ { @@ -414,12 +441,185 @@ def test_restore_removed_track_to_playlist(app, mocker): .playlists_containing_track == [] ) + assert ( + session.query(Track) + .filter(Track.track_id == 10) + .first() + .playlists_previously_containing_track + == {} + ) assert session.query(Track).filter( Track.track_id == 20 ).first().playlists_containing_track == [PLAYLIST_ID_OFFSET] + assert ( + session.query(Track) + .filter(Track.track_id == 20) + .first() + .playlists_previously_containing_track + == {} + ) + assert session.query(Track).filter( + Track.track_id == 30 + ).first().playlists_containing_track == [PLAYLIST_ID_OFFSET] + assert ( + session.query(Track) + .filter(Track.track_id == 30) + .first() + .playlists_previously_containing_track + == {} + ) + assert ( + session.query(Track) + .filter(Track.track_id == 40) + .first() + .playlists_containing_track + == [] + ) + assert ( + session.query(Track) + .filter(Track.track_id == 40) + .first() + .playlists_previously_containing_track + == {} + ) + + +# Remove a track from a playlist, restore it to same playlist, then remove again +# and confirm there are no duplicates in playlists_previously_containing_track +remove_from_playlist_twice_tx_receipts = { + "UpdatePlaylistTracklistUpdate": [ + { + "args": AttributeDict( + { + "_entityId": PLAYLIST_ID_OFFSET, + "_entityType": "Playlist", + "_userId": 1, + "_action": "Update", + "_metadata": f'{{"cid": "PlaylistTracklistUpdate", "data": {json.dumps(test_metadata["PlaylistTracklistUpdate"])}, "timestamp": {datetime.timestamp(now)}}}', + "_signer": "user1wallet", + } + ) + } + ], + "RemoveTrackFromPlaylistUpdate": [ + { + "args": AttributeDict( + { + "_entityId": PLAYLIST_ID_OFFSET, + "_entityType": "Playlist", + "_userId": 1, + "_action": "Update", + "_metadata": f'{{"cid": "PlaylistTracklistUpdate", "data": {json.dumps(test_metadata["RemoveFromPlaylistTracklistUpdate"])}, "timestamp": {datetime.timestamp(now)}}}', + "_signer": "user1wallet", + } + ) + } + ], + "RestoreTrackToPlaylist": [ + { + "args": AttributeDict( + { + "_entityId": PLAYLIST_ID_OFFSET, + "_entityType": "Playlist", + "_userId": 1, + "_action": "Update", + "_metadata": f'{{"cid": "PlaylistTracklistUpdate", "data": {json.dumps(test_metadata["PlaylistTracklistUpdate"])}, "timestamp": {datetime.timestamp(now)}}}', + "_signer": "user1wallet", + } + ) + } + ], + "RemoveTrackFromPlaylistUpdate2": [ + { + "args": AttributeDict( + { + "_entityId": PLAYLIST_ID_OFFSET, + "_entityType": "Playlist", + "_userId": 1, + "_action": "Update", + "_metadata": f'{{"cid": "PlaylistTracklistUpdate", "data": {json.dumps(test_metadata["RemoveFromPlaylistTracklistUpdate"])}, "timestamp": {datetime.timestamp(now)}}}', + "_signer": "user1wallet", + } + ) + } + ], +} + + +def test_remove_from_playlist_twice(app, mocker): + db, update_task, entity_manager_txs = setup_db( + app, mocker, entities, remove_from_playlist_twice_tx_receipts + ) + + with db.scoped_session() as session: + entity_manager_update( + update_task, + session, + entity_manager_txs, + block_number=0, + block_timestamp=1585336422, + block_hash=hex(0), + ) + relations: List[PlaylistTrack] = session.query(PlaylistTrack).all() + assert len(relations) == 2 + for id in [30]: + assert any( + [ + relation.track_id == id and not relation.is_removed + for relation in relations + ] + ) + for id in [10, 40]: + assert not any( + [ + relation.track_id == id and relation.is_removed + for relation in relations + ] + ) + for id in [20]: + assert any( + [ + relation.track_id == id and relation.is_removed + for relation in relations + ] + ) + + assert ( + session.query(Track) + .filter(Track.track_id == 10) + .first() + .playlists_containing_track + == [] + ) + assert ( + session.query(Track) + .filter(Track.track_id == 10) + .first() + .playlists_previously_containing_track + == {} + ) + assert ( + session.query(Track) + .filter(Track.track_id == 20) + .first() + .playlists_containing_track + == [] + ) + assert session.query(Track).filter( + Track.track_id == 20 + ).first().playlists_previously_containing_track == { + str(PLAYLIST_ID_OFFSET): {"time": 1585336422} + } assert session.query(Track).filter( Track.track_id == 30 ).first().playlists_containing_track == [PLAYLIST_ID_OFFSET] + assert ( + session.query(Track) + .filter(Track.track_id == 30) + .first() + .playlists_previously_containing_track + == {} + ) assert ( session.query(Track) .filter(Track.track_id == 40) @@ -427,6 +627,13 @@ def test_restore_removed_track_to_playlist(app, mocker): .playlists_containing_track == [] ) + assert ( + session.query(Track) + .filter(Track.track_id == 40) + .first() + .playlists_previously_containing_track + == {} + ) # Create a playlist then reorder the tracks diff --git a/packages/discovery-provider/src/models/tracks/track.py b/packages/discovery-provider/src/models/tracks/track.py index 2d1d544f034..d3afcfb3b27 100644 --- a/packages/discovery-provider/src/models/tracks/track.py +++ b/packages/discovery-provider/src/models/tracks/track.py @@ -89,6 +89,11 @@ class Track(Base, RepresentableMixin): playlists_containing_track = Column( ARRAY(Integer()), server_default="ARRAY[]::INTEGER[]" ) + playlists_previously_containing_track = Column( + JSONB(True), + nullable=False, + server_default="json_build_object()", + ) ai_attribution_user_id = Column(Integer, nullable=True) placement_hosts = Column(String, nullable=True) diff --git a/packages/discovery-provider/src/tasks/entity_manager/entities/playlist.py b/packages/discovery-provider/src/tasks/entity_manager/entities/playlist.py index ec598a7525c..526981825b4 100644 --- a/packages/discovery-provider/src/tasks/entity_manager/entities/playlist.py +++ b/packages/discovery-provider/src/tasks/entity_manager/entities/playlist.py @@ -146,20 +146,57 @@ def update_playlist_tracks(params: ManageEntityParameters, playlist_record: Play if playlist_track.track_id not in updated_track_ids: playlist_track.is_removed = True playlist_track.updated_at = params.block_datetime - track = ( + track_record = ( session.query(Track) .filter(Track.track_id == playlist_track.track_id) .first() ) - if track: - track.updated_at = params.block_datetime - track.playlists_containing_track = [ - collection_id - for collection_id in (track.playlists_containing_track or []) - if collection_id != playlist["playlist_id"] + if track_record: + current_playlist_id = playlist["playlist_id"] + track = helpers.model_to_dictionary(track_record) + track_record.updated_at = params.block_datetime + # remove from playlists_containing_track + track_record.playlists_containing_track = [ + playlist_id + for playlist_id in track["playlists_containing_track"] + if playlist_id != current_playlist_id ] + # add to or overwrite existing entry in playlists_previously_containing_track + playlists_previously_containing_track = track[ + "playlists_previously_containing_track" + ].copy() + playlists_previously_containing_track[str(current_playlist_id)] = { + "time": params.block_integer_time + } + track_record.playlists_previously_containing_track = ( + playlists_previously_containing_track + ) + for track_id in updated_track_ids: + # add playlist_id to playlists_containing_track and remove from playlists_previously_containing_track + track_record = session.query(Track).filter(Track.track_id == track_id).first() + if track_record: + current_playlist_id = playlist["playlist_id"] + track = helpers.model_to_dictionary(track_record) + track_record.updated_at = params.block_datetime + + track_record.playlists_containing_track = list( + set((track["playlists_containing_track"]) + [current_playlist_id]) + ) + + if ( + str(current_playlist_id) + in track_record.playlists_previously_containing_track + ): + playlists_previously_containing_track = track[ + "playlists_previously_containing_track" + ].copy() + del playlists_previously_containing_track[str(current_playlist_id)] + track_record.playlists_previously_containing_track = ( + playlists_previously_containing_track + ) + # add row for each track that is not already in the table if track_id not in existing_tracks: new_playlist_track = PlaylistTrack( @@ -171,28 +208,10 @@ def update_playlist_tracks(params: ManageEntityParameters, playlist_record: Play ) # upsert to handle duplicates session.merge(new_playlist_track) - track = session.query(Track).filter(Track.track_id == track_id).first() - if track: - track.updated_at = params.block_datetime - track.playlists_containing_track = list( - set( - (track.playlists_containing_track or []) - + [playlist["playlist_id"]] - ) - ) elif existing_tracks[track_id].is_removed: # recover deleted relation (track was previously removed then re-added) existing_tracks[track_id].is_removed = False existing_tracks[track_id].updated_at = params.block_datetime - track = session.query(Track).filter(Track.track_id == track_id).first() - if track: - track.updated_at = params.block_datetime - track.playlists_containing_track = list( - set( - (track.playlists_containing_track or []) - + [playlist["playlist_id"]] - ) - ) params.logger.info( f"playlists.py | Updated playlist tracks for {playlist['playlist_id']}"