honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

multi get semantics of empty/null fields missing 

the semantics between null fields (asking for source), and empty fields (not asking for anything) is missing
also exposes the items in the request, relates to #3061
This commit is contained in:
Shay Banon 2013-05-20 05:09:36 -07:00
parent 31f0aca65d
commit d983389090
2 changed files with 76 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/main/java/org/elasticsearch/action/get/MultiGetRequest.java
View File

@ -20,6 +20,7 @@
package org.elasticsearch.action.get; package org.elasticsearch.action.get;
import org.elasticsearch.ElasticSearchIllegalArgumentException; import org.elasticsearch.ElasticSearchIllegalArgumentException;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.ValidateActions; import org.elasticsearch.action.ValidateActions;
@ -34,9 +35,10 @@ import org.elasticsearch.common.xcontent.XContentParser;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Iterator;
import java.util.List; import java.util.List;
public class MultiGetRequest extends ActionRequest<MultiGetRequest> { public class MultiGetRequest extends ActionRequest<MultiGetRequest> implements Iterable<MultiGetRequest.Item> {
/** /**
* A single get item. * A single get item.
@ -112,18 +114,26 @@ public class MultiGetRequest extends ActionRequest<MultiGetRequest> {
@Override @Override
public void readFrom(StreamInput in) throws IOException { public void readFrom(StreamInput in) throws IOException {
index = in.readString(); index = in.readString();
if (in.readBoolean()) { type = in.readOptionalString();
type = in.readString();
}
id = in.readString(); id = in.readString();
if (in.readBoolean()) { routing = in.readOptionalString();
routing = in.readString(); if (in.getVersion().before(Version.V_0_90_1)) {
} int size = in.readVInt();
int size = in.readVInt(); if (size > 0) {
if (size > 0) { fields = new String[size];
fields = new String[size]; for (int i = 0; i < size; i++) {
for (int i = 0; i < size; i++) { fields[i] = in.readString();
fields[i] = in.readString(); }
}
} else {
// post 0.90.1, we maintain the semantic difference between not setting fields (default to _source)
// and setting fields to empty array (no data to return)
int size = in.readInt();
if (size >= 0) {
fields = new String[size];
for (int i = 0; i < size; i++) {
fields[i] = in.readString();
}
} }
} }
} }
@ -131,25 +141,26 @@ public class MultiGetRequest extends ActionRequest<MultiGetRequest> {
@Override @Override
public void writeTo(StreamOutput out) throws IOException { public void writeTo(StreamOutput out) throws IOException {
out.writeString(index); out.writeString(index);
if (type == null) { out.writeOptionalString(type);
out.writeBoolean(false);
} else {
out.writeBoolean(true);
out.writeString(type);
}
out.writeString(id); out.writeString(id);
if (routing == null) { out.writeOptionalString(routing);
out.writeBoolean(false); if (out.getVersion().before(Version.V_0_90_1)) {
if (fields == null) {
out.writeVInt(0);
} else {
out.writeVInt(fields.length);
for (String field : fields) {
out.writeString(field);
}
}
} else { } else {
out.writeBoolean(true); if (fields == null) {
out.writeString(routing); out.writeInt(-1);
} } else {
if (fields == null) { out.writeInt(fields.length);
out.writeVInt(0); for (String field : fields) {
} else { out.writeString(field);
out.writeVInt(fields.length); }
for (String field : fields) {
out.writeString(field);
} }
} }
} }
@ -163,6 +174,11 @@ public class MultiGetRequest extends ActionRequest<MultiGetRequest> {
List<Item> items = new ArrayList<Item>(); List<Item> items = new ArrayList<Item>();
@Override
public Iterator<Item> iterator() {
return items.iterator();
}
public MultiGetRequest add(Item item) { public MultiGetRequest add(Item item) {
items.add(item); items.add(item);
return this; return this;

53
src/test/java/org/elasticsearch/test/integration/get/GetActionTests.java
View File

@ -37,8 +37,6 @@ import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test; import org.testng.annotations.Test;
import java.util.List;
import static org.elasticsearch.client.Requests.clusterHealthRequest; import static org.elasticsearch.client.Requests.clusterHealthRequest;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
@ -201,6 +199,17 @@ public class GetActionTests extends AbstractNodesTests {
assertThat(response.getResponses().length, equalTo(2)); assertThat(response.getResponses().length, equalTo(2));
assertThat(response.getResponses()[0].getResponse().getSourceAsBytes(), nullValue()); assertThat(response.getResponses()[0].getResponse().getSourceAsBytes(), nullValue());
assertThat(response.getResponses()[0].getResponse().getField("field").getValues().get(0).toString(), equalTo("value1")); assertThat(response.getResponses()[0].getResponse().getField("field").getValues().get(0).toString(), equalTo("value1"));
// multi get with "no" fields, nothing should return, exists indication should still be around
response = client.prepareMultiGet()
.add(new MultiGetRequest.Item("test", "type1", "1").fields(Strings.EMPTY_ARRAY))
.add(new MultiGetRequest.Item("test", "type1", "3").fields(Strings.EMPTY_ARRAY))
.execute().actionGet();
assertThat(response.getResponses().length, equalTo(2));
assertThat(response.getResponses()[0].getResponse().isExists(), equalTo(true));
assertThat(response.getResponses()[0].getResponse().getSourceAsBytes(), nullValue());
assertThat(response.getResponses()[0].getResponse().getFields().size(), equalTo(0));
} }
@Test @Test
@ -412,13 +421,13 @@ public class GetActionTests extends AbstractNodesTests {
String mapping = jsonBuilder() String mapping = jsonBuilder()
.startObject() .startObject()
.startObject("source_excludes") .startObject("source_excludes")
.startObject("_source") .startObject("_source")
.array("excludes", "excluded") .array("excludes", "excluded")
.endObject()
.endObject()
.endObject() .endObject()
.string(); .endObject()
.endObject()
.string();
client.admin().indices().prepareCreate(index) client.admin().indices().prepareCreate(index)
.addMapping(type, mapping) .addMapping(type, mapping)
@ -447,14 +456,14 @@ public class GetActionTests extends AbstractNodesTests {
String type = "type1"; String type = "type1";
String mapping = jsonBuilder() String mapping = jsonBuilder()
.startObject() .startObject()
.startObject("source_excludes") .startObject("source_excludes")
.startObject("_source") .startObject("_source")
.array("includes", "included") .array("includes", "included")
.endObject()
.endObject() .endObject()
.endObject() .endObject()
.string(); .endObject()
.string();
client.admin().indices().prepareCreate(index) client.admin().indices().prepareCreate(index)
.addMapping(type, mapping) .addMapping(type, mapping)
@ -483,15 +492,15 @@ public class GetActionTests extends AbstractNodesTests {
String type = "type1"; String type = "type1";
String mapping = jsonBuilder() String mapping = jsonBuilder()
.startObject() .startObject()
.startObject("source_excludes") .startObject("source_excludes")
.startObject("_source") .startObject("_source")
.array("includes", "included") .array("includes", "included")
.array("exlcudes", "excluded") .array("exlcudes", "excluded")
.endObject()
.endObject() .endObject()
.endObject() .endObject()
.string(); .endObject()
.string();
client.admin().indices().prepareCreate(index) client.admin().indices().prepareCreate(index)
.addMapping(type, mapping) .addMapping(type, mapping)
@ -503,7 +512,7 @@ public class GetActionTests extends AbstractNodesTests {
.field("field", "1", "2") .field("field", "1", "2")
.field("included", "should be seen") .field("included", "should be seen")
.field("excluded", "should not be seen") .field("excluded", "should not be seen")
.endObject()) .endObject())
.execute().actionGet(); .execute().actionGet();
GetResponse responseBeforeFlush = client.prepareGet(index, type, "1").setFields("_source", "included", "excluded").execute().actionGet(); GetResponse responseBeforeFlush = client.prepareGet(index, type, "1").setFields("_source", "included", "excluded").execute().actionGet();