mkrum / Jamocracy

Jamocracy - a web service that allows parties to interact with a spotify playlist through text
http://www.jamocracy.xyz
6 stars 4 forks source link

Refreshing access tokens #11

Open DanielKerrigan opened 8 years ago

DanielKerrigan commented 8 years ago

If I add a song, wait an hour or so without using jamocracy, and then try to remove the song, it will not work due to an expired access token. Before we interact with a playlist, we should make sure that the access token is not expired.

In playlist_services,js, we should be calling SpotifyService.refreshAccessToken() in both addSongToPlaylist() and removeSong(). I don't think we need it in getSong().

Also, we should see if there is a way to check if an access token needs to be refreshed so that we can avoid refreshing it every time a song is added/removed.

samcal commented 8 years ago

Is there a reason not to refresh the token every time? Then we would only need to store the refresh token in the database. Sounds like a simpler way to ensure that we always have a valid access token while making calls to Spotify.

DanielKerrigan commented 8 years ago

Other than saving a call to the spotify api, no. I was hoping it would just be a simple if-statement, but it looks like it would be more complicated than that, so I am fine with refreshing every time.

DanielKerrigan commented 8 years ago

How do these functions look for the playlist service? The SMS route would require a few minor changes.

function getSong(text) {
    return SpotifyService.searchTracks(text);
}

function addSongToPlaylist(song, playlist, number, partyCode) {
    updateSong(number, song.uri);
    return SpotifyService.refreshAccessToken()
        .then(token => {
            playlist.access_token = token;
            DBService.update('parties', partyCode, 'access_token', token);
            DBService.increment('songs', song.name, 'playCount', 1)
                .then(() => {
                    DBService.append('songs', song.name, 'numbers', number);
                }, () => {
                    DBService.update('songs', song.name, {
                        'playCount': 1,
                        'numbers': [number]
                    });
                });
            return SpotifyService.addSongToPlaylist(song, playlist);
        });
}

function removeSong(song, playlist, number, partyCode) {
    updateSong(number, 'null');
    return SpotifyService.refreshAccessToken()
        .then(token => {
            playlist.access_token = token;
            DBService.update('parties', partyCode, 'access_token', token);
            return SpotifyService.removeSong(song, playlist);
        });
}
samcal commented 8 years ago

Yeah, looks like a good start, would solve the expired token problem. Are the Spotify tokens still being set in the route? It seems like token management should only be within PlaylistService to me.

Another approach is to rename refreshAccessToken() to withRefreshedToken(refresh_token), that returns a promise containing the new tokens. This would be a combination of setTokens and refreshAccessToken

function addSongToPlaylist(song, playlist, number, partyCode) {
    updateSong(number, song.uri);
    return SpotifyService.withRefreshedToken(playlist.refresh_token)
        .then(token => {
            // do some stuff with the token
        });
}
DanielKerrigan commented 8 years ago

Spotify tokens are being set in the Spotify API Service. The SMS route doesn't do anything with tokens.

samcal commented 8 years ago

Oh alright, that possible new version of the PlaylistService just confused me. Which module should be setting the access tokens?