From 89f92e960ed05df68554ed864211796a00c83316 Mon Sep 17 00:00:00 2001 From: Tom Page Date: Thu, 19 Dec 2019 12:13:45 +0000 Subject: [PATCH] Merge branch 'feature/SEARCH-2012_NullDocsWithField_14x' into 'release/V1.4.x' Feature/search 2012 null docs with field 14x See merge request search_discovery/insightengine!280 (cherry picked from commit 680eae4b6aaa8e04cc280af0c2057f6a2d868c2a) --- .../AlfrescoCollatableMLTextFieldType.java | 49 +++-- .../solr/AlfrescoCollatableTextFieldType.java | 18 +- ...AlfrescoCollatableMLTextFieldTypeTest.java | 200 ++++++++++++++++++ .../AlfrescoCollatableTextFieldTypeTest.java | 168 +++++++++++++++ .../solr/query/AlfrescoSolrSortIT.java | 108 ++++++++++ 5 files changed, 513 insertions(+), 30 deletions(-) create mode 100644 search-services/alfresco-search/src/test/java/org/alfresco/solr/AlfrescoCollatableMLTextFieldTypeTest.java create mode 100644 search-services/alfresco-search/src/test/java/org/alfresco/solr/AlfrescoCollatableTextFieldTypeTest.java create mode 100644 search-services/alfresco-search/src/test/java/org/alfresco/solr/query/AlfrescoSolrSortIT.java diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCollatableMLTextFieldType.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCollatableMLTextFieldType.java index 03d31c672..89d171642 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCollatableMLTextFieldType.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCollatableMLTextFieldType.java @@ -39,12 +39,9 @@ import org.springframework.extensions.surf.util.I18NUtil; /** * @author Andy - * */ public class AlfrescoCollatableMLTextFieldType extends StrField { - - /* (non-Javadoc) * @see org.apache.solr.schema.StrField#getSortField(org.apache.solr.schema.SchemaField, boolean) */ @@ -75,7 +72,6 @@ public class AlfrescoCollatableMLTextFieldType extends StrField } - public static class MLTextSortFieldComparatorSource extends FieldComparatorSource { @@ -101,16 +97,20 @@ public class AlfrescoCollatableMLTextFieldType extends StrField private final String[] values; private BinaryDocValues docTerms; - - private Bits docsWithField; + + /** + * An array of flags - one for each document in the segment. Each bit is set to true if the document has the + * field or false otherwise. If this is set to null then all docs in the segment have the field. + */ + Bits docsWithField; private final String field; - final Collator collator; + Collator collator; - private String bottom; - - private String top; + String bottom; + + String top; Locale collatorLocale; @@ -138,7 +138,7 @@ public class AlfrescoCollatableMLTextFieldType extends StrField { final String comparableString = findBestValue(doc, docTerms.get(doc)); return compareValues(bottom, comparableString); - + } public void copy(int slot, int doc) @@ -153,13 +153,14 @@ public class AlfrescoCollatableMLTextFieldType extends StrField private String findBestValue(int doc, BytesRef term) { - if (term.length == 0 && docsWithField.get(doc) == false) { + if (term.length == 0 && docsWithField != null && docsWithField.get(doc) == false) + { return null; } - + String withLocale = term.utf8ToString(); - - // split strin into MLText object + + // split string into MLText object if (withLocale == null) { return withLocale; @@ -231,14 +232,15 @@ public class AlfrescoCollatableMLTextFieldType extends StrField { docTerms = DocValues.getBinary(context.reader(), field); docsWithField = DocValues.getDocsWithField(context.reader(), field); - if (docsWithField instanceof Bits.MatchAllBits) { - docsWithField = null; + if (docsWithField instanceof Bits.MatchAllBits) + { + docsWithField = null; } return this; } - + @Override - public int compareValues(String val1, String val2) + public int compareValues(String val1, String val2) { if (val1 == null) { @@ -254,9 +256,10 @@ public class AlfrescoCollatableMLTextFieldType extends StrField } return collator.compare(val1, val2); } - - @Override - public void setScorer(Scorer scorer) {} - } + @Override + public void setScorer(Scorer scorer) + { + } + } } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCollatableTextFieldType.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCollatableTextFieldType.java index d3d54cd63..604dbcc27 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCollatableTextFieldType.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCollatableTextFieldType.java @@ -104,16 +104,20 @@ public class AlfrescoCollatableTextFieldType extends StrField private final String[] values; private BinaryDocValues docTerms; - - private Bits docsWithField; + + /** + * An array of flags - one for each document in the segment. Each bit is set to true if the document has the + * field or false otherwise. If this is set to null then all docs in the segment have the field. + */ + Bits docsWithField; private final String field; - final Collator collator; + Collator collator; - private String bottom; + String bottom; - private String top; + String top; Locale collatorLocale; @@ -141,7 +145,6 @@ public class AlfrescoCollatableTextFieldType extends StrField { final String comparableString = findBestValue(doc, docTerms.get(doc)); return compareValues(bottom, comparableString); - } public void copy(int slot, int doc) @@ -156,7 +159,8 @@ public class AlfrescoCollatableTextFieldType extends StrField private String findBestValue(int doc, BytesRef term) { - if (term.length == 0 && docsWithField.get(doc) == false) { + if (term.length == 0 && docsWithField != null && docsWithField.get(doc) == false) + { return null; } diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/AlfrescoCollatableMLTextFieldTypeTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/AlfrescoCollatableMLTextFieldTypeTest.java new file mode 100644 index 000000000..dcd57f47e --- /dev/null +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/AlfrescoCollatableMLTextFieldTypeTest.java @@ -0,0 +1,200 @@ +/* + * Copyright (C) 2005-2014 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ +package org.alfresco.solr; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +import java.text.Collator; +import java.util.Locale; + +import org.alfresco.solr.AlfrescoCollatableMLTextFieldType.MLTextSortFieldComparator; +import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; + +/** Unit tests for {@link AlfrescoCollatableMLTextFieldType}. */ +public class AlfrescoCollatableMLTextFieldTypeTest +{ + private static final int NUM_HITS = 3; + private static final String FIELD = "field"; + private static final Locale LOCALE = Locale.getDefault(); + /** A document id. */ + private static final int DOC = 0; + /** A value for the current bottom document. */ + private static final String BOTTOM_STRING = "Bottom"; + + @InjectMocks + MLTextSortFieldComparator textSortFieldComparator = new MLTextSortFieldComparator(NUM_HITS, FIELD, LOCALE); + @Mock + BinaryDocValues mockDocTerms; + @Mock + Bits mockDocsWithField; + @Mock + Collator mockCollator; + + @Before + public void setUp() + { + initMocks(this); + reset(mockDocTerms, mockDocsWithField); + textSortFieldComparator.bottom = BOTTOM_STRING; + } + + /** Check that a zero length term is sorted before a populated field. */ + @Test + public void testCompareBottom_termLengthZeroAndDocDoesntHaveField() + { + // Set up the document to have an empty term. + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef()); + when(mockDocsWithField.get(DOC)).thenReturn(false); + + // Call the method under test. + int result = textSortFieldComparator.compareBottom(DOC); + + assertEquals("Expected value for doc to be null, and so it to be sorted before BOTTOM_TERM", 1, result); + } + + /** + * Check the behaviour of compareBottom when docsWithField is null (this happens when all documents contain the + * field). + */ + @Test + public void testCompareBottom_nullDocsWithField() + { + // Set docsWithField to null to simulate all documents containing the field. + Bits oldValue = textSortFieldComparator.docsWithField; + textSortFieldComparator.docsWithField = null; + + // Set up the document to have an empty term. + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef()); + + // Call the method under test. + textSortFieldComparator.compareBottom(DOC); + + // Expect the EMPTY_TERM to be compared + verify(mockCollator).compare(BOTTOM_STRING, ""); + + // Reset docsWithField with the mock after the test. + textSortFieldComparator.docsWithField = oldValue; + } + + /** Check that if the doc has a value then it is compared with the existing value. */ + @Test + public void testCompareBottom_populatedTerm() + { + // Set up the document to have "Some value" for the field. + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef("Some value")); + when(mockDocsWithField.get(DOC)).thenReturn(false); + + // Call the method under test. + textSortFieldComparator.compareBottom(DOC); + + verify(mockCollator).compare(BOTTOM_STRING, "Some value"); + } + + /** Check the behaviour if the multilanguage term is encoded. */ + @Test + public void testCompareBottom_encodedTerm_localeFound() + { + // Create an encoded multilanguage string with Russian, US English and Thai with Thai digits. + String mlText = "\u0000ru\u0000First\u0000Ignored" + + "\u0000en_US\u0000Second\u0000IgnoredToo" + + "\u0000th_TH_TH\u0000Third\u0000AlsoIgnored"; + // Set up the document to have an encoded value for the field. + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef(mlText)); + when(mockDocsWithField.get(DOC)).thenReturn(false); + + // Check that the Russian text can be extracted. + textSortFieldComparator.collatorLocale = Locale.forLanguageTag("ru"); + textSortFieldComparator.compareBottom(DOC); + verify(mockCollator).compare(BOTTOM_STRING, "First"); + + // Check that the English text can be extracted. + textSortFieldComparator.collatorLocale = Locale.forLanguageTag("en"); + textSortFieldComparator.compareBottom(DOC); + verify(mockCollator).compare(BOTTOM_STRING, "Second"); + + // Check that the Thai text can be extracted. + textSortFieldComparator.collatorLocale = Locale.forLanguageTag("th"); + textSortFieldComparator.compareBottom(DOC); + verify(mockCollator).compare(BOTTOM_STRING, "Third"); + + // Reset the locale for other tests. + textSortFieldComparator.collatorLocale = LOCALE; + } + + /** Check the behaviour if the term has a locale but no text. */ + @Test + public void testCompareBottom_badlyEncodedTerm() + { + // Set the value to have a locale but no text. + String mlText = "\u0000ru"; + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef(mlText)); + + // Call the method under test. + textSortFieldComparator.compareBottom(DOC); + + // Check that an empty string is assumed. + verify(mockCollator).compare(BOTTOM_STRING, ""); + } + + @Test + public void testCompareValues_nullLessThanString() + { + int result = textSortFieldComparator.compareValues(null, "NotNull"); + assertEquals("Expected null to be 'less' than string.", -1, result); + } + + @Test + public void testCompareValues_stringGreaterThanNull() + { + int result = textSortFieldComparator.compareValues("NotNull", null); + assertEquals("Expected string to be 'greater' than null.", 1, result); + } + + @Test + public void testCompareValues_nullEqualToNull() + { + int result = textSortFieldComparator.compareValues(null, null); + assertEquals("Expected two null values to be equal.", 0, result); + } + + /** Check that when two non-null strings are compared then the underlying collator is used to get the result. */ + @Test + public void testCompareValues_twoStringsCompared() + { + // An arbitrary value to be returned by the collator. + int comparisonResult = 10; + when(mockCollator.compare("NotNull1", "NotNull2")).thenReturn(comparisonResult); + + // Call the method under test. + int result = textSortFieldComparator.compareValues("NotNull1", "NotNull2"); + + verify(mockCollator).compare("NotNull1", "NotNull2"); + assertEquals("Expected result to be obtained from collator.", comparisonResult, result); + } +} diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/AlfrescoCollatableTextFieldTypeTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/AlfrescoCollatableTextFieldTypeTest.java new file mode 100644 index 000000000..be8609c27 --- /dev/null +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/AlfrescoCollatableTextFieldTypeTest.java @@ -0,0 +1,168 @@ +/* + * Copyright (C) 2005-2014 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ +package org.alfresco.solr; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +import java.text.Collator; +import java.util.Locale; + +import org.alfresco.solr.AlfrescoCollatableTextFieldType.TextSortFieldComparator; +import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; + +/** Unit tests for {@link AlfrescoCollatableTextFieldType}. */ +public class AlfrescoCollatableTextFieldTypeTest +{ + private static final int NUM_HITS = 3; + private static final String FIELD = "field"; + private static final Locale LOCALE = Locale.getDefault(); + /** A document id. */ + private static final int DOC = 0; + /** A value for the current bottom document. */ + private static final String BOTTOM_STRING = "Bottom"; + + @InjectMocks + TextSortFieldComparator textSortFieldComparator = new TextSortFieldComparator(NUM_HITS, FIELD, LOCALE); + @Mock + BinaryDocValues mockDocTerms; + @Mock + Bits mockDocsWithField; + @Mock + Collator mockCollator; + + @Before + public void setUp() + { + initMocks(this); + reset(mockDocTerms, mockDocsWithField); + textSortFieldComparator.bottom = BOTTOM_STRING; + } + + /** Check that a zero length term is sorted before a populated field. */ + @Test + public void testCompareBottom_termLengthZeroAndDocDoesntHaveField() + { + // Set up the document to have an empty term. + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef()); + when(mockDocsWithField.get(DOC)).thenReturn(false); + + // Call the method under test. + int result = textSortFieldComparator.compareBottom(DOC); + + assertEquals("Expected value for doc to be null, and so it to be sorted before BOTTOM_TERM", 1, result); + } + + /** + * Check the behaviour of compareBottom when docsWithField is null (this happens when all documents contain the + * field). + */ + @Test + public void testCompareBottom_nullDocsWithField() + { + // Set docsWithField to null to simulate all documents containing the field. + Bits oldValue = textSortFieldComparator.docsWithField; + textSortFieldComparator.docsWithField = null; + + // Set up the document to have an empty term. + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef()); + + // Call the method under test. + textSortFieldComparator.compareBottom(DOC); + + // Expect the EMPTY_TERM to be compared + verify(mockCollator).compare(BOTTOM_STRING, ""); + + // Reset docsWithField with the mock after the test. + textSortFieldComparator.docsWithField = oldValue; + } + + /** Check that if the doc has a value then it is compared with the existing value. */ + @Test + public void testCompareBottom_populatedTerm() + { + // Set up the document to have "Some value" for the field. + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef("Some value")); + when(mockDocsWithField.get(DOC)).thenReturn(false); + + // Call the method under test. + textSortFieldComparator.compareBottom(DOC); + + verify(mockCollator).compare(BOTTOM_STRING, "Some value"); + } + + /** Check the behaviour if the term is encoded. */ + @Test + public void testCompareBottom_encodedTerm() + { + // Set up the document to have an encoded value for the field. + when(mockDocTerms.get(DOC)).thenReturn(new BytesRef("\u0000Value\u0000Ignored")); + when(mockDocsWithField.get(DOC)).thenReturn(false); + + // Call the method under test. + textSortFieldComparator.compareBottom(DOC); + + verify(mockCollator).compare(BOTTOM_STRING, "Value"); + } + + @Test + public void testCompareValues_nullLessThanString() + { + int result = textSortFieldComparator.compareValues(null, "NotNull"); + assertEquals("Expected null to be 'less' than string.", -1, result); + } + + @Test + public void testCompareValues_stringGreaterThanNull() + { + int result = textSortFieldComparator.compareValues("NotNull", null); + assertEquals("Expected string to be 'greater' than null.", 1, result); + } + + @Test + public void testCompareValues_nullEqualToNull() + { + int result = textSortFieldComparator.compareValues(null, null); + assertEquals("Expected two null values to be equal.", 0, result); + } + + /** Check that when two non-null strings are compared then the underlying collator is used to get the result. */ + @Test + public void testCompareValues_twoStringsCompared() + { + // An arbitrary value to be returned by the collator. + int comparisonResult = 10; + when(mockCollator.compare("NotNull1", "NotNull2")).thenReturn(comparisonResult); + + // Call the method under test. + int result = textSortFieldComparator.compareValues("NotNull1", "NotNull2"); + + verify(mockCollator).compare("NotNull1", "NotNull2"); + assertEquals("Expected result to be obtained from collator.", comparisonResult, result); + } +} diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/query/AlfrescoSolrSortIT.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/query/AlfrescoSolrSortIT.java new file mode 100644 index 000000000..dd269a25b --- /dev/null +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/query/AlfrescoSolrSortIT.java @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2005-2014 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ +package org.alfresco.solr.query; + +import org.alfresco.solr.AbstractAlfrescoDistributedIT; +import org.apache.lucene.util.LuceneTestCase; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.util.NamedList; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; + +import static org.hamcrest.core.Is.is; + +/** + * https://issues.alfresco.com/jira/browse/SEARCH-2012 + */ +@SolrTestCaseJ4.SuppressSSL +@LuceneTestCase.SuppressCodecs({"Appending","Lucene3x","Lucene40","Lucene41","Lucene42","Lucene43", "Lucene44", "Lucene45","Lucene46","Lucene47","Lucene48","Lucene49"}) +public class AlfrescoSolrSortIT extends AbstractAlfrescoDistributedIT +{ + @BeforeClass + private static void initData() throws Throwable + { + initSolrServers(1, getClassName(), null); + } + + @AfterClass + private static void destroyData() + { + dismissSolrServers(); + } + + @After + public void clearData() throws Exception + { + deleteByQueryAllClients("*:*"); + } + + @Test + public void AlfrescoCollatableFieldType_emptyValuesSortingAsc__shouldBeRankedFirst() throws Exception { + prepareIndexSegmentWithAllNonNullFieldValues("text@s__sort@{http://www.alfresco.org/model/content/1.0}title"); + putHandleDefaults(); + // Docs with id 1, 3, 5 and 6 should be first (note that these will be sorted by indexing time). + String[] expectedRanking = new String[]{"1","3","5","6","4","2"}; + + QueryResponse response = query(getDefaultTestClient(), true, + "{\"query\":\"(id:(1 2 3 4 5 6))\",\"locales\":[\"en\"], \"templates\": [{\"name\":\"t1\", \"template\":\"%cm:content\"}], \"authorities\": [\"joel\"], \"tenants\": []}", + params("qt", "/afts", "shards.qt", "/afts", "start", "0", "rows", "100", "sort", "text@s__sort@{http://www.alfresco.org/model/content/1.0}title asc")); + + NamedList res = response.getResponse(); + SolrDocumentList searchResults = (SolrDocumentList)res.get("response"); + for(int i=0;i