Albums: Add sorting criteria to tracks listing

include/IAlbum.h

@@ -25,7 +25,7 @@
 #include <vector>
 #include <string>
 
-#include "Types.h"
+#include "IMediaLibrary.h"
 
 class IAlbum
 {
@@ -42,13 +42,13 @@ public:
     /**
      * @brief tracks fetches album tracks from the database
      */
-    virtual std::vector<std::shared_ptr<IMedia>> tracks() const = 0;
+    virtual std::vector<std::shared_ptr<IMedia>> tracks( medialibrary::SortingCriteria sort, bool desc ) const = 0;
     /**
      * @brief tracks fetches album tracks, filtered by genre
      * @param genre A musical genre. Only tracks of this genre will be returned
      * @return
      */
-    virtual std::vector<std::shared_ptr<IMedia>> tracks( GenrePtr genre ) const = 0;
+    virtual std::vector<std::shared_ptr<IMedia>> tracks( GenrePtr genre, medialibrary::SortingCriteria sort, bool desc ) const = 0;
     /**
      * @brief albumArtist Returns the album main artist (generally tagged as album-artist)
      */

src/Album.cpp

@@ -137,25 +137,51 @@ bool Album::setArtworkMrl( const std::string& artworkMrl )
     return true;
 }
 
-std::vector<MediaPtr> Album::tracks() const
+std::string Album::orderBy( medialibrary::SortingCriteria sort, bool desc ) const
+{
+    std::string req = " ORDER BY ";
+    switch ( sort )
+    {
+    case medialibrary::SortingCriteria::Alpha:
+        req += "med.title";
+        break;
+    case medialibrary::SortingCriteria::Duration:
+        req += "med.duration";
+        break;
+    case medialibrary::SortingCriteria::ReleaseDate:
+        req +=  "med.release_date";
+        break;
+    default:
+        req += "att.disc_number, att.track_number";
+        break;
+    }
+
+    if ( desc == true )
+        req += " DESC";
+    return req;
+}
+
+
+std::vector<MediaPtr> Album::tracks( medialibrary::SortingCriteria sort, bool desc ) const
 {
     // This doesn't return the cached version, because it would be fairly complicated, if not impossible or
     // counter productive, to maintain a cache that respects all orderings.
-    static const std::string req = "SELECT med.* FROM " + policy::MediaTable::Name + " med "
-            " INNER JOIN " + policy::AlbumTrackTable::Name + " att ON att.media_id = med.id_media "
-            " WHERE att.album_id = ? AND med.is_present = 1 ORDER BY att.disc_number, att.track_number";
+    std::string req = "SELECT med.* FROM " + policy::MediaTable::Name + " med "
+        " INNER JOIN " + policy::AlbumTrackTable::Name + " att ON att.media_id = med.id_media "
+        " WHERE att.album_id = ? AND med.is_present = 1";
+    req += orderBy( sort, desc );
     return Media::fetchAll<IMedia>( m_ml, req, m_id );
 }
 
-std::vector<std::shared_ptr<IMedia> > Album::tracks( GenrePtr genre ) const
+std::vector<std::shared_ptr<IMedia>> Album::tracks( GenrePtr genre, medialibrary::SortingCriteria sort, bool desc ) const
 {
     if ( genre == nullptr )
         return {};
-    static const std::string req = "SELECT med.* FROM " + policy::MediaTable::Name + " med "
+    std::string req = "SELECT med.* FROM " + policy::MediaTable::Name + " med "
             " INNER JOIN " + policy::AlbumTrackTable::Name + " att ON att.media_id = med.id_media "
             " WHERE att.album_id = ? AND med.is_present = 1"
-            " AND genre_id = ?"
-            " ORDER BY att.disc_number, att.track_number";
+            " AND genre_id = ?";
+    req += orderBy( sort, desc );
     return Media::fetchAll<IMedia>( m_ml, req, m_id, genre->id() );
 }
 
@@ -163,7 +189,7 @@ std::vector<MediaPtr> Album::cachedTracks() const
 {
     auto lock = m_tracks.lock();
     if ( m_tracks.isCached() == false )
-        m_tracks = tracks();
+        m_tracks = tracks( medialibrary::SortingCriteria::Default, false );
     return m_tracks.get();
 }
 

src/Album.h

@@ -72,8 +72,8 @@ class Album : public IAlbum, public DatabaseHelpers<Album, policy::AlbumTable>
         bool setShortSummary( const std::string& summary );
         virtual const std::string& artworkMrl() const override;
         bool setArtworkMrl( const std::string& artworkMrl );
-        virtual std::vector<MediaPtr> tracks() const override;
-        virtual std::vector<std::shared_ptr<IMedia>> tracks( GenrePtr genre ) const override;
+        virtual std::vector<MediaPtr> tracks( medialibrary::SortingCriteria sort, bool desc ) const override;
+        virtual std::vector<std::shared_ptr<IMedia>> tracks( GenrePtr genre, medialibrary::SortingCriteria sort, bool desc ) const override;
         ///
         /// \brief cachedTracks Returns a cached list of tracks
         /// This has no warranty of ordering, validity, or anything else.
@@ -105,6 +105,9 @@ class Album : public IAlbum, public DatabaseHelpers<Album, policy::AlbumTable>
         ///
         static std::vector<AlbumPtr> search( MediaLibraryPtr ml, const std::string& pattern );
 
+    private:
+        std::string orderBy( medialibrary::SortingCriteria sort, bool desc ) const;
+
     protected:
         MediaLibraryPtr m_ml;
         unsigned int m_id;

test/samples/Tester.cpp

@@ -170,7 +170,7 @@ void Tests::checkAlbums( const rapidjson::Value& expectedAlbums, std::vector<Alb
             }
             if ( expectedAlbum.HasMember( "nbTracks" ) || expectedAlbum.HasMember( "tracks" ) )
             {
-                const auto tracks = a->tracks();
+                const auto tracks = a->tracks( medialibrary::SortingCriteria::Default, false );
                 if ( expectedAlbum.HasMember( "nbTracks" ) )
                 {
                     if ( expectedAlbum["nbTracks"].GetUint() != tracks.size() )

test/unittest/AlbumTests.cpp

@@ -65,13 +65,13 @@ TEST_F( Albums, AddTrack )
     f->save();
     ASSERT_NE( track, nullptr );
 
-    auto tracks = a->tracks();
+    auto tracks = a->tracks( medialibrary::SortingCriteria::Default, false );
     ASSERT_EQ( tracks.size(), 1u );
 
     Reload();
 
     a = std::static_pointer_cast<Album>( ml->album( a->id() ) );
-    tracks = a->tracks();
+    tracks = a->tracks( medialibrary::SortingCriteria::Default, false );
     ASSERT_EQ( tracks.size(), 1u );
     ASSERT_EQ( tracks[0]->albumTrack()->trackNumber(), track->trackNumber() );
 }
@@ -86,13 +86,13 @@ TEST_F( Albums, NbTracks )
         f->save();
         ASSERT_NE( track, nullptr );
     }
-    auto tracks = a->tracks();
+    auto tracks = a->tracks( medialibrary::SortingCriteria::Default, false );
     ASSERT_EQ( tracks.size(), a->nbTracks() );
 
     Reload();
 
     a = std::static_pointer_cast<Album>( ml->album( a->id() ) );
-    tracks = a->tracks();
+    tracks = a->tracks( medialibrary::SortingCriteria::Default, false );
     ASSERT_EQ( tracks.size(), a->nbTracks() );
 }
 
@@ -110,13 +110,13 @@ TEST_F( Albums, TracksByGenre )
         if ( i <= 5 )
             track->setGenre( g );
     }
-    auto tracks = a->tracks( g );
+    auto tracks = a->tracks( g, medialibrary::SortingCriteria::Default, false );
     ASSERT_EQ( 5u, tracks.size() );
 
     Reload();
 
     a = std::static_pointer_cast<Album>( ml->album( a->id() ) );
-    tracks = a->tracks( g );
+    tracks = a->tracks( g, medialibrary::SortingCriteria::Default, false );
     ASSERT_NE( tracks.size(), a->nbTracks() );
     ASSERT_EQ( 5u, tracks.size() );
 }
@@ -321,3 +321,30 @@ TEST_F( Albums, AutoDelete )
     album = ml->album( a->id() );
     ASSERT_EQ( nullptr, album );
 }
+
+TEST_F( Albums, SortTracks )
+{
+    auto a = ml->createAlbum( "album" );
+    auto m1 = ml->addFile( "B-track1.mp3" );
+    auto m2 = ml->addFile( "A-track2.mp3" );
+    auto t1 = a->addTrack( m1, 1, 1 );
+    auto t2 = a->addTrack( m2, 2, 1 );
+
+    // Default order is by disc number & track number
+    auto tracks = a->tracks( medialibrary::SortingCriteria::Default, false );
+    ASSERT_EQ( 2u, tracks.size() );
+    ASSERT_EQ( t1->id(), tracks[0]->id() );
+    ASSERT_EQ( t2->id(), tracks[1]->id() );
+
+    // Reverse order
+    tracks = a->tracks( medialibrary::SortingCriteria::Default, true );
+    ASSERT_EQ( 2u, tracks.size() );
+    ASSERT_EQ( t1->id(), tracks[1]->id() );
+    ASSERT_EQ( t2->id(), tracks[0]->id() );
+
+    // Try a media based criteria
+    tracks = a->tracks( medialibrary::SortingCriteria::Alpha, false );
+    ASSERT_EQ( 2u, tracks.size() );
+    ASSERT_EQ( t1->id(), tracks[1]->id() ); // B-track -> first
+    ASSERT_EQ( t2->id(), tracks[0]->id() ); // A-track -> second
+}

test/unittest/AlbumTrackTests.cpp

@@ -88,7 +88,7 @@ TEST_F( AlbumTracks, SetGenre )
     Reload();
 
     a = std::static_pointer_cast<Album>( ml->album( a->id() ) );
-    auto tracks = a->tracks();
+    auto tracks = a->tracks( medialibrary::SortingCriteria::Default, false );
     ASSERT_EQ( tracks.size(), 1u );
     auto t2 = tracks[0];
     ASSERT_EQ( t->genre()->id(), t2->albumTrack()->genre()->id() );

test/unittest/DeviceTests.cpp

@@ -324,7 +324,7 @@ TEST_F( DeviceFs, PartialAlbumRemoval )
     ASSERT_EQ( 1u, albums.size() );
     artists = ml->artists();
     ASSERT_EQ( 1u, artists.size() );
-    ASSERT_EQ( 1u, albums[0]->tracks().size() );
+    ASSERT_EQ( 1u, albums[0]->tracks( medialibrary::SortingCriteria::Default, false ).size() );
     ASSERT_EQ( 1u, artists[0]->media().size() );
 }