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

add SearchRaw to return raw JSON from search #377

Merged
merged 2 commits into from Nov 8, 2022

Conversation

ginglis13
Copy link
Contributor

adds a method SearchRaw with the same signature as Search except it returns json.RawMessage

Pull Request

Related issue

Fixes #337

What does this PR do?

  • add SearchIndexRaw to index interface as a method to return a json.RawMessage from a search
  • adds unit test for the method. The test has a want of type SearchResponse; this is because to verify the method, the test is unmarshalling the raw response returned into a SearchResponse

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

adds a method SearchRaw with the same signature as
SearchIndex except it returns json.RawMessage
Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @ginglis13 for this PR it will be a really great addition 🚀
I just made a suggestion 😊

Comment on lines +50 to +62
req := internalRequest{
endpoint: "/indexes/" + i.UID + "/search",
method: http.MethodPost,
contentType: contentTypeJSON,
withRequest: searchPostRequestParams,
withResponse: resp,
acceptedStatusCodes: []int{http.StatusOK},
functionName: "Search",
}

if err := i.client.executeRequest(req); err != nil {
return nil, err
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this method can call directly SearchRaw and Unmarshal the result in SearchResponse to avoid duplicate code, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah great point... I was essentially doing that in the unit test but not the actual implementation 😅 I'll take your suggestion and make that change

@alallema
Copy link
Contributor

Hi @ginglis13,
FYI, I will not be available from tonight until November 7th. Also, I already put the label hacktoberfest-accepted on your PR in case I won't have time to merge it before the end of the hacktoberfest 🥳

@trim21
Copy link
Contributor

trim21 commented Nov 4, 2022

Hi, have you consider just change Hits field to json.RawMessage to use other fields? So if want to get EstimatedTotalHits, so no need to unmarshal SearchResponse again.

// SearchResponseRaw is the response body for search method without decoding Hits.
type SearchResponseRaw struct {
	Hits               []json.RawMessage `json:"hits"`
	EstimatedTotalHits int64             `json:"estimatedTotalHits"`
	Offset             int64             `json:"offset"`
	Limit              int64             `json:"limit"`
	ProcessingTimeMs   int64             `json:"processingTimeMs"`
	Query              string            `json:"query"`
	FacetDistribution  json.RawMessage   `json:"facetDistribution,omitempty"`
}

@alallema
Copy link
Contributor

alallema commented Nov 8, 2022

Hi, have you consider just change Hits field to json.RawMessage to use other fields? So if want to get EstimatedTotalHits, so no need to unmarshal SearchResponse again.

Yes, but for me, it was simpler to propose a method that returns either a ready object with the existing structure in the SDK or returns a complete object in json. RawMessage so that the user has the choice to receive either a complete raw object or a pre-formatted object.

Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @ginglis13 for this PR and the great addition to this SDK 🎉
LGTM! 🔥
Thanks again for contributing with Meilisearch ❤️
If you are participating in Hacktoberfest, and you would like to receive some swag from Meilisearch too, please complete this form.

@alallema
Copy link
Contributor

alallema commented Nov 8, 2022

bors merge

@bors
Copy link
Contributor

bors bot commented Nov 8, 2022

@bors bors bot merged commit 7fc467d into meilisearch:main Nov 8, 2022
@alallema alallema added the enhancement New feature or request label Nov 8, 2022
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

Successfully merging this pull request may close these issues.

a json.RawMessage search response's hit fields
3 participants