Skip to content

Commit e8c2bc5

Browse files
RobCherrybrusic
RobCherry
authored and
brusic
committed
excluding all fields of an object should not remove parent.
When excluding '*.f1' from `{ "obj": { "f1": 1, "f2": 2 } }` XContentMapValues.filter returns `{ "obj": { "f2": 2}}`. When run on `{ "obj": { "f1" : 1 }}` we should return `{ "obj": { }}` to maintain object structure. People currently need to always check whether `obj` is there or not. Closes elastic#4715 Closes elastic#4047 Related to elastic#4491
1 parent fc96f3c commit e8c2bc5

File tree

3 files changed

+114
-34
lines changed

3 files changed

+114
-34
lines changed

‎src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -154,24 +154,18 @@ private static void filter(Map<String, Object> map, Map<String, Object> into, St
154154
}
155155
sb.append(key);
156156
String path = sb.toString();
157-
boolean excluded = false;
158-
for (String exclude : excludes) {
159-
if (Regex.simpleMatch(exclude, path)) {
160-
excluded = true;
161-
break;
162-
}
163-
}
164-
if (excluded) {
157+
158+
if (Regex.simpleMatch(excludes, path)) {
165159
sb.setLength(mark);
166160
continue;
167161
}
168-
boolean exactIncludeMatch;
162+
163+
boolean exactIncludeMatch = false; // true if the current position was specifically mentioned
164+
boolean pathIsPrefixOfAnInclude = false; // true if potentially a sub scope can be included
169165
if (includes.length == 0) {
170166
// implied match anything
171167
exactIncludeMatch = true;
172168
} else {
173-
exactIncludeMatch = false;
174-
boolean pathIsPrefixOfAnInclude = false;
175169
for (String include : includes) {
176170
// check for prefix matches as well to see if we need to zero in, something like: obj1.arr1.* or *.field
177171
// note, this does not work well with middle matches, like obj1.*.obj3
@@ -198,19 +192,20 @@ private static void filter(Map<String, Object> map, Map<String, Object> into, St
198192
break;
199193
}
200194
}
201-
if (!pathIsPrefixOfAnInclude && !exactIncludeMatch) {
202-
// skip subkeys, not interesting.
203-
sb.setLength(mark);
204-
continue;
205-
}
195+
}
196+
197+
if (!(pathIsPrefixOfAnInclude || exactIncludeMatch)) {
198+
// skip subkeys, not interesting.
199+
sb.setLength(mark);
200+
continue;
206201
}
207202

208203

209204
if (entry.getValue() instanceof Map) {
210205
Map<String, Object> innerInto = Maps.newHashMap();
211206
// if we had an exact match, we want give deeper excludes their chance
212207
filter((Map<String, Object>) entry.getValue(), innerInto, exactIncludeMatch ? Strings.EMPTY_ARRAY : includes, excludes, sb);
213-
if (!innerInto.isEmpty()) {
208+
if (exactIncludeMatch || !innerInto.isEmpty()) {
214209
into.put(entry.getKey(), innerInto);
215210
}
216211
} else if (entry.getValue() instanceof List) {

‎src/test/java/org/elasticsearch/common/xcontent/support/XContentMapValuesTests.java

Lines changed: 101 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@
2626
import org.elasticsearch.common.xcontent.XContentHelper;
2727
import org.elasticsearch.common.xcontent.XContentType;
2828
import org.elasticsearch.test.ElasticsearchTestCase;
29+
import org.hamcrest.Matchers;
2930
import org.junit.Test;
3031

3132
import java.util.Arrays;
3233
import java.util.HashMap;
3334
import java.util.List;
3435
import java.util.Map;
3536

36-
import static org.hamcrest.MatcherAssert.assertThat;
3737
import static org.hamcrest.Matchers.*;
3838
import static org.hamcrest.core.IsEqual.equalTo;
3939

@@ -46,6 +46,7 @@ public void testFilter() throws Exception {
4646
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
4747
.field("test1", "value1")
4848
.field("test2", "value2")
49+
.field("something_else", "value3")
4950
.endObject();
5051

5152
Map<String, Object> source = XContentFactory.xContent(XContentType.JSON).createParser(builder.string()).mapAndClose();
@@ -59,8 +60,9 @@ public void testFilter() throws Exception {
5960
assertThat(filter.get("test2").toString(), equalTo("value2"));
6061

6162
filter = XContentMapValues.filter(source, Strings.EMPTY_ARRAY, new String[]{"test1"});
62-
assertThat(filter.size(), equalTo(1));
63+
assertThat(filter.size(), equalTo(2));
6364
assertThat(filter.get("test2").toString(), equalTo("value2"));
65+
assertThat(filter.get("something_else").toString(), equalTo("value3"));
6466

6567
// more complex object...
6668
builder = XContentFactory.jsonBuilder().startObject()
@@ -200,20 +202,6 @@ public void testExtractRawValue() throws Exception {
200202
assertThat(XContentMapValues.extractRawValues("path1.xxx.path2.yyy.test", map).get(0).toString(), equalTo("value"));
201203
}
202204

203-
@Test
204-
public void testThatFilteringWithNestedArrayAndExclusionWorks() throws Exception {
205-
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
206-
.startArray("coordinates")
207-
.startArray().value("foo").endArray()
208-
.endArray()
209-
.endObject();
210-
211-
Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
212-
Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"nonExistingField"});
213-
214-
assertThat(mapTuple.v2(), equalTo(filteredSource));
215-
}
216-
217205
@Test
218206
public void prefixedNamesFilteringTest() {
219207
Map<String, Object> map = new HashMap<String, Object>();
@@ -368,4 +356,101 @@ public void filterWithEmptyIncludesExcludes() {
368356
assertThat(filteredMap.get("field").toString(), equalTo("value"));
369357

370358
}
359+
360+
@SuppressWarnings({"unchecked"})
361+
@Test
362+
public void testThatFilterIncludesEmptyObjectWhenUsingIncludes() throws Exception {
363+
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
364+
.startObject("obj")
365+
.endObject()
366+
.endObject();
367+
368+
Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
369+
Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), new String[]{"obj"}, Strings.EMPTY_ARRAY);
370+
371+
assertThat(mapTuple.v2(), equalTo(filteredSource));
372+
}
373+
374+
@Test
375+
public void testThatFilterIncludesEmptyObjectWhenUsingExcludes() throws Exception {
376+
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
377+
.startObject("obj")
378+
.endObject()
379+
.endObject();
380+
381+
Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
382+
Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"nonExistingField"});
383+
384+
assertThat(mapTuple.v2(), equalTo(filteredSource));
385+
}
386+
387+
@Test
388+
public void testNotOmittingObjectsWithExcludedProperties() throws Exception {
389+
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
390+
.startObject("obj")
391+
.field("f1", "v1")
392+
.endObject()
393+
.endObject();
394+
395+
Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
396+
Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"obj.f1"});
397+
398+
assertThat(filteredSource.size(), equalTo(1));
399+
assertThat(filteredSource, hasKey("obj"));
400+
assertThat(((Map) filteredSource.get("obj")).size(), equalTo(0));
401+
}
402+
403+
@SuppressWarnings({"unchecked"})
404+
@Test
405+
public void testNotOmittingObjectWithNestedExcludedObject() throws Exception {
406+
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
407+
.startObject("obj1")
408+
.startObject("obj2")
409+
.startObject("obj3")
410+
.endObject()
411+
.endObject()
412+
.endObject()
413+
.endObject();
414+
415+
// implicit include
416+
Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
417+
Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), Strings.EMPTY_ARRAY, new String[]{"*.obj2"});
418+
419+
assertThat(filteredSource.size(), equalTo(1));
420+
assertThat(filteredSource, hasKey("obj1"));
421+
assertThat(((Map) filteredSource.get("obj1")).size(), Matchers.equalTo(0));
422+
423+
// explicit include
424+
filteredSource = XContentMapValues.filter(mapTuple.v2(), new String[]{"obj1"}, new String[]{"*.obj2"});
425+
assertThat(filteredSource.size(), equalTo(1));
426+
assertThat(filteredSource, hasKey("obj1"));
427+
assertThat(((Map) filteredSource.get("obj1")).size(), Matchers.equalTo(0));
428+
429+
// wild card include
430+
filteredSource = XContentMapValues.filter(mapTuple.v2(), new String[]{"*.obj2"}, new String[]{"*.obj3"});
431+
assertThat(filteredSource.size(), equalTo(1));
432+
assertThat(filteredSource, hasKey("obj1"));
433+
assertThat(((Map<String, Object>) filteredSource.get("obj1")), hasKey("obj2"));
434+
assertThat(((Map) ((Map) filteredSource.get("obj1")).get("obj2")).size(), Matchers.equalTo(0));
435+
}
436+
437+
@SuppressWarnings({"unchecked"})
438+
@Test
439+
public void testIncludingObjectWithNestedIncludedObject() throws Exception {
440+
XContentBuilder builder = XContentFactory.jsonBuilder().startObject()
441+
.startObject("obj1")
442+
.startObject("obj2")
443+
.endObject()
444+
.endObject()
445+
.endObject();
446+
447+
Tuple<XContentType, Map<String, Object>> mapTuple = XContentHelper.convertToMap(builder.bytes(), true);
448+
Map<String, Object> filteredSource = XContentMapValues.filter(mapTuple.v2(), new String[]{"*.obj2"}, Strings.EMPTY_ARRAY);
449+
450+
assertThat(filteredSource.size(), equalTo(1));
451+
assertThat(filteredSource, hasKey("obj1"));
452+
assertThat(((Map) filteredSource.get("obj1")).size(), equalTo(1));
453+
assertThat(((Map<String, Object>) filteredSource.get("obj1")), hasKey("obj2"));
454+
assertThat(((Map) ((Map) filteredSource.get("obj1")).get("obj2")).size(), equalTo(0));
455+
}
371456
}

‎src/test/java/org/elasticsearch/search/fields/SearchFieldsTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ public void testPartialFields() throws Exception {
253253

254254
SearchResponse response = client().prepareSearch("test")
255255
.addPartialField("partial1", "obj1.arr1.*", null)
256-
.addPartialField("partial2", null, "obj1.*")
256+
.addPartialField("partial2", null, "obj1")
257257
.execute().actionGet();
258258
assertThat("Failures " + Arrays.toString(response.getShardFailures()), response.getShardFailures().length, equalTo(0));
259259

0 commit comments

Comments
 (0)