Merge pull request #19329 from nextcloud/feature/noid/warn-admins-about-delayed-cron

Warn admins about delayed cron executions
This commit is contained in:
Joas Schilling 2020-02-17 09:06:27 +01:00 committed by GitHub
commit a1fb57d2e2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 89 additions and 25 deletions

View File

@ -28,17 +28,25 @@
namespace OCA\Settings\Settings\Admin;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Settings\ISettings;
class Server implements ISettings {
/** @var IDBConnection */
private $connection;
/** @var ITimeFactory */
private $timeFactory;
/** @var IConfig */
private $config;
/**
* @param IConfig $config
*/
public function __construct(IConfig $config) {
public function __construct(IDBConnection $connection,
ITimeFactory $timeFactory,
IConfig $config) {
$this->connection = $connection;
$this->timeFactory = $timeFactory;
$this->config = $config;
}
@ -50,7 +58,8 @@ class Server implements ISettings {
// Background jobs
'backgroundjobs_mode' => $this->config->getAppValue('core', 'backgroundjobs_mode', 'ajax'),
'lastcron' => $this->config->getAppValue('core', 'lastcron', false),
'cronErrors' => $this->config->getAppValue('core', 'cronErrors'),
'cronMaxAge' => $this->cronMaxAge(),
'cronErrors' => $this->config->getAppValue('core', 'cronErrors'),
'cli_based_cron_possible' => function_exists('posix_getpwuid'),
'cli_based_cron_user' => function_exists('posix_getpwuid') ? posix_getpwuid(fileowner(\OC::$configDir . 'config.php'))['name'] : '',
];
@ -58,6 +67,24 @@ class Server implements ISettings {
return new TemplateResponse('settings', 'settings/admin/server', $parameters, '');
}
protected function cronMaxAge(): int {
$query = $this->connection->getQueryBuilder();
$query->select('last_checked')
->from('jobs')
->orderBy('last_checked', 'ASC')
->setMaxResults(1);
$result = $query->execute();
if ($row = $result->fetch()) {
$maxAge = (int) $row['last_checked'];
} else {
$maxAge = $this->timeFactory->getTime();
}
$result->closeCursor();
return $maxAge;
}
/**
* @return string the section ID, e.g. 'sharing'
*/

View File

@ -29,26 +29,40 @@
<div class="section" id="backgroundjobs">
<h2 class="inlineblock"><?php p($l->t('Background jobs'));?></h2>
<p class="cronlog inlineblock">
<?php if ($_['lastcron'] !== false):
<?php if ($_['lastcron'] !== false) {
$relative_time = relative_modified_date($_['lastcron']);
$maxAgeRelativeTime = relative_modified_date($_['cronMaxAge']);
$formatter = \OC::$server->getDateTimeFormatter();
$absolute_time = $formatter->formatDateTime($_['lastcron'], 'long', 'long');
if (time() - $_['lastcron'] <= 600): ?>
<span class="status success"></span>
<span class="crondate" title="<?php p($absolute_time);?>">
<?php p($l->t("Last job ran %s.", [$relative_time]));?>
</span>
<?php else: ?>
$maxAgeAbsoluteTime = $formatter->formatDateTime($_['cronMaxAge'], 'long', 'long');
if (time() - $_['lastcron'] > 600) { ?>
<span class="status error"></span>
<span class="crondate" title="<?php p($absolute_time);?>">
<?php p($l->t("Last job execution ran %s. Something seems wrong.", [$relative_time]));?>
</span>
<?php endif;
else: ?>
<?php p($l->t("Last job execution ran %s. Something seems wrong.", [$relative_time]));?>
</span>
<?php } else if (time() - $_['cronMaxAge'] > 12*3600) {
if ($_['backgroundjobs_mode'] === 'cron') { ?>
<span class="status warning"></span>
<span class="crondate" title="<?php p($maxAgeAbsoluteTime);?>">
<?php p($l->t("Some jobs havent been executed since %s. Please consider increasing the execution frequency.", [$maxAgeRelativeTime]));?>
</span>
<?php } else { ?>
<span class="status error"></span>
<span class="crondate" title="<?php p($maxAgeAbsoluteTime);?>">
<?php p($l->t("Some jobs didnt execute since %s. Please consider switching to system cron.", [$maxAgeRelativeTime]));?>
</span>
<?php }
} else { ?>
<span class="status success"></span>
<span class="crondate" title="<?php p($absolute_time);?>">
<?php p($l->t("Last job ran %s.", [$relative_time]));?>
</span>
<?php }
} else { ?>
<span class="status error"></span>
<?php p($l->t("Background job didnt run yet!"));
endif; ?>
} ?>
</p>
<a target="_blank" rel="noreferrer noopener" class="icon-info"
title="<?php p($l->t('Open documentation'));?>"

View File

@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2016 Lukas Reschke <lukas@statuscode.ch>
*
@ -31,25 +32,46 @@ namespace OCA\Settings\Tests\Settings\Admin;
use OCA\Settings\Settings\Admin\Server;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\IDBConnection;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
/**
* @group DB
*/
class ServerTest extends TestCase {
/** @var Server */
private $admin;
/** @var IConfig */
/** @var IDBConnection */
private $connection;
/** @var ITimeFactory|MockObject */
private $timeFactory;
/** @var IConfig|MockObject */
private $config;
protected function setUp(): void {
parent::setUp();
$this->connection = \OC::$server->getDatabaseConnection();
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->config = $this->createMock(IConfig::class);
$this->admin = new Server(
$this->config
);
$this->admin = $this->getMockBuilder(Server::class)
->onlyMethods(['cronMaxAge'])
->setConstructorArgs([
$this->connection,
$this->timeFactory,
$this->config,
])
->getMock();
}
public function testGetForm() {
public function testGetForm(): void {
$this->admin->expects($this->once())
->method('cronMaxAge')
->willReturn(1337);
$this->config
->expects($this->at(0))
->method('getAppValue')
@ -71,7 +93,8 @@ class ServerTest extends TestCase {
[
'backgroundjobs_mode' => 'ajax',
'lastcron' => false,
'cronErrors' => '',
'cronErrors' => '',
'cronMaxAge' => 1337,
'cli_based_cron_possible' => true,
'cli_based_cron_user' => function_exists('posix_getpwuid') ? posix_getpwuid(fileowner(\OC::$configDir . 'config.php'))['name'] : '', // to not explode here because of posix extension not being disabled - which is already checked in the line above
],
@ -81,11 +104,11 @@ class ServerTest extends TestCase {
$this->assertEquals($expected, $this->admin->getForm());
}
public function testGetSection() {
public function testGetSection(): void {
$this->assertSame('server', $this->admin->getSection());
}
public function testGetPriority() {
public function testGetPriority(): void {
$this->assertSame(0, $this->admin->getPriority());
}
}