-
Notifications
You must be signed in to change notification settings - Fork 25k
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
LLClient: Add String flavored setEntity #30447
Changes from 1 commit
9e54247
01e983c
84def91
04e9700
2fe40c5
ea564a1
6122f94
96ea9bc
d946a6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,8 +19,10 @@ | |
|
||
package org.elasticsearch.client; | ||
|
||
import org.apache.http.entity.ContentType; | ||
import org.apache.http.Header; | ||
import org.apache.http.HttpEntity; | ||
import org.apache.http.nio.entity.NStringEntity; | ||
import org.apache.http.nio.protocol.HttpAsyncResponseConsumer; | ||
|
||
import java.util.Arrays; | ||
|
@@ -103,6 +105,17 @@ public void setEntity(HttpEntity entity) { | |
this.entity = entity; | ||
} | ||
|
||
/** | ||
* Set the body of the request to a string. If not set or set to | ||
* {@code null} then no body is sent with the request. The | ||
* {@code Content-Type} will be sent as {@code application/json}. | ||
* If you need a different content type then use | ||
* {@link #setEntity(HttpEntity)}. | ||
*/ | ||
public void setEntity(String entity) { | ||
setEntity(entity == null ? null : new NStringEntity(entity, ContentType.APPLICATION_JSON)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add a small unit test for this method? |
||
} | ||
|
||
/** | ||
* The body of the request. If {@code null} then no body | ||
* is sent with the request. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,10 +19,8 @@ | |
|
||
package org.elasticsearch.index.reindex.remote; | ||
|
||
import org.apache.http.entity.ByteArrayEntity; | ||
import org.apache.http.entity.ContentType; | ||
import org.apache.http.entity.StringEntity; | ||
import org.apache.lucene.util.BytesRef; | ||
import org.apache.http.nio.entity.NStringEntity; | ||
import org.elasticsearch.ElasticsearchException; | ||
import org.elasticsearch.Version; | ||
import org.elasticsearch.action.search.SearchRequest; | ||
|
@@ -151,8 +149,7 @@ static Request initialSearch(SearchRequest searchRequest, BytesReference query, | |
} | ||
|
||
entity.endObject(); | ||
BytesRef bytes = BytesReference.bytes(entity).toBytesRef(); | ||
request.setEntity(new ByteArrayEntity(bytes.bytes, bytes.offset, bytes.length, ContentType.APPLICATION_JSON)); | ||
request.setEntity(Strings.toString(entity)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we support as well the method for bytes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ha! Just saw your other comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on this one I am on the fence. It is odd that we have to go from bytes to string here. I would rather leave what you had, or add a method to set a byte array, even if we are the only ones using it. |
||
} catch (IOException e) { | ||
throw new ElasticsearchException("unexpected error building entity", e); | ||
} | ||
|
@@ -199,15 +196,15 @@ static Request scroll(String scroll, TimeValue keepAlive, Version remoteVersion) | |
|
||
if (remoteVersion.before(Version.fromId(2000099))) { | ||
// Versions before 2.0.0 extract the plain scroll_id from the body | ||
request.setEntity(new StringEntity(scroll, ContentType.TEXT_PLAIN)); | ||
request.setEntity(new NStringEntity(scroll, ContentType.TEXT_PLAIN)); | ||
return request; | ||
} | ||
|
||
try (XContentBuilder entity = JsonXContent.contentBuilder()) { | ||
entity.startObject() | ||
.field("scroll_id", scroll) | ||
.endObject(); | ||
request.setEntity(new StringEntity(Strings.toString(entity), ContentType.APPLICATION_JSON)); | ||
request.setEntity(Strings.toString(entity)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we support the method for XContent as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. low-level client doesn't depend on xcontent, we can't do that |
||
} catch (IOException e) { | ||
throw new ElasticsearchException("failed to build scroll entity", e); | ||
} | ||
|
@@ -219,14 +216,14 @@ static Request clearScroll(String scroll, Version remoteVersion) { | |
|
||
if (remoteVersion.before(Version.fromId(2000099))) { | ||
// Versions before 2.0.0 extract the plain scroll_id from the body | ||
request.setEntity(new StringEntity(scroll, ContentType.TEXT_PLAIN)); | ||
request.setEntity(new NStringEntity(scroll, ContentType.TEXT_PLAIN)); | ||
return request; | ||
} | ||
try (XContentBuilder entity = JsonXContent.contentBuilder()) { | ||
entity.startObject() | ||
.array("scroll_id", scroll) | ||
.endObject(); | ||
request.setEntity(new StringEntity(Strings.toString(entity), ContentType.APPLICATION_JSON)); | ||
request.setEntity(Strings.toString(entity)); | ||
} catch (IOException e) { | ||
throw new ElasticsearchException("failed to build clear scroll entity", e); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: call it setJsonEntity?