From 3ce05b691998c0fe87ea6ba7a988d985c40017a4 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 22 Jan 2015 05:48:46 +0100 Subject: [PATCH] inner hits: Fix bug that resolves parent docs properly as inner hit when inner hit is defined on has_parent query. --- .../fetch/innerhits/InnerHitsContext.java | 26 ++++++++++--- .../search/innerhits/InnerHitsTests.java | 39 +++++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsContext.java b/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsContext.java index c1d0b9c3fb0..5ca516e346e 100644 --- a/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsContext.java +++ b/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsContext.java @@ -29,7 +29,9 @@ import org.apache.lucene.search.join.BitDocIdSetFilter; import org.apache.lucene.util.BitSet; import org.apache.lucene.util.Bits; import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.lucene.search.AndFilter; +import org.elasticsearch.index.fieldvisitor.SingleFieldsVisitor; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.Uid; import org.elasticsearch.index.mapper.internal.ParentFieldMapper; @@ -37,6 +39,7 @@ import org.elasticsearch.index.mapper.internal.UidFieldMapper; import org.elasticsearch.index.mapper.object.ObjectMapper; import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.index.search.nested.NonNestedDocsFilter; +import org.elasticsearch.search.SearchHitField; import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.internal.FilteredSearchContext; import org.elasticsearch.search.internal.SearchContext; @@ -249,16 +252,27 @@ public final class InnerHitsContext { topDocsCollector = TopScoreDocCollector.create(topN); } - String field; - ParentFieldMapper hitParentFieldMapper = documentMapper.parentFieldMapper(); - if (hitParentFieldMapper.active()) { - // Hit has a active _parent field and it is a child doc, so we want a parent doc as inner hits. + final String term; + final String field; + if (documentMapper.parentFieldMapper().active()) { + // Active _parent field has been selected, so we want a children doc as inner hits. field = ParentFieldMapper.NAME; + term = Uid.createUid(hitContext.hit().type(), hitContext.hit().id()); } else { - // Hit has no active _parent field and it is a parent doc, so we want children docs as inner hits. + // No active _parent field has been selected, so we want parent docs as inner hits. field = UidFieldMapper.NAME; + SearchHitField parentField = hitContext.hit().field(ParentFieldMapper.NAME); + if (parentField != null) { + term = parentField.getValue(); + } else { + SingleFieldsVisitor fieldsVisitor = new SingleFieldsVisitor(ParentFieldMapper.NAME); + hitContext.reader().document(hitContext.docId(), fieldsVisitor); + if (fieldsVisitor.fields().isEmpty()) { + return Lucene.EMPTY_TOP_DOCS; + } + term = (String) fieldsVisitor.fields().get(ParentFieldMapper.NAME).get(0); + } } - String term = Uid.createUid(hitContext.hit().type(), hitContext.hit().id()); Filter filter = new TermFilter(new Term(field, term)); // Only include docs that have the current hit as parent Filter typeFilter = documentMapper.typeFilter(); // Only include docs that have this inner hits type. context.searcher().search( diff --git a/src/test/java/org/elasticsearch/search/innerhits/InnerHitsTests.java b/src/test/java/org/elasticsearch/search/innerhits/InnerHitsTests.java index 67b87d5225a..fd210adb499 100644 --- a/src/test/java/org/elasticsearch/search/innerhits/InnerHitsTests.java +++ b/src/test/java/org/elasticsearch/search/innerhits/InnerHitsTests.java @@ -444,6 +444,45 @@ public class InnerHitsTests extends ElasticsearchIntegrationTest { } + @Test + public void testInnerHitsOnHasParent() throws Exception { + assertAcked(prepareCreate("stack") + .addMapping("question", "body", "type=string") + .addMapping("answer", "_parent", "type=question", "body", "type=string") + ); + List requests = new ArrayList<>(); + requests.add(client().prepareIndex("stack", "question", "1").setSource("body", "I'm using HTTPS + Basic authentication to protect a resource. How can I throttle authentication attempts to protect against brute force attacks?")); + requests.add(client().prepareIndex("stack", "answer", "1").setParent("1").setSource("body", "install fail2ban and enable rules for apache")); + requests.add(client().prepareIndex("stack", "question", "2").setSource("body", "I have firewall rules set up and also denyhosts installed.\\ndo I also need to install fail2ban?")); + requests.add(client().prepareIndex("stack", "answer", "2").setParent("2").setSource("body", "Denyhosts protects only ssh; Fail2Ban protects all daemons.")); + indexRandom(true, requests); + + SearchResponse response = client().prepareSearch("stack") + .setTypes("answer") + .addSort("_uid", SortOrder.ASC) + .setQuery( + boolQuery() + .must(matchQuery("body", "fail2ban")) + .must(hasParentQuery("question", matchAllQuery()).innerHit(new QueryInnerHitBuilder())) + ).get(); + assertNoFailures(response); + assertHitCount(response, 2); + + SearchHit searchHit = response.getHits().getAt(0); + assertThat(searchHit.getId(), equalTo("1")); + assertThat(searchHit.getType(), equalTo("answer")); + assertThat(searchHit.getInnerHits().get("question").getTotalHits(), equalTo(1l)); + assertThat(searchHit.getInnerHits().get("question").getAt(0).getType(), equalTo("question")); + assertThat(searchHit.getInnerHits().get("question").getAt(0).id(), equalTo("1")); + + searchHit = response.getHits().getAt(1); + assertThat(searchHit.getId(), equalTo("2")); + assertThat(searchHit.getType(), equalTo("answer")); + assertThat(searchHit.getInnerHits().get("question").getTotalHits(), equalTo(1l)); + assertThat(searchHit.getInnerHits().get("question").getAt(0).getType(), equalTo("question")); + assertThat(searchHit.getInnerHits().get("question").getAt(0).id(), equalTo("2")); + } + @Test public void testParentChildMultipleLayers() throws Exception { assertAcked(prepareCreate("articles")