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