fix: more spotify potential null fields, adding better handling for d…

…uplicates

src/spotify/model.rs

@@ -57,7 +57,7 @@ pub struct SpotifyPlaylistResponse {
 
 #[derive(Deserialize, Debug)]
 pub struct SpotifySongItemResponse {
-    pub track: SpotifySongResponse,
+    pub track: Option<SpotifySongResponse>,
 }
 
 #[derive(Deserialize, Debug)]

src/spotify/response.rs

@@ -1,6 +1,6 @@
 use std::convert::TryInto;
 
-use color_eyre::eyre::{Error, Result};
+use color_eyre::eyre::{Error, OptionExt, Result};
 use serde::Deserialize;
 use tracing::{debug, error};
 
@@ -60,7 +60,7 @@ where
             };
             // either an invalid or deleted song
             if song.id.is_empty() || song.duration_ms == 0 || song.isrc.is_none() {
-                debug!("song with invalid metadata, skipping it: {}", song.name);
+                debug!("song with invalid metadata, skipping it: '{}'", song.name);
                 continue;
             }
             res.push(song);
@@ -87,7 +87,7 @@ impl TryInto<Song> for SpotifySongItemResponse {
     type Error = Error;
 
     fn try_into(self) -> Result<Song, Self::Error> {
-        self.track.try_into()
+        self.track.ok_or_eyre("null track metadata")?.try_into()
     }
 }
 

src/sync.rs

@@ -3,6 +3,7 @@ use serde_json::json;
 use tracing::{debug, info, warn};
 
 use crate::music_api::{DynMusicApi, MusicApiType, Song};
+use crate::utils::dedup_songs;
 use crate::ConfigArgs;
 
 // TODO: Parse playlist owner to ignore platform-specific playlists?
@@ -50,8 +51,8 @@ pub async fn synchronize(
     let mut no_albums = json!({});
     let mut stats = json!({});
 
-    for src_playlist in src_playlists
-        .iter()
+    for mut src_playlist in src_playlists
+        .into_iter()
         .filter(|p| !SKIPPED_PLAYLISTS.contains(&p.name.as_str()) && !p.songs.is_empty())
     {
         if src_playlist.songs.is_empty() {
@@ -73,6 +74,13 @@ pub async fn synchronize(
         let mut success = 0;
         let mut attempts = 0;
 
+        if dedup_songs(&mut src_playlist.songs) {
+            warn!(
+                "duplicates found in source playlist \"{}\", they will be skipped",
+                src_playlist.name
+            );
+        }
+
         info!("synchronizing playlist \"{}\" ...", src_playlist.name);
         for src_song in src_playlist.songs.iter() {
             if dst_playlist.songs.contains(src_song) {
@@ -105,7 +113,16 @@ pub async fn synchronize(
             };
             // HACK: takes into account discrepancy for YtMusic with no ISRC
             if dst_playlist.songs.contains(&dst_song) {
-                debug!("discrepancy, song already in playlist: {}", dst_song.name);
+                debug!(
+                    "discrepancy, song already in destination playlist: {}",
+                    dst_song.name
+                );
+                continue;
+            }
+            // Edge case: same song on different album/single that all resolve to the same
+            // song on the destination platform
+            if dst_songs.contains(&dst_song) {
+                debug!("song already in dst_songs: {}", dst_song.name);
                 continue;
             }
 

src/utils.rs

@@ -1,5 +1,7 @@
 use regex::Regex;
 
+use crate::music_api::Song;
+
 pub fn clean_enclosure(name: &str, start_tag: char, end_tag: char) -> String {
     if name.contains(start_tag) {
         let mut res = vec![];
@@ -51,6 +53,23 @@ pub fn generic_name_clean(name: &str) -> String {
     name.trim_end().to_string()
 }
 
+pub fn dedup_songs(songs: &mut Vec<Song>) -> bool {
+    let mut seen = std::collections::HashSet::new();
+    let mut dups = false;
+    let mut i = 0;
+    let mut len = songs.len();
+    while i < len {
+        if !seen.insert(songs[i].id.clone()) {
+            songs.remove(i);
+            dups = true;
+            len -= 1;
+        } else {
+            i += 1;
+        }
+    }
+    dups
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;