Skip to content

Provide a generic to http get methods #466

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

Merged
merged 13 commits into from
Oct 26, 2022
Merged

Provide a generic to http get methods #466

merged 13 commits into from
Oct 26, 2022

Conversation

alallema
Copy link
Contributor

@alallema alallema commented Oct 24, 2022

Pull Request

What does this PR do?

Most of the methods had to use the jsonHandler after the call to the get method through the client. The purpose of this PR is to bypass this step to simplify the call to the get method via the client:

    public String[] getRankingRuleSettings(String uid) throws MeilisearchException {
        String urlPath = "/indexes/" + uid + "/settings/synonyms";
        return httpClient.jsonHandler.decode(httpClient.get(urlPath, String.class), String[].class);
    }

became

    public String[] getRankingRuleSettings(String uid) throws MeilisearchException {
        String urlPath = "/indexes/" + uid + "/settings/ranking-rules";
        return httpClient.get(urlPath, String[].class);
    }

Note:

This method still need a call to the decode method from jsonHandler

    public Map<String, String[]> getSynonymsSettings(String uid) throws MeilisearchException {
        String urlPath = "/indexes/" + uid + "/settings/synonyms";
        return httpClient.jsonHandler.decode(httpClient.get(urlPath, String.class), Map.class);
    }

@alallema alallema force-pushed the patch-get-method branch 3 times, most recently from 6f0b0e3 to 7eadf5c Compare October 24, 2022 10:03
@alallema alallema requested a review from brunoocasali October 24, 2022 10:08
@alallema alallema mentioned this pull request Oct 24, 2022
@alallema alallema linked an issue Oct 24, 2022 that may be closed by this pull request
Result<Task> result =
httpClient.jsonHandler.decode(
this.httpClient.get(urlPath), Result.class, Task.class);
Result<Task> result = httpClient.get(urlPath, Result.class, Task.class);
Copy link
Member

Choose a reason for hiding this comment

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

I think you can return directly like you did in the other places :)

Copy link
Contributor Author

@alallema alallema Oct 25, 2022

Choose a reason for hiding this comment

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

I hesitate because it is a complex type and this is the line that generates the Type safety warning so maybe it's better to remove it when the warning will be fix

@@ -20,6 +20,6 @@ public boolean hasContent() {
}

public byte[] getContentAsBytes() {
return content.getBytes();
return ((String) content).getBytes();
Copy link
Member

Choose a reason for hiding this comment

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

How can you ensure this content will be castable to a String?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be, anyway I don't use this method maybe it's better to remove it? WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Of course, if you don't use it, remove it! ;D

Result<Task> result =
httpClient.jsonHandler.decode(
this.httpClient.get(urlPath), Result.class, Task.class);
Result<Task> result = httpClient.get(urlPath, Result.class, Task.class);
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as I sent below :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same I hesitate because it is a complex type and this is the line that generates the Type safety warning

@alallema alallema requested a review from brunoocasali October 25, 2022 09:04
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

🎉 🎉 🎉

@alallema alallema merged commit 5380e4e into refactor Oct 26, 2022
@alallema alallema deleted the patch-get-method branch October 26, 2022 14:47
@alallema alallema added the enhancement New feature or request label Nov 9, 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.

Add possibility to provide a generic to http methods
2 participants