Merge branch 'master' into production - v-2019-04-03.0

listenbrainz/domain/spotify.py

@@ -229,6 +229,21 @@ def _make_authorization_headers(client_id, client_secret):
     return r.json()
 
 
+def get_user_dict(user_id):
+    """ Get spotify user details in the form of a dict
+
+    Args:
+        user_id (int): the row ID of the user in ListenBrainz
+    """
+    user = get_user(user_id)
+    if not user:
+        return {}
+    return {
+        'access_token': user.user_token,
+        'permission': user.permission,
+    }
+
+
 class SpotifyImporterException(Exception):
     pass
 

listenbrainz/tests/integration/test_api.py

@@ -66,6 +66,18 @@ def test_get_listens(self):
         # check for latest listen timestamp
         self.assertEqual(data['latest_listen_ts'], ts)
 
+        # request with min_ts should work
+        response = self.client.get(url, query_string = {'min_ts': int(time.time())})
+        self.assert200(response)
+
+        # request with max_ts lesser than the timestamp of the submitted listen
+        # should not send back any listens, should report a good latest_listen timestamp
+        response = self.client.get(url, query_string = {'max_ts': ts - 2})
+        self.assert200(response)
+        self.assertListEqual(response.json['payload']['listens'], [])
+        self.assertEqual(response.json['payload']['latest_listen_ts'], ts)
+
+
         # checkt that recent listens are fectched correctly
         url = url_for('api_v1.get_recent_listens_for_user_list', user_list = self.user['musicbrainz_id'])
         response = self.client.get(url, query_string = {'count': '1'})

listenbrainz/webserver/static/js/jsx/playback-controls.jsx

@@ -57,7 +57,7 @@ export class PlaybackControls extends React.Component {
             </div>
             {this.props.direction !== "hidden" &&
               <div className="right btn btn-xs" onClick={this.props.toggleDirection} title={`Play ${this.props.direction === 'up' ? 'down' : 'up'}`}>
-                <FontAwesomeIcon icon={this.state.direction === 'up' ? faSortAmountUp : faSortAmountDown}/>
+                <FontAwesomeIcon icon={this.props.direction === 'up' ? faSortAmountUp : faSortAmountDown}/>
               </div>
             }
           </div>

listenbrainz/webserver/static/js/jsx/profile.jsx

@@ -32,12 +32,12 @@ class RecentListens extends React.Component {
       listId: props.follow_list_id,
       direction: "down"
     };
-    this.handleSpotifyAccountError = this.handleSpotifyAccountError.bind(this);
     this.connectWebsockets = this.connectWebsockets.bind(this);
     this.getRecentListensForFollowList = this.getRecentListensForFollowList.bind(this);
     this.handleCurrentListenChange = this.handleCurrentListenChange.bind(this);
     this.handleFollowUserListChange = this.handleFollowUserListChange.bind(this);
     this.handleSpotifyAccountError = this.handleSpotifyAccountError.bind(this);
+    this.handleSpotifyPermissionError = this.handleSpotifyPermissionError.bind(this);
     this.isCurrentListen = this.isCurrentListen.bind(this);
     this.newAlert = this.newAlert.bind(this);
     this.onAlertDismissed = this.onAlertDismissed.bind(this);
@@ -108,9 +108,15 @@ class RecentListens extends React.Component {
       }
     })
   }
+
   handleSpotifyAccountError(error){
     this.newAlert("danger","Spotify account error", error);
-    this.setState({isSpotifyPremium: false})
+    this.setState({canPlayMusic: false})
+  }
+
+  handleSpotifyPermissionError(error) {
+    console.error(error);
+    this.setState({canPlayMusic: false});
   }
 
   playListen(listen){
@@ -351,15 +357,16 @@ class RecentListens extends React.Component {
             }
           </div>
           <div className="col-md-4" style={{ position: "-webkit-sticky", position: "sticky", top: 20 }}>
-            {this.props.spotify_access_token && this.state.isSpotifyPremium !== false ?
+            {this.props.spotify.access_token && this.state.canPlayMusic !== false ?
               <SpotifyPlayer
                 APIService={this.APIService}
                 ref={this.spotifyPlayer}
                 listens={spotifyListens}
                 direction={this.state.direction}
-                spotify_access_token={this.props.spotify_access_token}
+                spotify_user={this.props.spotify}
                 onCurrentListenChange={this.handleCurrentListenChange}
                 onAccountError={this.handleSpotifyAccountError}
+                onPermissionError={this.handleSpotifyPermissionError}
                 currentListen={this.state.currentListen}
                 newAlert={this.newAlert}
               /> :

listenbrainz/webserver/static/js/jsx/spotify-player.jsx

@@ -26,13 +26,15 @@ export class SpotifyPlayer extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
-      accessToken: props.spotify_access_token,
+      accessToken: props.spotify_user.access_token,
+      permission: props.spotify_user.permission,
       currentSpotifyTrack: {},
       playerPaused: true,
       progressMs: 0,
       durationMs: 0,
       direction: props.direction || "down"
     };
+
     this.connectSpotifyPlayer = this.connectSpotifyPlayer.bind(this);
     this.disconnectSpotifyPlayer = this.disconnectSpotifyPlayer.bind(this);
     this.getAlbumArt = this.getAlbumArt.bind(this);
@@ -49,12 +51,21 @@ export class SpotifyPlayer extends React.Component {
     this.stopPlayerStateTimer = this.stopPlayerStateTimer.bind(this);
     this.togglePlay = this.togglePlay.bind(this);
     this.toggleDirection = this.toggleDirection.bind(this);
-    window.onSpotifyWebPlaybackSDKReady = this.connectSpotifyPlayer;
-    const spotifyPlayerSDKLib = require('../lib/spotify-player-sdk-1.6.0');
+    // Do an initial check of the spotify token permissions (scopes) before loading the SDK library
+    this.checkSpotifyToken(this.state.accessToken, this.state.permission).then(success => {
+      if(success){
+        window.onSpotifyWebPlaybackSDKReady = this.connectSpotifyPlayer;
+        const spotifyPlayerSDKLib = require('../lib/spotify-player-sdk-1.6.0');
+      }
+    })
     // ONLY FOR TESTING PURPOSES
     window.disconnectSpotifyPlayer = this.disconnectSpotifyPlayer;
   }
 
+  componentWillUnmount() {
+    this.disconnectSpotifyPlayer();
+  }
+
   play_spotify_uri(spotify_uri) {
     if (!this._spotifyPlayer)
     {
@@ -87,6 +98,32 @@ export class SpotifyPlayer extends React.Component {
     .catch(this.handleError);
   };
 
+  async checkSpotifyToken(accessToken, permission){
+
+    if(!accessToken || !permission){
+      this.handleAccountError(noTokenErrorMessage);
+      return false;
+    }
+    try {
+      const scopes = permission.split(" ");
+      const requiredScopes = ["streaming", "user-read-birthdate", "user-read-email", "user-read-private"];
+      for (var i in requiredScopes) {
+        if (!scopes.includes(requiredScopes[i])) {
+          if(typeof this.props.onPermissionError === "function") {
+            this.props.onPermissionError("Permission to play songs not granted");
+          }
+          return false;
+        }
+      }
+      return true;
+
+    } catch (error) {
+      this.handleError(error);
+      return false;
+    }
+
+  }
+
   playListen(listen) {
     if (listen.track_metadata.additional_info.spotify_id)
     {
@@ -157,6 +194,9 @@ export class SpotifyPlayer extends React.Component {
 
   async handleTokenError(error, callbackFunction) {
     console.error(error);
+    if(error && error.message === "Invalid token scopes.") {
+      this.handleAccountError(error.message)
+    }
     try {
       const userToken = await this.props.APIService.refreshSpotifyToken();
       this.setState({accessToken: userToken},()=>{
@@ -169,10 +209,10 @@ export class SpotifyPlayer extends React.Component {
   }
 
   handleAccountError(error) {
-    const errorMessage = 'Failed to validate Spotify account: ';
-    console.error(errorMessage, error);
+    const errorMessage = <p>In order to play music, it is required that you link your Spotify Premium account.<br/>Please try to <a href="/profile/connect-spotify" target="_blank">link for "playing music" feature</a> and refresh this page</p>;
+    console.error('Failed to validate Spotify account', error);
     if(typeof this.props.onAccountError === "function") {
-      this.props.onAccountError(`${errorMessage} ${error}`);
+      this.props.onAccountError(errorMessage);
     }
   }
 
@@ -200,26 +240,21 @@ export class SpotifyPlayer extends React.Component {
     }
     if (typeof this._spotifyPlayer.disconnect === "function")
     {
-      this._spotifyPlayer.disconnect();
       this._spotifyPlayer.removeListener('initialization_error');
       this._spotifyPlayer.removeListener('authentication_error');
       this._spotifyPlayer.removeListener('account_error');
       this._spotifyPlayer.removeListener('playback_error');
       this._spotifyPlayer.removeListener('ready');
       this._spotifyPlayer.removeListener('player_state_changed');
+      this._spotifyPlayer.disconnect();
     }
     this._spotifyPlayer = null;
     this._firstRun = true;
   }
 
   connectSpotifyPlayer(callbackFunction) {
     this.disconnectSpotifyPlayer();
-    if (!this.state.accessToken)
-    {
-      const noTokenErrorMessage = <span> Please try to <a href="/profile/connect-spotify" target="_blank">link your account</a> and refresh this page</span>;
-      this.handleError(noTokenErrorMessage, "No Spotify access token");
-      return;
-    }
+
     this._spotifyPlayer = new window.Spotify.Player({
       name: 'ListenBrainz Player',
       getOAuthToken: authCallback => {
@@ -291,7 +326,7 @@ export class SpotifyPlayer extends React.Component {
   startPlayerStateTimer() {
     this._playerStateTimerID = setInterval(()=>{
       this._spotifyPlayer.getCurrentState().then(this.handlePlayerStateChanged)
-    }, 200);
+    }, 500);
   }
   stopPlayerStateTimer() {
     clearInterval(this._playerStateTimerID);
@@ -301,7 +336,6 @@ export class SpotifyPlayer extends React.Component {
     if(!state) {
       return;
     }
-    console.debug('Spotify player state', state);
     const {
       paused,
       position,
@@ -316,11 +350,11 @@ export class SpotifyPlayer extends React.Component {
     }
     // How do we accurately detect the end of a song?
     // From https://github.com/spotify/web-playback-sdk/issues/35#issuecomment-469834686
-    if (position === 0 && paused === true &&
-      _.has(state, "restrictions.disallow_resuming_reasons") && state.restrictions.disallow_resuming_reasons[0] === "not_paused")
+    if (position === 0 && paused === true)
     {
       // Track finished, play next track
-      console.debug("Detected Spotify end of track, playing next track")
+      console.debug("Detected Spotify end of track, playing next track");
+      console.debug('Spotify player state', state);
       this.debouncedPlayNextTrack();
       return;
     }

listenbrainz/webserver/views/api.py

@@ -77,6 +77,7 @@ def submit_listen():
 
 
 @api_bp.route("/user/<user_name>/listens")
+@crossdomain()
 @ratelimit()
 def get_listens(user_name):
     """
@@ -93,6 +94,7 @@ def get_listens(user_name):
     :resheader Content-Type: *application/json*
     """
 
+    current_time = int(time.time())
     max_ts = _parse_int_arg("max_ts")
     min_ts = _parse_int_arg("min_ts")
 
@@ -103,7 +105,7 @@ def get_listens(user_name):
 
     # If none are given, start with now and go down
     if max_ts == None and min_ts == None:
-        max_ts = int(time.time())
+        max_ts = current_time
 
     db_conn = webserver.create_influx(current_app)
     listens = db_conn.fetch_listens(
@@ -119,7 +121,7 @@ def get_listens(user_name):
     latest_listen = db_conn.fetch_listens(
         user_name,
         limit=1,
-        to_ts=max_ts,
+        to_ts=current_time,
     )
     latest_listen_ts = latest_listen[0].ts_since_epoch if len(latest_listen) > 0 else 0
 
@@ -136,6 +138,7 @@ def get_listens(user_name):
 
 
 @api_bp.route("/user/<user_name>/playing-now")
+@crossdomain()
 @ratelimit()
 def get_playing_now(user_name):
     """

listenbrainz/webserver/views/follow.py

@@ -1,8 +1,9 @@
 import ujson
 from flask import Blueprint, render_template, current_app
 from flask_login import current_user, login_required
+from listenbrainz.domain import spotify
+
 import listenbrainz.db.follow_list as db_follow_list
-import listenbrainz.db.spotify as db_spotify
 
 
 follow_bp = Blueprint("follow", __name__)
@@ -39,12 +40,12 @@ def follow(user_list):
         "name": current_user.musicbrainz_id,
         "auth_token": current_user.auth_token,
     }
-    spotify_access_token = db_spotify.get_token_for_user(current_user.id)
+    spotify_data = spotify.get_user_dict(current_user.id)
     props = {
         "user": user_data,
         "mode": "follow",
         "follow_list": follow_list_members,
-        "spotify_access_token": spotify_access_token,
+        "spotify": spotify_data,
         "web_sockets_server_url": current_app.config["WEBSOCKETS_SERVER_URL"],
         "api_url": current_app.config["API_URL"],
         "save_url": "{}/1/follow/save".format(current_app.config["API_URL"]),

listenbrainz/webserver/views/index.py

@@ -11,9 +11,9 @@
 import locale
 import ujson
 import listenbrainz.db.user as db_user
-import listenbrainz.db.spotify as db_spotify
 from listenbrainz.db.exceptions import DatabaseException
 from listenbrainz import webserver
+from listenbrainz.domain import spotify
 from listenbrainz.webserver import flash
 from listenbrainz.webserver.influx_connection import _influx
 from listenbrainz.webserver.redis_connection import _redis
@@ -141,20 +141,15 @@ def recent_listens():
                 "listened_at_iso": listen.timestamp.isoformat() + "Z",
             })
 
+    spotify_user = {}
     if current_user.is_authenticated:
-        token = db_spotify.get_token_for_user(current_user.id)
-        if token:
-            spotify_access_token = token
-        else:
-            spotify_access_token = ''
-    else:
-        spotify_access_token = ''
+        spotify_user = spotify.get_user_dict(current_user.id)
 
     props = {
-        "listens"              : recent,
-        "mode"                 : "recent",
-        "spotify_access_token" : spotify_access_token,
-        "api_url"               : current_app.config['API_URL'],
+        "listens": recent,
+        "mode": "recent",
+        "spotify": spotify_user,
+        "api_url": current_app.config["API_URL"],
     }
 
     return render_template("index/recent.html",

listenbrainz/webserver/views/profile.py

@@ -280,4 +280,5 @@ def refresh_spotify_token():
         'id': current_user.id,
         'musicbrainz_id': current_user.musicbrainz_id,
         'user_token': spotify_user.user_token,
+        'permission': spotify_user.permission,
     })

listenbrainz/webserver/views/test/test_index.py

@@ -308,4 +308,4 @@ def test_recent_listens_page(self):
         self.assertTemplateUsed('index/recent.html')
         props = ujson.loads(self.get_context_variable('props'))
         self.assertEqual(props['mode'], 'recent')
-        self.assertEqual(props['spotify_access_token'], '')
+        self.assertDictEqual(props['spotify'], {})

listenbrainz/webserver/views/test/test_profile.py

@@ -153,7 +153,7 @@ def test_spotify_refresh_token_which_has_expired(self, mock_refresh_user_token,
             record_listens=True,
             error_message=None,
             latest_listened_at=None,
-            permission='user-read-recently-played',
+            permission='user-read-recently-played some-other-permission',
         )
         r = self.client.post(url_for('profile.refresh_spotify_token'))
         self.assert200(r)
@@ -162,6 +162,7 @@ def test_spotify_refresh_token_which_has_expired(self, mock_refresh_user_token,
             'id': self.user['id'],
             'musicbrainz_id': self.user['musicbrainz_id'],
             'user_token': 'old-token',
+            'permission': 'user-read-recently-played some-other-permission',
         })
 
 
@@ -194,4 +195,5 @@ def test_spotify_refresh_token_which_has_not_expired(self, mock_refresh_user_tok
             'id': self.user['id'],
             'musicbrainz_id': self.user['musicbrainz_id'],
             'user_token': 'new-token',
+            'permission': 'user-read-recently-played',
         })