Skip to content
Snippets Groups Projects
Commit d0b3cea1 authored by Jan-Hendrik Willms's avatar Jan-Hendrik Willms
Browse files

cronjobs: remove scheduling once and priority, fixes #4078 

Closes #4078

Merge request studip/studip!2922
parent 260b6ac6
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 111 additions and 306 deletions
RELEASE-NOTES.md
+ 1
− 0
View file @ d0b3cea1
......@@ -18,6 +18,7 @@
- Die Funktionen `studip_json_encode()` und `studip_json_decode()` wurden entfernt. Stattdessen müssen die Methode `json_encode()` und `json_decode()` verwendet werden. ([Issue #3814](https://gitlab.studip.de/studip/studip/-/issues/3814))
- Die `MembersModel.php` wurde entfernt ([Issue #3811](https://gitlab.studip.de/studip/studip/-/issues/3811))
- Die `admission.inc.php` wurde entfernt. ([Issue #3812](https://gitlab.studip.de/studip/studip/-/issues/3812))
- Die Methoden `CronjobScheduler::scheduleOnce()` sowie `CronjobTask::scheduleOnce()` wurden ersatzlos entfernt. ([Issue #4078](https://gitlab.studip.de/studip/studip/-/issues/4078))
## Security related issues
......
app/controllers/admin/cronjobs/schedules.php
+ 14
− 25
View file @ d0b3cea1
......@@ -29,7 +29,7 @@ class Admin_Cronjobs_SchedulesController extends AuthenticatedController
if (empty($_SESSION['cronjob-filter'])) {
$_SESSION['cronjob-filter'] = [
'where' => '1',
'values' => array_fill_keys(['type', 'status', 'task_id'], null),
'values' => array_fill_keys(['status', 'task_id'], null),
];
}
......@@ -102,9 +102,6 @@ class Admin_Cronjobs_SchedulesController extends AuthenticatedController
$filter = array_filter(Request::optionArray('filter'));
$conditions = [];
if (!empty($filter['type'])) {
$conditions[] = "type = " . DBManager::get()->quote($filter['type']);
}
if (!empty($filter['status'])) {
$active = (int)($filter['status'] === 'active');
$conditions[] = "active = " . DBManager::get()->quote($active);
......@@ -130,7 +127,6 @@ class Admin_Cronjobs_SchedulesController extends AuthenticatedController
if (Request::submitted('store')) {
$parameters = Request::getArray('parameters');
$schedule->priority = Request::option('priority', 'normal');
$schedule->title = Request::get('title');
$schedule->description = Request::get('description');
$schedule->active = Request::int('active', 0);
......@@ -138,27 +134,20 @@ class Admin_Cronjobs_SchedulesController extends AuthenticatedController
$schedule->task_id = Request::option('task_id');
}
$schedule->parameters = $parameters[$schedule->task_id];
$schedule->type = Request::option('type') === 'once'
? 'once'
: 'periodic';
if ($schedule->type === 'once') {
$temp = Request::getArray('once');
$schedule->next_execution = strtotime($temp['date'] . ' ' . $temp['time']);
} else {
$temp = Request::getArray('periodic');
$schedule->minute = $this->extractCronItem($temp['minute']);
$schedule->hour = $this->extractCronItem($temp['hour']);
$schedule->day = $this->extractCronItem($temp['day']);
$schedule->month = $this->extractCronItem($temp['month']);
$schedule->day_of_week = mb_strlen($temp['day_of_week']['value'])
? (int) $temp['day_of_week']['value']
: null;
if ($schedule->active) {
$schedule->next_execution = $schedule->calculateNextExecution();
}
$temp = Request::getArray('periodic');
$schedule->minute = $this->extractCronItem($temp['minute']);
$schedule->hour = $this->extractCronItem($temp['hour']);
$schedule->day = $this->extractCronItem($temp['day']);
$schedule->month = $this->extractCronItem($temp['month']);
$schedule->day_of_week = mb_strlen($temp['day_of_week']['value'])
? (int) $temp['day_of_week']['value']
: null;
if ($schedule->active) {
$schedule->next_execution = $schedule->calculateNextExecution();
}
$schedule->store();
PageLayout::postSuccess(_('Die Änderungen wurden gespeichert.'));
......
app/views/admin/cronjobs/schedules/display.php
+ 1
− 21
View file @ d0b3cea1
......@@ -16,9 +16,6 @@
<dt><?= _('Aktiv') ?></dt>
<dd><?= $schedule->active ? _('Ja') : _('Nein') ?></dd>
<dt><?= _('Priorität') ?></dt>
<dd><?= CronjobSchedule::describePriority($schedule->priority) ?></dd>
<? if (count($schedule->parameters) > 0): ?>
<dt><?= _('Parameter') ?></dt>
<dd>
......@@ -33,23 +30,8 @@
<dt><?= _('Aufgabe') ?></dt>
<dd><?= htmlReady($schedule->task->name) ?></dd>
<dt><?= _('Typ') ?></dt>
<? if ($schedule->type === 'once'): ?>
<dd>
<?= sprintf(_('Einmalig am %s um %s'), date('d.m.Y', $schedule->next_execution), date('H:i', $schedule->next_execution)) ?>
</dd>
<dt><?= _('Ausgeführt') ?>?</dt>
<dd>
<? if ($schedule->execution_count > 0): ?>
<?= _('Ja') ?>, <?= sprintf(_('am %s um %s'), date('d.m.Y', $schedule->last_execution), date('H:i:s', $schedule->last_execution)) ?>
<? else: ?>
<?= _('Nein') ?>
<? endif; ?>
</dd>
<? else: ?>
<dt><?= _('Ausführungsrhytmus') ?></dt>
<dd>
<?= _('Regelmässig') ?>
<?= $this->render_partial('admin/cronjobs/schedules/periodic-schedule', $schedule->toArray()) ?>
</dd>
......@@ -68,8 +50,6 @@
<dt><?= _('Letztes Ergebnis') ?></dt>
<dd><code><?= htmlReady($schedule->last_result) ?></code></dd>
<? endif; ?>
<? endif; ?>
</dl>
<div data-dialog-button>
......
app/views/admin/cronjobs/schedules/edit.php
+ 0
− 39
View file @ d0b3cea1
......@@ -41,17 +41,6 @@ $days_of_week = [
<?= _('Aktiv') ?>
</label>
<label>
<?= _('Priorität') ?>
<select name="priority" id="priority">
<? foreach (CronjobSchedule::getPriorities() as $priority => $label): ?>
<option value="<?= $priority ?>" <? if ((!$schedule->priority && $priority === CronjobSchedule::PRIORITY_NORMAL) || $schedule->priority === $priority) echo 'selected'; ?>>
<?= htmlReady($label) ?>
</option>
<? endforeach; ?>
</select>
</label>
<label>
<?= _('Titel') ?>
<input type="text" name="title" id="title" value="<?= htmlReady($schedule->title ?: '') ?>">
......@@ -118,14 +107,6 @@ $days_of_week = [
<fieldset>
<legend><?= _('Zeitplan') ?></legend>
<label>
<input type="radio" name="type" value="periodic"
data-activates="[name^='periodic']"
data-deactivates="[name^='once']"
<? if ($schedule->type === 'periodic') echo 'checked'; ?>>
<?= _('Wiederholt') ?>
</label>
<section>
<table class="default">
<colgroup>
......@@ -218,26 +199,6 @@ $days_of_week = [
</tbody>
</table>
</section>
<label>
<input type="radio" name="type" value="once"
data-activates="input[name^='once']"
data-deactivates="[name^='periodic']"
<? if ($schedule->type === 'once') echo 'checked'; ?>>
<?= _('Einmalig') ?>
</label>
<label class="col-1">
<?= _('Datum') ?>
<input type="text" name="once[date]" data-date-picker class="size-s"
value="<? if ($schedule->type === 'once' && $schedule->next_execution) echo date('d.m.Y', $schedule->next_execution); ?>">
</label>
<label class="col-1">
<?= _('Uhrzeit') ?>
<input type="text" name="once[time]" data-time-picker class="size-s"
value="<? if ($schedule->type === 'once' && $schedule->next_execution) echo date('H:i', $schedule->next_execution) ?>">
</label>
</fieldset>
<footer class="buttons">
......
app/views/admin/cronjobs/schedules/index.php
+ 4
− 25
View file @ d0b3cea1
......@@ -18,19 +18,7 @@ use Studip\Button, Studip\LinkButton;
<?= sprintf(_('Passend: %u von %u Cronjobs'), count($schedules), $total) ?>
<? endif; ?>
</legend>
<label class="col-2">
<?= _('Typ') ?>
<select name="filter[type]" id="type" class="submit-upon-select">
<option value=""><?= _('Alle Cronjobs anzeigen') ?></option>
<option value="once" <? if ($filter['type'] === 'once') echo 'selected'; ?>>
<?= _('Nur einmalige Cronjobs anzeigen') ?>
</option>
<option value="periodic" <? if ($filter['type'] === 'periodic') echo 'selected'; ?>>
<?= _('Nur regelmässige Cronjobs anzeigen') ?>
</option>
</select>
</label>
<label class="col-2">
<label class="col-3">
<?= _('Aufgabe') ?>
<select name="filter[task_id]" id="task_id" class="submit-upon-select">
<option value=""><?= _('Alle Cronjobs anzeigen') ?></option>
......@@ -41,7 +29,7 @@ use Studip\Button, Studip\LinkButton;
<? endforeach; ?>
</select>
</label>
<label class="col-2">
<label class="col-3">
<?= _('Status') ?>
<select name="filter[status]" id="status" class="submit-upon-select">
<option value=""><?= _('Alle Cronjobs anzeigen') ?></option>
......@@ -74,7 +62,6 @@ use Studip\Button, Studip\LinkButton;
<col style="width: 20px">
<col>
<col style="width: 40px">
<col style="width: 100px">
<col style="width: 30px">
<col style="width: 30px">
<col style="width: 30px">
......@@ -91,7 +78,6 @@ use Studip\Button, Studip\LinkButton;
</th>
<th data-sort="text"><?= _('Cronjob') ?></th>
<th data-sort="htmldata"><?= _('Aktiv') ?></th>
<th data-sort="text"><?= _('Typ') ?></th>
<th colspan="5" data-sort="false"><?= _('Ausführung') ?></th>
<th data-sort="false"><?= _('Optionen') ?></th>
</tr>
......@@ -124,14 +110,7 @@ use Studip\Button, Studip\LinkButton;
</a>
<? endif; ?>
</td>
<td><?= $schedule->type === 'once' ? _('Einmalig') : _('Regelmässig') ?></td>
<? if ($schedule->type === 'once'): ?>
<td colspan="5">
<?= strftime('%x %R', $schedule->next_execution) ?>
</td>
<? else: ?>
<?= $this->render_partial('admin/cronjobs/schedules/periodic-schedule', $schedule->toArray() + ['display' => 'table-cells']) ?>
<? endif; ?>
<?= $this->render_partial('admin/cronjobs/schedules/periodic-schedule', $schedule->toArray() + ['display' => 'table-cells']) ?>
<td style="text-align: right">
<a data-dialog href="<?= $controller->display($schedule) ?>">
<?= Icon::create('admin')->asImg(['title' => _('Cronjob anzeigen')]) ?>
......@@ -152,7 +131,7 @@ use Studip\Button, Studip\LinkButton;
</tbody>
<tfoot>
<tr>
<td colspan="10">
<td colspan="9">
<select name="action" data-activates=".cronjobs button[name=bulk]" aria-label="<?= _('Aktion auswählen')?>">
<option value="">- <?= _('Aktion auswählen') ?> -</option>
<option value="activate"><?= _('Aktivieren') ?></option>
......
db/migrations/1.231_add_files_search_index.php
+ 1
− 6
View file @ d0b3cea1
......@@ -181,12 +181,7 @@ class AddFilesSearchIndex extends Migration
private function installCronjob()
{
$scheduler = CronjobScheduler::getInstance();
require_once 'lib/classes/FilesSearch/Cronjob.php';
$task = new \FilesSearch\Cronjob();
$taskId = $scheduler->registerTask($task);
$scheduler->scheduleOnce($taskId, strtotime('+1 minute'))->activate();
$scheduler->schedulePeriodic($taskId, 55, 0)->activate();
\FilesSearch\Cronjob::register()->schedule(55, 0)->activate(true);
}
}
db/migrations/6.0.1_remove_cronjobs_scheduling_once_and_priority.php 0 → 100644
+ 19
− 0
View file @ d0b3cea1
<?php
return new class extends Migration
{
protected function up()
{
$query = "ALTER TABLE `cronjobs_schedules`
DROP COLUMN `priority`,
DROP COLUMN `type`";
DBManager::get()->exec($query);
}
protected function down()
{
$query = "ALTER TABLE `cronjobs_schedules`
ADD COLUMN `priority` ENUM('low', 'normal', 'high') COLLATE `latin1_bin` DEFAULT NULL AFTER `parameters`,
ADD COLUMN `type` ENUM('periodic', 'once') COLLATE `latin1_bin` DEFAULT NULL AFTER `priority`";
DBManager::get()->exec($query);
}
};
lib/classes/CronjobScheduler.class.php
+ 28
− 39
View file @ d0b3cea1
......@@ -131,35 +131,6 @@ class CronjobScheduler
return $this;
}
/**
* Schedules a task for a single execution at the provided time.
*
* @param String $task_id The id of the task to be executed
* @param int $timestamp When the task should be executed
* @param String $priority Priority of the execution (low, normal, high),
* defaults to normal
* @param Array $parameters Optional parameters passed to the task
* @return CronjobSchedule The generated schedule object.
*/
public function scheduleOnce($task_id, $timestamp, $priority = CronjobSchedule::PRIORITY_NORMAL,
$parameters = [])
{
$schedule = new CronjobSchedule();
$schedule->type = 'once';
$schedule->task_id = $task_id;
$schedule->parameters = $parameters;
$schedule->priority = $priority;
$schedule->next_execution = $timestamp;
$schedule->store();
$task = $schedule->task;
$task->assigned_count += 1;
$task->store();
return $schedule;
}
/**
* Schedules a task for periodic execution with the provided schedule.
*
......@@ -185,21 +156,21 @@ class CronjobScheduler
* - 1 >= x >= 7 for "exactly at day of week x"
* (x starts with monday at 1 and ends with
* sunday at 7)
* @param String $priority Priority of the execution (low, normal, high),
* defaults to normal
* @param Array $parameters Optional parameters passed to the task
* @return CronjobSchedule The generated schedule object.
*/
public function schedulePeriodic($task_id, $minute = null, $hour = null,
$day = null, $month = null, $day_of_week = null,
$priority = CronjobSchedule::PRIORITY_NORMAL,
$parameters = [])
{
public function schedule(
string $task_id,
?int $minute = null,
?int $hour = null,
?int $day = null,
?int $month = null,
?int $day_of_week = null,
array $parameters = []
): CronjobSchedule {
$schedule = new CronjobSchedule();
$schedule->type = 'periodic';
$schedule->task_id = $task_id;
$schedule->parameters = $parameters;
$schedule->priority = $priority;
$schedule->minute = $minute;
$schedule->hour = $hour;
......@@ -216,6 +187,24 @@ class CronjobScheduler
return $schedule;
}
/**
* An alias for schedule for backwards compatibility.
*
* @see CronjobScheduler::schedule()
*/
public function schedulePeriodic(
$task_id,
$minute = null,
$hour = null,
$day = null,
$month = null,
$day_of_week = null,
$priority = null,
$parameters = []
) {
return $this->schedule($task_id, $minute, $hour, $day, $month, $day_of_week, $parameters);
}
/**
* Cancels the provided schedule.
*
......@@ -259,7 +248,7 @@ class CronjobScheduler
// Find all schedules that are due to execute and which task is active
$temp = CronjobSchedule::findBySQL('active = 1 AND next_execution <= UNIX_TIMESTAMP() '
.'ORDER BY priority DESC, next_execution ASC');
.'ORDER next_execution');
$schedules = array_filter($temp, function ($schedule) { return $schedule->task->active; });
if (count($schedules) === 0) {
......
lib/models/CronjobSchedule.class.php
+ 10
− 62
View file @ d0b3cea1
......@@ -34,8 +34,6 @@
* @property string|null $title database column
* @property string|null $description database column
* @property string|null $parameters database column
* @property string $priority database column
* @property string $type database column
* @property int|null $minute database column
* @property int|null $hour database column
* @property int|null $day database column
......@@ -53,10 +51,6 @@
class CronjobSchedule extends SimpleORMap
{
const PRIORITY_LOW = 'low';
const PRIORITY_NORMAL = 'normal';
const PRIORITY_HIGH = 'high';
protected static function configure($config = [])
{
$config['db_table'] = 'cronjobs_schedules';
......@@ -78,41 +72,6 @@ class CronjobSchedule extends SimpleORMap
parent::configure($config);
}
/**
* Returns a mapped version of the priorities (key = priority value,
* value = localized priority label).
*
* @return Array The mapped priorities
*/
public static function getPriorities()
{
$mapping = [];
$mapping[self::PRIORITY_LOW] = _('niedrig');
$mapping[self::PRIORITY_NORMAL] = _('normal');
$mapping[self::PRIORITY_HIGH] = _('hoch');
return $mapping;
}
/**
* Maps a priority value to it's localized label.
*
* @param String $priority Priority value
* @return String The localized label
* @throws RuntimeException when an unknown priority value is passed
*/
public static function describePriority($priority)
{
$priority = $priority ?? 'normal';
$mapping = self::getPriorities();
if (!isset($mapping[$priority])) {
throw new RuntimeException('Access to unknown priority "' . $priority . '"');
}
return $mapping[$priority];
}
/**
* replaces title with task name if title is empty.
*
......@@ -171,10 +130,12 @@ class CronjobSchedule extends SimpleORMap
*
* @return CronjobSchedule Returns itself to allow chaining
*/
public function activate()
public function activate(bool $run_immediately = false)
{
$this->active = 1;
$this->next_execution = $this->calculateNextExecution();
$next_execution = $run_immediately ? strtotime('-1 minute') : $this->calculateNextExecution();
$this->active = true;
$this->next_execution = $next_execution;
$this->store();
return $this;
......@@ -187,7 +148,7 @@ class CronjobSchedule extends SimpleORMap
*/
public function deactivate()
{
$this->active = 0;
$this->active = false;
$this->store();
return $this;
......@@ -221,9 +182,6 @@ class CronjobSchedule extends SimpleORMap
$result = $this->task->engage($this->last_result, $this->parameters);
if ($this->type === 'once') {
$this->active = 0;
}
$this->last_result = $result;
$this->store();
......@@ -245,16 +203,12 @@ class CronjobSchedule extends SimpleORMap
/**
* Calculates the next execution for this schedule.
*
* For schedules of type 'once' the check solely tests whether the
* timestamp has already passed and will return false in that case.
* Otherwise the defined timestamp will be returned.
*
* For schedules of type 'periodic' the next execution
* is calculated by increasing the current timestamp and testing
* whether all conditions match. This is not the best method to test
* and should be optimized sooner or later.
* The next execution is calculated by increasing the current timestamp
* and testing whether all conditions match. This is not the best method
* to test and should be optimized sooner or later.
*
* @param mixed $now Defines the temporal fix point
*
* @return int Timestamp of calculated next execution
* @throws RuntimeException When calculation takes too long (you should
* check the conditions for validity in that case)
......@@ -263,12 +217,6 @@ class CronjobSchedule extends SimpleORMap
{
$now = $now ?: time();
if ($this->type === 'once') {
return $now <= $this->next_execution
? $this->next_execution
: false;
}
$result = $now;
$result -= $result % 60;
......
lib/models/CronjobTask.class.php
+ 33
− 35
View file @ d0b3cea1
......@@ -159,72 +159,70 @@ class CronjobTask extends SimpleORMap
// Convenience methods to ease the usage
/**
* Schedules this task for a single execution at the provided time.
*
* @param int $timestamp When the task should be executed
* @param String $priority Priority of the execution (low, normal, high),
* defaults to normal
* @param Array $parameters Optional parameters passed to the task
* @return CronjobSchedule The generated schedule object.
*/
public function scheduleOnce($timestamp, $priority = CronjobSchedule::PRIORITY_NORMAL,
$parameters = [])
{
return CronjobScheduler::getInstance()->scheduleOnce(
$this->id,
$timestamp,
$priority,
$parameters
);
}
/**
* Schedules this task for periodic execution with the provided schedule.
*
* @param mixed $minute Minute part of the schedule:
* @param int|null $minute Minute part of the schedule:
* - null for "every minute" a.k.a. "don't care"
* - x < 0 for "every x minutes"
* - x >= 0 for "only at minute x"
* @param mixed $hour Hour part of the schedule:
* @param int|null $hour Hour part of the schedule:
* - null for "every hour" a.k.a. "don't care"
* - x < 0 for "every x hours"
* - x >= 0 for "only at hour x"
* @param mixed $day Day part of the schedule:
* @param int|null $day Day part of the schedule:
* - null for "every day" a.k.a. "don't care"
* - x < 0 for "every x days"
* - x > 0 for "only at day x"
* @param mixed $month Month part of the schedule:
* @param int|null $month Month part of the schedule:
* - null for "every month" a.k.a. "don't care"
* - x < 0 for "every x months"
* - x > 0 for "only at month x"
* @param mixed $day_of_week Day of week part of the schedule:
* @param int|null $day_of_week Day of week part of the schedule:
* - null for "every day" a.k.a. "don't care"
* - 1 >= x >= 7 for "exactly at day of week x"
* (x starts with monday at 1 and ends with
* sunday at 7)
* @param String $priority Priority of the execution (low, normal, high),
* defaults to normal
* @param Array $parameters Optional parameters passed to the task
* @param array $parameters Optional parameters passed to the task
* @return CronjobSchedule The generated schedule object.
*/
public function schedulePeriodic($minute = null, $hour = null,
$day = null, $month = null, $day_of_week = null,
$priority = CronjobSchedule::PRIORITY_NORMAL,
$parameters = [])
{
return CronjobScheduler::getInstance()->schedulePeriodic(
public function schedule(
?int $minute = null,
?int $hour = null,
?int $day = null,
?int $month = null,
?int $day_of_week = null,
array $parameters = []
): CronjobSchedule {
return CronjobScheduler::getInstance()->schedule(
$this->id,
$minute,
$hour,
$day,
$month,
$day_of_week,
$priority,
$parameters
);
}
/**
* An alias for schedule for backwards compatibility.
*
* @see CronjobTask::schedule()
* @return CronjobSchedule
*/
public function schedulePeriodic(
$minute = null,
$hour = null,
$day = null,
$month = null,
$day_of_week = null,
$priority = '',
$parameters = []
) {
return $this->schedule($minute, $hour, $day, $month, $day_of_week, $parameters);
}
/**
* Extracts the default parameter values from the associated task.
*
......
tests/unit/lib/classes/CronjobScheduleTest.php
+ 0
− 54
View file @ d0b3cea1
......@@ -26,69 +26,15 @@ class CronjobScheduleTest extends \Codeception\Test\Unit
StudipTestHelper::tear_down_tables();
}
function testOnceSchedule()
{
$schedule = new CronjobSchedule();
$schedule->type = 'once';
$this->assertEquals('once', $schedule->type);
return $schedule;
}
/**
* @depends testOnceSchedule
*/
function testNextExecutionOncePast($schedule)
{
$now = strtotime('10.11.2013 01:02:00');
$then = strtotime('-2 weeks', $now);
$schedule->next_execution = $then;
$schedule->calculateNextExecution();
$this->assertEquals($then, $schedule->next_execution);
}
/**
* @depends testOnceSchedule
*/
function testNextExecutionOncePresent($schedule)
{
$now = strtotime('10.11.2013 01:02:00');
$schedule->next_execution = $now;
$schedule->calculateNextExecution();
$this->assertEquals($now, $schedule->next_execution);
}
/**
* @depends testOnceSchedule
*/
function testNextExecutionOnceFuture(CronjobSchedule $schedule)
{
$now = strtotime('10.11.2013 01:02:00');
$then = strtotime('+2 weeks', $now);
$schedule->next_execution = $then;
$schedule->calculateNextExecution($now);
$this->assertEquals($then, $schedule->next_execution);
}
function testPeriodicSchedule()
{
$schedule = new CronjobSchedule();
$schedule->type = 'periodic';
$schedule->minute = null;
$schedule->hour = null;
$schedule->day = null;
$schedule->month = null;
$schedule->day_of_week = null;
$this->assertEquals('periodic', $schedule->type);
return $schedule;
}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment