From 8722458d2a2342517e45674129ec5c7d071a4077 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 11 Oct 2017 17:25:21 +0200
Subject: [PATCH] ensure that sorting is stable

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 .../lib/Collaboration/CommentersSorter.php    | 17 ++++++++++++++---
 .../Collaboration/CommentersSorterTest.php    |  2 +-
 .../Collaboration/ShareRecipientSorter.php    | 19 +++++++++++++++++--
 .../ShareRecipientSorterTest.php              | 10 +++++-----
 4 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/apps/comments/lib/Collaboration/CommentersSorter.php b/apps/comments/lib/Collaboration/CommentersSorter.php
index e89a66f148..8a24592c30 100644
--- a/apps/comments/lib/Collaboration/CommentersSorter.php
+++ b/apps/comments/lib/Collaboration/CommentersSorter.php
@@ -58,12 +58,23 @@ class CommentersSorter implements ISorter {
 				continue;
 			}
 
-			usort($byType, function ($a, $b) use ($commenters, $type) {
-				$r = $this->compare($a, $b, $commenters[$type]);
+			// at least on PHP 5.6 usort turned out to be not stable. So we add
+			// the current index to the value and compare it on a draw
+			$i = 0;
+			$workArray = array_map(function($element) use (&$i) {
+				return [$i++, $element];
+			}, $byType);
+
+			usort($workArray, function ($a, $b) use ($commenters, $type) {
+				$r = $this->compare($a[1], $b[1], $commenters[$type]);
+				if($r === 0) {
+					$r = $a[0] - $b[0];
+				}
 				return $r;
 			});
 
-			$s = '';
+			// and remove the index values again
+			$byType = array_column($workArray, 1);
 		}
 	}
 
diff --git a/apps/comments/tests/Unit/Collaboration/CommentersSorterTest.php b/apps/comments/tests/Unit/Collaboration/CommentersSorterTest.php
index 495dee1f41..95a74f118c 100644
--- a/apps/comments/tests/Unit/Collaboration/CommentersSorterTest.php
+++ b/apps/comments/tests/Unit/Collaboration/CommentersSorterTest.php
@@ -59,7 +59,7 @@ class CommentersSorterTest extends TestCase {
 		$workArray = $data['input'];
 		$this->sorter->sort($workArray, ['itemType' => 'files', 'itemId' => '24']);
 
-		$this->assertSame($data['expected'], $workArray);
+		$this->assertEquals($data['expected'], $workArray);
 	}
 
 	public function sortDataProvider() {
diff --git a/apps/files_sharing/lib/Collaboration/ShareRecipientSorter.php b/apps/files_sharing/lib/Collaboration/ShareRecipientSorter.php
index ba07d9d44d..1a722817cb 100644
--- a/apps/files_sharing/lib/Collaboration/ShareRecipientSorter.php
+++ b/apps/files_sharing/lib/Collaboration/ShareRecipientSorter.php
@@ -61,9 +61,24 @@ class ShareRecipientSorter implements ISorter {
 			if(!isset($al[$type]) || !is_array($al[$type])) {
 				continue;
 			}
-			usort($byType, function ($a, $b) use ($al, $type) {
-				return  $this->compare($a, $b, $al[$type]);
+
+			// at least on PHP 5.6 usort turned out to be not stable. So we add
+			// the current index to the value and compare it on a draw
+			$i = 0;
+			$workArray = array_map(function($element) use (&$i) {
+				return [$i++, $element];
+			}, $byType);
+
+			usort($workArray, function ($a, $b) use ($al, $type) {
+				$result = $this->compare($a[1], $b[1], $al[$type]);
+				if($result === 0) {
+					$result = $a[0] - $b[0];
+				}
+				return $result;
 			});
+
+			// and remove the index values again
+			$byType = array_column($workArray, 1);
 		}
 	}
 
diff --git a/apps/files_sharing/tests/Collaboration/ShareRecipientSorterTest.php b/apps/files_sharing/tests/Collaboration/ShareRecipientSorterTest.php
index edcc9b9ae3..42487e6162 100644
--- a/apps/files_sharing/tests/Collaboration/ShareRecipientSorterTest.php
+++ b/apps/files_sharing/tests/Collaboration/ShareRecipientSorterTest.php
@@ -74,7 +74,7 @@ class ShareRecipientSorterTest extends TestCase {
 		$workArray = $data['input'];
 		$this->sorter->sort($workArray, $data['context']);
 
-		$this->assertSame($data['expected'], $workArray);
+		$this->assertEquals($data['expected'], $workArray);
 	}
 
 	public function testSortNoNodes() {
@@ -98,7 +98,7 @@ class ShareRecipientSorterTest extends TestCase {
 	public function sortDataProvider() {
 		return [[
 			[
-				#1 – sort properly and otherwise keep existing order
+				#0 – sort properly and otherwise keep existing order
 				'context' => ['itemType' => 'files', 'itemId' => 42],
 				'accessList' => ['users' => ['celia', 'darius', 'faruk', 'gail'], 'bots' => ['r2-d2']],
 				'input' => [
@@ -135,7 +135,7 @@ class ShareRecipientSorterTest extends TestCase {
 				],
 			],
 			[
-				# 2 – no recipients
+				#1 – no recipients
 				'context' => ['itemType' => 'files', 'itemId' => 42],
 				'accessList' => ['users' => false],
 				'input' => [
@@ -172,7 +172,7 @@ class ShareRecipientSorterTest extends TestCase {
 				],
 			],
 			[
-				#3 – unsupported item  type
+				#2 – unsupported item  type
 				'context' => ['itemType' => 'announcements', 'itemId' => 42],
 				'accessList' => null, // not needed
 				'input' => [
@@ -209,7 +209,7 @@ class ShareRecipientSorterTest extends TestCase {
 				],
 			],
 			[
-				#4 – no nothing
+				#3 – no nothing
 				'context' => ['itemType' => 'files', 'itemId' => 42],
 				'accessList' => [],
 				'input' => [],