Bläddra i källkod

refactor: less verbose next/previous song queries

Fela Maslen 5 år sedan
förälder
incheckning
1778e24fe2

+ 1 - 0
gmus-backend/pkg/read/types.go

@@ -1,6 +1,7 @@
 package read
 
 type Song struct {
+	Id           int    `db:"id"`
 	TrackNumber  int    `db:"track_number"`
 	Title        string `db:"title"`
 	Artist       string `db:"artist"`

+ 97 - 181
gmus-backend/pkg/repository/player.go

@@ -3,202 +3,118 @@ package repository
 import (
 	"database/sql"
 
+	"github.com/felamaslen/gmus-backend/pkg/read"
 	"github.com/jmoiron/sqlx"
 )
 
-func getSongIdOrZero(db *sqlx.DB, query string, songId int64) (assocSongId int64, err error) {
-	err = db.QueryRowx(query, songId).Scan(&assocSongId)
-	if err != nil && err == sql.ErrNoRows {
-		err = nil
-		assocSongId = 0
-	}
-	return
-}
-
-func GetNextSongId(db *sqlx.DB, prevSongId int64) (nextSongId int64, err error) {
-	nextSongId, err = getSongIdOrZero(
-		db,
+func GetNextSong(db *sqlx.DB, prevSongId int) (nextSong *read.Song, err error) {
+	nextSong = &read.Song{}
+	err = db.QueryRowx(
 		`
-    select coalesce(id_next, 0) as id from (
-      select
-        prev.track_number, prev.title, prev.artist, prev.album
-          ,coalesce(prev.id_next, next_artist.id) as id_next
-      from (
-        select
-          prev.track_number, prev.title, prev.artist, prev.album
-            ,coalesce(prev.id_next, next_album.id) as id_next
-        from (
-          select
-            prev.track_number, prev.title, prev.artist, prev.album
-              ,coalesce(prev.id_next, next_title.id) as id_next
-          from (
-            select
-              prev.track_number, prev.title, prev.artist, prev.album
-                ,coalesce(prev.id_next, next_track_number.id) as id_next
-            from (
-              select
-                prev.track_number, prev.title, prev.artist, prev.album
-                  ,next_no_info.id as id_next
-
-              from (
-                select track_number, title, artist, album from songs
-                where id = $1
-              ) prev
-
-              left join songs next_no_info on 1=1
-                and prev.artist = ''
-                and prev.album = ''
-                and next_no_info.id > $1
-
-              order by
-                next_no_info.artist
-                ,next_no_info.album
-                ,next_no_info.track_number
-                ,next_no_info.title
-                ,next_no_info.id
-
-              limit 1
-
-            ) prev
-
-            left join songs next_track_number on 1=1
-              and prev.id_next is null
-              and next_track_number.artist = prev.artist
-              and next_track_number.album = prev.album
-              and (
-                (prev.track_number > 0 and next_track_number.track_number > prev.track_number)
-                or (prev.track_number = 0 and next_track_number.id > $1)
-              )
-
-            order by next_track_number.track_number, next_track_number.id
-            limit 1
-
-          ) prev
-
-          left join songs next_title on 1=1
-            and prev.id_next is null
-            and next_title.artist = prev.artist
-            and next_title.album = prev.album
-            and next_title.track_number is null
-            and next_title.title > prev.title
-
-          order by next_title.title
-          limit 1
-
-        ) prev
-
-        left join songs next_album on 1=1
-          and prev.id_next is null
-          and next_album.artist = prev.artist
-          and next_album.album > prev.album
-
-        order by next_album.album, next_album.track_number, next_album.title
-        limit 1
-      ) prev
+select
+  s1.id
+  ,s1.track_number
+  ,s1.title
+  ,s1.artist
+  ,s1.album
+  ,s1.duration
+
+from (
+  select * from songs where id = $1
+) s0
+
+left join songs s1 on (
+  s1.artist > s0.artist
+  or (s1.artist = s0.artist
+    and s1.album > s0.album
+  )
+  or (s1.artist = s0.artist
+    and s1.album = s0.album
+    and s1.track_number > s0.track_number
+  )
+  or (s1.artist = s0.artist
+    and s1.album = s0.album
+    and s1.track_number = s0.track_number
+    and s1.title > s0.title
+  )
+  or (s1.artist = s0.artist
+    and s1.album = s0.album
+    and s1.track_number = s0.track_number
+    and s1.title = s0.title
+    and s1.id > s0.id
+  )
+)
 
-      left join songs next_artist on 1=1
-        and prev.id_next is null
-        and next_artist.artist > prev.artist
+order by
+  s1.artist
+  ,s1.album
+  ,s1.track_number
+  ,s1.title
+  ,s1.id
 
-      order by next_artist.artist, next_artist.album, next_artist.track_number, next_artist.title
-      limit 1
-    ) result
+limit 1
     `,
 		prevSongId,
-	)
+	).StructScan(nextSong)
+	if err != nil && err == sql.ErrNoRows {
+		err = nil
+		nextSong = &read.Song{Id: 0}
+	}
 	return
 }
 
-func GetPrevSongId(db *sqlx.DB, nextSongId int64) (prevSongId int64, err error) {
-	prevSongId, err = getSongIdOrZero(
-		db,
+func GetPrevSong(db *sqlx.DB, nextSongId int) (prevSong *read.Song, err error) {
+	prevSong = &read.Song{}
+	err = db.QueryRowx(
 		`
-    select coalesce(id_next, 0) as id from (
-      select
-        prev.track_number, prev.title, prev.artist, prev.album
-          ,coalesce(prev.id_next, next_no_info.id) as id_next
-      from (
-        select
-          prev.track_number, prev.title, prev.artist, prev.album
-            ,coalesce(prev.id_next, next_artist.id) as id_next
-        from (
-          select
-            prev.track_number, prev.title, prev.artist, prev.album
-              ,coalesce(prev.id_next, next_album.id) as id_next
-          from (
-            select
-              prev.track_number, prev.title, prev.artist, prev.album
-                ,coalesce(prev.id_next, next_title.id) as id_next
-            from (
-              select
-                prev.track_number, prev.title, prev.artist, prev.album
-                  ,next_track_number.id as id_next
-              from (
-                select track_number, title, artist, album from songs
-                where id = $1
-              ) prev
-
-              left join songs next_track_number on 1=1
-                and next_track_number.artist = prev.artist
-                and next_track_number.album = prev.album
-                and next_track_number.track_number < prev.track_number
-
-              order by next_track_number.track_number desc
-              limit 1
-
-            ) prev
-
-            left join songs next_title on 1=1
-              and prev.id_next is null
-              and next_title.artist = prev.artist
-              and next_title.album = prev.album
-              and next_title.track_number is null
-              and next_title.title < prev.title
-
-            order by next_title.title desc
-            limit 1
-
-          ) prev
-
-          left join songs next_album on 1=1
-            and prev.id_next is null
-            and next_album.artist = prev.artist
-            and next_album.album < prev.album
-
-          order by
-            next_album.album desc
-            ,next_album.track_number desc
-            ,next_album.title desc
-          limit 1
-        ) prev
-
-        left join songs next_artist on 1=1
-          and prev.id_next is null
-          and next_artist.artist < prev.artist
-
-        order by
-          next_artist.artist desc
-          ,next_artist.album desc
-          ,next_artist.track_number desc
-          ,next_artist.title desc
-        limit 1
-      ) prev
-
-      inner join songs from_song on from_song.id = $1
-
-      left join songs next_no_info on 1=1
-        and prev.id_next is null
-        and next_no_info.artist = ''
-        and next_no_info.album = ''
-        and next_no_info.id != $1
-        and (from_song.artist != '' or from_song.album != '' or next_no_info.id < $1)
+select
+  s1.id
+  ,s1.track_number
+  ,s1.title
+  ,s1.artist
+  ,s1.album
+  ,s1.duration
+
+from (
+  select * from songs where id = $1
+) s0
+
+left join songs s1 on (
+  s1.artist < s0.artist
+  or (s1.artist = s0.artist
+    and s1.album < s0.album
+  )
+  or (s1.artist = s0.artist
+    and s1.album = s0.album
+    and s1.track_number < s0.track_number
+  )
+  or (s1.artist = s0.artist
+    and s1.album = s0.album
+    and s1.track_number = s0.track_number
+    and s1.title < s0.title
+  )
+  or (s1.artist = s0.artist
+    and s1.album = s0.album
+    and s1.track_number = s0.track_number
+    and s1.title = s0.title
+    and s1.id < s0.id
+  )
+)
 
-      order by next_no_info.id desc
-      limit 1
+order by
+  s1.artist desc
+  ,s1.album desc
+  ,s1.track_number desc
+  ,s1.title desc
+  ,s1.id desc
 
-    ) result
+limit 1
     `,
 		nextSongId,
-	)
+	).StructScan(prevSong)
+	if err != nil && err == sql.ErrNoRows {
+		err = nil
+		prevSong = &read.Song{Id: 0}
+	}
 	return
 }

+ 29 - 29
gmus-backend/pkg/repository/player_test.go

@@ -12,7 +12,7 @@ import (
 var _ = Describe("Player repository", func() {
 	db := database.GetConnection()
 
-	var ids []int64
+	var ids []int
 
 	BeforeEach(func() {
 		testing.PrepareDatabaseForTesting()
@@ -46,8 +46,8 @@ var _ = Describe("Player repository", func() {
 			panic(err)
 		}
 
-		var id int64
-		ids = []int64{}
+		var id int
+		ids = []int{}
 
 		for rows.Next() {
 			rows.Scan(&id)
@@ -56,94 +56,94 @@ var _ = Describe("Player repository", func() {
 		rows.Close()
 	})
 
-	Describe("GetNextSongId", func() {
+	Describe("GetNextSong", func() {
 		Context("when another song exists in the same album", func() {
 			It("should return the correct song ID", func() {
-				id, _ := repository.GetNextSongId(db, ids[0])
-				Expect(id).To(Equal(ids[2]))
+				song, _ := repository.GetNextSong(db, ids[0])
+				Expect(song.Id).To(Equal(ids[2]))
 			})
 		})
 
 		Context("when another song exists from the same artist", func() {
 			It("should return the correct song ID", func() {
-				id, _ := repository.GetNextSongId(db, ids[2])
-				Expect(id).To(Equal(ids[3]))
+				song, _ := repository.GetNextSong(db, ids[2])
+				Expect(song.Id).To(Equal(ids[3]))
 			})
 		})
 
 		Context("when another song exists by a different artist", func() {
 			It("should return the correct song ID", func() {
-				id, _ := repository.GetNextSongId(db, ids[3])
-				Expect(id).To(Equal(ids[1]))
+				song, _ := repository.GetNextSong(db, ids[3])
+				Expect(song.Id).To(Equal(ids[1]))
 			})
 		})
 
 		Context("when no further songs exist", func() {
 			It("should return zero", func() {
-				id, _ := repository.GetNextSongId(db, ids[1])
-				Expect(id).To(Equal(int64(0)))
+				song, _ := repository.GetNextSong(db, ids[1])
+				Expect(song.Id).To(Equal(0))
 			})
 		})
 
 		Context("when the song has no information", func() {
 			It("should return the next song by ID", func() {
-				id, _ := repository.GetNextSongId(db, ids[4])
-				Expect(id).To(Equal(ids[5]))
+				song, _ := repository.GetNextSong(db, ids[4])
+				Expect(song.Id).To(Equal(ids[5]))
 			})
 		})
 
 		Context("when the ID does not exist", func() {
 			It("should return nil", func() {
-				id, err := repository.GetNextSongId(db, 10000000)
+				song, err := repository.GetNextSong(db, 10000000)
 
 				Expect(err).To(BeNil())
-				Expect(id).To(BeZero())
+				Expect(song.Id).To(BeZero())
 			})
 		})
 	})
 
-	Describe("GetPrevSongId", func() {
+	Describe("GetPrevSong", func() {
 		Context("when another song exists in the same album", func() {
 			It("should return the correct song ID", func() {
-				id, _ := repository.GetPrevSongId(db, ids[2])
-				Expect(id).To(Equal(ids[0]))
+				song, _ := repository.GetPrevSong(db, ids[2])
+				Expect(song.Id).To(Equal(ids[0]))
 			})
 		})
 
 		Context("when another song exists from the same artist", func() {
 			It("should return the correct song ID", func() {
-				id, _ := repository.GetPrevSongId(db, ids[3])
-				Expect(id).To(Equal(ids[2]))
+				song, _ := repository.GetPrevSong(db, ids[3])
+				Expect(song.Id).To(Equal(ids[2]))
 			})
 		})
 
 		Context("when another song exists by a different artist", func() {
 			It("should return the correct song ID", func() {
-				id, _ := repository.GetPrevSongId(db, ids[1])
-				Expect(id).To(Equal(ids[3]))
+				song, _ := repository.GetPrevSong(db, ids[1])
+				Expect(song.Id).To(Equal(ids[3]))
 			})
 		})
 
 		Context("when the song has no information", func() {
 			It("should return the previous song by ID", func() {
-				id, _ := repository.GetPrevSongId(db, ids[5])
-				Expect(id).To(Equal(ids[4]))
+				song, _ := repository.GetPrevSong(db, ids[5])
+				Expect(song.Id).To(Equal(ids[4]))
 			})
 		})
 
 		Context("when no further songs exist", func() {
 			It("should return zero", func() {
-				id, _ := repository.GetPrevSongId(db, ids[4])
-				Expect(id).To(Equal(int64(0)))
+				song, _ := repository.GetPrevSong(db, ids[4])
+				Expect(song.Id).To(Equal(0))
 			})
 		})
 
 		Context("when the ID does not exist", func() {
 			It("should return nil", func() {
-				id, err := repository.GetPrevSongId(db, 10000000)
+				song, err := repository.GetPrevSong(db, 10000000)
 
 				Expect(err).To(BeNil())
-				Expect(id).To(BeZero())
+				Expect(song.Id).To(BeZero())
 			})
 		})
 	})

+ 1 - 1
gmus-backend/pkg/repository/songs.go

@@ -96,7 +96,7 @@ func SelectSongsByArtist(db *sqlx.DB, artist string) (songs *[]*read.SongExterna
     ,duration
   from songs
   where artist = $1
-  order by album, track_number, id, title
+  order by album, track_number, title, id
   `, artist)
 
 	return

+ 9 - 14
gmus-backend/pkg/server/fetch.go

@@ -137,20 +137,15 @@ type NullResponse struct {
 	Id int `json:"id"`
 }
 
-func respondWithSongOrNull(db *sqlx.DB, w http.ResponseWriter, id int) error {
-	if id == 0 {
+func respondWithSongOrNull(db *sqlx.DB, w http.ResponseWriter, song *read.Song) error {
+	if song.Id == 0 {
 		response, _ := json.Marshal(NullResponse{})
 		w.Write(response)
 		return nil
 	}
 
-	song, err := repository.SelectSong(db, id)
-	if err != nil {
-		return err
-	}
-
 	response, err := json.Marshal(read.SongExternal{
-		Id:          id,
+		Id:          song.Id,
 		TrackNumber: song.TrackNumber,
 		Title:       song.Title,
 		Artist:      song.Artist,
@@ -166,37 +161,37 @@ func respondWithSongOrNull(db *sqlx.DB, w http.ResponseWriter, id int) error {
 	return nil
 }
 
-func routeFetchNextSongId(l *logger.Logger, rdb *redis.Client, w http.ResponseWriter, r *http.Request) error {
+func routeFetchNextSong(l *logger.Logger, rdb *redis.Client, w http.ResponseWriter, r *http.Request) error {
 	id, err := validateSongId(w, r)
 	if err != nil {
 		return nil
 	}
 
 	db := database.GetConnection()
-	nextId, err := repository.GetNextSongId(db, int64(id))
+	nextSong, err := repository.GetNextSong(db, int64(id))
 	if err != nil {
 		return err
 	}
 
-	if err := respondWithSongOrNull(db, w, int(nextId)); err != nil {
+	if err := respondWithSongOrNull(db, w, nextSong); err != nil {
 		return err
 	}
 	return nil
 }
 
-func routeFetchPrevSongId(l *logger.Logger, rdb *redis.Client, w http.ResponseWriter, r *http.Request) error {
+func routeFetchPrevSong(l *logger.Logger, rdb *redis.Client, w http.ResponseWriter, r *http.Request) error {
 	id, err := validateSongId(w, r)
 	if err != nil {
 		return nil
 	}
 
 	db := database.GetConnection()
-	prevId, err := repository.GetPrevSongId(db, int64(id))
+	prevSong, err := repository.GetPrevSong(db, int64(id))
 	if err != nil {
 		return err
 	}
 
-	if err := respondWithSongOrNull(db, w, int(prevId)); err != nil {
+	if err := respondWithSongOrNull(db, w, prevSong); err != nil {
 		return err
 	}
 	return nil

+ 2 - 2
gmus-backend/pkg/server/server.go

@@ -31,8 +31,8 @@ func StartServer() {
 
 	router.Path("/song-info").Methods("GET").HandlerFunc(routeHandler(l, rdb, routeFetchSongInfo))
 
-	router.Path("/next-song").Methods("GET").HandlerFunc(routeHandler(l, rdb, routeFetchNextSongId))
-	router.Path("/prev-song").Methods("GET").HandlerFunc(routeHandler(l, rdb, routeFetchPrevSongId))
+	router.Path("/next-song").Methods("GET").HandlerFunc(routeHandler(l, rdb, routeFetchNextSong))
+	router.Path("/prev-song").Methods("GET").HandlerFunc(routeHandler(l, rdb, routeFetchPrevSong))
 
 	port := conf.Port