Skip to content
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

Query builder does not fit musicbrainz lucene query for certain entities #58

Closed
oknozor opened this issue Jul 8, 2021 · 3 comments
Closed
Labels
enhancement New feature or request

Comments

@oknozor
Copy link
Owner

oknozor commented Jul 8, 2021

The QueryBuilder derive macro we use for lucene query does not fit with some of the search queries :

For instance release-groups search accept the following fields :

  • artist : (part of) the combined credited artist name for the release group, including join phrases (e.g. "Artist X feat.")
  • artistname : (part of) the name of any of the release group artists
  • creditname : (part of) the credited name of any of the release group artists on this particular release group

While top level fields of the ReleaseGroup entity (on musicbrainz_rs side) only refers to artists via artist_credit.

We end up with a single query parameter artist_credit in the release group generated builder. We can rename it with the current implementation but we still lacks two artists related fields.

In my opinion the best approach would be to implement this manually and get rid of the query builder macro.

@oknozor oknozor added the enhancement New feature or request label Jul 8, 2021
@ritiek
Copy link
Collaborator

ritiek commented Jul 8, 2021

I see. If this is the case, would it be a good idea to create a different struct for each entity made specifically for search purpose?

I mean ReleaseGroup has all the searchable fields on the api defined here. So we'd create a new struct containing all these searchable fields:

use lucene_query_builder::QueryBuilder;
use crate::entity::alias::Alias;
...

#[derive(Debug, Serialize, Deserialize, PartialEq, Clone, QueryBuilder, Default)]
struct ReleaseGroupSearch {
    alias: Alias,
    arid: String,
    artist: String,
    ...
}

and then we could make use of these structs when making searches:

use musicbrainz_rs::entity::release_group::ReleaseGroupSearch;

let query = ReleaseGroupSearch::query_builder()
    .arid("3377f3bb-60fc-4403-aea9-7e800612e060")
    .and()
    .artist("the chainsmokers")
    .build();

(here arid refers to Halsey)

which would result this query: query=arid:3377f3bb-60fc-4403-aea9-7e800612e060 AND artist:"the chainsmokers"

Does this sound like a good idea? Otherwise we can already write our own queries manually this way:

let query_param = "arid:3377f3bb-60fc-4403-aea9-7e800612e060 AND artist:\"the chainsmokers\"".to_string();
let query = format!("query={}", query_param);

@oknozor
Copy link
Owner Author

oknozor commented Jul 14, 2021

Implementing a separate search struct and still using the query builder derives seems to be a great compromise, let's go for it !

@oknozor
Copy link
Owner Author

oknozor commented Aug 20, 2021

implemented in #69

@oknozor oknozor closed this as completed Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants