From 1b72d9b60fbeff3d4d2247e93c3becdf3658c15f Mon Sep 17 00:00:00 2001 From: ta264 Date: Sun, 8 Sep 2019 21:44:25 +0100 Subject: [PATCH] Fixed: NRE importing Spotify saved albums / followed artists (#962) --- .../Spotify/SpotifyFollowedArtistsFixture.cs | 173 +++++++++++++ .../Spotify/SpotifyPlaylistFixture.cs | 239 ++++++++++++++++++ .../Spotify/SpotifySavedAlbumsFixture.cs | 166 ++++++++++++ .../Spotify/SpotifyFollowedArtists.cs | 47 ++-- .../Spotify/SpotifyImportListBase.cs | 53 ++-- .../ImportLists/Spotify/SpotifyPlaylist.cs | 93 ++++--- .../ImportLists/Spotify/SpotifyProxy.cs | 109 ++++++++ .../ImportLists/Spotify/SpotifySavedAlbums.cs | 57 +++-- 8 files changed, 831 insertions(+), 106 deletions(-) create mode 100644 src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifyFollowedArtistsFixture.cs create mode 100644 src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifyPlaylistFixture.cs create mode 100644 src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifySavedAlbumsFixture.cs create mode 100644 src/NzbDrone.Core/ImportLists/Spotify/SpotifyProxy.cs diff --git a/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifyFollowedArtistsFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifyFollowedArtistsFixture.cs new file mode 100644 index 000000000..982df6e61 --- /dev/null +++ b/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifyFollowedArtistsFixture.cs @@ -0,0 +1,173 @@ +using System.Collections.Generic; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.ImportLists.Spotify; +using NzbDrone.Core.Test.Framework; +using SpotifyAPI.Web; +using SpotifyAPI.Web.Models; + +namespace NzbDrone.Core.Test.ImportListTests +{ + [TestFixture] + public class SpotifyFollowedArtistsFixture : CoreTest + { + // placeholder, we don't use real API + private readonly SpotifyWebAPI api = null; + + [Test] + public void should_not_throw_if_followed_is_null() + { + Mocker.GetMock(). + Setup(x => x.GetFollowedArtists(It.IsAny(), + It.IsAny())) + .Returns(default(FollowedArtists)); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + } + + [Test] + public void should_not_throw_if_followed_artists_is_null() + { + var followed = new FollowedArtists { + Artists = null + }; + + Mocker.GetMock(). + Setup(x => x.GetFollowedArtists(It.IsAny(), + It.IsAny())) + .Returns(followed); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + } + + [Test] + public void should_not_throw_if_followed_artist_items_is_null() + { + var followed = new FollowedArtists { + Artists = new CursorPaging { + Items = null + } + }; + + Mocker.GetMock(). + Setup(x => x.GetFollowedArtists(It.IsAny(), + It.IsAny())) + .Returns(followed); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + Subject.Fetch(api); + } + + [Test] + public void should_not_throw_if_artist_is_null() + { + var followed = new FollowedArtists { + Artists = new CursorPaging { + Items = new List { + null + } + } + }; + + Mocker.GetMock(). + Setup(x => x.GetFollowedArtists(It.IsAny(), + It.IsAny())) + .Returns(followed); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + Subject.Fetch(api); + } + + [Test] + public void should_parse_followed_artist() + { + var followed = new FollowedArtists { + Artists = new CursorPaging { + Items = new List { + new FullArtist { + Name = "artist" + } + } + } + }; + + Mocker.GetMock(). + Setup(x => x.GetFollowedArtists(It.IsAny(), + It.IsAny())) + .Returns(followed); + + var result = Subject.Fetch(api); + + result.Should().HaveCount(1); + } + + [Test] + public void should_not_throw_if_get_next_page_returns_null() + { + var followed = new FollowedArtists { + Artists = new CursorPaging { + Items = new List { + new FullArtist { + Name = "artist" + } + }, + Next = "DummyToMakeHasNextTrue" + } + }; + + Mocker.GetMock(). + Setup(x => x.GetFollowedArtists(It.IsAny(), + It.IsAny())) + .Returns(followed); + + Mocker.GetMock() + .Setup(x => x.GetNextPage(It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Returns(default(CursorPaging)); + + var result = Subject.Fetch(api); + + result.Should().HaveCount(1); + + Mocker.GetMock() + .Verify(v => v.GetNextPage(It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Once()); + } + + [TestCase(null)] + [TestCase("")] + public void should_skip_bad_artist_names(string name) + { + var followed = new FollowedArtists { + Artists = new CursorPaging { + Items = new List { + new FullArtist { + Name = name + } + } + } + }; + + Mocker.GetMock(). + Setup(x => x.GetFollowedArtists(It.IsAny(), + It.IsAny())) + .Returns(followed); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + } + } +} diff --git a/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifyPlaylistFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifyPlaylistFixture.cs new file mode 100644 index 000000000..6b82d43cb --- /dev/null +++ b/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifyPlaylistFixture.cs @@ -0,0 +1,239 @@ +using System.Collections.Generic; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.ImportLists.Spotify; +using NzbDrone.Core.Test.Framework; +using SpotifyAPI.Web; +using SpotifyAPI.Web.Models; + +namespace NzbDrone.Core.Test.ImportListTests +{ + [TestFixture] + public class SpotifyPlaylistFixture : CoreTest + { + // placeholder, we don't use real API + private readonly SpotifyWebAPI api = null; + + [Test] + public void should_not_throw_if_playlist_tracks_is_null() + { + Mocker.GetMock(). + Setup(x => x.GetPlaylistTracks(It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(default(Paging)); + + var result = Subject.Fetch(api, "playlistid"); + + result.Should().BeEmpty(); + } + + [Test] + public void should_not_throw_if_playlist_tracks_items_is_null() + { + var playlistTracks = new Paging { + Items = null + }; + + Mocker.GetMock(). + Setup(x => x.GetPlaylistTracks(It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(playlistTracks); + + var result = Subject.Fetch(api, "playlistid"); + + result.Should().BeEmpty(); + } + + [Test] + public void should_not_throw_if_playlist_track_is_null() + { + var playlistTracks = new Paging { + Items = new List { + null + } + }; + + Mocker.GetMock(). + Setup(x => x.GetPlaylistTracks(It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(playlistTracks); + + var result = Subject.Fetch(api, "playlistid"); + + result.Should().BeEmpty(); + } + + [Test] + public void should_use_album_artist_when_it_exists() + { + var playlistTracks = new Paging { + Items = new List { + new PlaylistTrack { + Track = new FullTrack { + Album = new SimpleAlbum { + Name = "Album", + Artists = new List { + new SimpleArtist { + Name = "AlbumArtist" + } + } + }, + Artists = new List { + new SimpleArtist { + Name = "TrackArtist" + } + } + } + } + } + }; + + Mocker.GetMock(). + Setup(x => x.GetPlaylistTracks(It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(playlistTracks); + + var result = Subject.Fetch(api, "playlistid"); + + result.Should().HaveCount(1); + result[0].Artist.Should().Be("AlbumArtist"); + } + + [Test] + public void should_fall_back_to_track_artist_if_album_artist_missing() + { + var playlistTracks = new Paging { + Items = new List { + new PlaylistTrack { + Track = new FullTrack { + Album = new SimpleAlbum { + Name = "Album", + Artists = new List { + new SimpleArtist { + Name = null + } + } + }, + Artists = new List { + new SimpleArtist { + Name = "TrackArtist" + } + } + } + } + } + }; + + Mocker.GetMock(). + Setup(x => x.GetPlaylistTracks(It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(playlistTracks); + + var result = Subject.Fetch(api, "playlistid"); + + result.Should().HaveCount(1); + result[0].Artist.Should().Be("TrackArtist"); + } + + + [TestCase(null, null, "Album")] + [TestCase("AlbumArtist", null, null)] + [TestCase(null, "TrackArtist", null)] + public void should_skip_bad_artist_or_album_names(string albumArtistName, string trackArtistName, string albumName) + { + var playlistTracks = new Paging { + Items = new List { + new PlaylistTrack { + Track = new FullTrack { + Album = new SimpleAlbum { + Name = albumName, + Artists = new List { + new SimpleArtist { + Name = albumArtistName + } + } + }, + Artists = new List { + new SimpleArtist { + Name = trackArtistName + } + } + } + } + } + }; + + Mocker.GetMock(). + Setup(x => x.GetPlaylistTracks(It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(playlistTracks); + + var result = Subject.Fetch(api, "playlistid"); + + result.Should().BeEmpty(); + } + + [Test] + public void should_not_throw_if_get_next_page_returns_null() + { + var playlistTracks = new Paging { + Items = new List { + new PlaylistTrack { + Track = new FullTrack { + Album = new SimpleAlbum { + Name = "Album", + Artists = new List { + new SimpleArtist { + Name = null + } + } + }, + Artists = new List { + new SimpleArtist { + Name = "TrackArtist" + } + } + } + } + }, + Next = "DummyToMakeHasNextTrue" + }; + + Mocker.GetMock(). + Setup(x => x.GetPlaylistTracks(It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns(playlistTracks); + + Mocker.GetMock() + .Setup(x => x.GetNextPage(It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Returns(default(Paging)); + + var result = Subject.Fetch(api, "playlistid"); + + result.Should().HaveCount(1); + + Mocker.GetMock() + .Verify(x => x.GetNextPage(It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Once()); + } + } +} diff --git a/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifySavedAlbumsFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifySavedAlbumsFixture.cs new file mode 100644 index 000000000..1444b08bf --- /dev/null +++ b/src/NzbDrone.Core.Test/ImportListTests/Spotify/SpotifySavedAlbumsFixture.cs @@ -0,0 +1,166 @@ +using System.Collections.Generic; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.ImportLists.Spotify; +using NzbDrone.Core.Test.Framework; +using SpotifyAPI.Web; +using SpotifyAPI.Web.Models; + +namespace NzbDrone.Core.Test.ImportListTests +{ + [TestFixture] + public class SpotifySavedAlbumsFixture : CoreTest + { + // placeholder, we don't use real API + private readonly SpotifyWebAPI api = null; + + [Test] + public void should_not_throw_if_saved_albums_is_null() + { + Mocker.GetMock(). + Setup(x => x.GetSavedAlbums(It.IsAny(), + It.IsAny())) + .Returns(default(Paging)); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + } + + [Test] + public void should_not_throw_if_saved_album_items_is_null() + { + var savedAlbums = new Paging { + Items = null + }; + + Mocker.GetMock(). + Setup(x => x.GetSavedAlbums(It.IsAny(), + It.IsAny())) + .Returns(savedAlbums); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + } + + [Test] + public void should_not_throw_if_saved_album_is_null() + { + var savedAlbums = new Paging { + Items = new List { + null + } + }; + + Mocker.GetMock(). + Setup(x => x.GetSavedAlbums(It.IsAny(), + It.IsAny())) + .Returns(savedAlbums); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + } + + [TestCase("Artist", "Album")] + public void should_parse_saved_album(string artistName, string albumName) + { + var savedAlbums = new Paging { + Items = new List { + new SavedAlbum { + Album = new FullAlbum { + Name = albumName, + Artists = new List { + new SimpleArtist { + Name = artistName + } + } + } + } + } + }; + + Mocker.GetMock(). + Setup(x => x.GetSavedAlbums(It.IsAny(), + It.IsAny())) + .Returns(savedAlbums); + + var result = Subject.Fetch(api); + + result.Should().HaveCount(1); + } + + [Test] + public void should_not_throw_if_get_next_page_returns_null() + { + var savedAlbums = new Paging { + Items = new List { + new SavedAlbum { + Album = new FullAlbum { + Name = "Album", + Artists = new List { + new SimpleArtist { + Name = "Artist" + } + } + } + } + }, + Next = "DummyToMakeHasNextTrue" + }; + + Mocker.GetMock(). + Setup(x => x.GetSavedAlbums(It.IsAny(), + It.IsAny())) + .Returns(savedAlbums); + + Mocker.GetMock() + .Setup(x => x.GetNextPage(It.IsAny(), + It.IsAny(), + It.IsAny>())) + .Returns(default(Paging)); + + var result = Subject.Fetch(api); + + result.Should().HaveCount(1); + + Mocker.GetMock() + .Verify(x => x.GetNextPage(It.IsAny(), + It.IsAny(), + It.IsAny>()), + Times.Once()); + } + + [TestCase(null, "Album")] + [TestCase("Artist", null)] + [TestCase(null, null)] + public void should_skip_bad_artist_or_album_names(string artistName, string albumName) + { + var savedAlbums = new Paging { + Items = new List { + new SavedAlbum { + Album = new FullAlbum { + Name = albumName, + Artists = new List { + new SimpleArtist { + Name = artistName + } + } + } + } + } + }; + + Mocker.GetMock(). + Setup(x => x.GetSavedAlbums(It.IsAny(), + It.IsAny())) + .Returns(savedAlbums); + + var result = Subject.Fetch(api); + + result.Should().BeEmpty(); + } + } +} diff --git a/src/NzbDrone.Core/ImportLists/Spotify/SpotifyFollowedArtists.cs b/src/NzbDrone.Core/ImportLists/Spotify/SpotifyFollowedArtists.cs index 112a78594..6a9bdd92a 100644 --- a/src/NzbDrone.Core/ImportLists/Spotify/SpotifyFollowedArtists.cs +++ b/src/NzbDrone.Core/ImportLists/Spotify/SpotifyFollowedArtists.cs @@ -6,7 +6,7 @@ using NzbDrone.Core.Configuration; using NzbDrone.Core.Parser; using NzbDrone.Core.Parser.Model; using SpotifyAPI.Web; -using SpotifyAPI.Web.Enums; +using SpotifyAPI.Web.Models; namespace NzbDrone.Core.ImportLists.Spotify { @@ -17,13 +17,14 @@ namespace NzbDrone.Core.ImportLists.Spotify public class SpotifyFollowedArtists : SpotifyImportListBase { - public SpotifyFollowedArtists(IImportListStatusService importListStatusService, + public SpotifyFollowedArtists(ISpotifyProxy spotifyProxy, + IImportListStatusService importListStatusService, IImportListRepository importListRepository, IConfigService configService, IParsingService parsingService, - HttpClient httpClient, + IHttpClient httpClient, Logger logger) - : base(importListStatusService, importListRepository, configService, parsingService, httpClient, logger) + : base(spotifyProxy, importListStatusService, importListRepository, configService, parsingService, httpClient, logger) { } @@ -32,27 +33,43 @@ namespace NzbDrone.Core.ImportLists.Spotify public override IList Fetch(SpotifyWebAPI api) { var result = new List(); - - var followed = Execute(api, (x) => x.GetFollowedArtists(FollowType.Artist, 50)); - var artists = followed.Artists; + + var followedArtists = _spotifyProxy.GetFollowedArtists(this, api); + var artists = followedArtists?.Artists; + while (true) { + if (artists?.Items == null) + { + return result; + } + foreach (var artist in artists.Items) { - if (artist.Name.IsNotNullOrWhiteSpace()) - { - result.AddIfNotNull(new ImportListItemInfo - { - Artist = artist.Name, - }); - } + result.AddIfNotNull(ParseFullArtist(artist)); } + if (!artists.HasNext()) + { break; - artists = Execute(api, (x) => x.GetNextPage(artists)); + } + + artists = _spotifyProxy.GetNextPage(this, api, artists); } return result; } + + private ImportListItemInfo ParseFullArtist(FullArtist artist) + { + if (artist?.Name.IsNotNullOrWhiteSpace() ?? false) + { + return new ImportListItemInfo { + Artist = artist.Name, + }; + } + + return null; + } } } diff --git a/src/NzbDrone.Core/ImportLists/Spotify/SpotifyImportListBase.cs b/src/NzbDrone.Core/ImportLists/Spotify/SpotifyImportListBase.cs index d9c8c7564..2a1a57853 100644 --- a/src/NzbDrone.Core/ImportLists/Spotify/SpotifyImportListBase.cs +++ b/src/NzbDrone.Core/ImportLists/Spotify/SpotifyImportListBase.cs @@ -20,21 +20,27 @@ namespace NzbDrone.Core.ImportLists.Spotify private IHttpClient _httpClient; private IImportListRepository _importListRepository; - public SpotifyImportListBase(IImportListStatusService importListStatusService, - IImportListRepository importListRepository, - IConfigService configService, - IParsingService parsingService, - HttpClient httpClient, - Logger logger) + protected ISpotifyProxy _spotifyProxy; + + protected SpotifyImportListBase(ISpotifyProxy spotifyProxy, + IImportListStatusService importListStatusService, + IImportListRepository importListRepository, + IConfigService configService, + IParsingService parsingService, + IHttpClient httpClient, + Logger logger) : base(importListStatusService, configService, parsingService, logger) { _httpClient = httpClient; _importListRepository = importListRepository; + _spotifyProxy = spotifyProxy; } public override ImportListType ListType => ImportListType.Spotify; - private void RefreshToken() + public string AccessToken => Settings.AccessToken; + + public void RefreshToken() { _logger.Trace("Refreshing Token"); @@ -68,7 +74,7 @@ namespace NzbDrone.Core.ImportLists.Spotify } - protected SpotifyWebAPI GetApi() + public SpotifyWebAPI GetApi() { Settings.Validate().Filter("AccessToken", "RefreshToken").ThrowOnError(); _logger.Trace($"Access token expires at {Settings.Expires}"); @@ -85,35 +91,6 @@ namespace NzbDrone.Core.ImportLists.Spotify }; } - protected T Execute(SpotifyWebAPI api, Func method, bool allowReauth = true) where T : BasicModel - { - T result = method(api); - if (result.HasError()) - { - // If unauthorized, refresh token and try again - if (result.Error.Status == 401) - { - if (allowReauth) - { - _logger.Debug("Spotify authorization error, refreshing token and retrying"); - RefreshToken(); - api.AccessToken = Settings.AccessToken; - return Execute(api, method, false); - } - else - { - throw new SpotifyAuthorizationException(result.Error.Message); - } - } - else - { - throw new SpotifyException("[{0}] {1}", result.Error.Status, result.Error.Message); - } - } - - return result; - } - public override IList Fetch() { using (var api = GetApi()) @@ -162,7 +139,7 @@ namespace NzbDrone.Core.ImportLists.Spotify { using (var api = GetApi()) { - var profile = Execute(api, (x) => x.GetPrivateProfile()); + var profile = _spotifyProxy.GetPrivateProfile(this, api); _logger.Debug($"Connected to spotify profile {profile.DisplayName} [{profile.Id}]"); return null; } diff --git a/src/NzbDrone.Core/ImportLists/Spotify/SpotifyPlaylist.cs b/src/NzbDrone.Core/ImportLists/Spotify/SpotifyPlaylist.cs index 0af1ac55b..dc8daa267 100644 --- a/src/NzbDrone.Core/ImportLists/Spotify/SpotifyPlaylist.cs +++ b/src/NzbDrone.Core/ImportLists/Spotify/SpotifyPlaylist.cs @@ -15,60 +15,77 @@ namespace NzbDrone.Core.ImportLists.Spotify { public class SpotifyPlaylist : SpotifyImportListBase { - public SpotifyPlaylist(IImportListStatusService importListStatusService, + public SpotifyPlaylist(ISpotifyProxy spotifyProxy, + IImportListStatusService importListStatusService, IImportListRepository importListRepository, IConfigService configService, IParsingService parsingService, - HttpClient httpClient, + IHttpClient httpClient, Logger logger) - : base(importListStatusService, importListRepository, configService, parsingService, httpClient, logger) + : base(spotifyProxy, importListStatusService, importListRepository, configService, parsingService, httpClient, logger) { } public override string Name => "Spotify Playlists"; public override IList Fetch(SpotifyWebAPI api) + { + return Settings.PlaylistIds.SelectMany(x => Fetch(api, x)).ToList(); + } + + public IList Fetch(SpotifyWebAPI api, string playlistId) { var result = new List(); - foreach (var id in Settings.PlaylistIds) - { - _logger.Trace($"Processing playlist {id}"); + _logger.Trace($"Processing playlist {playlistId}"); + + var playlistTracks = _spotifyProxy.GetPlaylistTracks(this, api, playlistId, "next, items(track(name, album(name,artists)))"); - var playlistTracks = Execute(api, (x) => x.GetPlaylistTracks(id, fields: "next, items(track(name, album(name,artists)))")); - while (true) + while (true) + { + if (playlistTracks?.Items == null) { - foreach (var track in playlistTracks.Items) - { - var fullTrack = track.Track; - // From spotify docs: "Note, a track object may be null. This can happen if a track is no longer available." - if (fullTrack != null) - { - var album = fullTrack.Album?.Name; - var artist = fullTrack.Album?.Artists?.FirstOrDefault()?.Name ?? fullTrack.Artists.FirstOrDefault()?.Name; + return result; + } - if (album.IsNotNullOrWhiteSpace() && artist.IsNotNullOrWhiteSpace()) - { - result.AddIfNotNull(new ImportListItemInfo - { - Artist = artist, - Album = album, - ReleaseDate = ParseSpotifyDate(fullTrack.Album.ReleaseDate, fullTrack.Album.ReleaseDatePrecision) - }); - - } - } - } + foreach (var playlistTrack in playlistTracks.Items) + { + result.AddIfNotNull(ParsePlaylistTrack(playlistTrack)); + } - if (!playlistTracks.HasNextPage()) - break; - playlistTracks = Execute(api, (x) => x.GetNextPage(playlistTracks)); + if (!playlistTracks.HasNextPage()) + { + break; } + + playlistTracks = _spotifyProxy.GetNextPage(this, api, playlistTracks); } return result; } + private ImportListItemInfo ParsePlaylistTrack(PlaylistTrack playlistTrack) + { + // From spotify docs: "Note, a track object may be null. This can happen if a track is no longer available." + if (playlistTrack?.Track?.Album != null) + { + var album = playlistTrack.Track.Album; + var albumName = album.Name; + var artistName = album.Artists?.FirstOrDefault()?.Name ?? playlistTrack.Track?.Artists?.FirstOrDefault()?.Name; + + if (albumName.IsNotNullOrWhiteSpace() && artistName.IsNotNullOrWhiteSpace()) + { + return new ImportListItemInfo { + Artist = artistName, + Album = albumName, + ReleaseDate = ParseSpotifyDate(album.ReleaseDate, album.ReleaseDatePrecision) + }; + } + } + + return null; + } + public override object RequestAction(string action, IDictionary query) { if (action == "getPlaylists") @@ -87,18 +104,26 @@ namespace NzbDrone.Core.ImportLists.Spotify { try { - var profile = Execute(api, (x) => x.GetPrivateProfile()); - var playlistPage = Execute(api, (x) => x.GetUserPlaylists(profile.Id)); + var profile = _spotifyProxy.GetPrivateProfile(this, api); + var playlistPage = _spotifyProxy.GetUserPlaylists(this, api, profile.Id); _logger.Trace($"Got {playlistPage.Total} playlists"); var playlists = new List(playlistPage.Total); while (true) { + if (playlistPage == null) + { + break; + } + playlists.AddRange(playlistPage.Items); if (!playlistPage.HasNextPage()) + { break; - playlistPage = Execute(api, (x) => x.GetNextPage(playlistPage)); + } + + playlistPage = _spotifyProxy.GetNextPage(this, api, playlistPage); } return new diff --git a/src/NzbDrone.Core/ImportLists/Spotify/SpotifyProxy.cs b/src/NzbDrone.Core/ImportLists/Spotify/SpotifyProxy.cs new file mode 100644 index 000000000..bcaa5cca9 --- /dev/null +++ b/src/NzbDrone.Core/ImportLists/Spotify/SpotifyProxy.cs @@ -0,0 +1,109 @@ +using System; +using NLog; +using SpotifyAPI.Web; +using SpotifyAPI.Web.Enums; +using SpotifyAPI.Web.Models; + +namespace NzbDrone.Core.ImportLists.Spotify +{ + public interface ISpotifyProxy + { + PrivateProfile GetPrivateProfile(SpotifyImportListBase list, SpotifyWebAPI api) + where TSettings : SpotifySettingsBase, new(); + Paging GetUserPlaylists(SpotifyImportListBase list, SpotifyWebAPI api, string id) + where TSettings : SpotifySettingsBase, new(); + FollowedArtists GetFollowedArtists(SpotifyImportListBase list, SpotifyWebAPI api) + where TSettings : SpotifySettingsBase, new(); + Paging GetSavedAlbums(SpotifyImportListBase list, SpotifyWebAPI api) + where TSettings : SpotifySettingsBase, new(); + Paging GetPlaylistTracks(SpotifyImportListBase list, SpotifyWebAPI api, string id, string fields) + where TSettings : SpotifySettingsBase, new(); + Paging GetNextPage(SpotifyImportListBase list, SpotifyWebAPI api, Paging item) + where TSettings : SpotifySettingsBase, new(); + CursorPaging GetNextPage(SpotifyImportListBase list, SpotifyWebAPI api, CursorPaging item) + where TSettings : SpotifySettingsBase, new(); + } + + public class SpotifyProxy : ISpotifyProxy + { + private readonly Logger _logger; + + public SpotifyProxy(Logger logger) + { + _logger = logger; + } + + public PrivateProfile GetPrivateProfile(SpotifyImportListBase list, SpotifyWebAPI api) + where TSettings : SpotifySettingsBase, new() + { + return Execute(list, api, x => x.GetPrivateProfile()); + } + + public Paging GetUserPlaylists(SpotifyImportListBase list, SpotifyWebAPI api, string id) + where TSettings : SpotifySettingsBase, new() + { + return Execute(list, api, x => x.GetUserPlaylists(id)); + } + + public FollowedArtists GetFollowedArtists(SpotifyImportListBase list, SpotifyWebAPI api) + where TSettings : SpotifySettingsBase, new() + { + return Execute(list, api, x => x.GetFollowedArtists(FollowType.Artist)); + } + + public Paging GetSavedAlbums(SpotifyImportListBase list, SpotifyWebAPI api) + where TSettings : SpotifySettingsBase, new() + { + return Execute(list, api, x => x.GetSavedAlbums(50)); + } + + public Paging GetPlaylistTracks(SpotifyImportListBase list, SpotifyWebAPI api, string id, string fields) + where TSettings : SpotifySettingsBase, new() + { + return Execute(list, api, x => x.GetPlaylistTracks(id, fields: fields)); + } + + public Paging GetNextPage(SpotifyImportListBase list, SpotifyWebAPI api, Paging item) + where TSettings : SpotifySettingsBase, new() + { + return Execute(list, api, (x) => x.GetNextPage(item)); + } + + public CursorPaging GetNextPage(SpotifyImportListBase list, SpotifyWebAPI api, CursorPaging item) + where TSettings : SpotifySettingsBase, new() + { + return Execute(list, api, (x) => x.GetNextPage(item)); + } + + public T Execute(SpotifyImportListBase list, SpotifyWebAPI api, Func method, bool allowReauth = true) + where T : BasicModel + where TSettings : SpotifySettingsBase, new() + { + T result = method(api); + if (result.HasError()) + { + // If unauthorized, refresh token and try again + if (result.Error.Status == 401) + { + if (allowReauth) + { + _logger.Debug("Spotify authorization error, refreshing token and retrying"); + list.RefreshToken(); + api.AccessToken = list.AccessToken; + return Execute(list, api, method, false); + } + else + { + throw new SpotifyAuthorizationException(result.Error.Message); + } + } + else + { + throw new SpotifyException("[{0}] {1}", result.Error.Status, result.Error.Message); + } + } + + return result; + } + } +} diff --git a/src/NzbDrone.Core/ImportLists/Spotify/SpotifySavedAlbums.cs b/src/NzbDrone.Core/ImportLists/Spotify/SpotifySavedAlbums.cs index f0ee40374..bc250da45 100644 --- a/src/NzbDrone.Core/ImportLists/Spotify/SpotifySavedAlbums.cs +++ b/src/NzbDrone.Core/ImportLists/Spotify/SpotifySavedAlbums.cs @@ -7,6 +7,7 @@ using NzbDrone.Core.Configuration; using NzbDrone.Core.Parser; using NzbDrone.Core.Parser.Model; using SpotifyAPI.Web; +using SpotifyAPI.Web.Models; namespace NzbDrone.Core.ImportLists.Spotify { @@ -17,13 +18,14 @@ namespace NzbDrone.Core.ImportLists.Spotify public class SpotifySavedAlbums : SpotifyImportListBase { - public SpotifySavedAlbums(IImportListStatusService importListStatusService, + public SpotifySavedAlbums(ISpotifyProxy spotifyProxy, + IImportListStatusService importListStatusService, IImportListRepository importListRepository, IConfigService configService, IParsingService parsingService, - HttpClient httpClient, + IHttpClient httpClient, Logger logger) - : base(importListStatusService, importListRepository, configService, parsingService, httpClient, logger) + : base(spotifyProxy, importListStatusService, importListRepository, configService, parsingService, httpClient, logger) { } @@ -33,32 +35,49 @@ namespace NzbDrone.Core.ImportLists.Spotify { var result = new List(); - var albums = Execute(api, (x) => x.GetSavedAlbums(50)); - _logger.Trace($"Got {albums.Total} saved albums"); + var savedAlbums = _spotifyProxy.GetSavedAlbums(this, api); + + _logger.Trace($"Got {savedAlbums?.Total ?? 0} saved albums"); while (true) { - foreach (var album in albums.Items) + if (savedAlbums?.Items == null) { - var artistName = album.Album.Artists.FirstOrDefault()?.Name; - var albumName = album.Album.Name; - _logger.Trace($"Adding {artistName} - {albumName}"); + return result; + } - if (artistName.IsNotNullOrWhiteSpace() && albumName.IsNotNullOrWhiteSpace()) - { - result.AddIfNotNull(new ImportListItemInfo - { - Artist = artistName, - Album = albumName - }); - } + foreach (var savedAlbum in savedAlbums.Items) + { + result.AddIfNotNull(ParseSavedAlbum(savedAlbum)); } - if (!albums.HasNextPage()) + + if (!savedAlbums.HasNextPage()) + { break; - albums = Execute(api, (x) => x.GetNextPage(albums)); + } + + savedAlbums = _spotifyProxy.GetNextPage(this, api, savedAlbums); } return result; } + + private ImportListItemInfo ParseSavedAlbum(SavedAlbum savedAlbum) + { + var artistName = savedAlbum?.Album?.Artists?.FirstOrDefault()?.Name; + var albumName = savedAlbum?.Album?.Name; + _logger.Trace($"Adding {artistName} - {albumName}"); + + if (artistName.IsNotNullOrWhiteSpace() && albumName.IsNotNullOrWhiteSpace()) + { + return new ImportListItemInfo { + Artist = artistName, + Album = albumName, + ReleaseDate = ParseSpotifyDate(savedAlbum?.Album?.ReleaseDate, savedAlbum?.Album?.ReleaseDatePrecision) + }; + } + + return null; + } } }