Merge pull request #1 from safierinx-a/fix/maloja-importer

Fix Maloja importer resilience: nil MBZ client, null album, error han…

engine/engine.go

@@ -263,47 +263,51 @@ func RunImporter(l *zerolog.Logger, store db.DB, mbzc mbz.MusicBrainzCaller) {
 	} else {
 		return
 	}
-	defer func() {
-		if r := recover(); r != nil {
-			l.Error().Interface("recover", r).Msg("Importer: Panic when importing files")
-		}
-	}()
 	for _, file := range files {
 		if file.IsDir() {
 			continue
 		}
-		if strings.Contains(file.Name(), "Streaming_History_Audio") {
+		importFile(l, store, mbzc, file.Name())
-			l.Info().Msgf("Importer: Import file %s detecting as being Spotify export", file.Name())
-			err := importer.ImportSpotifyFile(logger.NewContext(l), store, file.Name())
-			if err != nil {
-				l.Err(err).Msgf("Importer: Failed to import file: %s", file.Name())
 	}
-		} else if strings.Contains(file.Name(), "maloja") {
-			l.Info().Msgf("Importer: Import file %s detecting as being Maloja export", file.Name())
-			err := importer.ImportMalojaFile(logger.NewContext(l), store, file.Name())
-			if err != nil {
-				l.Err(err).Msgf("Importer: Failed to import file: %s", file.Name())
 }
-		} else if strings.Contains(file.Name(), "recenttracks") {
+
-			l.Info().Msgf("Importer: Import file %s detecting as being ghan.nl LastFM export", file.Name())
+func importFile(l *zerolog.Logger, store db.DB, mbzc mbz.MusicBrainzCaller, filename string) {
-			err := importer.ImportLastFMFile(logger.NewContext(l), store, mbzc, file.Name())
+	defer func() {
-			if err != nil {
+		if r := recover(); r != nil {
-				l.Err(err).Msgf("Importer: Failed to import file: %s", file.Name())
+			l.Error().Interface("recover", r).Msgf("Importer: Panic when importing file: %s", filename)
 		}
-		} else if strings.Contains(file.Name(), "listenbrainz") {
+	}()
-			l.Info().Msgf("Importer: Import file %s detecting as being ListenBrainz export", file.Name())
+	if strings.Contains(filename, "Streaming_History_Audio") {
-			err := importer.ImportListenBrainzExport(logger.NewContext(l), store, mbzc, file.Name())
+		l.Info().Msgf("Importer: Import file %s detecting as being Spotify export", filename)
+		err := importer.ImportSpotifyFile(logger.NewContext(l), store, mbzc, filename)
 		if err != nil {
-				l.Err(err).Msgf("Importer: Failed to import file: %s", file.Name())
+			l.Err(err).Msgf("Importer: Failed to import file: %s", filename)
 		}
-		} else if strings.Contains(file.Name(), "koito") {
+	} else if strings.Contains(filename, "maloja") {
-			l.Info().Msgf("Importer: Import file %s detecting as being Koito export", file.Name())
+		l.Info().Msgf("Importer: Import file %s detecting as being Maloja export", filename)
-			err := importer.ImportKoitoFile(logger.NewContext(l), store, file.Name())
+		err := importer.ImportMalojaFile(logger.NewContext(l), store, mbzc, filename)
 		if err != nil {
-				l.Err(err).Msgf("Importer: Failed to import file: %s", file.Name())
+			l.Err(err).Msgf("Importer: Failed to import file: %s", filename)
+		}
+	} else if strings.Contains(filename, "recenttracks") {
+		l.Info().Msgf("Importer: Import file %s detecting as being ghan.nl LastFM export", filename)
+		err := importer.ImportLastFMFile(logger.NewContext(l), store, mbzc, filename)
+		if err != nil {
+			l.Err(err).Msgf("Importer: Failed to import file: %s", filename)
+		}
+	} else if strings.Contains(filename, "listenbrainz") {
+		l.Info().Msgf("Importer: Import file %s detecting as being ListenBrainz export", filename)
+		err := importer.ImportListenBrainzExport(logger.NewContext(l), store, mbzc, filename)
+		if err != nil {
+			l.Err(err).Msgf("Importer: Failed to import file: %s", filename)
+		}
+	} else if strings.Contains(filename, "koito") {
+		l.Info().Msgf("Importer: Import file %s detecting as being Koito export", filename)
+		err := importer.ImportKoitoFile(logger.NewContext(l), store, filename)
+		if err != nil {
+			l.Err(err).Msgf("Importer: Failed to import file: %s", filename)
 		}
 	} else {
-			l.Warn().Msgf("Importer: File %s not recognized as a valid import file; make sure it is valid and named correctly", file.Name())
+		l.Warn().Msgf("Importer: File %s not recognized as a valid import file; make sure it is valid and named correctly", filename)
-		}
 	}
 }

engine/import_test.go

@@ -44,6 +44,61 @@ func TestImportMaloja(t *testing.T) {
 	truncateTestData(t)
 }
 
+func TestImportMaloja_NullAlbum(t *testing.T) {
+
+	src := path.Join("..", "test_assets", "maloja_import_null_album_test.json")
+	destDir := filepath.Join(cfg.ConfigDir(), "import")
+	dest := filepath.Join(destDir, "maloja_import_null_album_test.json")
+
+	input, err := os.ReadFile(src)
+	require.NoError(t, err)
+
+	require.NoError(t, os.WriteFile(dest, input, os.ModePerm))
+
+	engine.RunImporter(logger.Get(), store, &mbz.MbzErrorCaller{})
+
+	// The null-album track should be imported with album falling back to track name
+	a, err := store.GetArtist(context.Background(), db.GetArtistOpts{Name: "Null Album Artist"})
+	require.NoError(t, err)
+	assert.Equal(t, "Null Album Artist", a.Name)
+	assert.EqualValues(t, 1, a.ListenCount)
+
+	// The valid-album track should also be imported
+	a, err = store.GetArtist(context.Background(), db.GetArtistOpts{Name: "Valid Album Artist"})
+	require.NoError(t, err)
+	assert.Equal(t, "Valid Album Artist", a.Name)
+	assert.EqualValues(t, 1, a.ListenCount)
+
+	// The empty artists item should be skipped
+	count, err := store.CountArtists(context.Background(), db.Timeframe{Period: db.PeriodAllTime})
+	require.NoError(t, err)
+	assert.EqualValues(t, 2, count)
+
+	truncateTestData(t)
+}
+
+func TestImportMaloja_ApiFormat(t *testing.T) {
+
+	src := path.Join("..", "test_assets", "maloja_api_format_test.json")
+	destDir := filepath.Join(cfg.ConfigDir(), "import")
+	dest := filepath.Join(destDir, "maloja_api_format_test.json")
+
+	input, err := os.ReadFile(src)
+	require.NoError(t, err)
+
+	require.NoError(t, os.WriteFile(dest, input, os.ModePerm))
+
+	engine.RunImporter(logger.Get(), store, &mbz.MbzErrorCaller{})
+
+	// API format uses "list" key instead of "scrobbles"
+	a, err := store.GetArtist(context.Background(), db.GetArtistOpts{Name: "API Format Artist"})
+	require.NoError(t, err)
+	assert.Equal(t, "API Format Artist", a.Name)
+	assert.EqualValues(t, 2, a.ListenCount)
+
+	truncateTestData(t)
+}
+
 func TestImportSpotify(t *testing.T) {
 
 	src := path.Join("..", "test_assets", "Streaming_History_Audio_spotify_import_test.json")

internal/importer/maloja.go

@@ -17,8 +17,13 @@ import (
 	"github.com/gabehf/koito/internal/utils"
 )
 
-type MalojaExport struct {
+type MalojaAlbum struct {
+	Title string `json:"albumtitle"`
+}
+
+type MalojaFile struct {
 	Scrobbles []MalojaExportItem `json:"scrobbles"`
+	List      []MalojaExportItem `json:"list"`
 }
 type MalojaExportItem struct {
 	Time  int64       `json:"time"`
@@ -27,12 +32,10 @@ type MalojaExportItem struct {
 type MalojaTrack struct {
 	Artists []string     `json:"artists"`
 	Title   string       `json:"title"`
-	Album   struct {
+	Album   *MalojaAlbum `json:"album"`
-		Title string `json:"albumtitle"`
-	} `json:"album"`
 }
 
-func ImportMalojaFile(ctx context.Context, store db.DB, filename string) error {
+func ImportMalojaFile(ctx context.Context, store db.DB, mbzc mbz.MusicBrainzCaller, filename string) error {
 	l := logger.FromContext(ctx)
 	l.Info().Msgf("Beginning maloja import on file: %s", filename)
 	file, err := os.Open(path.Join(cfg.ConfigDir(), "import", filename))
@@ -47,14 +50,20 @@ func ImportMalojaFile(ctx context.Context, store db.DB, filename string) error {
 			time.Sleep(time.Duration(ms) * time.Millisecond)
 		}
 	}
-	export := new(MalojaExport)
+	export := new(MalojaFile)
 	err = json.NewDecoder(file).Decode(&export)
 	if err != nil {
 		return fmt.Errorf("ImportMalojaFile: %w", err)
 	}
-	for _, item := range export.Scrobbles {
+	items := export.Scrobbles
+	if len(items) == 0 {
+		items = export.List
+	}
+	count := 0
+	total := len(items)
+	for i, item := range items {
 		martists := make([]string, 0)
-		// Maloja has a tendency to have the the artist order ['feature', 'main \u2022 feature'], so
+		// Maloja has a tendency to have the the artist order ['feature', 'main ● feature'], so
 		// here we try to turn that artist array into ['main', 'feature']
 		item.Track.Artists = utils.MoveFirstMatchToFront(item.Track.Artists, " \u2022 ")
 		for _, an := range item.Track.Artists {
@@ -71,12 +80,16 @@ func ImportMalojaFile(ctx context.Context, store db.DB, filename string) error {
 			l.Debug().Msgf("Skipping import due to import time rules")
 			continue
 		}
+		releaseTitle := ""
+		if item.Track.Album != nil {
+			releaseTitle = item.Track.Album.Title
+		}
 		opts := catalog.SubmitListenOpts{
-			MbzCaller:      &mbz.MusicBrainzClient{},
+			MbzCaller:      mbzc,
 			Artist:         item.Track.Artists[0],
 			ArtistNames:    artists,
 			TrackTitle:     item.Track.Title,
-			ReleaseTitle:   item.Track.Album.Title,
+			ReleaseTitle:   releaseTitle,
 			Time:           ts.Local(),
 			Client:         "maloja",
 			UserID:         1,
@@ -84,10 +97,14 @@ func ImportMalojaFile(ctx context.Context, store db.DB, filename string) error {
 		}
 		err = catalog.SubmitListen(ctx, store, opts)
 		if err != nil {
-			l.Err(err).Msg("Failed to import maloja playback item")
+			l.Err(err).Msgf("Failed to import maloja item %d/%d", i+1, total)
-			return fmt.Errorf("ImportMalojaFile: %w", err)
+			continue
+		}
+		count++
+		if count%500 == 0 {
+			l.Info().Msgf("Maloja import progress: %d/%d", count, total)
 		}
 		throttleFunc()
 	}
-	return finishImport(ctx, filename, len(export.Scrobbles))
+	return finishImport(ctx, filename, count)
 }

internal/importer/spotify.go

@@ -24,7 +24,7 @@ type SpotifyExportItem struct {
 	MsPlayed   int32     `json:"ms_played"`
 }
 
-func ImportSpotifyFile(ctx context.Context, store db.DB, filename string) error {
+func ImportSpotifyFile(ctx context.Context, store db.DB, mbzc mbz.MusicBrainzCaller, filename string) error {
 	l := logger.FromContext(ctx)
 	l.Info().Msgf("Beginning spotify import on file: %s", filename)
 	file, err := os.Open(path.Join(cfg.ConfigDir(), "import", filename))
@@ -59,7 +59,7 @@ func ImportSpotifyFile(ctx context.Context, store db.DB, filename string) error
 			continue
 		}
 		opts := catalog.SubmitListenOpts{
-			MbzCaller:      &mbz.MusicBrainzClient{},
+			MbzCaller:      mbzc,
 			Artist:         item.ArtistName,
 			TrackTitle:     item.TrackName,
 			ReleaseTitle:   item.AlbumName,

test_assets/maloja_api_format_test.json

@@ -0,0 +1,41 @@
+{
+    "status": "ok",
+    "list": [
+      {
+        "time": 1746434410,
+        "track": {
+          "artists": [
+            "API Format Artist"
+          ],
+          "title": "API Format Track",
+          "album": {
+            "artists": [
+              "API Format Artist"
+            ],
+            "albumtitle": "API Format Album"
+          },
+          "length": null
+        },
+        "duration": null,
+        "origin": "client:default"
+      },
+      {
+        "time": 1746434682,
+        "track": {
+          "artists": [
+            "API Format Artist"
+          ],
+          "title": "API Format Track 2",
+          "album": {
+            "artists": [
+              "API Format Artist"
+            ],
+            "albumtitle": "API Format Album"
+          },
+          "length": null
+        },
+        "duration": null,
+        "origin": "client:default"
+      }
+    ]
+}

test_assets/maloja_import_null_album_test.json

@@ -0,0 +1,52 @@
+{
+    "maloja": {
+      "export_time": 1748738994
+    },
+    "scrobbles": [
+      {
+        "time": 1746434410,
+        "track": {
+          "artists": [
+            "Null Album Artist"
+          ],
+          "title": "Track With Null Album",
+          "album": null,
+          "length": null
+        },
+        "duration": null,
+        "origin": "client:default"
+      },
+      {
+        "time": 1746434682,
+        "track": {
+          "artists": [
+            "Valid Album Artist"
+          ],
+          "title": "Track With Valid Album",
+          "album": {
+            "artists": [
+              "Valid Album Artist"
+            ],
+            "albumtitle": "My Great Album"
+          },
+          "length": null
+        },
+        "duration": null,
+        "origin": "client:default"
+      },
+      {
+        "time": 1746434900,
+        "track": {
+          "artists": [],
+          "title": "Track With Empty Artists",
+          "album": {
+            "artists": [],
+            "albumtitle": "Some Album"
+          },
+          "length": null
+        },
+        "duration": null,
+        "origin": "client:default"
+      }
+    ]
+}