Get() methods from all repositories now return a ErrNotFound when the id is nonexistent

api/stream.go

@@ -21,14 +21,13 @@ func (c *StreamController) Prepare() {
 	c.id = c.RequiredParamString("id", "id parameter required")
 
 	mf, err := c.repo.Get(c.id)
-	if err != nil {
+	switch {
-		beego.Error("Error reading mediafile", c.id, "from the database", ":", err)
+	case err == domain.ErrNotFound:
-		c.SendError(responses.ERROR_GENERIC, "Internal error")
-	}
-
-	if mf == nil {
 		beego.Error("MediaFile", c.id, "not found!")
 		c.SendError(responses.ERROR_DATA_NOT_FOUND)
+	case err != nil:
+		beego.Error("Error reading mediafile", c.id, "from the database", ":", err)
+		c.SendError(responses.ERROR_GENERIC, "Internal error")
 	}
 
 	c.mf = mf

domain/base.go

@@ -1,11 +1,17 @@
 package domain
 
+import "errors"
+
 type BaseRepository interface {
 	NewId(fields ...string) string
 	CountAll() (int64, error)
 	Exists(id string) (bool, error)
 }
 
+var (
+	ErrNotFound = errors.New("Data not found")
+)
+
 type QueryOptions struct {
 	SortBy string
 	Alpha  bool

engine/cover.go

@@ -1,16 +1,13 @@
 package engine
 
 import (
-	"io"
+	"bytes"
-	"os"
-
 	"image"
 	_ "image/gif"
-	_ "image/jpeg"
-	_ "image/png"
-
-	"bytes"
 	"image/jpeg"
+	_ "image/png"
+	"io"
+	"os"
 
 	"github.com/deluan/gosonic/domain"
 	"github.com/dhowden/tag"
@@ -31,16 +28,17 @@ func NewCover(mr domain.MediaFileRepository) Cover {
 
 func (c cover) Get(id string, size int, out io.Writer) error {
 	mf, err := c.mfileRepo.Get(id)
-	if err != nil {
+	if err != nil && err != domain.ErrNotFound {
 		return err
 	}
 
 	var reader io.Reader
 
-	if mf != nil && mf.HasCoverArt {
+	if err == nil && mf.HasCoverArt {
 		reader, err = readFromTag(mf.Path)
 	} else {
-		f, err := os.Open("static/default_cover.jpg")
+		var f *os.File
+		f, err = os.Open("static/default_cover.jpg")
 		if err == nil {
 			defer f.Close()
 			reader = f

engine/search.go

@@ -3,6 +3,7 @@ package engine
 import (
 	"strings"
 
+	"github.com/astaxie/beego"
 	"github.com/deluan/gomate"
 	"github.com/deluan/gosonic/domain"
 	"github.com/kennygrant/sanitize"
@@ -73,7 +74,7 @@ func (s search) SearchArtist(q string, offset int, size int) (*Results, error) {
 	res := make(Results, len(resp))
 	for i, id := range resp {
 		a, err := s.artistRepo.Get(id)
-		if err != nil {
+		if criticalError("Artist", id, err) {
 			return nil, err
 		}
 		res[i] = Entry{Id: a.Id, Title: a.Name, IsDir: true}
@@ -92,7 +93,7 @@ func (s search) SearchAlbum(q string, offset int, size int) (*Results, error) {
 	res := make(Results, len(resp))
 	for i, id := range resp {
 		al, err := s.albumRepo.Get(id)
-		if err != nil {
+		if criticalError("Album", id, err) {
 			return nil, err
 		}
 		res[i] = FromAlbum(al)
@@ -111,10 +112,20 @@ func (s search) SearchSong(q string, offset int, size int) (*Results, error) {
 	res := make(Results, len(resp))
 	for i, id := range resp {
 		mf, err := s.mfileRepo.Get(id)
-		if err != nil {
+		if criticalError("Song", id, err) {
 			return nil, err
 		}
 		res[i] = FromMediaFile(mf)
 	}
 	return &res, nil
 }
+
+func criticalError(kind, id string, err error) bool {
+	switch {
+	case err != nil:
+		return true
+	case err == domain.ErrNotFound:
+		beego.Warn(kind, "Id", id, "not in the DB. Need a reindex?")
+	}
+	return false
+}

persistence/ledis_repository.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"reflect"
 	"strings"
-
 	"time"
 
 	"github.com/deluan/gosonic/domain"
@@ -228,6 +227,9 @@ func (r *ledisRepository) readEntity(id string) (interface{}, error) {
 	if err != nil {
 		return nil, err
 	}
+	if len(res[0]) == 0 {
+		return nil, domain.ErrNotFound
+	}
 	err = r.toEntity(res, entity)
 	return entity, err
 }

persistence/ledis_repository_test.go

@@ -4,9 +4,9 @@ import (
 	"fmt"
 	"strconv"
 	"testing"
-
 	"time"
 
+	"github.com/deluan/gosonic/domain"
 	"github.com/deluan/gosonic/tests"
 	"github.com/deluan/gosonic/utils"
 	. "github.com/smartystreets/goconvey/convey"
@@ -27,7 +27,8 @@ func shouldBeEqual(actualStruct interface{}, expectedStruct ...interface{}) stri
 	return ShouldEqual(actual, expected)
 }
 
-func createRepo() *ledisRepository {
+func createEmptyRepo() *ledisRepository {
+	dropDb()
 	repo := &ledisRepository{}
 	repo.init("test", &TestEntity{})
 	return repo
@@ -37,7 +38,7 @@ func TestBaseRepository(t *testing.T) {
 	tests.Init(t, false)
 
 	Convey("Subject: Annotations", t, func() {
-		repo := createRepo()
+		repo := createEmptyRepo()
 		Convey("It should parse the parent table definition", func() {
 			So(repo.parentTable, ShouldEqual, "parent")
 			So(repo.parentIdField, ShouldEqual, "ParentId")
@@ -51,7 +52,7 @@ func TestBaseRepository(t *testing.T) {
 	})
 
 	Convey("Subject: calcScore", t, func() {
-		repo := createRepo()
+		repo := createEmptyRepo()
 
 		Convey("It should create an int score", func() {
 			def := repo.indexes["ByCount"]
@@ -86,7 +87,7 @@ func TestBaseRepository(t *testing.T) {
 	})
 
 	Convey("Subject: NewId", t, func() {
-		repo := createRepo()
+		repo := createEmptyRepo()
 
 		Convey("When I call NewId with a name", func() {
 			Id := repo.NewId("a name")
@@ -120,7 +121,14 @@ func TestBaseRepository(t *testing.T) {
 	Convey("Subject: saveOrUpdate/loadEntity/CountAll", t, func() {
 
 		Convey("Given an empty DB", func() {
-			repo := createRepo()
+			repo := createEmptyRepo()
+
+			Convey("When I try to retrieve an nonexistent ID", func() {
+				_, err := repo.readEntity("NOT_FOUND")
+				Convey("Then I should get a NotFound error", func() {
+					So(err, ShouldEqual, domain.ErrNotFound)
+				})
+			})
 
 			Convey("When I save a new entity and a parent", func() {
 				entity := &TestEntity{Id: "123", Name: "My Name", ParentId: "ABC", Year: time.Now()}
@@ -151,7 +159,7 @@ func TestBaseRepository(t *testing.T) {
 		})
 
 		Convey("Given a table with one entity", func() {
-			repo := createRepo()
+			repo := createEmptyRepo()
 			entity := &TestEntity{Id: "111", Name: "One Name", ParentId: "AAA"}
 			repo.saveOrUpdate(entity.Id, entity)
 
@@ -186,7 +194,7 @@ func TestBaseRepository(t *testing.T) {
 		})
 
 		Convey("Given a table with 3 entities", func() {
-			repo := createRepo()
+			repo := createEmptyRepo()
 			for i := 1; i <= 3; i++ {
 				e := &TestEntity{Id: strconv.Itoa(i), Name: fmt.Sprintf("Name %d", i), ParentId: "AAA"}
 				repo.saveOrUpdate(e.Id, e)
@@ -216,7 +224,7 @@ func TestBaseRepository(t *testing.T) {
 				})
 				Convey("And I get all saved ids", func() {
 					So(len(ids), ShouldEqual, 3)
-					for k, _ := range ids {
+					for k := range ids {
 						So(k, ShouldBeIn, []string{"1", "2", "3"})
 					}
 				})
@@ -245,10 +253,5 @@ func TestBaseRepository(t *testing.T) {
 
 			})
 		})
-
-		Reset(func() {
-			dropDb()
-		})
-
 	})
 }

persistence/mock_album_repo.go

@@ -45,7 +45,10 @@ func (m *MockAlbum) Get(id string) (*domain.Album, error) {
 	if m.err {
 		return nil, errors.New("Error!")
 	}
-	return m.data[id], nil
+	if d, ok := m.data[id]; ok {
+		return d, nil
+	}
+	return nil, domain.ErrNotFound
 }
 
 func (m *MockAlbum) GetAll(qo domain.QueryOptions) (*domain.Albums, error) {

persistence/mock_artist_repo.go

@@ -43,5 +43,8 @@ func (m *MockArtist) Get(id string) (*domain.Artist, error) {
 	if m.err {
 		return nil, errors.New("Error!")
 	}
-	return m.data[id], nil
+	if d, ok := m.data[id]; ok {
+		return d, nil
+	}
+	return nil, domain.ErrNotFound
 }

persistence/mock_mediafile_repo.go

@@ -46,8 +46,10 @@ func (m *MockMediaFile) Get(id string) (*domain.MediaFile, error) {
 	if m.err {
 		return nil, errors.New("Error!")
 	}
-	mf := m.data[id]
+	if d, ok := m.data[id]; ok {
-	return mf, nil
+		return d, nil
+	}
+	return nil, domain.ErrNotFound
 }
 
 func (m *MockMediaFile) FindByAlbum(artistId string) (*domain.MediaFiles, error) {

persistence/property_repository.go

@@ -3,6 +3,7 @@ package persistence
 import (
 	"errors"
 
+	"github.com/deluan/gosonic/domain"
 	"github.com/deluan/gosonic/engine"
 )
 
@@ -33,8 +34,8 @@ func (r *propertyRepository) Get(id string) (string, error) {
 func (r *propertyRepository) DefaultGet(id string, defaultValue string) (string, error) {
 	v, err := r.Get(id)
 
-	if v == "" {
+	if err == domain.ErrNotFound {
-		v = defaultValue
+		return defaultValue, nil
 	}
 
 	return v, err

scanner/itunes_scanner.go

@@ -66,6 +66,8 @@ func (s *ItunesScanner) ScanLibrary(lastModifiedSince time.Time, path string) (i
 	i := 0
 	for _, t := range l.Tracks {
 		if !s.skipTrack(&t) {
+			s.calcCheckSum(&t)
+
 			ar := s.collectArtists(&t)
 			mf := s.collectMediaFiles(&t)
 			s.collectAlbums(&t, mf, ar)
@@ -200,8 +202,6 @@ func (s *ItunesScanner) calcCheckSum(t *itl.Track) string {
 }
 
 func (s *ItunesScanner) collectMediaFiles(t *itl.Track) *domain.MediaFile {
-	s.calcCheckSum(t)
-
 	mf := &domain.MediaFile{}
 	mf.Id = strconv.Itoa(t.TrackID)
 	mf.Album = unescape(t.Album)