-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
response_type! macro doesn't allow for top level Array responses #33
Comments
Thanks for raising this! Not sure if it'd be of any help, but there is the following documentation on writing macros: https://veykril.github.io/tlborm/ |
Temporary fixed it with a generic struct. Which may(?) be better than a macro, as the responses aren't edited at all from the LB documentaion But yeah... Learning macro is something that needs a lot of time still /// A generic response type for listenbrainz responses
#[derive(Deserialize)]
pub struct ListenbrainzResponse<T: Deserialize> {
pub rate_limit: Option<RateLimit>,
pub data: T
}
impl<T: Deserialize> ResponseType for ListenbrainzResponse<T> {
fn from_response(response: Response) -> Result<Self, Error> {
let response = Error::try_from_error_response(response)?;
let mut inner_data: T = response.json()?;
let rate_limit = RateLimit::from_headers(&response);
Ok(Self {
rate_limit,
data: inner_data,
})
}
} |
Ooh I like that approach, does this work for every response? Because the less macro use the better, honestly 😄 |
It clippy doesn't complain... So It's fine. Then you just need to put your Vec into a type: // ---------- popularity/recording
pub type PopularityRecordingResponse = Vec<PopularityTopRecordingsForArtistResponseItem>;
#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)]
pub struct PopularityRecordingResponseItem {
pub recording_mbid: String,
pub total_listen_count: u64,
pub total_user_count: u64
} But well... A PR is better than words, so I'm going to make a draft PR to expose the changes |
Perfect! |
I really like this approach. I detest macros, generics FTW! Really pleased with the code sample above @RustyNova016 - great job. Let's get the remaining PRs in, then work out a strategy on refactoring to use generics. We should open a tracking issue, and link each PR in with that. |
Macros are great, but not developer friendly. I got an issue in the musicbrainz crate and can't fix it because of a complicated macro system... Eitherway, I need to do the generic way for #34, so that's one less to do. |
The current response_type! macro is built in such a way that it only JSON that has an object at its top level can be parsed. However, new endpoints were made that return a top level Array instead: https://listenbrainz.readthedocs.io/en/latest/users/api/popularity.html
While I'm able to implement the endpoints, I have no experience in macros. Would be nice to have it fixed so I can implement them.
The text was updated successfully, but these errors were encountered: