fixes and test cases for GetPlaylist

store/files.go

@@ -54,7 +54,10 @@ func (st *Store) CreateFile(group string, file File) (*File, error) {
 
 func (st *Store) GetFile(group string, id uint64) (file *File, err error) {
 	file = &File{}
-	err = st.db.First(file, id).Error
+	// we have to make sure that the file actually belongs to <group>
+	// otherwise a bad user can trick us into returning files for groups it
+	// normally has no access to
+	err = st.db.Where("group_name = ?", group).First(file, id).Error
 	return
 }
 
@@ -79,6 +82,8 @@ func (st *Store) DeleteFile(group string, id uint64) error {
 		return err
 	}
 	if err := tx.Delete(&File{ID: id}).Error; err != nil {
+		// TODO: this is likely due to a FK constraint:
+		//       - fetch all playlists that are using this file...
 		tx.Rollback()
 		return err
 	}

store/playlists.go

@@ -24,16 +24,58 @@
 
 package store
 
-func (p *Playlist) BeforeSave() (err error) {
+import (
+	"fmt"
+	"net/url"
+	"strconv"
+
+	"github.com/jinzhu/gorm"
+)
+
+func (p *Playlist) BeforeSave() error {
 	for idx := range p.Entries {
 		p.Entries[idx].LineNum = uint(idx)
-		// TODO: fix URI String if p.Entries[idx].File is not nil
+		if p.Entries[idx].File != nil {
+			p.Entries[idx].URI = "" // this will be regenerated in AfterFind()
+		} else if p.Entries[idx].URI != "" {
+			uri, err := url.Parse(p.Entries[idx].URI)
+			if err != nil {
+				return fmt.Errorf("playlist entry #%d is invalid: %v", idx, err)
+			}
+			if uri.Scheme == FileURIScheme {
+				if uri.Host == "" || uri.Path == "" {
+					return fmt.Errorf("playlist entry #%d is invalid: uri must be in the format %s://<group>/<file-id>", idx, FileURIScheme)
+				}
+				fileID, err := strconv.ParseUint(uri.Path, 10, 64)
+				if err != nil {
+					return fmt.Errorf("playlist entry #%d is invalid: %v", idx, err)
+				}
+				p.Entries[idx].File = &File{ID: fileID, GroupName: uri.Host}
+				p.Entries[idx].URI = "" // this will be regenerated in AfterFind()
+			}
+		} else {
+			return fmt.Errorf("playlist entry #%d is invalid, entries must either contain a File or have a URI set", idx)
+		}
 	}
-	return
+	return nil
+}
+
+func (p *Playlist) AfterFind() error {
+	for idx := range p.Entries {
+		if p.Entries[idx].File != nil {
+			urihost := p.Entries[idx].File.GroupName
+			uripath := strconv.FormatUint(p.Entries[idx].File.ID, 10)
+			uri := url.URL{Scheme: FileURIScheme, Host: urihost, Path: uripath}
+			p.Entries[idx].URI = uri.String()
+		}
+	}
+	return nil
 }
 
 func (st *Store) ListPlaylists(group string) (playlists Playlists, err error) {
-	err = st.db.Where("group_name = ?", group).Find(&playlists).Error
+	err = st.db.Where("group_name = ?", group).Preload("Entries", func(db *gorm.DB) *gorm.DB {
+		return db.Order("playlist_entries.line_num asc")
+	}).Preload("Entries.File").Find(&playlists).Error
 	return
 }
 
@@ -49,7 +91,12 @@ func (st *Store) CreatePlaylist(group string, playlist Playlist) (*Playlist, err
 
 func (st *Store) GetPlaylist(group string, id uint64) (playlist *Playlist, err error) {
 	playlist = &Playlist{}
-	err = st.db.First(playlist, id).Error
+	// we have to make sure that the playlist actually belongs to <group>
+	// otherwise a bad user can trick us into returning playlists for groups it
+	// normally has no access to
+	err = st.db.Where("group_name = ?", group).Preload("Entries", func(db *gorm.DB) *gorm.DB {
+		return db.Order("playlist_entries.line_num ASC")
+	}).Preload("Entries.File").First(playlist, id).Error
 	return
 }
 

store/store_test.go

@@ -494,7 +494,7 @@ func TestFilesDelete(t *testing.T) {
 
 func generateTestPlaylist(uris ...string) (p Playlist) {
 	for _, u := range uris {
-		e := PlaylistEntry{Uri: u}
+		e := PlaylistEntry{URI: u}
 		p.Entries = append(p.Entries, e)
 	}
 	return p
@@ -533,12 +533,12 @@ func TestPlaylistsListCreateDelete(t *testing.T) {
 		t.Fatalf("ListPlaylists should return a single playlist but returned: %v", playlists)
 	}
 
-	in1 := generateTestPlaylist("audioin://1", "http://stream.example.com/live.mp")
+	in1 := generateTestPlaylist("audioin://1", "http://stream.example.com/live.mp3")
 	_, err = store.CreatePlaylist(testGroup1, in1)
 	if err != nil {
 		t.Fatalf("creating playlist in not existing group shouldn't throw an error but CreatePlaylist returned: %v", err)
 	}
-	in2 := generateTestPlaylist("http://stream.example.com/live.ogg", "audioin://2")
+	in2 := generateTestPlaylist("https://stream.example.com/other.ogg", "audioin://2")
 	_, err = store.CreatePlaylist(testGroup1, in2)
 	if err != nil {
 		t.Fatalf("creating playlist in not existing group shouldn't throw an error but CreatePlaylist returned: %v", err)
@@ -567,3 +567,32 @@ func TestPlaylistsListCreateDelete(t *testing.T) {
 		t.Fatalf("deleteing playlist %d of group '%s' failed: %v", publicPlaylist.ID, publicGroupName, err)
 	}
 }
+
+func TestPlaylistsCreateAndGet(t *testing.T) {
+	store := newTestStore(t)
+
+	f := File{Size: 12345}
+	f.Source.FileName = testSourceFileName1
+	f.Metadata.Artist = testFileArtist1
+	f.Metadata.Album = testFileAlbum1
+	f.Metadata.Title = testFileTitle1
+	file1, err := store.CreateFile(testGroup1, f)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if _, err := store.GetPlaylist(testGroup1, 0); err != ErrNotFound {
+		t.Fatalf("getting playlist in not-existing group should return ErrNotFound, but GetPlaylist returned: %v", err)
+	}
+
+	p := generateTestPlaylist("audioin://1", "http://stream.example.com/live.mp3")
+	p.Entries = append(p.Entries, PlaylistEntry{File: &File{GroupName: file1.GroupName, ID: file1.ID}})
+	list1, err := store.CreatePlaylist(testGroup1, p)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if _, err := store.GetPlaylist(testGroup1, list1.ID); err != nil {
+		t.Fatalf("getting existing playlist from store shouldn't return an error, but GetPlaylist returned: %v", err)
+	}
+}

store/types.go

@@ -32,6 +32,8 @@ import (
 )
 
 const (
+	FileURIScheme = "file"
+
 	// well-known table names
 	migrationsTn = "__migrations__"
 
@@ -164,7 +166,7 @@ type PlaylistEntry struct {
 	ID         uint64  `json:"id" gorm:"primary_key"`
 	PlaylistID uint64  `json:"-" gorm:"not null;index;unique_index:unique_playlist_line_numbers"`
 	LineNum    uint    `json:"-" gorm:"not null;unique_index:unique_playlist_line_numbers"`
-	Uri        string  `json:"uri"`
+	URI        string  `json:"uri"`
 	File       *File   `json:"file" gorm:"association_autoupdate:false;association_autocreate:false"`
 	FileID     *uint64 `json:"-" gorm:"index"`
 }