From 0352f8d3ff898060148c1d0b588562e3a0a17bfe Mon Sep 17 00:00:00 2001 From: ta264 Date: Sat, 17 Aug 2019 08:04:59 +0100 Subject: [PATCH] Fixed: Faster artist endpoint (#874) * Fixed: Speed up AllArtist API endpoint * New: Display UI before artists have loaded * Add test of new repository methods --- .../src/Activity/Blacklist/BlacklistRow.js | 4 ++ frontend/src/Activity/History/HistoryRow.js | 2 +- frontend/src/AlbumStudio/AlbumStudio.js | 3 +- frontend/src/Artist/Editor/ArtistEditor.js | 3 +- frontend/src/Artist/Index/ArtistIndex.css | 6 +++ frontend/src/Artist/Index/ArtistIndex.js | 5 ++- frontend/src/Calendar/CalendarPage.css | 6 +++ frontend/src/Calendar/CalendarPage.js | 39 ++++++++++------ .../src/Calendar/CalendarPageConnector.js | 3 +- frontend/src/Components/Page/PageConnector.js | 7 --- .../Selectors/createArtistCountSelector.js | 8 +++- .../src/Wanted/CutoffUnmet/CutoffUnmetRow.js | 4 ++ src/Lidarr.Api.V1/Artist/ArtistModule.cs | 8 ++-- src/Lidarr.Api.V1/Artist/ArtistResource.cs | 4 ++ .../AlbumRepositoryFixture.cs | 45 +++++++++++++++++++ .../Datastore/BasicRepository.cs | 2 +- src/NzbDrone.Core/Music/AlbumRepository.cs | 28 ++++++++++++ src/NzbDrone.Core/Music/AlbumService.cs | 12 +++++ 18 files changed, 158 insertions(+), 31 deletions(-) diff --git a/frontend/src/Activity/Blacklist/BlacklistRow.js b/frontend/src/Activity/Blacklist/BlacklistRow.js index 0665122f9..31813a41d 100644 --- a/frontend/src/Activity/Blacklist/BlacklistRow.js +++ b/frontend/src/Activity/Blacklist/BlacklistRow.js @@ -50,6 +50,10 @@ class BlacklistRow extends Component { onRemovePress } = this.props; + if (!artist) { + return null; + } + return ( { diff --git a/frontend/src/Activity/History/HistoryRow.js b/frontend/src/Activity/History/HistoryRow.js index ba438ecb9..62b83ed93 100644 --- a/frontend/src/Activity/History/HistoryRow.js +++ b/frontend/src/Activity/History/HistoryRow.js @@ -67,7 +67,7 @@ class HistoryRow extends Component { onMarkAsFailedPress } = this.props; - if (!album) { + if (!artist || !album) { return null; } diff --git a/frontend/src/AlbumStudio/AlbumStudio.js b/frontend/src/AlbumStudio/AlbumStudio.js index 2506015a6..39222a9e2 100644 --- a/frontend/src/AlbumStudio/AlbumStudio.js +++ b/frontend/src/AlbumStudio/AlbumStudio.js @@ -1,5 +1,6 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; +import getErrorMessage from 'Utilities/Object/getErrorMessage'; import getSelectedIds from 'Utilities/Table/getSelectedIds'; import selectAll from 'Utilities/Table/selectAll'; import toggleSelected from 'Utilities/Table/toggleSelected'; @@ -145,7 +146,7 @@ class AlbumStudio extends Component { { !isFetching && !!error && -
Unable to load the Album Studio
+
{getErrorMessage(error, 'Failed to load artist from API')}
} { diff --git a/frontend/src/Artist/Editor/ArtistEditor.js b/frontend/src/Artist/Editor/ArtistEditor.js index 670afc327..d4f6b282c 100644 --- a/frontend/src/Artist/Editor/ArtistEditor.js +++ b/frontend/src/Artist/Editor/ArtistEditor.js @@ -1,5 +1,6 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; +import getErrorMessage from 'Utilities/Object/getErrorMessage'; import getSelectedIds from 'Utilities/Table/getSelectedIds'; import selectAll from 'Utilities/Table/selectAll'; import toggleSelected from 'Utilities/Table/toggleSelected'; @@ -209,7 +210,7 @@ class ArtistEditor extends Component { { !isFetching && !!error && -
Unable to load the calendar
+
{getErrorMessage(error, 'Failed to load artist from API')}
} { diff --git a/frontend/src/Artist/Index/ArtistIndex.css b/frontend/src/Artist/Index/ArtistIndex.css index ad47ce8c4..43b445c3c 100644 --- a/frontend/src/Artist/Index/ArtistIndex.css +++ b/frontend/src/Artist/Index/ArtistIndex.css @@ -4,6 +4,12 @@ overflow: hidden; } +.errorMessage { + margin-top: 20px; + text-align: center; + font-size: 20px; +} + .contentBody { composes: contentBody from '~Components/Page/PageContentBody.css'; diff --git a/frontend/src/Artist/Index/ArtistIndex.js b/frontend/src/Artist/Index/ArtistIndex.js index b4152a5e9..f88ffda52 100644 --- a/frontend/src/Artist/Index/ArtistIndex.js +++ b/frontend/src/Artist/Index/ArtistIndex.js @@ -2,6 +2,7 @@ import _ from 'lodash'; import PropTypes from 'prop-types'; import React, { Component } from 'react'; import hasDifferentItems from 'Utilities/Object/hasDifferentItems'; +import getErrorMessage from 'Utilities/Object/getErrorMessage'; import { align, icons, sortDirections } from 'Helpers/Props'; import LoadingIndicator from 'Components/Loading/LoadingIndicator'; import PageContent from 'Components/Page/PageContent'; @@ -340,7 +341,9 @@ class ArtistIndex extends Component { { !isFetching && !!error && -
Unable to load artist
+
+ {getErrorMessage(error, 'Failed to load artist from API')} +
} { diff --git a/frontend/src/Calendar/CalendarPage.css b/frontend/src/Calendar/CalendarPage.css index 90ba5c505..b6839c467 100644 --- a/frontend/src/Calendar/CalendarPage.css +++ b/frontend/src/Calendar/CalendarPage.css @@ -12,3 +12,9 @@ flex-grow: 1; width: 100%; } + +.errorMessage { + margin-top: 20px; + text-align: center; + font-size: 20px; +} diff --git a/frontend/src/Calendar/CalendarPage.js b/frontend/src/Calendar/CalendarPage.js index ea795cc52..9dfe3229e 100644 --- a/frontend/src/Calendar/CalendarPage.js +++ b/frontend/src/Calendar/CalendarPage.js @@ -1,5 +1,6 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; +import getErrorMessage from 'Utilities/Object/getErrorMessage'; import { align, icons } from 'Helpers/Props'; import PageContent from 'Components/Page/PageContent'; import Measure from 'Components/Measure'; @@ -75,6 +76,7 @@ class CalendarPage extends Component { selectedFilterKey, filters, hasArtist, + artistError, missingAlbumIds, isSearchingForMissing, useCurrentPage, @@ -131,21 +133,31 @@ class CalendarPage extends Component { className={styles.calendarPageBody} innerClassName={styles.calendarInnerPageBody} > - - { - isMeasured ? - : -
- } - + { + artistError && +
+ {getErrorMessage(artistError, 'Failed to load artist from API')} +
+ } { - hasArtist && + !artistError && + + { + isMeasured ? + : +
+ } + + } + + { + hasArtist && !!artistError && } @@ -169,6 +181,7 @@ CalendarPage.propTypes = { selectedFilterKey: PropTypes.string.isRequired, filters: PropTypes.arrayOf(PropTypes.object).isRequired, hasArtist: PropTypes.bool.isRequired, + artistError: PropTypes.object, missingAlbumIds: PropTypes.arrayOf(PropTypes.number).isRequired, isSearchingForMissing: PropTypes.bool.isRequired, useCurrentPage: PropTypes.bool.isRequired, diff --git a/frontend/src/Calendar/CalendarPageConnector.js b/frontend/src/Calendar/CalendarPageConnector.js index 655275b00..db0f827c1 100644 --- a/frontend/src/Calendar/CalendarPageConnector.js +++ b/frontend/src/Calendar/CalendarPageConnector.js @@ -72,7 +72,8 @@ function createMapStateToProps() { selectedFilterKey, filters, colorImpairedMode: uiSettings.enableColorImpairedMode, - hasArtist: !!artistCount, + hasArtist: !!artistCount.count, + artistError: artistCount.error, missingAlbumIds, isSearchingForMissing }; diff --git a/frontend/src/Components/Page/PageConnector.js b/frontend/src/Components/Page/PageConnector.js index 57836a749..7d0dded6b 100644 --- a/frontend/src/Components/Page/PageConnector.js +++ b/frontend/src/Components/Page/PageConnector.js @@ -43,7 +43,6 @@ const selectAppProps = createSelector( ); const selectIsPopulated = createSelector( - (state) => state.artist.isPopulated, (state) => state.customFilters.isPopulated, (state) => state.tags.isPopulated, (state) => state.settings.ui.isPopulated, @@ -52,7 +51,6 @@ const selectIsPopulated = createSelector( (state) => state.settings.importLists.isPopulated, (state) => state.system.status.isPopulated, ( - artistIsPopulated, customFiltersIsPopulated, tagsIsPopulated, uiSettingsIsPopulated, @@ -62,7 +60,6 @@ const selectIsPopulated = createSelector( systemStatusIsPopulated ) => { return ( - artistIsPopulated && customFiltersIsPopulated && tagsIsPopulated && uiSettingsIsPopulated && @@ -75,7 +72,6 @@ const selectIsPopulated = createSelector( ); const selectErrors = createSelector( - (state) => state.artist.error, (state) => state.customFilters.error, (state) => state.tags.error, (state) => state.settings.ui.error, @@ -84,7 +80,6 @@ const selectErrors = createSelector( (state) => state.settings.importLists.error, (state) => state.system.status.error, ( - artistError, customFiltersError, tagsError, uiSettingsError, @@ -94,7 +89,6 @@ const selectErrors = createSelector( systemStatusError ) => { const hasError = !!( - artistError || customFiltersError || tagsError || uiSettingsError || @@ -106,7 +100,6 @@ const selectErrors = createSelector( return { hasError, - artistError, customFiltersError, tagsError, uiSettingsError, diff --git a/frontend/src/Store/Selectors/createArtistCountSelector.js b/frontend/src/Store/Selectors/createArtistCountSelector.js index 5920b4099..203a5cb94 100644 --- a/frontend/src/Store/Selectors/createArtistCountSelector.js +++ b/frontend/src/Store/Selectors/createArtistCountSelector.js @@ -4,8 +4,12 @@ import createAllArtistSelector from './createAllArtistSelector'; function createArtistCountSelector() { return createSelector( createAllArtistSelector(), - (artists) => { - return artists.length; + (state) => state.artist.error, + (artists, error) => { + return { + count: artists.length, + error + }; } ); } diff --git a/frontend/src/Wanted/CutoffUnmet/CutoffUnmetRow.js b/frontend/src/Wanted/CutoffUnmet/CutoffUnmetRow.js index 34e1d501f..6cf592fa6 100644 --- a/frontend/src/Wanted/CutoffUnmet/CutoffUnmetRow.js +++ b/frontend/src/Wanted/CutoffUnmet/CutoffUnmetRow.js @@ -26,6 +26,10 @@ function CutoffUnmetRow(props) { onSelectedChange } = props; + if (!artist) { + return null; + } + return ( x.ArtistMetadataId)); + var lastAlbums = _albumService.GetLastAlbumsByArtistMetadataId(artists.Select(x => x.ArtistMetadataId)); + foreach (var artistResource in artists) { - var artistAlbums = _albumService.GetAlbumsByArtist(artistResource.Id).OrderBy(s=>s.ReleaseDate); - artistResource.NextAlbum = artistAlbums.Where(s => s.ReleaseDate >= DateTime.UtcNow && s.Monitored).FirstOrDefault(); - artistResource.LastAlbum = artistAlbums.Where(s => s.ReleaseDate <= DateTime.UtcNow && s.Monitored).LastOrDefault(); + artistResource.NextAlbum = nextAlbums.FirstOrDefault(x => x.ArtistMetadataId == artistResource.ArtistMetadataId); + artistResource.LastAlbum = lastAlbums.FirstOrDefault(x => x.ArtistMetadataId == artistResource.ArtistMetadataId); } } diff --git a/src/Lidarr.Api.V1/Artist/ArtistResource.cs b/src/Lidarr.Api.V1/Artist/ArtistResource.cs index 264beb981..8b4981f20 100644 --- a/src/Lidarr.Api.V1/Artist/ArtistResource.cs +++ b/src/Lidarr.Api.V1/Artist/ArtistResource.cs @@ -5,6 +5,7 @@ using NzbDrone.Common.Extensions; using NzbDrone.Core.MediaCover; using NzbDrone.Core.Music; using Lidarr.Http.REST; +using Newtonsoft.Json; namespace Lidarr.Api.V1.Artist { @@ -14,6 +15,8 @@ namespace Lidarr.Api.V1.Artist //Todo: Is there an easy way to keep IgnoreArticlesWhenSorting in sync between, Series, History, Missing? //Todo: We should get the entire Profile instead of ID and Name separately + [JsonIgnore] + public int ArtistMetadataId { get; set; } public ArtistStatusType Status { get; set; } public bool Ended => Status == ArtistStatusType.Ended; @@ -70,6 +73,7 @@ namespace Lidarr.Api.V1.Artist return new ArtistResource { Id = model.Id, + ArtistMetadataId = model.ArtistMetadataId, ArtistName = model.Name, //AlternateTitles diff --git a/src/NzbDrone.Core.Test/MusicTests/AlbumRepositoryTests/AlbumRepositoryFixture.cs b/src/NzbDrone.Core.Test/MusicTests/AlbumRepositoryTests/AlbumRepositoryFixture.cs index f904346c8..66e82f0bf 100644 --- a/src/NzbDrone.Core.Test/MusicTests/AlbumRepositoryTests/AlbumRepositoryFixture.cs +++ b/src/NzbDrone.Core.Test/MusicTests/AlbumRepositoryTests/AlbumRepositoryFixture.cs @@ -3,7 +3,9 @@ using FluentAssertions; using NUnit.Framework; using NzbDrone.Core.Music; using NzbDrone.Core.Test.Framework; +using System; using System.Collections.Generic; +using System.Linq; namespace NzbDrone.Core.Test.MusicTests.AlbumRepositoryTests { @@ -13,6 +15,7 @@ namespace NzbDrone.Core.Test.MusicTests.AlbumRepositoryTests private Artist _artist; private Album _album; private Album _albumSpecial; + private List _albums; private AlbumRelease _release; private AlbumRepository _albumRepo; private ReleaseRepository _releaseRepo; @@ -150,5 +153,47 @@ namespace NzbDrone.Core.Test.MusicTests.AlbumRepositoryTests album.Should().BeNull(); } + + private void GivenMultipleAlbums() + { + _albums = Builder.CreateListOfSize(4) + .All() + .With(x => x.Id = 0) + .With(x => x.Artist = _artist) + .With(x => x.ArtistMetadataId = _artist.ArtistMetadataId) + .TheFirst(1) + // next + .With(x => x.ReleaseDate = DateTime.UtcNow.AddDays(1)) + .TheNext(1) + // another future one + .With(x => x.ReleaseDate = DateTime.UtcNow.AddDays(2)) + .TheNext(1) + // most recent + .With(x => x.ReleaseDate = DateTime.UtcNow.AddDays(-1)) + .TheNext(1) + // an older one + .With(x => x.ReleaseDate = DateTime.UtcNow.AddDays(-2)) + .BuildList(); + + _albumRepo.InsertMany(_albums); + } + + [Test] + public void get_next_albums_should_return_next_album() + { + GivenMultipleAlbums(); + + var result = _albumRepo.GetNextAlbums(new [] { _artist.ArtistMetadataId }); + result.Should().BeEquivalentTo(_albums.Take(1)); + } + + [Test] + public void get_last_albums_should_return_next_album() + { + GivenMultipleAlbums(); + + var result = _albumRepo.GetLastAlbums(new [] { _artist.ArtistMetadataId }); + result.Should().BeEquivalentTo(_albums.Skip(2).Take(1)); + } } } diff --git a/src/NzbDrone.Core/Datastore/BasicRepository.cs b/src/NzbDrone.Core/Datastore/BasicRepository.cs index f94aec2a0..3c823b531 100644 --- a/src/NzbDrone.Core/Datastore/BasicRepository.cs +++ b/src/NzbDrone.Core/Datastore/BasicRepository.cs @@ -58,7 +58,7 @@ namespace NzbDrone.Core.Datastore public IEnumerable All() { - return DataMapper.Query().ToList(); + return Query.ToList(); } public int Count() diff --git a/src/NzbDrone.Core/Music/AlbumRepository.cs b/src/NzbDrone.Core/Music/AlbumRepository.cs index 6cb2f43d5..252af816a 100644 --- a/src/NzbDrone.Core/Music/AlbumRepository.cs +++ b/src/NzbDrone.Core/Music/AlbumRepository.cs @@ -14,6 +14,8 @@ namespace NzbDrone.Core.Music public interface IAlbumRepository : IBasicRepository { List GetAlbums(int artistId); + List GetLastAlbums(IEnumerable artistMetadataIds); + List GetNextAlbums(IEnumerable artistMetadataIds); List GetAlbumsByArtistMetadataId(int artistMetadataId); List GetAlbumsForRefresh(int artistId, IEnumerable foreignIds); Album FindByTitle(int artistMetadataId, string title); @@ -47,6 +49,32 @@ namespace NzbDrone.Core.Music .Where(a => a.Id == artistId).ToList(); } + public List GetLastAlbums(IEnumerable artistMetadataIds) + { + string query = string.Format("SELECT Albums.* " + + "FROM Albums " + + "WHERE Albums.ArtistMetadataId IN ({0}) " + + "AND Albums.ReleaseDate < datetime('now') " + + "GROUP BY Albums.ArtistMetadataId " + + "HAVING Albums.ReleaseDate = MAX(Albums.ReleaseDate)", + string.Join(", ", artistMetadataIds)); + + return Query.QueryText(query); + } + + public List GetNextAlbums(IEnumerable artistMetadataIds) + { + string query = string.Format("SELECT Albums.* " + + "FROM Albums " + + "WHERE Albums.ArtistMetadataId IN ({0}) " + + "AND Albums.ReleaseDate > datetime('now') " + + "GROUP BY Albums.ArtistMetadataId " + + "HAVING Albums.ReleaseDate = MIN(Albums.ReleaseDate)", + string.Join(", ", artistMetadataIds)); + + return Query.QueryText(query); + } + public List GetAlbumsByArtistMetadataId(int artistMetadataId) { return Query.Where(s => s.ArtistMetadataId == artistMetadataId); diff --git a/src/NzbDrone.Core/Music/AlbumService.cs b/src/NzbDrone.Core/Music/AlbumService.cs index 394bfc1e6..6165b1c98 100644 --- a/src/NzbDrone.Core/Music/AlbumService.cs +++ b/src/NzbDrone.Core/Music/AlbumService.cs @@ -15,6 +15,8 @@ namespace NzbDrone.Core.Music Album GetAlbum(int albumId); List GetAlbums(IEnumerable albumIds); List GetAlbumsByArtist(int artistId); + List GetNextAlbumsByArtistMetadataId(IEnumerable artistMetadataIds); + List GetLastAlbumsByArtistMetadataId(IEnumerable artistMetadataIds); List GetAlbumsByArtistMetadataId(int artistMetadataId); List GetAlbumsForRefresh(int artistMetadataId, IEnumerable foreignIds); Album AddAlbum(Album newAlbum); @@ -170,6 +172,16 @@ namespace NzbDrone.Core.Music return _albumRepository.GetAlbums(artistId).ToList(); } + public List GetNextAlbumsByArtistMetadataId(IEnumerable artistMetadataIds) + { + return _albumRepository.GetNextAlbums(artistMetadataIds).ToList(); + } + + public List GetLastAlbumsByArtistMetadataId(IEnumerable artistMetadataIds) + { + return _albumRepository.GetLastAlbums(artistMetadataIds).ToList(); + } + public List GetAlbumsByArtistMetadataId(int artistMetadataId) { return _albumRepository.GetAlbumsByArtistMetadataId(artistMetadataId).ToList();