diff --git a/amps/share-services/src/main/resources/alfresco/templates/webscripts/org/alfresco/slingshot/search/search.lib.js b/amps/share-services/src/main/resources/alfresco/templates/webscripts/org/alfresco/slingshot/search/search.lib.js index 14328b4c0f8..0b594e5ae45 100644 --- a/amps/share-services/src/main/resources/alfresco/templates/webscripts/org/alfresco/slingshot/search/search.lib.js +++ b/amps/share-services/src/main/resources/alfresco/templates/webscripts/org/alfresco/slingshot/search/search.lib.js @@ -674,6 +674,11 @@ function processResults(nodes, maxPageResults, startIndex, rootNode, meta) } } + if (failed !== 0 && logger.isWarnLoggingEnabled() === true) + { + logger.warn("Faceting disabled as hits are innacurate due to unknown nodeRefs returned"); + } + if (logger.isLoggingEnabled()) logger.log("Filtered resultset to length: " + results.length + ". Discarded item count: " + failed); @@ -684,9 +689,9 @@ function processResults(nodes, maxPageResults, startIndex, rootNode, meta) totalRecords: results.length, totalRecordsUpper: nodes.length - failed, startIndex: startIndex, - numberFound: meta ? meta.numberFound : -1 + numberFound: meta ? meta.numberFound - failed : -1 }, - facets: meta ? meta.facets : null, + facets: (meta && failed === 0) ? meta.facets : null, highlighting: meta ? meta.highlighting : null, items: results, spellcheck: meta ? meta.spellcheck : null @@ -741,6 +746,11 @@ function processResultsSinglePage(nodes, startIndex, rootNode, meta) } } + if (failed !== 0 && logger.isWarnLoggingEnabled() === true) + { + logger.warn("Faceting disabled as hits are innacurate due to unknown nodeRefs returned"); + } + if (logger.isLoggingEnabled()) logger.log("Filtered resultset to length: " + results.length + ". Discarded item count: " + failed); @@ -751,9 +761,9 @@ function processResultsSinglePage(nodes, startIndex, rootNode, meta) totalRecords: results.length, totalRecordsUpper: -1, startIndex: startIndex, - numberFound: meta ? meta.numberFound : -1 + numberFound: meta ? meta.numberFound - failed : -1 }, - facets: meta ? meta.facets : null, + facets: (meta && failed === 0) ? meta.facets : null, highlighting: meta ? meta.highlighting : null, items: results, spellcheck: meta ? meta.spellcheck : null diff --git a/remote-api/src/main/java/org/alfresco/rest/api/search/impl/ResultMapper.java b/remote-api/src/main/java/org/alfresco/rest/api/search/impl/ResultMapper.java index 1433b8b7180..65c4ed9b24f 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/search/impl/ResultMapper.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/search/impl/ResultMapper.java @@ -191,12 +191,20 @@ public CollectionWithPagingInfo toCollectionWithPagingInfo(Params params, } } + // If we have unknown noderefs in the resultset, the faceting hits will be wrong. Disable facets instead of displaying incorrect hits + boolean disableFaceting = unknownNodeRefsCount.get() > 0; + + if(disableFaceting) + { + logger.warn("Disabled faceting, stats and pivots as the hits are innacurate due to unknown nodeRefs returned"); + } + SearchContext context = toSearchEngineResultSet(results) - .map(resultSet -> toSearchContext(resultSet, searchRequestContext, searchQuery)) + .map(resultSet -> toSearchContext(resultSet, searchRequestContext, searchQuery, disableFaceting)) .orElse(null); - return CollectionWithPagingInfo.asPaged(params.getPaging(), noderesults, results.hasMore(), setTotal(results), null, context); + return CollectionWithPagingInfo.asPaged(params.getPaging(), noderesults, results.hasMore(), getTotalAvailableResults(results, unknownNodeRefsCount.get()), null, context); } /** @@ -288,6 +296,18 @@ public Integer setTotal(ResultSet results) return total; } + /** + * Sets the total number found of available nodes + * @param results + * @return An integer total + */ + public Integer getTotalAvailableResults(ResultSet results, int unavailableResultsCount) + { + Long totalItems = results.getNumberFound(); + Integer total = totalItems.intValue() - unavailableResultsCount; + return total; + } + /** * Uses the results from Solr to set the Search Context * @@ -295,12 +315,16 @@ public Integer setTotal(ResultSet results) * @return SearchContext */ public SearchContext toSearchContext(SearchEngineResultSet resultSet, SearchRequestContext searchRequestContext, SearchQuery searchQuery) + { + return toSearchContext(resultSet, searchRequestContext, searchQuery, false); + } + + public SearchContext toSearchContext(SearchEngineResultSet resultSet, SearchRequestContext searchRequestContext, SearchQuery searchQuery, boolean disableFaceting) { SearchContext context = null; - Map facetQueries = resultSet.getFacetQueries(); + List facets = new ArrayList<>(); List facetResults = null; - SpellCheckContext spellCheckContext = null; List ffcs = new ArrayList(); if (searchQuery == null) @@ -308,64 +332,91 @@ public SearchContext toSearchContext(SearchEngineResultSet resultSet, SearchRequ throw new IllegalArgumentException("searchQuery can't be null"); } - //Facet queries - if(facetQueries!= null && !facetQueries.isEmpty()) + SpellCheckContext spellCheckContext = getSpellCheckContext(resultSet); + + if (!disableFaceting) { - //If group by field populated in query facet return bucketing into facet field. - List facetQueryForFields = getFacetBucketsFromFacetQueries(facetQueries,searchQuery); - if(hasGroup(searchQuery) || FacetFormat.V2 == searchQuery.getFacetFormat()) + + Map facetQueries = resultSet.getFacetQueries(); + Map>> facetFields = resultSet.getFieldFacets(); + Map>> facetInterval = resultSet.getFacetIntervals(); + Map>> facetRanges = resultSet.getFacetRanges(); + + if (useBuckets(searchQuery)) { - facets.addAll(facetQueryForFields); + facets.addAll(getFacetQueriesForFields(facetQueries, searchQuery)); + facets.addAll(getFacetBucketsForFacetFieldsAsFacets(facetFields, searchQuery)); } else { - // Return the old way facet query with no bucketing. - facetResults = new ArrayList<>(facetQueries.size()); - for (Entry fq:facetQueries.entrySet()) - { - String filterQuery = null; - if (searchQuery.getFacetQueries() != null) - { - Optional found = searchQuery.getFacetQueries().stream().filter(facetQuery -> fq.getKey().equals(facetQuery.getLabel())).findFirst(); - filterQuery = found.isPresent()? found.get().getQuery():fq.getKey(); - } - facetResults.add(new FacetQueryContext(fq.getKey(), filterQuery, fq.getValue())); - } + facetResults = getFacetResults(facetQueries, searchQuery); + ffcs.addAll(getFacetBucketsForFacetFields(facetFields, searchQuery)); } + + facets.addAll(getGenericFacetsForIntervals(facetInterval, searchQuery)); + facets.addAll(RangeResultMapper.getGenericFacetsForRanges(facetRanges, searchQuery.getFacetRanges())); + + List stats = getFieldStats(searchRequestContext, resultSet.getStats()); + List pimped = getPivots(searchRequestContext, resultSet.getPivotFacets(), stats); + facets.addAll(pimped); + facets.addAll(stats); } - //Field Facets - Map>> facetFields = resultSet.getFieldFacets(); - if(FacetFormat.V2 == searchQuery.getFacetFormat()) + // Put it all together + context = new SearchContext(resultSet.getLastIndexedTxId(), facets, facetResults, ffcs, spellCheckContext, + searchRequestContext.includeRequest() ? searchQuery : null); + + return isNullContext(context) ? null : context; + } + + private boolean useBuckets(SearchQuery searchQuery) + { + return hasGroup(searchQuery) || FacetFormat.V2 == searchQuery.getFacetFormat(); + } + + private SpellCheckContext getSpellCheckContext(SearchEngineResultSet resultSet) + { + SpellCheckContext spellCheckContext = null; + SpellCheckResult spell = resultSet.getSpellCheckResult(); + if (spell != null && spell.getResultName() != null && !spell.getResults().isEmpty()) { - facets.addAll(getFacetBucketsForFacetFieldsAsFacets(facetFields, searchQuery)); + spellCheckContext = new SpellCheckContext(spell.getResultName(), spell.getResults()); } - else + + return spellCheckContext; + } + + private List getFacetQueriesForFields(Map facetQueries, SearchQuery searchQuery) + { + List facetQueryForFields = new ArrayList<>(); + if (facetQueries != null && !facetQueries.isEmpty()) { - ffcs.addAll(getFacetBucketsForFacetFields(facetFields, searchQuery)); + facetQueryForFields = getFacetBucketsFromFacetQueries(facetQueries, searchQuery); } - Map>> facetInterval = resultSet.getFacetIntervals(); - facets.addAll(getGenericFacetsForIntervals(facetInterval, searchQuery)); - - Map>> facetRanges = resultSet.getFacetRanges(); - facets.addAll(RangeResultMapper.getGenericFacetsForRanges(facetRanges, searchQuery.getFacetRanges())); - - List stats = getFieldStats(searchRequestContext, resultSet.getStats()); - List pimped = getPivots(searchRequestContext, resultSet.getPivotFacets(), stats); - facets.addAll(pimped); - facets.addAll(stats); + return facetQueryForFields; + } - //Spelling - SpellCheckResult spell = resultSet.getSpellCheckResult(); - if (spell != null && spell.getResultName() != null && !spell.getResults().isEmpty()) + private List getFacetResults(Map facetQueries, SearchQuery searchQuery) + { + List facetResults = null; + if (facetQueries != null && !facetQueries.isEmpty()) { - spellCheckContext = new SpellCheckContext(spell.getResultName(),spell.getResults()); + facetResults = new ArrayList<>(facetQueries.size()); + for (Entry fq : facetQueries.entrySet()) + { + String filterQuery = null; + if (searchQuery.getFacetQueries() != null) + { + Optional found = searchQuery.getFacetQueries().stream() + .filter(facetQuery -> fq.getKey().equals(facetQuery.getLabel())).findFirst(); + filterQuery = found.isPresent() ? found.get().getQuery() : fq.getKey(); + } + facetResults.add(new FacetQueryContext(fq.getKey(), filterQuery, fq.getValue())); + } } - //Put it all together - context = new SearchContext(resultSet.getLastIndexedTxId(), facets, facetResults, ffcs, spellCheckContext, searchRequestContext.includeRequest()?searchQuery:null); - return isNullContext(context)?null:context; + return facetResults; } public static boolean hasGroup(SearchQuery searchQuery) @@ -399,8 +450,6 @@ protected List getFacetBucketsFromFacetQueries(Map metrics = new HashSet<>(1); @@ -416,7 +465,7 @@ protected List getFacetBucketsFromFacetQueries(Map facetResults.add(new GenericFacetResponse(FACET_TYPE.query, a, v))); @@ -553,8 +602,10 @@ protected List getFacetBucketsForFacetFields(Map getGenericFacetsForIntervals(Map ffcs = new ArrayList<>(facetFields.size()); - for (Entry>> facet:facetFields.entrySet()) + for (Entry>> facet : facetFields.entrySet()) { if (facet.getValue() != null && !facet.getValue().isEmpty()) { List buckets = new ArrayList<>(facet.getValue().size()); - for (Pair buck:facet.getValue()) + for (Pair buck : facet.getValue()) { - String filterQuery = null; - Map bucketInfo = new HashMap<>(); - - if (searchQuery != null - && searchQuery.getFacetIntervals() != null - && searchQuery.getFacetIntervals().getIntervals() != null - && !searchQuery.getFacetIntervals().getIntervals().isEmpty()) - { - Optional found = searchQuery.getFacetIntervals().getIntervals().stream().filter( - interval -> facet.getKey().equals(interval.getLabel()!=null?interval.getLabel():interval.getField())).findFirst(); - if (found.isPresent()) - { - if (found.get().getSets() != null) - { - Optional foundSet = found.get().getSets().stream().filter(aSet -> buck.getFirst().equals(aSet.getLabel())).findFirst(); - if (foundSet.isPresent()) - { - filterQuery = found.get().getField() + ":" + foundSet.get().toAFTSQuery(); - bucketInfo.put(GenericFacetResponse.START, foundSet.get().getStart()); - bucketInfo.put(GenericFacetResponse.END, foundSet.get().getEnd()); - bucketInfo.put(GenericFacetResponse.START_INC, String.valueOf(foundSet.get().isStartInclusive())); - bucketInfo.put(GenericFacetResponse.END_INC, String.valueOf(foundSet.get().isEndInclusive())); - } - } - } - } - GenericBucket bucket = new GenericBucket(buck.getFirst(), filterQuery, null , new HashSet(Arrays.asList(new SimpleMetric(METRIC_TYPE.count,String.valueOf(buck.getSecond())))), null, bucketInfo); + GenericBucket bucket = createBucket(facet.getKey(), buck, searchQuery); buckets.add(bucket); } ffcs.add(new GenericFacetResponse(FACET_TYPE.interval, facet.getKey(), buckets)); @@ -609,6 +634,62 @@ protected static List getGenericFacetsForIntervals(Map buck, SearchQuery searchQuery) + { + String filterQuery = null; + Map bucketInfo = new HashMap<>(); + + Pair facetIntervalSet = getFacetIntervalSet(facetKey, buck, searchQuery); + + if (facetIntervalSet != null) + { + String field = facetIntervalSet.getFirst(); + IntervalSet intervalSet = facetIntervalSet.getSecond(); + + filterQuery = field + ":" + intervalSet.toAFTSQuery(); + bucketInfo.put(GenericFacetResponse.START, intervalSet.getStart()); + bucketInfo.put(GenericFacetResponse.END, intervalSet.getEnd()); + bucketInfo.put(GenericFacetResponse.START_INC, String.valueOf(intervalSet.isStartInclusive())); + bucketInfo.put(GenericFacetResponse.END_INC, String.valueOf(intervalSet.isEndInclusive())); + } + + return new GenericBucket(buck.getFirst(), filterQuery, null, + new HashSet<>(Arrays.asList(new SimpleMetric(METRIC_TYPE.count, String.valueOf(buck.getSecond())))), null, + bucketInfo); + } + + private static Pair getFacetIntervalSet(String facetKey, Pair buck, SearchQuery searchQuery) + { + if (!searchQueryHasFacetIntervals(searchQuery)) + { + return null; + } + + Optional found = searchQuery.getFacetIntervals().getIntervals().stream() + .filter(interval -> facetKey.equals(interval.getLabel() != null ? interval.getLabel() : interval.getField())) + .findFirst(); + + if (found.isPresent() && found.get().getSets() != null) + { + Optional foundSet = found.get().getSets().stream() + .filter(aSet -> buck.getFirst().equals(aSet.getLabel())).findFirst(); + + if (foundSet.isPresent()) + { + return new Pair<>(found.get().getField(), foundSet.get()); + } + } + + return null; + } + + private static boolean searchQueryHasFacetIntervals(SearchQuery searchQuery) + { + return searchQuery != null && searchQuery.getFacetIntervals() != null + && searchQuery.getFacetIntervals().getIntervals() != null + && !searchQuery.getFacetIntervals().getIntervals().isEmpty(); + } + /** * Is the context null? * @param context diff --git a/remote-api/src/test/java/org/alfresco/rest/api/search/ResultMapperTests.java b/remote-api/src/test/java/org/alfresco/rest/api/search/ResultMapperTests.java index 8bb7a576ca0..02ebf0869aa 100644 --- a/remote-api/src/test/java/org/alfresco/rest/api/search/ResultMapperTests.java +++ b/remote-api/src/test/java/org/alfresco/rest/api/search/ResultMapperTests.java @@ -264,7 +264,9 @@ public void testToCollectionWithPagingInfo() CollectionWithPagingInfo collectionWithPage = mapper.toCollectionWithPagingInfo(EMPTY_PARAMS, searchRequest, SearchQuery.EMPTY, results); assertNotNull(collectionWithPage); Long found = results.getNumberFound(); - assertEquals(found.intValue(), collectionWithPage.getTotalItems().intValue()); + // We have one deleted node that will not be part of the results + int shouldbeReturned = found.intValue()-1; + assertEquals(shouldbeReturned, collectionWithPage.getTotalItems().intValue()); Node firstNode = collectionWithPage.getCollection().stream().findFirst().get(); assertNotNull(firstNode.getSearch().getScore()); assertEquals(StoreMapper.LIVE_NODES, firstNode.getLocation()); @@ -886,4 +888,48 @@ public void testGetNode() assertNull("Expected node to be filtered due to permission exception.", node); } + + /** Verify counters and faceting is disabled when we have results that are no longer valid */ + @Test + public void testUnknownNodesInResults() + { + String jsonQuery = "{\"query\": {\"query\": \"alfresco\"}," + + "\"facetFields\": {\"facets\": [" + + "{\"field\": \"creator\", \"mincount\": 1}," + + "{\"field\": \"modifier\", \"mincount\": 1}]}," + + "\"facetQueries\": [" + + "{\"query\": \"content.size:[o TO 102400]\", \"label\": \"small\"}," + + "{\"query\": \"content.size:[102400 TO 1048576]\", \"label\": \"medium\"}," + + "{\"query\": \"content.size:[1048576 TO 16777216]\", \"label\": \"large\"}]" + + "}"; + + SearchQuery searchQuery = helper.extractFromJson(jsonQuery); + SearchRequestContext searchRequest = SearchRequestContext.from(searchQuery); + + // With 1 unknown node in the results + ResultSet resultsWithUnkown = mockResultSet(Collections.emptyList(), asList(VERSIONED_ID)); + + // Total items must match the total number of documents returned + CollectionWithPagingInfo collectionWithUnkown = mapper.toCollectionWithPagingInfo(EMPTY_PARAMS, searchRequest, + searchQuery, resultsWithUnkown); + assertEquals(collectionWithUnkown.getTotalItems().intValue(), collectionWithUnkown.getCollection().size()); + + // No facets, facet queries or facet fields should be returned + assertTrue(collectionWithUnkown.getContext().getFacets().isEmpty()); + assertTrue(collectionWithUnkown.getContext().getFacetsFields().isEmpty()); + assertNull(collectionWithUnkown.getContext().getFacetQueries()); + + // With all nodes ok + ResultSet resultsWithoutUnknown = mockResultSet(Collections.emptyList(), Collections.emptyList()); + + // Total items still needs to match the total number of documents returned + CollectionWithPagingInfo collectionWithoutUnkown = mapper.toCollectionWithPagingInfo(EMPTY_PARAMS, searchRequest, + searchQuery, resultsWithoutUnknown); + assertEquals(collectionWithoutUnkown.getTotalItems().intValue(), collectionWithoutUnkown.getCollection().size()); + + // Facets, facet queries and facet fields should be returned + assertFalse(collectionWithoutUnkown.getContext().getFacets().isEmpty()); + assertFalse(collectionWithoutUnkown.getContext().getFacetsFields().isEmpty()); + assertNotNull(collectionWithoutUnkown.getContext().getFacetQueries()); + } }