Merge pull request #1079 from ealasu/feature/local-spotify-id

Fix bug where playlists containing a local track would fail to load
This commit is contained in:
Roderick van Domburg 2022-12-03 08:47:13 +01:00 committed by GitHub
commit 98c985ffab
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 11 deletions

View file

@ -90,7 +90,7 @@ jobs:
key: ${{ runner.os }}-${{ steps.get-rustc-version.outputs.version }}-${{ hashFiles('Cargo.lock') }} key: ${{ runner.os }}-${{ steps.get-rustc-version.outputs.version }}-${{ hashFiles('Cargo.lock') }}
- name: Install developer package dependencies - name: Install developer package dependencies
run: sudo apt-get update && sudo apt-get install libpulse-dev portaudio19-dev libasound2-dev libsdl2-dev gstreamer1.0-dev libgstreamer-plugins-base1.0-dev libavahi-compat-libdnssd-dev run: sudo apt-get update && sudo apt install -y libunwind-dev && sudo apt-get install libpulse-dev portaudio19-dev libasound2-dev libsdl2-dev gstreamer1.0-dev libgstreamer-plugins-base1.0-dev libavahi-compat-libdnssd-dev
- run: cargo build --workspace --examples - run: cargo build --workspace --examples
- run: cargo test --workspace - run: cargo test --workspace
@ -231,7 +231,7 @@ jobs:
key: ${{ runner.os }}-${{ steps.get-rustc-version.outputs.version }}-${{ hashFiles('Cargo.lock') }} key: ${{ runner.os }}-${{ steps.get-rustc-version.outputs.version }}-${{ hashFiles('Cargo.lock') }}
- name: Install developer package dependencies - name: Install developer package dependencies
run: sudo apt-get update && sudo apt-get install libpulse-dev portaudio19-dev libasound2-dev libsdl2-dev gstreamer1.0-dev libgstreamer-plugins-base1.0-dev libavahi-compat-libdnssd-dev run: sudo apt-get update && sudo apt install -y libunwind-dev && sudo apt-get install libpulse-dev portaudio19-dev libasound2-dev libsdl2-dev gstreamer1.0-dev libgstreamer-plugins-base1.0-dev libavahi-compat-libdnssd-dev
- run: cargo install cargo-hack - run: cargo install cargo-hack
- run: cargo hack --workspace --remove-dev-deps - run: cargo hack --workspace --remove-dev-deps

View file

@ -84,6 +84,7 @@ https://github.com/librespot-org/librespot
It supports a lot of functionality, including audio previews and image It supports a lot of functionality, including audio previews and image
downloads even if librespot doesn't use that for playback itself. downloads even if librespot doesn't use that for playback itself.
- [core] Support downloading of lyrics - [core] Support downloading of lyrics
- [core] Support parsing `SpotifyId` for local files
- [main] Add all player events to `player_event_handler.rs` - [main] Add all player events to `player_event_handler.rs`
- [main] Add an event worker thread that runs async to the main thread(s) but - [main] Add an event worker thread that runs async to the main thread(s) but
sync to itself to prevent potential data races for event consumers sync to itself to prevent potential data races for event consumers

View file

@ -21,6 +21,7 @@ pub enum SpotifyItemType {
Playlist, Playlist,
Show, Show,
Track, Track,
Local,
Unknown, Unknown,
} }
@ -33,6 +34,7 @@ impl From<&str> for SpotifyItemType {
"playlist" => Self::Playlist, "playlist" => Self::Playlist,
"show" => Self::Show, "show" => Self::Show,
"track" => Self::Track, "track" => Self::Track,
"local" => Self::Local,
_ => Self::Unknown, _ => Self::Unknown,
} }
} }
@ -47,6 +49,7 @@ impl From<SpotifyItemType> for &str {
SpotifyItemType::Playlist => "playlist", SpotifyItemType::Playlist => "playlist",
SpotifyItemType::Show => "show", SpotifyItemType::Show => "show",
SpotifyItemType::Track => "track", SpotifyItemType::Track => "track",
SpotifyItemType::Local => "local",
_ => "unknown", _ => "unknown",
} }
} }
@ -167,24 +170,30 @@ impl SpotifyId {
/// ///
/// [Spotify URI]: https://developer.spotify.com/documentation/web-api/#spotify-uris-and-ids /// [Spotify URI]: https://developer.spotify.com/documentation/web-api/#spotify-uris-and-ids
pub fn from_uri(src: &str) -> SpotifyIdResult { pub fn from_uri(src: &str) -> SpotifyIdResult {
let mut uri_parts: Vec<&str> = src.split(':').collect();
// At minimum, should be `spotify:{type}:{id}` // At minimum, should be `spotify:{type}:{id}`
if uri_parts.len() < 3 { let (scheme, tail) = src.split_once(':').ok_or(SpotifyIdError::InvalidFormat)?;
return Err(SpotifyIdError::InvalidFormat.into()); let (item_type, id) = tail.split_once(':').ok_or(SpotifyIdError::InvalidFormat)?;
}
if uri_parts[0] != "spotify" { if scheme != "spotify" {
return Err(SpotifyIdError::InvalidRoot.into()); return Err(SpotifyIdError::InvalidRoot.into());
} }
let id = uri_parts.pop().unwrap_or_default(); let item_type = item_type.into();
// Local files have a variable-length ID: https://developer.spotify.com/documentation/general/guides/local-files-spotify-playlists/
// TODO: find a way to add this local file ID to SpotifyId.
// One possible solution would be to copy the contents of `id` to a new String field in SpotifyId,
// but then we would need to remove the derived Copy trait, which would be a breaking change.
if item_type == SpotifyItemType::Local {
return Ok(Self { item_type, id: 0 });
}
if id.len() != Self::SIZE_BASE62 { if id.len() != Self::SIZE_BASE62 {
return Err(SpotifyIdError::InvalidId.into()); return Err(SpotifyIdError::InvalidId.into());
} }
Ok(Self { Ok(Self {
item_type: uri_parts.pop().unwrap_or_default().into(), item_type,
..Self::from_base62(id)? ..Self::from_base62(id)?
}) })
} }
@ -534,7 +543,7 @@ mod tests {
raw: &'static [u8], raw: &'static [u8],
} }
static CONV_VALID: [ConversionCase; 4] = [ static CONV_VALID: [ConversionCase; 5] = [
ConversionCase { ConversionCase {
id: 238762092608182713602505436543891614649, id: 238762092608182713602505436543891614649,
kind: SpotifyItemType::Track, kind: SpotifyItemType::Track,
@ -575,6 +584,14 @@ mod tests {
154, 27, 28, 251, 198, 242, 68, 86, 154, 224, 53, 108, 119, 187, 233, 216, 154, 27, 28, 251, 198, 242, 68, 86, 154, 224, 53, 108, 119, 187, 233, 216,
], ],
}, },
ConversionCase {
id: 0,
kind: SpotifyItemType::Local,
uri: "spotify:local:0000000000000000000000",
base16: "00000000000000000000000000000000",
base62: "0000000000000000000000",
raw: &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
},
]; ];
static CONV_INVALID: [ConversionCase; 3] = [ static CONV_INVALID: [ConversionCase; 3] = [
@ -676,6 +693,14 @@ mod tests {
} }
} }
#[test]
fn from_local_uri() {
let actual = SpotifyId::from_uri("spotify:local:xyz:123").unwrap();
assert_eq!(actual.id, 0);
assert_eq!(actual.item_type, SpotifyItemType::Local);
}
#[test] #[test]
fn to_uri() { fn to_uri() {
for c in &CONV_VALID { for c in &CONV_VALID {