fix(scanner): fix records with album name same as artist

and never use db.Where() with a struct

gorm was seeing a query like

    db.Where(Album{Left: left, Right: right})

but if the `left` variable was empty, gorm couldn't differentiate it with an empty field in the struct
so it generated SQL that we weren't expected

like

    SELECT * FROM albums WHERE right=?

instead of

    SELECT * FROM albums WHERE left=? AND right=?

fixes #230

db/db.go

@@ -55,17 +55,17 @@ func NewMock() (*DB, error) {
 }
 
 func (db *DB) GetSetting(key string) (string, error) {
-	setting := &Setting{}
-	if err := db.Where("key=?", key).First(setting).Error; err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
+	var setting Setting
+	if err := db.Where("key=?", key).First(&setting).Error; err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
 		return "", err
 	}
 	return setting.Value, nil
 }
 
 func (db *DB) SetSetting(key, value string) error {
 	return db.
-		Where(Setting{Key: key}).
-		Assign(Setting{Value: value}).
+		Where("key=?", key).
+		Assign(Setting{Key: key, Value: value}).
 		FirstOrCreate(&Setting{}).
 		Error
 }
@@ -89,27 +89,27 @@ func (db *DB) InsertBulkLeftMany(table string, head []string, left int, col []in
 }
 
 func (db *DB) GetUserByID(id int) *User {
-	user := &User{}
+	var user User
 	err := db.
 		Where("id=?", id).
-		First(user).
+		First(&user).
 		Error
 	if errors.Is(err, gorm.ErrRecordNotFound) {
 		return nil
 	}
-	return user
+	return &user
 }
 
 func (db *DB) GetUserByName(name string) *User {
-	user := &User{}
+	var user User
 	err := db.
 		Where("name=?", name).
-		First(user).
+		First(&user).
 		Error
 	if errors.Is(err, gorm.ErrRecordNotFound) {
 		return nil
 	}
-	return user
+	return &user
 }
 
 func (db *DB) Begin() *DB {

scanner/scanner.go

@@ -166,7 +166,7 @@ func (s *Scanner) scanDir(tx *db.DB, c *Context, musicDir string, absPath string
 	relPath, _ := filepath.Rel(musicDir, absPath)
 	pdir, pbasename := filepath.Split(filepath.Dir(relPath))
 	var parent db.Album
-	if err := tx.Where(db.Album{RootDir: musicDir, LeftPath: pdir, RightPath: pbasename}).FirstOrCreate(&parent).Error; err != nil {
+	if err := tx.Where("root_dir=? AND left_path=? AND right_path=?", musicDir, pdir, pbasename).Assign(db.Album{RootDir: musicDir, LeftPath: pdir, RightPath: pbasename}).FirstOrCreate(&parent).Error; err != nil {
 		return fmt.Errorf("first or create parent: %w", err)
 	}
 
@@ -197,8 +197,8 @@ func (s *Scanner) populateTrackAndAlbumArtists(tx *db.DB, c *Context, i int, par
 		return fmt.Errorf("stating %q: %w", basename, err)
 	}
 
-	track := &db.Track{AlbumID: album.ID, Filename: filepath.Base(basename)}
-	if err := tx.Where(track).First(track).Error; err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
+	var track db.Track
+	if err := tx.Where("album_id=? AND filename=?", album.ID, filepath.Base(basename)).First(&track).Error; err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
 		return fmt.Errorf("query track: %w", err)
 	}
 
@@ -213,7 +213,7 @@ func (s *Scanner) populateTrackAndAlbumArtists(tx *db.DB, c *Context, i int, par
 	}
 
 	genreNames := strings.Split(trags.SomeGenre(), s.genreSplit)
-	genreIDs, err := populateGenres(tx, track, genreNames)
+	genreIDs, err := populateGenres(tx, &track, genreNames)
 	if err != nil {
 		return fmt.Errorf("populate genres: %w", err)
 	}
@@ -232,10 +232,10 @@ func (s *Scanner) populateTrackAndAlbumArtists(tx *db.DB, c *Context, i int, par
 		}
 	}
 
-	if err := populateTrack(tx, album, track, trags, basename, int(stat.Size())); err != nil {
+	if err := populateTrack(tx, album, &track, trags, basename, int(stat.Size())); err != nil {
 		return fmt.Errorf("process %q: %w", basename, err)
 	}
-	if err := populateTrackGenres(tx, track, genreIDs); err != nil {
+	if err := populateTrackGenres(tx, &track, genreIDs); err != nil {
 		return fmt.Errorf("populate track genres: %w", err)
 	}
 
@@ -266,7 +266,7 @@ func populateAlbum(tx *db.DB, album *db.Album, albumArtist *db.Artist, trags tag
 }
 
 func populateAlbumBasics(tx *db.DB, musicDir string, parent, album *db.Album, dir, basename string, cover string) error {
-	if err := tx.Where(db.Album{RootDir: musicDir, LeftPath: dir, RightPath: basename}).First(album).Error; err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
+	if err := tx.Where("root_dir=? AND left_path=? AND right_path=?", musicDir, dir, basename).First(album).Error; err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
 		return fmt.Errorf("find album: %w", err)
 	}
 

scanner/scanner_test.go

@@ -585,3 +585,28 @@ func TestIncrementalScanNoChangeNoUpdatedAt(t *testing.T) {
 
 	is.Equal(albumA.UpdatedAt, albumB.UpdatedAt)
 }
+
+// https://github.com/sentriz/gonic/issues/230
+func TestAlbumAndArtistSameNameWeirdness(t *testing.T) {
+	t.Parallel()
+	is := is.NewRelaxed(t)
+	m := mockfs.New(t)
+
+	const name = "same"
+
+	add := func(path string, a ...interface{}) {
+		m.AddTrack(fmt.Sprintf(path, a...))
+		m.SetTags(fmt.Sprintf(path, a...), func(tags *mockfs.Tags) error { return nil })
+	}
+
+	add("an-artist/%s/track-1.flac", name)
+	add("an-artist/%s/track-2.flac", name)
+	add("%s/an-album/track-1.flac", name)
+	add("%s/an-album/track-2.flac", name)
+
+	m.ScanAndClean()
+
+	var albums []*db.Album
+	is.NoErr(m.DB().Find(&albums).Error)
+	is.Equal(len(albums), 5) // root, 2 artists, 2 albums
+}

server/ctrlsubsonic/handlers_common.go

@@ -10,12 +10,12 @@ import (
 
 	"github.com/jinzhu/gorm"
 
+	"go.senan.xyz/gonic/db"
 	"go.senan.xyz/gonic/multierr"
+	"go.senan.xyz/gonic/scanner"
 	"go.senan.xyz/gonic/server/ctrlsubsonic/params"
 	"go.senan.xyz/gonic/server/ctrlsubsonic/spec"
 	"go.senan.xyz/gonic/server/ctrlsubsonic/specid"
-	"go.senan.xyz/gonic/db"
-	"go.senan.xyz/gonic/scanner"
 )
 
 func lowerUDecOrHash(in string) string {
@@ -128,7 +128,7 @@ func (c *Controller) ServeNotFound(r *http.Request) *spec.Response {
 
 func (c *Controller) ServeGetPlayQueue(r *http.Request) *spec.Response {
 	user := r.Context().Value(CtxUser).(*db.User)
-	queue := db.PlayQueue{}
+	var queue db.PlayQueue
 	err := c.DB.
 		Where("user_id=?", user.ID).
 		Find(&queue).
@@ -170,8 +170,9 @@ func (c *Controller) ServeSavePlayQueue(r *http.Request) *spec.Response {
 		}
 	}
 	user := r.Context().Value(CtxUser).(*db.User)
-	queue := &db.PlayQueue{UserID: user.ID}
-	c.DB.Where(queue).First(queue)
+	var queue db.PlayQueue
+	c.DB.Where("user_id=?, user.ID").First(&queue)
+	queue.UserID = user.ID
 	queue.Current = params.GetOrID("current", specid.ID{}).Value
 	queue.Position = params.GetOrInt("position", 0)
 	queue.ChangedBy = params.GetOr("c", "") // must exist, middleware checks
@@ -186,18 +187,18 @@ func (c *Controller) ServeGetSong(r *http.Request) *spec.Response {
 	if err != nil {
 		return spec.NewError(10, "provide an `id` parameter")
 	}
-	track := &db.Track{}
+	var track db.Track
 	err = c.DB.
 		Where("id=?", id.Value).
 		Preload("Album").
 		Preload("Album.TagArtist").
-		First(track).
+		First(&track).
 		Error
 	if errors.Is(err, gorm.ErrRecordNotFound) {
 		return spec.NewError(10, "couldn't find a track with that id")
 	}
 	sub := spec.NewResponse()
-	sub.Track = spec.NewTrackByTags(track, track.Album)
+	sub.Track = spec.NewTrackByTags(&track, track.Album)
 	return sub
 }
 

server/ctrlsubsonic/handlers_raw.go

@@ -72,22 +72,22 @@ func streamGetAudio(dbc *db.DB, podcastsPath string, user *db.User, id specid.ID
 }
 
 func streamUpdateStats(dbc *db.DB, userID, albumID int, playTime time.Time) error {
-	play := db.Play{
-		AlbumID: albumID,
-		UserID:  userID,
-	}
+	var play db.Play
 	err := dbc.
-		Where(play).
+		Where("album_id=? AND user_id=?", albumID, userID).
 		First(&play).
 		Error
 	if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
 		return fmt.Errorf("find stat: %w", err)
 	}
 
+	play.AlbumID = albumID
+	play.UserID = userID
 	play.Count++ // for getAlbumList?type=frequent
 	if playTime.After(play.Time) {
 		play.Time = playTime // for getAlbumList?type=recent
 	}
+
 	if err := dbc.Save(&play).Error; err != nil {
 		return fmt.Errorf("save stat: %w", err)
 	}