starred/rspotify
1
0
Fork 0
mirror of https://github.com/ramsayleung/rspotify.git synced 2026-04-25 23:45:52 +03:00
Code Issues 15 Projects Releases 5 Packages Wiki Activity

[PR #145] [MERGED] Refactor model #269

New issue
Closed
opened 2026-02-27 20:24:02 +03:00 by kerem · 0 comments
kerem commented 2026-02-27 20:24:02 +03:00
Owner
Copy link

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/145
Author: @ramsayleung
Created: 10/28/2020
Status: ✅ Merged
Merged: 11/17/2020
Merged by: @marioortizmanero

Base: master ← Head: ramsay_refactor_model

📝 Commits (10+)

  • 1fa25d4 Make copyright as a struct instead of HashMap<String,String>
  • 41f1ce0 Merge branch 'master' into ramsay_refactor_model
  • 2ebe01b Add missing field is_private_session for Device.
  • c9a2de5 Fix broken model reference link.
  • 2f94301 Change PublicUser.images from Option<Vec<Image>> to Vec<Image>
  • 20e2e8f Add three missing fields for SimplifiedTrack.
  • ee5ea95 Update CHANGELOG.
  • 87764fa Set default value if PublicUser.images isn't present.
  • 92f8ed0 Add missing field product for PrivateUser.
  • 38cef90 Merge branch 'master' into ramsay_refactor_model

📊 Changes

26 files changed (+1282 additions, -322 deletions)

View changed files

📝 CHANGELOG.md (+86 -2)
📝 src/client.rs (+8 -6)
📝 src/model/album.rs (+22 -19)
📝 src/model/artist.rs (+17 -10)
📝 src/model/audio.rs (+40 -28)
📝 src/model/category.rs (+8 -5)
📝 src/model/context.rs (+39 -25)
➖ src/model/cud_result.rs (+0 -7)
📝 src/model/device.rs (+12 -6)
📝 src/model/enums/country.rs (+252 -14)
📝 src/model/enums/misc.rs (+27 -42)
📝 src/model/enums/mod.rs (+2 -2)
📝 src/model/enums/types.rs (+53 -44)
📝 src/model/image.rs (+4 -2)
📝 src/model/mod.rs (+26 -5)
📝 src/model/offset.rs (+4 -2)
📝 src/model/page.rs (+12 -9)
📝 src/model/playing.rs (+4 -13)
📝 src/model/playlist.rs (+28 -12)
📝 src/model/recommend.rs (+10 -14)

...and 6 more files

📄 Description

Refactor rspotify model to fix issues refering in #127

Most of newly added code are test cases.

  • Change copyright from HashMap<String, String> to struct Copyright
  • Derive PartialEq (and Eq if possible) for everything.
  • Device is missing the field is_private_session: bool
  • SimplfiiedTrack is missing the is_playable, linked_from and restrictions fields.
  • PublicUser::images should be a Vec<Image> and represent None as an empty Vec.
  • PrivateUser is missing the product field, which should be an enum Subscription { Premium, Free }.
  • IncludeExternal can just be replaced with a bool in the search function.
  • Restrictions is located in the album module, despite it not having any particular relevance to albums.
  • Single-item modules like image should have their items placed in the above scope.
  • Device::id and Device::volume_percent can be null in the Spotify API, so they should be an Option.
  • Playing should be replaced with CurrentlyPlayingContext, since it is the same.
  • PlaylistTrack::added_at can be null so should be Option.
  • PlaylistTrack is poorly named in the Spotify API. It should be called PlaylistItem since it can also contain episodes.
  • language shouldn't be in SimplifiedEpisode if it's deprecated.
  • This is more opinionated but AudioAnalysisSection and AudioAnalysisSegment could contain a #[serde::flatten]ed TimeInterval instead of the having the fields inline.
  • AudioAnalysisMeasure is identical to TimeInterval, and so should be replaced by it.
  • FullPlayingContext doesn't exist in the Spotify API. It should be replaced with CurrentlyPlaybackContext.
  • SimplifiedPlayingContext also doesn't exist, and should be replaced with CurrentlyPlayingContext.
  • CUDResult is oddly named; it should refer to playlists in its title. PlaylistResult might be a better name. But it's not even necessary at all, the functions can just return String directly.
  • {FullArtist, FullPlaylist}::followers is a HashMap<String, Option<Value>>. It should be a struct Followers { total: usize }.
  • Actions::disallows can be replaced with a Vec<DisallowKey> or HashSet<DisallowKey> by removing all entires whose value is false, which will result in a simpler API.
  • FullAlbum::release_date and FullAlbum::release_date_precision are Strings. Instead, it should be deserialized into a proper date type like NaiveDate and a enum DatePrecision { Year, Month, Day }. PS: release_date can't deserialized into a date type like NaiveDate, since it might only has year e.g. 2020, which can't convert to a date. String probably is a right choice.
  • The same applies to SimplifiedEpisode and FullEpisode.
  • Types like FullAlbums, PageSimplifiedAlbums, etc exist as wrappers. This makes the API more complex. Instead, have that type exist solely in the API function that needs it, and then return a Vec<FullAlbum>. This drastically reduces the number of public API types.

🔄 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/145 **Author:** [@ramsayleung](https://github.com/ramsayleung) **Created:** 10/28/2020 **Status:** ✅ Merged **Merged:** 11/17/2020 **Merged by:** [@marioortizmanero](https://github.com/marioortizmanero) **Base:** `master` ← **Head:** `ramsay_refactor_model` --- ### 📝 Commits (10+) - [`1fa25d4`](https://github.com/ramsayleung/rspotify/commit/1fa25d4f374f1ebbb280b88b5e93bd8afa9a21eb) Make copyright as a struct instead of HashMap<String,String> - [`41f1ce0`](https://github.com/ramsayleung/rspotify/commit/41f1ce014f871fcf1ecf9610c7eaeed5cdb3ad76) Merge branch 'master' into ramsay_refactor_model - [`2ebe01b`](https://github.com/ramsayleung/rspotify/commit/2ebe01b2a25efd649b2e0220eabd8d475fae79f5) Add missing field `is_private_session` for `Device`. - [`c9a2de5`](https://github.com/ramsayleung/rspotify/commit/c9a2de58a8b843b932c5b9563f7ee1a5da38d152) Fix broken model reference link. - [`2f94301`](https://github.com/ramsayleung/rspotify/commit/2f943017d300da3f3defc448440e9eeaa9a0682d) Change `PublicUser.images` from `Option<Vec<Image>>` to `Vec<Image>` - [`20e2e8f`](https://github.com/ramsayleung/rspotify/commit/20e2e8fc2aa124af1717ec1fe70465a15c73445e) Add three missing fields for `SimplifiedTrack`. - [`ee5ea95`](https://github.com/ramsayleung/rspotify/commit/ee5ea95b82830799f5783e6c921e909909ce951f) Update CHANGELOG. - [`87764fa`](https://github.com/ramsayleung/rspotify/commit/87764fa81c05a7c3f6e613a7c3262f5031ca00d5) Set default value if PublicUser.images isn't present. - [`92f8ed0`](https://github.com/ramsayleung/rspotify/commit/92f8ed01041b3ed23a9a1fb99c13ea9581bccaf5) Add missing field `product` for `PrivateUser`. - [`38cef90`](https://github.com/ramsayleung/rspotify/commit/38cef9091f30285667ccc42984952e6af328138b) Merge branch 'master' into ramsay_refactor_model ### 📊 Changes **26 files changed** (+1282 additions, -322 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+86 -2) 📝 `src/client.rs` (+8 -6) 📝 `src/model/album.rs` (+22 -19) 📝 `src/model/artist.rs` (+17 -10) 📝 `src/model/audio.rs` (+40 -28) 📝 `src/model/category.rs` (+8 -5) 📝 `src/model/context.rs` (+39 -25) ➖ `src/model/cud_result.rs` (+0 -7) 📝 `src/model/device.rs` (+12 -6) 📝 `src/model/enums/country.rs` (+252 -14) 📝 `src/model/enums/misc.rs` (+27 -42) 📝 `src/model/enums/mod.rs` (+2 -2) 📝 `src/model/enums/types.rs` (+53 -44) 📝 `src/model/image.rs` (+4 -2) 📝 `src/model/mod.rs` (+26 -5) 📝 `src/model/offset.rs` (+4 -2) 📝 `src/model/page.rs` (+12 -9) 📝 `src/model/playing.rs` (+4 -13) 📝 `src/model/playlist.rs` (+28 -12) 📝 `src/model/recommend.rs` (+10 -14) _...and 6 more files_ </details> ### 📄 Description Refactor `rspotify` model to fix issues refering in #127 Most of newly added code are test cases. + [x] Change `copyright` from `HashMap<String, String>` to `struct Copyright` + [x] Derive `PartialEq` (and `Eq` if possible) for everything. + [x] `Device` is missing the field `is_private_session: bool` + [x] `SimplfiiedTrack` is missing the `is_playable`, `linked_from` and `restrictions` fields. + [x] `PublicUser::images` should be a `Vec<Image>` and represent `None` as an empty `Vec`. + [x] `PrivateUser` is missing the `product` field, which should be an `enum Subscription { Premium, Free }`. + [ ] ~~`IncludeExternal` can just be replaced with a `bool` in the search function.~~ + [x] `Restrictions` is located in the `album` module, despite it not having any particular relevance to `albums`. + [ ] ~~Single-item modules like `image` should have their items placed in the above scope.~~ + [x] `Device::id` and `Device::volume_percent` can be null in the Spotify API, so they should be an Option. + [x] `Playing` should be replaced with `CurrentlyPlayingContext`, since it is the same. + [x] `PlaylistTrack::added_at` can be null so should be Option. + [x] `PlaylistTrack` is poorly named in the Spotify API. It should be called `PlaylistItem` since it can also contain episodes. + [x] `language` shouldn't be in `SimplifiedEpisode` if it's deprecated. + [x] This is more opinionated but `AudioAnalysisSection` and `AudioAnalysisSegment` could contain a `#[serde::flatten]`ed `TimeInterval` instead of the having the fields inline. + [x] `AudioAnalysisMeasure` is identical to `TimeInterval`, and so should be replaced by it. + [x] `FullPlayingContext` doesn't exist in the Spotify API. It should be replaced with `CurrentlyPlaybackContext`. + [x] `SimplifiedPlayingContext` also doesn't exist, and should be replaced with `CurrentlyPlayingContext`. + [x] `CUDResult` is oddly named; it should refer to playlists in its title. `PlaylistResult` might be a better name. But it's not even necessary at all, the functions can just return String directly. + [x] `{FullArtist, FullPlaylist}::followers` is a` HashMap<String, Option<Value>>`. It should be a struct `Followers { total: usize }`. + [x] `Actions::disallows` can be replaced with a `Vec<DisallowKey>` or `HashSet<DisallowKey>` by removing all entires whose value is false, which will result in a simpler API. + [x] `FullAlbum::release_date` and `FullAlbum::release_date_precision` are Strings. Instead, ~~it should be deserialized into a proper date type like `NaiveDate`~~ and a `enum DatePrecision { Year, Month, Day }`. PS: `release_date` can't deserialized into a date type like `NaiveDate`, since it might only has `year` e.g. `2020`, which can't convert to a date. `String` probably is a right choice. + [x] The same applies to `SimplifiedEpisode` and `FullEpisode`. + [ ] Types like `FullAlbums`, `PageSimplifiedAlbums`, etc exist as wrappers. This makes the API more complex. Instead, have that type exist solely in the API function that needs it, and then return a `Vec<FullAlbum>`. This drastically reduces the number of public API types. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-02-27 20:24:02 +03:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
ramsay/upgrade-0.16.1
dependabot/cargo/sha2-0.11.0
ramsay/upgrade-to-v0.16
ramsay/deprecate-removed-renamed-field
ramsay/migrate-to-new-endpoint
ramsay/deprecate-removed-endpoints
dependabot/cargo/strum-0.28.0
dependabot/cargo/reqwest-middleware-0.5.1
dependabot/cargo/reqwest-0.13.1
ramsay/upgrade-to-0.15.3
ramsay/upgrade-to-0.15.2
dependabot/cargo/ureq-3.1.2
ramsay/upgrade-dependencies
ramsay/upgrade-to-0.15.1
ramsay/fix-malformed-json
ramsay/upgrade-to-v0.15.0
ramsay/allow-enum-unknown-variant
ramsay/upgrade-to-0.14
ramsay/add_tcplistern_for_callback
ramsay/fix-clippy-error
ramsay/update-doc
ramsay/fix-endless-sequence
ramsay/publish-crate-workflow-test
ramsay/fix-pkce-refresh-token
ramsay/pkce_auth_refresh_token
ramsay_add_test_for_collectionyourepisodes
better-webapp
ramsay_fix_token_expired
percent-search
refresh_token_rwlock
ramsay_convert_result_inside_endpoint
streaming
v0.16.1
v0.16.0
v0.15.3
v0.15.2
v0.15.1
v0.15.0
v0.14.0
v0.13.3
v0.13.2
v0.13.1
v0.13.0
v0.12.0
v0.11.7
v0.11.6
v0.10.0
v0.11.5
v0.11.5.0
v0.11.4
v0.11.3
v0.11.2
v0.11.1
v0.11.0
v0.11.5.1
v0.9
v0.9-alpha
Labels
Clear labels   
Stale

   
bug

   
discussion

   
enhancement

   
good first issue

   
good first issue

   
help wanted

   
pull-request

Mirrored from GitHub Pull Request

  
question
No labels
Stale
bug
discussion
enhancement
good first issue
good first issue
help wanted
pull-request
question
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/rspotify#269
No description provided.