From 4d66575abe4278d6016873cf9ac33b1fcb891a9b Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Wed, 8 May 2013 17:54:21 -0400 Subject: [PATCH] Make GetField behavior more consitent for multivalued fields. Before this change, the GetField#getValue() method was returning a list of values of a multivalued fields if the field values were obtained from source or if the field was stored and real-time get was used. If the field was stored but non-realtime get was used, GetField#getValue() was returning only the first element and the GetField#getValues() was returning a list of elements. This change makes behavior consistent. GetField#getValue() now always returns only the first value of the field and GetField#getValues() returns the entire list. --- .../index/get/ShardGetService.java | 19 +++++---- .../test/integration/get/GetActionTests.java | 40 +++++++++---------- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/get/ShardGetService.java b/src/main/java/org/elasticsearch/index/get/ShardGetService.java index 18a7455ddc5..ae325a0d834 100644 --- a/src/main/java/org/elasticsearch/index/get/ShardGetService.java +++ b/src/main/java/org/elasticsearch/index/get/ShardGetService.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.get; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; import org.apache.lucene.index.Term; import org.elasticsearch.ElasticSearchException; @@ -267,12 +268,11 @@ public class ShardGetService extends AbstractIndexShardComponent { if (fields == null) { fields = newHashMapWithExpectedSize(2); } - GetField getField = fields.get(field); - if (getField == null) { - getField = new GetField(field, new ArrayList(2)); - fields.put(field, getField); + if (value instanceof List) { + fields.put(field, new GetField(field, (List) value)); + } else { + fields.put(field, new GetField(field, ImmutableList.of(value))); } - getField.getValues().add(value); } } } @@ -379,12 +379,11 @@ public class ShardGetService extends AbstractIndexShardComponent { if (fields == null) { fields = newHashMapWithExpectedSize(2); } - GetField getField = fields.get(field); - if (getField == null) { - getField = new GetField(field, new ArrayList(2)); - fields.put(field, getField); + if (value instanceof List) { + fields.put(field, new GetField(field, (List) value)); + } else { + fields.put(field, new GetField(field, ImmutableList.of(value))); } - getField.getValues().add(value); } } } diff --git a/src/test/java/org/elasticsearch/test/integration/get/GetActionTests.java b/src/test/java/org/elasticsearch/test/integration/get/GetActionTests.java index 5eff12abd86..a3b75a3b4b0 100644 --- a/src/test/java/org/elasticsearch/test/integration/get/GetActionTests.java +++ b/src/test/java/org/elasticsearch/test/integration/get/GetActionTests.java @@ -276,9 +276,9 @@ public class GetActionTests extends AbstractNodesTests { GetResponse getResponse = client.prepareGet("test", "type1", "1").setFields("str", "strs", "int", "ints", "date", "binary").execute().actionGet(); assertThat(getResponse.isExists(), equalTo(true)); assertThat((String) getResponse.getField("str").getValue(), equalTo("test")); - assertThat((List) getResponse.getField("strs").getValue(), contains("A", "B", "C")); + assertThat(getResponse.getField("strs").getValues(), contains((Object) "A", "B", "C")); assertThat((Long) getResponse.getField("int").getValue(), equalTo(42l)); - assertThat((List) getResponse.getField("ints").getValue(), contains(1L, 2L, 3L, 4L)); + assertThat(getResponse.getField("ints").getValues(), contains((Object) 1L, 2L, 3L, 4L)); assertThat((String) getResponse.getField("date").getValue(), equalTo("2012-11-13T15:26:14.000Z")); assertThat(getResponse.getField("binary").getValue(), instanceOf(String.class)); // its a String..., not binary mapped @@ -286,9 +286,9 @@ public class GetActionTests extends AbstractNodesTests { getResponse = client.prepareGet("test", "type2", "1").setFields("str", "strs", "int", "ints", "date", "binary").execute().actionGet(); assertThat(getResponse.isExists(), equalTo(true)); assertThat((String) getResponse.getField("str").getValue(), equalTo("test")); - assertThat((List) getResponse.getField("strs").getValue(), contains("A", "B", "C")); + assertThat(getResponse.getField("strs").getValues(), contains((Object) "A", "B", "C")); assertThat((Integer) getResponse.getField("int").getValue(), equalTo(42)); - assertThat((List) getResponse.getField("ints").getValue(), contains(1, 2, 3, 4)); + assertThat(getResponse.getField("ints").getValues(), contains((Object) 1, 2, 3, 4)); assertThat((String) getResponse.getField("date").getValue(), equalTo("2012-11-13T15:26:14.000Z")); assertThat((BytesReference) getResponse.getField("binary").getValue(), equalTo((BytesReference) new BytesArray(new byte[]{1, 2, 3}))); @@ -299,9 +299,9 @@ public class GetActionTests extends AbstractNodesTests { getResponse = client.prepareGet("test", "type1", "1").setFields("str", "strs", "int", "ints", "date", "binary").execute().actionGet(); assertThat(getResponse.isExists(), equalTo(true)); assertThat((String) getResponse.getField("str").getValue(), equalTo("test")); - assertThat((List) getResponse.getField("strs").getValue(), contains("A", "B", "C")); + assertThat(getResponse.getField("strs").getValues(), contains((Object) "A", "B", "C")); assertThat((Long) getResponse.getField("int").getValue(), equalTo(42l)); - assertThat((List) getResponse.getField("ints").getValue(), contains(1L, 2L, 3L, 4L)); + assertThat(getResponse.getField("ints").getValues(), contains((Object) 1L, 2L, 3L, 4L)); assertThat((String) getResponse.getField("date").getValue(), equalTo("2012-11-13T15:26:14.000Z")); assertThat(getResponse.getField("binary").getValue(), instanceOf(String.class)); // its a String..., not binary mapped @@ -364,10 +364,9 @@ public class GetActionTests extends AbstractNodesTests { assertThat(response.getId(), equalTo("1")); assertThat(response.getType(), equalTo("type1")); assertThat(response.getFields().size(), equalTo(1)); - assertThat(response.getFields().get("field").getValues().size(), equalTo(1)); - assertThat(((List) response.getFields().get("field").getValues().get(0)).size(), equalTo(2)); - assertThat(((List) response.getFields().get("field").getValues().get(0)).get(0).toString(), equalTo("1")); - assertThat(((List) response.getFields().get("field").getValues().get(0)).get(1).toString(), equalTo("2")); + assertThat(response.getFields().get("field").getValues().size(), equalTo(2)); + assertThat(response.getFields().get("field").getValues().get(0).toString(), equalTo("1")); + assertThat(response.getFields().get("field").getValues().get(1).toString(), equalTo("2")); response = client.prepareGet("test", "type2", "1") @@ -377,10 +376,9 @@ public class GetActionTests extends AbstractNodesTests { assertThat(response.getType(), equalTo("type2")); assertThat(response.getId(), equalTo("1")); assertThat(response.getFields().size(), equalTo(1)); - assertThat(response.getFields().get("field").getValues().size(), equalTo(1)); - assertThat(((List) response.getFields().get("field").getValues().get(0)).size(), equalTo(2)); - assertThat(((List) response.getFields().get("field").getValues().get(0)).get(0).toString(), equalTo("1")); - assertThat(((List) response.getFields().get("field").getValues().get(0)).get(1).toString(), equalTo("2")); + assertThat(response.getFields().get("field").getValues().size(), equalTo(2)); + assertThat(response.getFields().get("field").getValues().get(0).toString(), equalTo("1")); + assertThat(response.getFields().get("field").getValues().get(1).toString(), equalTo("2")); // Now test values being fetched from stored fields. client.admin().indices().prepareRefresh("test").execute().actionGet(); @@ -390,10 +388,9 @@ public class GetActionTests extends AbstractNodesTests { assertThat(response.isExists(), equalTo(true)); assertThat(response.getId(), equalTo("1")); assertThat(response.getFields().size(), equalTo(1)); - assertThat(response.getFields().get("field").getValues().size(), equalTo(1)); - assertThat(((List) response.getFields().get("field").getValues().get(0)).size(), equalTo(2)); - assertThat(((List) response.getFields().get("field").getValues().get(0)).get(0).toString(), equalTo("1")); - assertThat(((List) response.getFields().get("field").getValues().get(0)).get(1).toString(), equalTo("2")); + assertThat(response.getFields().get("field").getValues().size(), equalTo(2)); + assertThat(response.getFields().get("field").getValues().get(0).toString(), equalTo("1")); + assertThat(response.getFields().get("field").getValues().get(1).toString(), equalTo("2")); response = client.prepareGet("test", "type2", "1") @@ -402,10 +399,9 @@ public class GetActionTests extends AbstractNodesTests { assertThat(response.isExists(), equalTo(true)); assertThat(response.getId(), equalTo("1")); assertThat(response.getFields().size(), equalTo(1)); - assertThat(response.getFields().get("field").getValues().size(), equalTo(1)); - assertThat(((List) response.getFields().get("field").getValues().get(0)).size(), equalTo(2)); - assertThat(((List) response.getFields().get("field").getValues().get(0)).get(0).toString(), equalTo("1")); - assertThat(((List) response.getFields().get("field").getValues().get(0)).get(1).toString(), equalTo("2")); + assertThat(response.getFields().get("field").getValues().size(), equalTo(2)); + assertThat(response.getFields().get("field").getValues().get(0).toString(), equalTo("1")); + assertThat(response.getFields().get("field").getValues().get(1).toString(), equalTo("2")); } @Test