Ringobot / SpotifyApi.NetCore

Lightweight .NET Core wrapper for the Spotify Web API
MIT License
38 stars 16 forks source link

Playlist Tracks returning null values #8

Closed andrewevansumg closed 5 years ago

andrewevansumg commented 5 years ago

Hey Daniel. Wee problem with the playlistsApi

Behaviour When my code invokes playlistsApi.GetTracks("spotify", "37i9dQZF1DX1gcrZ1xC96D");

image

The result contains an array of items, with null values.

image

Debugging I'm trying to understand why this is. However in the spotify documentation I can't find an example for

/users/username/playlists/playlistId/tracks (weird).

https://developer.spotify.com/console/playlists/

return await GetModel<T>($"{BaseUrl}/users/{Uri.EscapeDataString(username)}/playlists/{Uri.EscapeDataString(playlistId)}/tracks");

However given the items array is the same length as the number of tracks in the example playlist, I would guess the response is fine.

Perhaps the issue is in the JsonConvert.DeserializeObject in GetModel, where the response json keys aren't exactly the same as the property names/keys on the TracksSearchResult object??? Just a guess :)

andrewevansumg commented 5 years ago

Okay I'm confused, I've got the project up and running locally. and I've written the following test which passes fine. Is the version published on nuget the same as the code on github??

` public async Task GetTracks_ReturnsValidPlaylistTracks() {

        // Arrange
        const string username = "spotify";
        const string playlistId = "37i9dQZF1DX3WvGXE8FqYX";

        var http = new HttpClient();
        var accounts = new AccountsService(http, TestsHelper.GetLocalConfig());

        var api = new PlaylistsApi(http, accounts);

        // Act
        var response = await api.GetTracks(username, playlistId);

        // Assert
        Assert.IsNotNull(response.Items);
        Assert.IsTrue(response.Items.Length > 0);
        Assert.IsTrue(response.Items[0].Track.Name.Length > 0);
        Assert.IsTrue(response.Items[0].Track.Album.Name.Length > 0);
    }   `

This test

aevansme commented 5 years ago

Apologies, I was using an out of date version. The test is now failing which is good. Using up to date version now to debug the issue. Will update pull request once in a moment

DanielLarsenNZ commented 5 years ago

Hi Andrew! I did a big refactor of the playlists models for v2.3.1. Looks like I have broken something 😟 - really sorry about that. Thanks for the test, did you/can you add that to the PR?

aevansme commented 5 years ago

Hey Daniel. Not a problem. I've found the problem. The response from /user/username/playlist/playlistId/tracks is ever so slightly different and wasn't mapping correctly. I've added a new Search Result class and the test is now passing :)

Here is an example JSON response for that endpoint (pasting as I'm pretty sure it's different then any other class, but only slightly)

image

aevansme commented 5 years ago

... Pull request updated to include fix _b

DanielLarsenNZ commented 5 years ago

Found it - /user/playlist/tracks was deprecated in September! https://developer.spotify.com/community/news/2018/06/12/changes-to-playlist-uris/

And, as per your PR, the playlist tracks responses are different.

So what I think we will do is merge your PR, mark the user + playlistId overload as obsolete (to be removed in v3).

I have to go to pesky work now - ok if I review and merge overnight (your time)?

Good work man! ✋

DanielLarsenNZ commented 5 years ago

Fixed in v2.3.2

dotnet add package SpotifyApi.NetCore --version 2.3.2