Added a builder like struct for the search function (Related with issue )

[GartoxFR]

Description

These changes add a struct to build well formatted query to pass to the search method (fixes #354).
These are not breaking changes as you can still pass a simple String to the method.
It can still be improve as it doesn't prevent users to pass wrong filter with wrong SearchType (give a track title to search an album for example, which is not supporte by Spotify API).

Motivation and Context

Make writing searches easier

Dependencies

Adding strum to the main project

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

How has this been tested?

I've modified one test case to use the struct instead of a basic &str. As the old test still passes, these changes are non breaking.

I also wrote a simple example which printed the String generated by the struct to verify that it was right with its inputs. This file is included in this PR but will probably be removed if it is accepted.

Please also list any relevant details for your test configuration

Is this change properly documented?

Please make sure you've properly documented the changes you're making.

Don't forget to add an entry to the CHANGELOG if necessary (new features, breaking changes, relevant internal improvements).

[ramsayleung]

Is it necessary to impose the strum crate? We are cautious about including a new crate, it will increase compile time.

[GartoxFR]

Will it really increase compile time ? As strum is built while building models which are built before the main crate. Anyway I think moving SearchFilter to the models is a good idea and will solve this problem

[ramsayleung]

I think it's not a full example, right? The SearchQuery should combine with the search endpoint?

[GartoxFR]

I tried an other approach to ensure type safety. Please tell me what you think about this before I try to implement testing and more example around it

[ramsayleung]

After rereading the Spotify's document, I think its explanation about the restriction between filter and result type is ambiguous, so I think it's better to leave filter and result type alone, roll back to your original design.

Just adding a builder for the query string, and we are unnecessary to include the search type into the builder.

[GartoxFR]

I reverted my previous changes to go back to the builder pattern.
I'm not used to writing test so can you give me ideas on how to write test for this ?
I can't just call the method into and compare the result to an expected query string because the order of the keys in the query string is undefined and can change

[ramsayleung]

In my opinion, there are two ways to test the SearchQuery builder.

I can't just call the method into and compare the result to an expected query string because the order of the keys in the query string is undefined and can change

1. Just making the order of the keys determined and predictable by replacing HashMap with BTreeMap which is a sorted map by itself. Then the complexity of querying if a key is in the map will reduce from O(1) to O(LogN), but I think it's acceptable.
2. Test the query string word by word, for example:

let query_string = SearchQuery::default().album("arrival").artist("abba").tag_new("test");

// the query_string could be:
// 1. `ablum:arrival artist:abba test`
// 2. `artist:abba ablum:arrival test`
// 3. `test artist:abba ablum:arrival`
// 4. ...

// we could split query string by whitespace and assert the splited strings contain `ablum:arrival` `artist:abba` `test`
let query_words: Vec<String> = query_string.split(' ').collect();
assert_eq!(3, query_words.len());
assert!(query_words.contains('ablum:arrival'));
assert!(query_words.contains('artist:abba'));
// ...

[GartoxFR]

I finally chose the BTreeMap because it's a bit tricky to split correctly the query as there can be spaces inside album, track..

[marioortizmanero]

Awesome idea, thanks for the contribution!!

[marioortizmanero]

To me, it'd be more intuitive if calling any multiple times appended its contents, instead of replace them.

[GartoxFR]

So any would comport differently from the other fields ? Or should I change this for all fields ?

[GartoxFR]

I'm a bit confused about what did just happen in the pipeline. It seems it complained about some code which is inside the documentation. I never really worked with rust doc so what happened ?

[ramsayleung]

The pipeline complained about that it's unable to find the definition of SearchQuery, so you should import the SearchQuery before using it:

/// Exemple
/// ```rust
/// use SearchQuery;
/// SearchQuery::default()
///     .any("foo")
///     .album("bar")
/// // Filter on album containing "bar" and anything containing "foo"
/// ```

[github-actions]

Message to comment on stale PRs. If none provided, will not mark PRs stale