From 3dfbd76a8359c7ab6c2ed7b856daa1a6c950fb3f Mon Sep 17 00:00:00 2001 From: John Cass Date: Tue, 19 Apr 2016 17:15:05 +0200 Subject: [PATCH] * Retrieve album cover and artist images using MusicBrianz ID, if available. * Retrieve album cover and artist images using MusicBrianz ID, if available. --- README.rst | 2 + .../static/js/functionsvars.js | 2 +- mopidy_musicbox_webclient/static/js/gui.js | 4 +- mopidy_musicbox_webclient/static/js/images.js | 260 ++++++-- .../static/js/process_ws.js | 8 +- .../static/js/synced_timer.js | 1 - tests/js/test_images.js | 575 +++++++++++++++--- 7 files changed, 705 insertions(+), 147 deletions(-) diff --git a/README.rst b/README.rst index 5d3a973..70de863 100644 --- a/README.rst +++ b/README.rst @@ -78,11 +78,13 @@ v2.3.0 (UNRELEASED) - Now remembers which backend was searched previously, and automatically selects that backend as the default search target. (Addresses: `#130 `_). - Upgrade Media Progress Timer to version 3.0.0. +- Now retrieves album cover and artist images using MusicBrainzID, if available. **Fixes** - Don't create Mopidy models manually. (Fixes: `#172 `_). - Context menu is now available for all tracks in browse pane. (Fixes: `#126 `_). +- last.fm artist image lookups should now always return the correct image for similarly named artists. v2.2.0 (2016-03-01) ------------------- diff --git a/mopidy_musicbox_webclient/static/js/functionsvars.js b/mopidy_musicbox_webclient/static/js/functionsvars.js index ae1634a..ea4ff05 100644 --- a/mopidy_musicbox_webclient/static/js/functionsvars.js +++ b/mopidy_musicbox_webclient/static/js/functionsvars.js @@ -269,7 +269,7 @@ function renderSongLiDivider (track, nextTrack, currentIndex, target) { renderSongLiTrackArtists(track) + '

' ) // Retrieve album covers - coverArt.getCover(track.uri, getjQueryID(target + '-cover', track.uri, true), 'small', mopidy) + images.setAlbumImage(track.uri, getjQueryID(target + '-cover', track.uri, true), mopidy, 'small') } else if (currentIndex > 0) { // Small divider $(target).before('
  •  
    • ') diff --git a/mopidy_musicbox_webclient/static/js/gui.js b/mopidy_musicbox_webclient/static/js/gui.js index 932ee1a..abe94d5 100644 --- a/mopidy_musicbox_webclient/static/js/gui.js +++ b/mopidy_musicbox_webclient/static/js/gui.js @@ -128,12 +128,10 @@ function setSongInfo (data) { } if (data.track.album && data.track.album.name) { $('#modalalbum').html('' + data.track.album.name + '') - coverArt.getCover(data.track.uri, '#infocover, #controlspopupimage', 'extralarge', mopidy) } else { $('#modalalbum').html('') - $('#infocover').attr('src', 'images/default_cover.png') - $('#controlspopupimage').attr('src', 'images/default_cover.png') } + images.setAlbumImage(data.track.uri, '#infocover, #controlspopupimage', mopidy) $('#modalartist').html(arttmp) diff --git a/mopidy_musicbox_webclient/static/js/images.js b/mopidy_musicbox_webclient/static/js/images.js index 8bef0c3..19f0c28 100644 --- a/mopidy_musicbox_webclient/static/js/images.js +++ b/mopidy_musicbox_webclient/static/js/images.js @@ -4,7 +4,7 @@ } else if (typeof module === 'object' && module.exports) { module.exports = factory() } else { - root.coverArt = factory() + root.images = factory() } }(this, function () { 'use strict' @@ -12,97 +12,271 @@ var API_KEY = 'b6d34c3af91d62ab0ae00ab1b6fa8733' var API_SECRET = '2c631802c2285d5d5d1502462fe42a2b' - var coverArt = { - lastfm: new LastFM({ + var images = { + + DEFAULT_ALBUM_URL: 'images/default_cover.png', + DEFAULT_ARTIST_URL: 'images/user_24x32.png', + + lastFM: new LastFM({ apiKey: API_KEY, apiSecret: API_SECRET, cache: new LastFMCache() }), - getCover: function (uri, images, size, mopidy) { - var defUrl = 'images/default_cover.png' - $(images).attr('src', defUrl) + /* Extract artist information from Mopidy track. */ + _getArtistInfo: function (track) { + var artistName = '' + var musicBrainzID = '' + + if (track && track.artists && (track.artists.length > 0)) { + // First look for the artist info in the track + artistName = track.artists[0].name + musicBrainzID = track.artists[0].musicbrainz_id + } + + if ((!artistName || !musicBrainzID) && (track && track.album && track.album.artists && track.album.artists.length > 0)) { + // Fallback to using artist info contained in the track's album + artistName = artistName || track.album.artists[0].name + musicBrainzID = musicBrainzID || track.album.artists[0].musicbrainz_id + } + + return {mbid: musicBrainzID, name: artistName} + }, + + /* Utility function for retrieving artist informaton for the given track from last.fm */ + _getLastFmArtistInfo: function (track) { + var artist = images._getArtistInfo(track) + var artistPromise = $.Deferred() + + if (!(track && (track.musicbrainz_id || (track.name && artist && artist.name)))) { + // Avoid expensive last.fm call if tag information is missing. + return artistPromise.reject('none', 'Not enough tag information available for track to make last.fm call.') + } + + var params = {} + // Only add arguments to parameter object if values are available for them. + if (track.musicbrainz_id) { + params.mbid = track.musicbrainz_id + } + if (track.name && artist.name) { + params.track = track.name + params.artist = artist.name + } + + images.lastFM.track.getInfo(params, {success: function (data) { + artistPromise.resolve(data.track.artist) + }, error: function (code, message) { + artistPromise.reject(code, message) + }}) + + return artistPromise + }, + + /* Utility function for retrieving information for the given track from last.fm. */ + _getLastFmAlbumInfo: function (track) { + var artist = images._getArtistInfo(track) + var albumPromise = $.Deferred() + + if (!(track && track.album && (track.album.musicbrainz_id || (track.album.name && artist && artist.name)))) { + // Avoid expensive last.fm call if tag information is missing. + return albumPromise.reject('none', 'Not enough tag information available for album to make last.fm call.') + } + + var musicBrainzID = track.album.musicbrainz_id + var albumName = track.album.name + var artistName = images._getArtistInfo(track).name + + var params = {} + // Only add arguments to parameter object if values are available for them. + if (musicBrainzID) { + params.mbid = musicBrainzID + } + if (artistName && albumName) { + params.artist = artistName + params.album = albumName + } + + images.lastFM.album.getInfo(params, {success: function (data) { + albumPromise.resolve(data) + }, error: function (code, message) { + albumPromise.reject(code, message) + }}) + + return albumPromise + }, + + /** + * Sets an HTML image element to contain the album cover art of the relevant Mopidy track. + * + * Potential sources for the album image will be interrogated in the following order until + * a suitable image URI is found: + * 1.) mopidy.library.getImages + * 2.) mopidy.models.Track.album.images (DEPRECATED) + * 3.) last.fm using the album MusicBrainz ID + * 4.) last.fm using the album name and track artist name + * 5.) last.fm using the album name and album artist name + * 6.) a default image + * + * @param {string} uri - The URI of the Mopidy track to retrieve the album cover image for. + * @param {string} img_element - The identifier of the HTML image element that will be used + * to render the image. + * @param {object} mopidy - The Mopidy.js object that should be used to communicate with the + * Mopidy server. + * @param {string} size - (Optional) The preferred size of the image. This parameter is only + * used in the last.fm lookups if Mopidy does not provide the image + * directly. Can be one of 'small', 'medium', 'large', + * 'extralarge' (default), or 'mega'. + */ + setAlbumImage: function (uri, img_element, mopidy, size) { + // Set default immediately while we're busy retrieving actual image. + $(img_element).attr('src', images.DEFAULT_ALBUM_URL) if (!uri) { return } + size = size || 'extralarge' + mopidy.library.getImages({'uris': [uri]}).then(function (imageResults) { var uri = Object.keys(imageResults)[0] if (imageResults[uri].length > 0) { - $(images).attr('src', imageResults[uri][0].uri) + $(img_element).attr('src', imageResults[uri][0].uri) } else { // Also check deprecated 'album.images' in case backend does not // implement mopidy.library.getImages yet... - coverArt.getCoverFromAlbum(uri, images, size, mopidy) + images._setDeprecatedAlbumImage(uri, img_element, mopidy, size) } }) }, // Note that this approach has been deprecated in Mopidy - // TODO: Remove when Mopidy no longer supports getting images - // with 'album.images'. - getCoverFromAlbum: function (uri, images, size, mopidy) { - var defUrl = 'images/default_cover.png' - $(images).attr('src', defUrl) + // TODO: Remove when Mopidy no longer supports retrieving images + // from 'album.images'. + /* Set album image using mopidy.album.images. */ + _setDeprecatedAlbumImage: function (uri, img_element, mopidy, size) { if (!uri) { + $(img_element).attr('src', images.DEFAULT_ALBUM_URL) return } + size = size || 'extralarge' + mopidy.library.lookup({'uris': [uri]}).then(function (resultDict) { var uri = Object.keys(resultDict)[0] var track = resultDict[uri][0] if (track && track.album && track.album.images && track.album.images.length > 0) { - $(images).attr('src', track.album.images[0]) - } else if (track && (track.album || track.artist)) { - // Fallback to last.fm - coverArt.getCoverFromLastFm(track, images, size) + $(img_element).attr('src', track.album.images[0]) } else { - return + // Fallback to last.fm + images._setLastFmAlbumImage(track, img_element, size) } }) }, - getCoverFromLastFm: function (track, images, size) { - var defUrl = 'images/default_cover.png' - $(images).attr('src', defUrl) + /* Lookup album image on last.fm using the provided Mopidy track. */ + _setLastFmAlbumImage: function (track, img_element, size) { if (!track || !(track.album || track.artists)) { + $(img_element).attr('src', images.DEFAULT_ALBUM_URL) return } - var albumname = (track.album && track.album.name) ? track.album.name : '' - var artistname = '' - if (track.album && track.album.artists && track.album.artists.length > 0) { - // First look for the artist in the album - artistname = track.album.artists[0].name - } else if (track.artists && (track.artists.length > 0)) { - // Fallback to using artists for specific track - artistname = track.artists[0].name - } + size = size || 'extralarge' - this.lastfm.album.getInfo({artist: artistname, album: albumname}, {success: function (data) { + images._getLastFmAlbumInfo(track).then(function (data) { for (var i = 0; i < data.album.image.length; i++) { if (data.album.image[i].size === size) { - $(images).attr('src', data.album.image[i]['#text'] || defUrl) + $(img_element).attr('src', data.album.image[i]['#text'] || images.DEFAULT_ALBUM_URL) + break } } - }, error: function (code, message) { - console.error('Error retrieving album info from last.fm', code, message) - }}) + }, function (code, message) { + $(img_element).attr('src', images.DEFAULT_ALBUM_URL) + console.error('Error getting album info from last.fm (%s: %s)', code, message) + }) }, - getArtistImage: function (artist, images, size) { - var defUrl = 'images/user_24x32.png' - $(images).attr('src', defUrl) - if (!artist || artist.length === 0) { + /** + * Sets an HTML image element to contain the artist image of the relevant Mopidy track. + * + * Potential sources of the artist image will be interrogated in the following order until + * a suitable image URI is found: + * 1.) mopidy.library.getImages + * 2.) last.fm using the artist MusicBrainz ID. If no artist ID is provided, it will be + * looked up on last.fm first using the track and album details. + * 3.) a default image + * + * @param {string} artist_uri - The URI of the Mopidy artist to retrieve the image for. + * @param {string} track_uri - The URI of the Mopidy track that will be used as a fallback + * if the artist URI does not provide any image results. + * @param {string} img_element - The identifier of the HTML image element that will be used + * to render the image. + * @param {object} mopidy - The Mopidy.js object that should be used to communicate with the + * Mopidy server. + * @param {string} size - (Optional) The preferred size of the image. This parameter is only + * used in the last.fm lookups if Mopidy does not provide the image + * directly. Can be one of 'small', 'medium', 'large', + * 'extralarge' (default), or 'mega'. + */ + setArtistImage: function (artist_uri, track_uri, img_element, mopidy, size) { + // Set default immediately while we're busy retrieving actual image. + $(img_element).attr('src', images.DEFAULT_ARTIST_URL) + if (!artist_uri && !track_uri) { return } - this.lastfm.artist.getInfo({artist: artist}, {success: function (data) { + size = size || 'extralarge' + + if (artist_uri) { + // Use artist as starting point for retrieving image. + mopidy.library.getImages({'uris': [artist_uri]}).then(function (imageResults) { + var uri = Object.keys(imageResults)[0] + if (imageResults[uri].length > 0) { + $(img_element).attr('src', imageResults[uri][0].uri) + } else { + // Fall back to using track as starting point for retrieving image. + images._setArtistImageFromTrack(track_uri, img_element, mopidy, size) + } + }) + } + }, + + /* Set artist image using the supplied Mopidy track URI. */ + _setArtistImageFromTrack: function (uri, img_element, mopidy, size) { + mopidy.library.lookup({'uris': [uri]}).then(function (resultDict) { + var uri = Object.keys(resultDict)[0] + var track = resultDict[uri][0] + var artist = images._getArtistInfo(track) + + if (artist.mbid) { + images._setLastFmArtistImage(artist.mbid, img_element, size) + } else { + // Look up unique MusicBrainz ID for artist first using the available track information + images._getLastFmArtistInfo(track).then(function (artist) { + images._setLastFmArtistImage(artist.mbid, img_element, size) + }, function (code, message) { + $(img_element).attr('src', images.DEFAULT_ARTIST_URL) + console.error('Error retrieving artist info from last.fm. (%s: %s)', code, message) + }) + } + }) + }, + + /* Set artist image using the supplied artist MusicBrainz ID. */ + _setLastFmArtistImage: function (mbid, img_element, size) { + if (!mbid) { + // Avoid expensive last.fm call if tag information is missing. + $(img_element).attr('src', images.DEFAULT_ARTIST_URL) + return + } + size = size || 'extralarge' + + images.lastFM.artist.getInfo({mbid: mbid}, {success: function (data) { for (var i = 0; i < data.artist.image.length; i++) { if (data.artist.image[i].size === size) { - $(images).attr('src', data.artist.image[i]['#text'] || defUrl) + $(img_element).attr('src', data.artist.image[i]['#text'] || images.DEFAULT_ARTIST_URL) + break } } }, error: function (code, message) { - console.error('Error retrieving artist info from last.fm', code, message) + $(img_element).attr('src', images.DEFAULT_ARTIST_URL) + console.error('Error retrieving artist info from last.fm. (%s: %s)', code, message) }}) } } - return coverArt + return images })) diff --git a/mopidy_musicbox_webclient/static/js/process_ws.js b/mopidy_musicbox_webclient/static/js/process_ws.js index a3ccba3..0b752c2 100644 --- a/mopidy_musicbox_webclient/static/js/process_ws.js +++ b/mopidy_musicbox_webclient/static/js/process_ws.js @@ -220,7 +220,7 @@ function processCurrentPlaylist (resultArr) { function processArtistResults (resultArr) { if (!resultArr || (resultArr.length === 0)) { $('#h_artistname').text('Artist not found...') - coverArt.getCover('', '#artistviewimage, #artistpopupimage', 'extralarge', mopidy) + images.setAlbumImage('', '#artistviewimage, #artistpopupimage', mopidy) showLoading(false) return } @@ -229,7 +229,7 @@ function processArtistResults (resultArr) { resultsToTables(resultArr, ARTIST_TABLE, resultArr.uri) var artistname = getArtist(resultArr) $('#h_artistname, #artistpopupname').html(artistname) - getArtistImage(artistname, '#artistviewimage, #artistpopupimage', 'extralarge') + images.setArtistImage(resultArr.uri, resultArr[0].uri, '#artistviewimage, #artistpopupimage', mopidy) showLoading(false) } @@ -239,7 +239,7 @@ function processArtistResults (resultArr) { function processAlbumResults (resultArr) { if (!resultArr || (resultArr.length === 0)) { $('#h_albumname').text('Album not found...') - coverArt.getCover('', '#albumviewcover, #coverpopupimage', 'extralarge', mopidy) + images.setAlbumImage('', '#albumviewcover, #coverpopupimage', mopidy) showLoading(false) return } @@ -252,6 +252,6 @@ function processAlbumResults (resultArr) { $('#h_albumartist').html(artistname) $('#coverpopupalbumname').html(albumname) $('#coverpopupartist').html(artistname) - coverArt.getCover(resultArr[0].uri, '#albumviewcover, #coverpopupimage', 'extralarge') + images.setAlbumImage(resultArr[0].uri, '#albumviewcover, #coverpopupimage', mopidy) showLoading(false) } diff --git a/mopidy_musicbox_webclient/static/js/synced_timer.js b/mopidy_musicbox_webclient/static/js/synced_timer.js index e66b0eb..aaa6c04 100644 --- a/mopidy_musicbox_webclient/static/js/synced_timer.js +++ b/mopidy_musicbox_webclient/static/js/synced_timer.js @@ -189,7 +189,6 @@ } SyncedProgressTimer.prototype.reset = function () { - // this._progressTimer.reset() this.stop() this.set(0, Infinity) diff --git a/tests/js/test_images.js b/tests/js/test_images.js index 4cd6ab8..11af74c 100644 --- a/tests/js/test_images.js +++ b/tests/js/test_images.js @@ -6,173 +6,558 @@ chai.use(require('chai-jquery')) var sinon = require('sinon') -var coverArt = require('../../mopidy_musicbox_webclient/static/js/images.js') +var images = require('../../mopidy_musicbox_webclient/static/js/images.js') -describe('CoverArt', function () { +describe('images', function () { var mopidy - var images - beforeEach(function () { + var img_element + before(function () { mopidy = sinon.stub(new Mopidy({callingConvention: 'by-position-or-by-name'})) - images = $('') - $(images).removeAttr('src') + img_element = $('') }) - describe('#getCover()', function () { - it('should use default image if no track URI is provided', function () { - coverArt.getCover('', images, '', mopidy) - expect($(images).prop('src')).to.endWith('images/default_cover.png') + beforeEach(function () { + $(img_element).removeAttr('src') + }) + + describe('#_getArtistInfo()', function () { + it('should get artist info from track', function () { + var track = { + artists: [{name: 'trackArtistMock', musicbrainz_id: 'trackArtistIDMock'}], + album: {artists: [{name: 'albumArtistMock', musicbrainz_id: 'albumArtistIDMock'}]} + } + + var artist = images._getArtistInfo(track) + assert.equal(artist.name, 'trackArtistMock') + assert.equal(artist.mbid, 'trackArtistIDMock') }) - it('should get image from Mopidy, if available', function () { - var getImagesResultMock = {'mock:track:uri': [{uri: 'mockImageUri'}]} - var library = { getImages: function () { return $.when(getImagesResultMock) } } - mopidy.library = library + it('should fall back to using album artist if track artist is not available', function () { + var track = { + album: {artists: [{name: 'albumArtistMock', musicbrainz_id: 'albumArtistIDMock'}]} + } - var getImagesSpy = sinon.spy(mopidy.library, 'getImages') - coverArt.getCover('mock:track:uri', images, '', mopidy) - - assert.isTrue(getImagesSpy.calledOnce) - expect($(images).prop('src')).to.endWith('mockImageUri') + var artist = images._getArtistInfo(track) + assert.equal(artist.name, 'albumArtistMock') + assert.equal(artist.mbid, 'albumArtistIDMock') }) - it('should fall back to retrieving image from deprecated track.album.images', function () { - var getImagesResultMock = {'mock:track:uri': []} - var lookupResultMock = {'mock:track:uri': [{album: {images: ['mockAlbumImageUri']}}]} - var library = { + it('should use any combination of artist info from tracks and albums', function () { + var track = { + artists: [{name: 'trackArtistMock'}], + album: {artists: [{name: 'albumArtistMock', musicbrainz_id: 'albumArtistIDMock'}]} + } + + var artist = images._getArtistInfo(track) + assert.equal(artist.name, 'trackArtistMock') + assert.equal(artist.mbid, 'albumArtistIDMock') + }) + }) + + describe('#_getLastFmArtistInfo', function () { + var getInfoStub + + before(function () { + getInfoStub = sinon.stub(images.lastFM.track, 'getInfo') + getInfoStub.yieldsTo('success', {track: {artist: {mbid: 'mockArtistID', name: 'mockArtistName'}}}) + }) + afterEach(function () { + getInfoStub.reset() + }) + after(function () { + getInfoStub.restore() + }) + + it('should retrieve artist info from last.fm using MusicBrainz ID', function () { + var track = { + musicbrainz_id: 'trackIDMock' + } + + images._getLastFmArtistInfo(track).then(function (artist) { + assert.equal(artist.mbid, 'mockArtistID') + assert.equal(artist.name, 'mockArtistName') + }, function (code, message) { + assert.fail(code, '', 'Async method call did not resolve as expected') + }) + assert(getInfoStub.calledWith({mbid: 'trackIDMock'})) + }) + + it('should retrieve artist info from last.fm using track and artist name', function () { + var track = { + name: 'trackNameMock', + artists: [{name: 'trackArtistMock'}] + } + + images._getLastFmArtistInfo(track).then(function (artist) { + assert.equal(artist.mbid, 'mockArtistID') + assert.equal(artist.name, 'mockArtistName') + }, function (code, message) { + assert.fail(code, '', 'Async method call did not resolve as expected') + }) + assert(getInfoStub.calledWith({track: 'trackNameMock', artist: 'trackArtistMock'})) + }) + + it('should raise error if neither track MusicBrainz ID OR track AND album names are available', function () { + var track = { + name: 'trackNameMock' + } + + images._getLastFmArtistInfo(track).then(function (data) { + assert.fail(data, undefined, 'Async method call with just track name did not reject as expected') + }, function (code, message) { + assert.equal(code, 'none') + assert.equal(message, 'Not enough tag information available for track to make last.fm call.') + }) + + track = { + artists: [{name: 'trackArtistMock'}] + } + + images._getLastFmArtistInfo(track).then(function (data) { + assert.fail(data, undefined, 'Async method call with just artist name did not reject as expected') + }, function (code, message) { + assert.equal(code, 'none') + assert.equal(message, 'Not enough tag information available for track to make last.fm call.') + }) + }) + + it('should re-raise last.fm errors', function () { + var track = { + musicbrainz_id: 'trackIDMock' + } + + getInfoStub.yieldsTo('error', 'code', 'message') + + images._getLastFmArtistInfo(track).then(function (data) { + assert.fail(data, undefined, 'Async method call did not re-raise reject as expected') + }, function (code, message) { + assert.equal(code, 'code') + assert.equal(message, 'message') + }) + }) + }) + + describe('#_getLastFmAlbumInfo', function () { + var getInfoStub + + before(function () { + getInfoStub = sinon.stub(images.lastFM.album, 'getInfo') + getInfoStub.yieldsTo('success', {album: {image: [{size: 'extralarge', '#text': 'mockURL'}]}}) + }) + afterEach(function () { + getInfoStub.reset() + }) + after(function () { + getInfoStub.restore() + }) + + it('should retrieve album info from last.fm using MusicBrainz ID', function () { + var track = { + album: {musicbrainz_id: 'albumIDMock'} + } + + images._getLastFmAlbumInfo(track).then(function (data) { + assert.equal(data.album.image[0].size, 'extralarge') + assert.equal(data.album.image[0]['#text'], 'mockURL') + }, function (code, message) { + assert.fail(code, '', 'Async method call did not resolve as expected') + }) + assert(getInfoStub.calledWith({mbid: 'albumIDMock'})) + }) + + it('should retrieve album info from last.fm using album name and either the track or artist name', function () { + var track = { + album: { + name: 'albumNameMock', + artists: [{name: 'albumArtistMock'}] + }, + artists: [{name: 'trackArtistMock'}] + } + + images._getLastFmAlbumInfo(track).then(function (data) { + assert.equal(data.album.image[0].size, 'extralarge') + assert.equal(data.album.image[0]['#text'], 'mockURL') + }, function (code, message) { + assert.fail(code, '', 'Async method call did not resolve as expected') + }) + assert(getInfoStub.calledWith({artist: 'trackArtistMock', album: 'albumNameMock'})) + }) + + it('should raise error if neither album MusicBrainz ID OR album AND artist names are available', function () { + var track = { + album: {} + } + + images._getLastFmAlbumInfo(track).then(function (data) { + assert.fail(data, undefined, 'Async method call with just track name did not reject as expected') + }, function (code, message) { + assert.equal(code, 'none') + assert.equal(message, 'Not enough tag information available for album to make last.fm call.') + }) + + track = { + artists: [{name: 'trackArtistMock'}] + } + + images._getLastFmAlbumInfo(track).then(function (data) { + assert.fail(data, undefined, 'Async method call with just artist name did not reject as expected') + }, function (code, message) { + assert.equal(code, 'none') + assert.equal(message, 'Not enough tag information available for album to make last.fm call.') + }) + }) + + it('should re-raise last.fm errors', function () { + var track = { + album: {musicbrainz_id: 'albumIDMock'} + } + + getInfoStub.yieldsTo('error', 'code', 'message') + + images._getLastFmAlbumInfo(track).then(function (data) { + assert.fail(data, undefined, 'Async method call did not re-raise reject as expected') + }, function (code, message) { + assert.equal(code, 'code') + assert.equal(message, 'message') + }) + }) + }) + + describe('#setAlbumImage()', function () { + var getImagesResultMock + var lookupResultMock + var library + var getImagesSpy + var setDeprecatedAlbumImageSpy + + before(function () { + library = { getImages: function () { return $.when(getImagesResultMock) }, lookup: function () { return $.when(lookupResultMock) } } mopidy.library = library - var getImagesSpy = sinon.spy(mopidy.library, 'getImages') - var getCoverFromAlbumSpy = sinon.spy(coverArt, 'getCoverFromAlbum') + getImagesSpy = sinon.spy(mopidy.library, 'getImages') + setDeprecatedAlbumImageSpy = sinon.spy(images, '_setDeprecatedAlbumImage') + }) + afterEach(function () { + getImagesSpy.reset() + setDeprecatedAlbumImageSpy.reset() + }) - coverArt.getCover('mock:track:uri', images, '', mopidy) + it('should use default image if no track URI is provided', function () { + images.setAlbumImage('', img_element, mopidy) + expect($(img_element).prop('src')).to.endWith(images.DEFAULT_ALBUM_URL) + }) + + it('should get image from Mopidy, if available', function () { + getImagesResultMock = {'mock:track:uri': [{uri: 'mockImageUri'}]} + + images.setAlbumImage('mock:track:uri', img_element, mopidy) assert.isTrue(getImagesSpy.calledOnce) - assert.isTrue(getCoverFromAlbumSpy.calledOnce) + expect($(img_element).prop('src')).to.endWith('mockImageUri') + }) + + it('should fall back to retrieving image from deprecated track.album.images', function () { + getImagesResultMock = {'mock:track:uri': []} + lookupResultMock = {'mock:track:uri': [{album: {images: ['mockAlbumImageUri']}}]} + + images.setAlbumImage('mock:track:uri', img_element, mopidy) + + assert.isTrue(getImagesSpy.calledOnce) + assert.isTrue(setDeprecatedAlbumImageSpy.calledOnce) + }) + + it('should default to retrieving "extralarge" album image', function () { + getImagesResultMock = {'mock:track:uri': []} + lookupResultMock = {'mock:track:uri': [{album: {images: ['mockAlbumImageUri']}}]} + + images.setAlbumImage('mock:track:uri', img_element, mopidy) + + expect(setDeprecatedAlbumImageSpy.args[0]).to.include('extralarge') }) }) - describe('#getCoverFromAlbum()', function () { - it('should use default image if no track URI is provided', function () { - coverArt.getCoverFromAlbum('', images, '', mopidy) - expect($(images).prop('src')).to.endWith('images/default_cover.png') - }) + describe('#_setDeprecatedAlbumImage()', function () { + var lookupResultMock + var library - it('should get image from Mopidy track.album.images, if available', function () { - var lookupResultMock = {'mock:track:uri': [{album: {images: ['mockAlbumImageUri']}}]} - var library = { + before(function () { + library = { lookup: function () { return $.when(lookupResultMock) } } mopidy.library = library + }) + + it('should use default image if no track URI is provided', function () { + images._setDeprecatedAlbumImage('', img_element, mopidy) + expect($(img_element).prop('src')).to.endWith(images.DEFAULT_ALBUM_URL) + }) + + it('should get image from Mopidy track.album.img_element, if available', function () { + lookupResultMock = {'mock:track:uri': [{album: {images: ['mockAlbumImageUri']}}]} var lookupSpy = sinon.spy(mopidy.library, 'lookup') - coverArt.getCoverFromAlbum('mock:track:uri', images, '', mopidy) + images._setDeprecatedAlbumImage('mock:track:uri', img_element, mopidy) assert.isTrue(lookupSpy.calledOnce) - expect($(images).prop('src')).to.endWith('mockAlbumImageUri') + expect($(img_element).prop('src')).to.endWith('mockAlbumImageUri') + lookupSpy.restore() }) it('should use default image if track.album or track.artist is not available', function () { - var lookupResultMock = {'mock:track:uri': []} - var library = { - lookup: function () { return $.when(lookupResultMock) } - } - mopidy.library = library + lookupResultMock = {'mock:track:uri': [{}]} var lookupSpy = sinon.spy(mopidy.library, 'lookup') - coverArt.getCoverFromAlbum('mock:track:uri', images, '', mopidy) + images._setDeprecatedAlbumImage('mock:track:uri', img_element, mopidy) assert.isTrue(lookupSpy.calledOnce) - expect($(images).prop('src')).to.endWith('images/default_cover.png') + expect($(img_element).prop('src')).to.endWith(images.DEFAULT_ALBUM_URL) + lookupSpy.restore() }) it('should fall back to retrieving image from last.fm if none provided by Mopidy', function () { - var lookupResultMock = {'mock:track:uri': [{album: {images: []}}]} - var library = { - lookup: function () { return $.when(lookupResultMock) } - } - mopidy.library = library + lookupResultMock = {'mock:track:uri': [{album: {images: []}}]} - var getCoverFromLastFmStub = sinon.stub(coverArt, 'getCoverFromLastFm') - coverArt.getCoverFromAlbum('mock:track:uri', images, '', mopidy) + var setLastFmAlbumImageStub = sinon.stub(images, '_setLastFmAlbumImage') + images._setDeprecatedAlbumImage('mock:track:uri', img_element, mopidy) - assert.isTrue(getCoverFromLastFmStub.calledOnce) - getCoverFromLastFmStub.restore() + assert.isTrue(setLastFmAlbumImageStub.calledOnce) + setLastFmAlbumImageStub.restore() + }) + + it('should default to retrieving "extralarge" album image', function () { + lookupResultMock = {'mock:track:uri': [{album: {}}]} + + var setLastFmAlbumImageStub = sinon.stub(images, '_setLastFmAlbumImage') + images._setDeprecatedAlbumImage('mock:track:uri', img_element, mopidy) + + expect(setLastFmAlbumImageStub.args[0]).to.include('extralarge') + setLastFmAlbumImageStub.restore() }) }) - describe('#getCoverFromLastFm()', function () { - it('should use default image if no track is provided', function () { - coverArt.getCoverFromLastFm(undefined, images, '') - expect($(images).prop('src')).to.endWith('images/default_cover.png') + describe('#_setLastFmAlbumImage()', function () { + var getInfoResultMock + var getInfoStub + + before(function () { + getInfoStub = sinon.stub(images, '_getLastFmAlbumInfo') + }) + beforeEach(function () { + getInfoStub.reset() + }) + after(function () { + getInfoStub.restore() }) - it('should fall back to using track artist if album artist is not available', function () { - var track = {artists: [{name: 'artistMock'}]} - var getInfoResultMock = {album: {image: []}} + it('should use default image if track album or track artists are not available', function () { + images._setLastFmAlbumImage({}, img_element) + expect($(img_element).prop('src')).to.endWith(images.DEFAULT_ALBUM_URL) + }) + + it('should use correct size for setting album image', function () { + var track = {album: {name: 'albumMock', artists: [{name: 'artistMock'}]}} + getInfoResultMock = {album: {image: [ + {'#text': 'mockAlbumSmallImageUri', size: 'small'}, + {'#text': 'mockAlbumMedImageUri', size: 'medium'}, + {'#text': 'mockAlbumLargeImageUri', size: 'large'} + ]}} - var getInfoStub = sinon.stub(coverArt.lastfm.album, 'getInfo') getInfoStub.returns($.when(getInfoResultMock)) - coverArt.getCoverFromLastFm(track, images, '') - var args = getInfoStub.args - assert.equal(args[0][0].artist, 'artistMock') - getInfoStub.restore() + images._setLastFmAlbumImage(track, img_element, 'medium') + expect($(img_element).prop('src')).to.endWith('mockAlbumMedImageUri') }) - it('should get album info from last.fm', function () { - var track = {album: {artists: [{name: 'albumMock'}]}} - var getInfoResultMock = {album: {image: [{'#text': 'mockAlbumImageUri', size: 'small'}]}} + it('should default to "extralarge" if no image size is specified', function () { + var track = {album: {name: 'albumMock', artists: [{name: 'artistMock'}]}} + getInfoResultMock = {album: {image: [ + {'#text': 'mockAlbumSmallImageUri', size: 'small'}, + {'#text': 'mockAlbumMedImageUri', size: 'medium'}, + {'#text': 'mockAlbumXLargeImageUri', size: 'extralarge'}, + {'#text': 'mockAlbumLargeImageUri', size: 'large'} + ]}} - var getInfoStub = sinon.stub(coverArt.lastfm.album, 'getInfo') - getInfoStub.yieldsTo('success', getInfoResultMock) + getInfoStub.returns($.when(getInfoResultMock)) - coverArt.getCoverFromLastFm(track, images, 'small') - expect($(images).prop('src')).to.endWith('mockAlbumImageUri') - getInfoStub.restore() + images._setLastFmAlbumImage(track, img_element) + expect($(img_element).prop('src')).to.endWith('mockAlbumXLargeImageUri') }) - it('should log errors', function () { - var track = {album: {artists: [{name: 'albumMock'}]}} - var getInfoStub = sinon.stub(coverArt.lastfm.album, 'getInfo') - getInfoStub.yieldsTo('error', 'code', 'message') + it('should log last.fm errors', function () { + var track = {album: {name: 'albumMock', artists: [{name: 'artistMock'}]}} + + getInfoStub.returns($.Deferred().reject('code', 'message')) var consoleStub = sinon.stub(console, 'error') - coverArt.getCoverFromLastFm(track, images, '') + images._setLastFmAlbumImage(track, img_element) - assert.isTrue(consoleStub.calledOnce) - getInfoStub.restore() + assert.isTrue(consoleStub.calledWith('Error getting album info from last.fm (%s: %s)', 'code', 'message')) consoleStub.restore() }) }) - describe('#getArtistImage()', function () { - it('should use default image if no artist is provided', function () { - coverArt.getArtistImage('', images, '') - expect($(images).prop('src')).to.endWith('images/user_24x32.png') + describe('#setArtistImage()', function () { + var getImagesResultMock + var library + var getImagesSpy + + before(function () { + library = { + getImages: function () { return $.when(getImagesResultMock) } + } + mopidy.library = library + getImagesSpy = sinon.spy(mopidy.library, 'getImages') + }) + afterEach(function () { + getImagesSpy.reset() + }) + after(function () { + getImagesSpy.restore() }) - it('should get artist info from last.fm', function () { - var getInfoResultMock = {artist: {image: [{'#text': 'mockArtistImageUri', size: 'small'}]}} + it('should use default image if no artist URI is provided', function () { + images.setArtistImage('', '', img_element, mopidy) + expect($(img_element).prop('src')).to.endWith(images.DEFAULT_ARTIST_URL) + }) - var getInfoStub = sinon.stub(coverArt.lastfm.artist, 'getInfo') - getInfoStub.yieldsTo('success', getInfoResultMock) + it('should get artist image from Mopidy, if available', function () { + getImagesResultMock = {'mock:artist:uri': [{uri: 'mockImageUri'}]} - coverArt.getArtistImage('mockArtist', images, 'small') - expect($(images).prop('src')).to.endWith('mockArtistImageUri') + var setArtistImageFromTrackStub = sinon.stub(images, '_setArtistImageFromTrack') + images.setArtistImage('mock:artist:uri', 'mock:track:uri', img_element, mopidy) + + assert.isTrue(getImagesSpy.calledOnce) + expect($(img_element).prop('src')).to.endWith('mockImageUri') + assert.isFalse(setArtistImageFromTrackStub.called) + + setArtistImageFromTrackStub.restore() + }) + + it('should fall back to retrieving artist image from last.fm', function () { + getImagesResultMock = {'mock:artist:uri': []} + + var setArtistImageFromTrackStub = sinon.stub(images, '_setArtistImageFromTrack') + images.setArtistImage('mock:artist:uri', 'mock:track:uri', img_element, mopidy) + + assert.isTrue(getImagesSpy.calledOnce) + assert.isTrue(setArtistImageFromTrackStub.calledOnce) + + setArtistImageFromTrackStub.restore() + }) + }) + + describe('#_setArtistImageFromTrack()', function () { + var lookupResultMock + var library + var getInfoStub + + before(function () { + library = { + lookup: function () { return $.when(lookupResultMock) } + } + mopidy.library = library + getInfoStub = sinon.stub(images, '_getLastFmArtistInfo') + }) + afterEach(function () { + getInfoStub.reset() + }) + after(function () { getInfoStub.restore() }) - it('should log errors', function () { - var getInfoStub = sinon.stub(coverArt.lastfm.artist, 'getInfo') - getInfoStub.yieldsTo('error', 'code', 'message') + it('should set artist image from last.fm using available Mopidy track information', function () { + lookupResultMock = {'mock:track:uri': [{album: {artists: [{name: 'artistMock'}]}}]} + + var getInfoResultMock = {mbid: 'artistIDLookupMock', name: 'artistNameMock'} + getInfoStub.returns($.when(getInfoResultMock)) + var setLastFmArtistImageStub = sinon.stub(images, '_setLastFmArtistImage') + + images._setArtistImageFromTrack('mock:track:uri', img_element, mopidy, 'small') + + assert(getInfoStub.called) + assert(setLastFmArtistImageStub.calledWithMatch('artistIDLookupMock')) + setLastFmArtistImageStub.restore() + }) + + it('should set artist info from last.fm using MusicBrainz ID, if available', function () { + lookupResultMock = {'mock:track:uri': [{album: {artists: [{musicbrainz_id: 'artistIDMock'}]}}]} + + var setLastFmArtistImageStub = sinon.stub(images, '_setLastFmArtistImage') + + images._setArtistImageFromTrack('mock:track:uri', img_element, mopidy, 'small') + + assert(setLastFmArtistImageStub.calledWithMatch('artistIDMock')) + setLastFmArtistImageStub.restore() + }) + + it('should log last.fm errors', function () { + lookupResultMock = {'mock:track:uri': [{album: {artists: [{name: 'artistMock'}]}}]} + + getInfoStub.returns($.Deferred().reject('code', 'message')) var consoleStub = sinon.stub(console, 'error') - coverArt.getArtistImage('mockArtist', images, 'small') - assert.isTrue(consoleStub.calledOnce) + images._setArtistImageFromTrack('mock:track:uri', img_element, mopidy, 'small') + assert.isTrue(consoleStub.calledWith('Error retrieving artist info from last.fm. (%s: %s)', 'code', 'message')) + consoleStub.restore() + }) + }) + + describe('#_setLastFmArtistImage()', function () { + var getInfoResultMock + var getInfoStub + + before(function () { + getInfoStub = sinon.stub(images.lastFM.artist, 'getInfo') + }) + beforeEach(function () { + getInfoStub.reset() + }) + after(function () { getInfoStub.restore() + }) + + it('should use default image if artist MusicBrainz ID is not available', function () { + images._setLastFmArtistImage('', img_element) + expect($(img_element).prop('src')).to.endWith(images.DEFAULT_ARTIST_URL) + }) + + it('should use correct size for setting album image', function () { + getInfoResultMock = {artist: {image: [ + {'#text': 'mockAlbumSmallImageUri', size: 'small'}, + {'#text': 'mockAlbumMedImageUri', size: 'medium'}, + {'#text': 'mockAlbumLargeImageUri', size: 'large'} + ]}} + + getInfoStub.yieldsTo('success', getInfoResultMock) + + images._setLastFmArtistImage('artistIDMock', img_element, 'medium') + expect($(img_element).prop('src')).to.endWith('mockAlbumMedImageUri') + }) + + it('should default to "extralarge" if no image size is specified', function () { + getInfoResultMock = {artist: {image: [ + {'#text': 'mockAlbumSmallImageUri', size: 'small'}, + {'#text': 'mockAlbumMedImageUri', size: 'medium'}, + {'#text': 'mockAlbumXLargeImageUri', size: 'extralarge'}, + {'#text': 'mockAlbumLargeImageUri', size: 'large'} + ]}} + + getInfoStub.yieldsTo('success', getInfoResultMock) + + images._setLastFmArtistImage('artistIDMock', img_element) + expect($(img_element).prop('src')).to.endWith('mockAlbumXLargeImageUri') + }) + + it('should log last.fm errors', function () { + getInfoStub.yieldsTo('error', 'code', 'message') + + var consoleStub = sinon.stub(console, 'error') + images._setLastFmArtistImage('artistIDMock', img_element) + + assert.isTrue(consoleStub.calledWith('Error retrieving artist info from last.fm. (%s: %s)', 'code', 'message')) consoleStub.restore() }) })