From 04cee6a7db2bc6cf48f6d873d92865c988539a86 Mon Sep 17 00:00:00 2001
From: Joas Schilling <nickvergessen@owncloud.com>
Date: Fri, 22 Apr 2016 10:49:55 +0200
Subject: [PATCH] Change the sort order of background jobs to be DESC instead
 of ASC

In theory, if your instance ever creates more jobs then your system cron can
handle, the default background jobs get never executed anymore. Because
everytime when the joblist returns the next job it looks for the next ID,
however there is always a new next ID, so it will never wrap back to execute
the low IDs. But when we change the sort order to be DESC, we make sure that
these low IDs are always executed, before the system jumps back up to
execute the new IDs.
---
 lib/private/backgroundjob/joblist.php |  6 +++---
 tests/lib/backgroundjob/joblist.php   | 17 +++++++++++++----
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/lib/private/backgroundjob/joblist.php b/lib/private/backgroundjob/joblist.php
index b8230fca4d..2429b83044 100644
--- a/lib/private/backgroundjob/joblist.php
+++ b/lib/private/backgroundjob/joblist.php
@@ -172,8 +172,8 @@ class JobList implements IJobList {
 		$query = $this->connection->getQueryBuilder();
 		$query->select('*')
 			->from('jobs')
-			->where($query->expr()->gt('id', $query->createNamedParameter($lastId, IQueryBuilder::PARAM_INT)))
-			->orderBy('id', 'ASC')
+			->where($query->expr()->lt('id', $query->createNamedParameter($lastId, IQueryBuilder::PARAM_INT)))
+			->orderBy('id', 'DESC')
 			->setMaxResults(1);
 		$result = $query->execute();
 		$row = $result->fetch();
@@ -187,7 +187,7 @@ class JobList implements IJobList {
 			$query = $this->connection->getQueryBuilder();
 			$query->select('*')
 				->from('jobs')
-				->orderBy('id', 'ASC')
+				->orderBy('id', 'DESC')
 				->setMaxResults(1);
 			$result = $query->execute();
 			$row = $result->fetch();
diff --git a/tests/lib/backgroundjob/joblist.php b/tests/lib/backgroundjob/joblist.php
index c0796d8253..fcc3c44081 100644
--- a/tests/lib/backgroundjob/joblist.php
+++ b/tests/lib/backgroundjob/joblist.php
@@ -9,6 +9,7 @@
 namespace Test\BackgroundJob;
 
 use OCP\BackgroundJob\IJob;
+use OCP\IDBConnection;
 use Test\TestCase;
 
 /**
@@ -28,10 +29,17 @@ class JobList extends TestCase {
 		parent::setUp();
 
 		$connection = \OC::$server->getDatabaseConnection();
+		$this->clearJobsList($connection);
 		$this->config = $this->getMock('\OCP\IConfig');
 		$this->instance = new \OC\BackgroundJob\JobList($connection, $this->config);
 	}
 
+	protected function clearJobsList(IDBConnection $connection) {
+		$query = $connection->getQueryBuilder();
+		$query->delete('jobs');
+		$query->execute();
+	}
+
 	protected function getAllSorted() {
 		$jobs = $this->instance->getAll();
 
@@ -149,11 +157,11 @@ class JobList extends TestCase {
 		$this->config->expects($this->once())
 			->method('getAppValue')
 			->with('backgroundjob', 'lastjob', 0)
-			->will($this->returnValue($savedJob1->getId()));
+			->will($this->returnValue($savedJob2->getId()));
 
 		$nextJob = $this->instance->getNext();
 
-		$this->assertEquals($savedJob2, $nextJob);
+		$this->assertEquals($savedJob1, $nextJob);
 
 		$this->instance->remove($job, 1);
 		$this->instance->remove($job, 2);
@@ -166,16 +174,17 @@ class JobList extends TestCase {
 
 		$jobs = $this->getAllSorted();
 
+		$savedJob1 = $jobs[count($jobs) - 2];
 		$savedJob2 = $jobs[count($jobs) - 1];
 
 		$this->config->expects($this->once())
 			->method('getAppValue')
 			->with('backgroundjob', 'lastjob', 0)
-			->will($this->returnValue($savedJob2->getId()));
+			->will($this->returnValue($savedJob1->getId()));
 
 		$nextJob = $this->instance->getNext();
 
-		$this->assertEquals($jobs[0], $nextJob);
+		$this->assertEquals($savedJob2, $nextJob);
 
 		$this->instance->remove($job, 1);
 		$this->instance->remove($job, 2);