[PR #317] [MERGED] Deserialize Id types via an ephemeral string slice instead of one borrowed from the input #393

New issue

Closed

opened 2026-02-27 20:24:32 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:24:32 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/317
Author: @semicoleon
Created: 5/21/2022
Status: ✅ Merged
Merged: 5/23/2022
Merged by: @ramsayleung

Base: master ← Head: borrowed-id-str

📝 Commits (1)

2af2f4b Deserialize Id types via an ephemeral string slice instead of one borrowed from the input

📊 Changes

2 files changed (+46 additions, -23 deletions)

View changed files

📝 rspotify-model/src/idtypes.rs (+10 -3)
📝 tests/test_models.rs (+36 -20)

📄 Description

Description

Call deserialize_str instead of deserializing a &str directly
Closes #316

Motivation and Context

Currently the id types can only be deserialized when they can be borrowed directly from the input, but they don't actually need to have a long lived borrow of the string. The ephemeral &str from deserialize_str is sufficient.

Dependencies

N/A

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)

How has this been tested?

I added a function to test_models.rs which calls both serde_json::from_str and serde_json::from_reader and replaced the calls to serde_json::from_str with it. I was originally asserting the outputs were equal but test_current_playback_context and test_currently_playing_context consistently failed, possibly due to floating point imprecision?

Is this change properly documented?

I'm not sure if this needs to be added to the changelog. It's relatively unlikely that anyone was hitting this issue in the first place.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/ramsayleung/rspotify/pull/317 **Author:** [@semicoleon](https://github.com/semicoleon) **Created:** 5/21/2022 **Status:** ✅ Merged **Merged:** 5/23/2022 **Merged by:** [@ramsayleung](https://github.com/ramsayleung) **Base:** `master` ← **Head:** `borrowed-id-str` --- ### 📝 Commits (1) - [`2af2f4b`](https://github.com/ramsayleung/rspotify/commit/2af2f4b79cff08f19fc553311a7f09bc11bdeb7d) Deserialize Id types via an ephemeral string slice instead of one borrowed from the input ### 📊 Changes **2 files changed** (+46 additions, -23 deletions) <details> <summary>View changed files</summary> 📝 `rspotify-model/src/idtypes.rs` (+10 -3) 📝 `tests/test_models.rs` (+36 -20) </details> ### 📄 Description ## Description Call deserialize_str instead of deserializing a &str directly Closes #316 ## Motivation and Context Currently the id types can only be deserialized when they can be borrowed directly from the input, but they don't actually need to have a long lived borrow of the string. The ephemeral &str from deserialize_str is sufficient. ## Dependencies N/A ## Type of change Please delete options that are not relevant. - [x] Bug fix (non-breaking change which fixes an issue) ## How has this been tested? I added a function to `test_models.rs` which calls both serde_json::from_str and serde_json::from_reader and replaced the calls to serde_json::from_str with it. I was originally asserting the outputs were equal but `test_current_playback_context` and `test_currently_playing_context` consistently failed, possibly due to floating point imprecision? ## Is this change properly documented? I'm not sure if this needs to be added to the changelog. It's relatively unlikely that anyone was hitting this issue in the first place. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>