refactor(playlists): simplify #6310 tests and update check comments

Builds the removeOrphans fixture with a single INSERT instead of shifting positions in two UPDATE passes, and has the Insert test keep the playlist id instead of reading a private repository field. Two comments now say that addTracks owns the existence check as well as the library check, and that Insert closes holes left by any id keepAccessible drops. No behavior change.

persistence/playlist_repository.go

@@ -281,7 +281,7 @@ func (r *playlistRepository) updatePlaylist(ctx context.Context, playlistId stri
 }
 
 // addTracks is the only path that writes playlist_tracks rows (smart playlists aside), so it owns
-// the library check: every caller, including a full replace through Put, goes through it.
+// the library and existence check: every caller, including a full replace through Put, goes through it.
 func (r *playlistRepository) addTracks(ctx context.Context, playlistId string, startingPos int, mediaFileIds []string) (int, error) {
 	mediaFileIds, err := r.keepAccessible(ctx, mediaFileIds)
 	if err != nil {

persistence/playlist_repository_test.go

@@ -472,17 +472,11 @@ var _ = Describe("PlaylistRepository", func() {
 	Describe("removeOrphans", func() {
 		It("deletes rows pointing to no track and renumbers the rest", func() {
 			pls := model.Playlist{Name: "Orphans", OwnerID: "userid"}
-			pls.AddMediaFilesByID([]string{"1001", "1002"})
 			Expect(repo.Put(ctx, &pls)).To(Succeed())
 			DeferCleanup(func() { Expect(repo.Delete(ctx, pls.ID)).To(Succeed()) })
-			// Rows an older version stored for unknown ids, interleaved with real tracks
-			for _, q := range []string{"-(id * 2)", "-id"} {
-				_, err := GetDBXBuilder().NewQuery(`UPDATE playlist_tracks SET id = ` + q + ` WHERE playlist_id = {:id}`).
-					Bind(dbx.Params{"id": pls.ID}).Execute()
-				Expect(err).ToNot(HaveOccurred())
-			}
+			// Rows as an older version stored them, with an unknown id between real tracks
 			_, err := GetDBXBuilder().NewQuery(`INSERT INTO playlist_tracks (playlist_id, media_file_id, id)
-				VALUES ({:id}, 'no-such-track', 1), ({:id}, '!!not-an-id!!', 3)`).
+				VALUES ({:id}, '1001', 1), ({:id}, 'no-such-track', 2), ({:id}, '1002', 3)`).
 				Bind(dbx.Params{"id": pls.ID}).Execute()
 			Expect(err).ToNot(HaveOccurred())
 

persistence/playlist_track_repository.go

@@ -213,7 +213,7 @@ func (r *playlistTrackRepository) Insert(ctx context.Context, mediaFileIds []str
 	if err != nil || inserted == n {
 		return inserted, err
 	}
-	// The shift above reserved a slot per requested id, so ids dropped by the library filter
+	// The shift above reserved a slot per requested id, so ids dropped by keepAccessible
 	// leave a hole. Close it.
 	return inserted, r.playlistRepo.renumber(ctx, r.playlistId)
 }

persistence/playlist_track_repository_test.go

@@ -83,12 +83,14 @@ var _ = Describe("PlaylistTrackRepository", func() {
 
 	Describe("Insert", func() {
 		var tracks model.PlaylistTrackRepository
+		var plsID string
 
 		BeforeEach(func() {
 			plsRepo := NewPlaylistRepository(GetDBXBuilder())
 			pls := model.Playlist{Name: "Insert", OwnerID: "userid", OwnerName: "userid"}
 			Expect(plsRepo.Put(ctx, &pls)).To(Succeed())
 			DeferCleanup(func() { Expect(plsRepo.Delete(ctx, pls.ID)).To(Succeed()) })
+			plsID = pls.ID
 
 			tracks = plsRepo.Tracks(ctx, pls.ID, false)
 			Expect(tracks.Add(ctx, []string{songDayInALife.ID, songRadioactivity.ID})).To(Equal(2))
@@ -127,7 +129,7 @@ var _ = Describe("PlaylistTrackRepository", func() {
 		It("drops ids that match no track, keeping positions contiguous", func() {
 			Expect(tracks.Insert(ctx, []string{"no-such-track", songComeTogether.ID}, 1)).To(Equal(1))
 
-			Expect(storedRows(tracks.(*playlistTrackRepository).playlistId)).To(Equal([]string{
+			Expect(storedRows(plsID)).To(Equal([]string{
 				"1:" + songComeTogether.ID, "2:" + songDayInALife.ID, "3:" + songRadioactivity.ID,
 			}))
 		})