Merge pull request #14213 from AW-UC/naturalsort_defaultcollator-patch-file-sorting

Update naturalsort_defaultcollator.php. Fixes #13982

3 files changed, 115 insertions, 12 deletions

diff --git a/lib/private/naturalsort.php b/lib/private/naturalsort.php
index ef242eb7ab9..535e4a7676f 100644
--- a/lib/private/naturalsort.php
+++ b/lib/private/naturalsort.php
@@ -28,6 +28,19 @@ class NaturalSort {
 	private $cache = array();
 
 	/**
+	 * Instantiate a new \OC\NaturalSort instance.
+	 * @param object $injectedCollator
+	 */
+	public function __construct($injectedCollator = null) {
+		// inject an instance of \Collator('en_US') to force using the php5-intl Collator
+		// or inject an instance of \OC\NaturalSort_DefaultCollator to force using Owncloud's default collator
+		if (isset($injectedCollator)) {
+			$this->collator = $injectedCollator;
+			\OC_Log::write('core', 'forced use of '.get_class($injectedCollator), \OC_Log::DEBUG);
+		}
+	}
+
+	/**
 	 * Split the given string in chunks of numbers and strings
 	 * @param string $t string
 	 * @return array of strings and number chunks
diff --git a/lib/private/naturalsort_defaultcollator.php b/lib/private/naturalsort_defaultcollator.php
index f777cfd794f..94db6dfbae5 100644
--- a/lib/private/naturalsort_defaultcollator.php
+++ b/lib/private/naturalsort_defaultcollator.php
@@ -22,9 +22,13 @@ namespace OC;
 
 class NaturalSort_DefaultCollator {
 	public function compare($a, $b) {
-		if ($a === $b) {
-			return 0;
+		$result = strcasecmp($a, $b);
+		if ($result === 0) {
+			if ($a === $b) {
+				return 0;
+			}
+			return ($a > $b) ? -1 : 1;
 		}
-		return ($a < $b) ? -1 : 1;
+		return ($result < 0) ? -1 : 1;
 	}
 }
diff --git a/tests/lib/naturalsort.php b/tests/lib/naturalsort.php
index e022a855309..8fcbc6f5fd3 100644
--- a/tests/lib/naturalsort.php
+++ b/tests/lib/naturalsort.php
@@ -8,27 +8,32 @@
 
 class Test_NaturalSort extends \Test\TestCase {
 
-	public function setUp() {
-		parent::setUp();
-
+	/**
+	 * @dataProvider naturalSortDataProvider
+	 */
+	public function testNaturalSortCompare($array, $sorted)
+	{
 		if(!class_exists('Collator')) {
-			$this->markTestSkipped('The intl module is not available, natural sorting will not work as expected.');
+			$this->markTestSkipped('The intl module is not available, natural sorting might not work as expected.');
 			return;
 		}
+		$comparator = \OC\NaturalSort::getInstance();
+		usort($array, array($comparator, 'compare'));
+		$this->assertEquals($sorted, $array);
 	}
 
 	/**
-	 * @dataProvider naturalSortDataProvider
-	 */
-	public function testNaturalSortCompare($array, $sorted)
+	* @dataProvider defaultCollatorDataProvider
+	*/
+	public function testDefaultCollatorCompare($array, $sorted)
 	{
-		$comparator = \OC\NaturalSort::getInstance();
+		$comparator = new \OC\NaturalSort(new \OC\NaturalSort_DefaultCollator());
 		usort($array, array($comparator, 'compare'));
 		$this->assertEquals($sorted, $array);
 	}
 
 	/**
-	 * Data provider for natural sort.
+	 * Data provider for natural sorting with php5-intl's Collator.
 	 * Must provide the same result as in core/js/tests/specs/coreSpec.js
 	 * @return array test cases
 	 */
@@ -181,4 +186,85 @@ class Test_NaturalSort extends \Test\TestCase {
 			),
 		);
 	}
+
+	/**
+	* Data provider for natural sorting with \OC\NaturalSort_DefaultCollator.
+	* Must provide the same result as in core/js/tests/specs/coreSpec.js
+	* @return array test cases
+	*/
+	public function defaultCollatorDataProvider()
+	{
+		return array(
+			// different casing
+			array(
+				// unsorted
+				array(
+					'aaa',
+					'bbb',
+					'BBB',
+					'AAA'
+				),
+				// sorted
+				array(
+					'aaa',
+					'AAA',
+					'bbb',
+					'BBB'
+				)
+			),
+			// numbers
+			array(
+				// unsorted
+				array(
+					'124.txt',
+					'abc1',
+					'123.txt',
+					'abc',
+					'abc2',
+					'def (2).txt',
+					'ghi 10.txt',
+					'abc12',
+					'def.txt',
+					'def (1).txt',
+					'ghi 2.txt',
+					'def (10).txt',
+					'abc10',
+					'def (12).txt',
+					'z',
+					'ghi.txt',
+					'za',
+					'ghi 1.txt',
+					'ghi 12.txt',
+					'zz',
+					'15.txt',
+					'15b.txt',
+				),
+				// sorted
+				array(
+					'15.txt',
+					'15b.txt',
+					'123.txt',
+					'124.txt',
+					'abc',
+					'abc1',
+					'abc2',
+					'abc10',
+					'abc12',
+					'def.txt',
+					'def (1).txt',
+					'def (2).txt',
+					'def (10).txt',
+					'def (12).txt',
+					'ghi.txt',
+					'ghi 1.txt',
+					'ghi 2.txt',
+					'ghi 10.txt',
+					'ghi 12.txt',
+					'z',
+					'za',
+					'zz',
+				)
+			),
+		);
+	}
 }