Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

added document index, type and id to crud operations #221

Merged
merged 2 commits into from
Jul 12, 2015
Merged

added document index, type and id to crud operations #221

merged 2 commits into from
Jul 12, 2015

Conversation

bartekbp
Copy link
Contributor

No description provided.

@@ -14,7 +14,7 @@

<T extends JestResult> T execute(Action<T> clientRequest) throws IOException;

<T extends JestResult> void executeAsync(Action<T> clientRequest, JestResultHandler<T> jestResultHandler);
<T extends JestResult> void executeAsync(Action<T> clientRequest, JestResultHandler<? super T> jestResultHandler);
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for this (as T is already defined as T extends JestResult)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That way we can use more generic handlers - e.g. JestResultHandler when executeAsync is parametrized with DocumentResult.
By introducing new type DocumentResult I wanted to keep currently working handlers intact.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I missed that, good point.

@kramer
Copy link
Member

kramer commented Jul 12, 2015

Could you also please add assertions for getIndex, getType, and getId on the result in the related actions' integration tests?

@bartekbp
Copy link
Contributor Author

Done;)

kramer pushed a commit that referenced this pull request Jul 12, 2015
added document index, type and id to (single document targeting) crud operations
@kramer kramer merged commit 2866e74 into searchbox-io:master Jul 12, 2015
@kramer
Copy link
Member

kramer commented Jul 12, 2015

Great, thanks for your contribution @bartekbp !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants