Skip to content

Commit

Permalink
Schedule periodic tasks with Laravel (#2222)
Browse files Browse the repository at this point in the history
NOTE: This PR moves cleanup logic out of the request-triggered daily
update script. To continue having these cleanup tasks performed
automatically, administrators of non-Docker systems must run `php
artisan schedule:run` via cron or similar.

The current "daily update" process is clunky, and in dire need of a
refactor. This PR takes the first step towards modernizing the daily
update process by separating out a few small portions of the process
into independent "tasks" managed by Laravel. This PR also develops the
infrastructure needed to eventually perform LDAP syncing for
#1983.

Laravel overhauled the task system in Laravel 11, but we can't upgrade
to Laravel 11 until Red Hat releases a UBI image with PHP 8.2+.
  • Loading branch information
williamjallen authored May 30, 2024
1 parent 2f02587 commit d1bf379
Show file tree
Hide file tree
Showing 12 changed files with 254 additions and 22 deletions.
14 changes: 13 additions & 1 deletion app/Console/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace App\Console;

use App\Jobs\PruneAuthTokens;
use App\Jobs\PruneJobs;
use Illuminate\Console\Scheduling\Schedule;
use Illuminate\Foundation\Console\Kernel as ConsoleKernel;

Expand All @@ -28,7 +30,17 @@ protected function schedule(Schedule $schedule)
$cdash_app_dir = realpath(app_path($cdash_directory_name));
$output_filename = $cdash_app_dir . "/AuditReport.log";

$schedule->command('dependencies:audit')->everySixHours()->sendOutputTo($output_filename);
$schedule->command('dependencies:audit')
->everySixHours()
->sendOutputTo($output_filename);

$schedule->job(new PruneJobs())
->hourly()
->withoutOverlapping();

$schedule->job(new PruneAuthTokens())
->hourly()
->withoutOverlapping();
}

/**
Expand Down
26 changes: 26 additions & 0 deletions app/Jobs/PruneAuthTokens.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace App\Jobs;

use App\Models\AuthToken;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;

/**
* Removes expired auth tokens.
*/
class PruneAuthTokens implements ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

/**
* Execute the job.
*/
public function handle(): void
{
AuthToken::expired()->delete();
}
}
34 changes: 34 additions & 0 deletions app/Jobs/PruneJobs.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace App\Jobs;

use App\Models\SuccessfulJob;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Artisan;

/**
* Removes job results which are expired. Job lifetime is controlled by the BACKUP_TIMEFRAME
* configuration option.
*/
class PruneJobs implements ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

/**
* Execute the job.
*/
public function handle(): void
{
$lifetime = config('cdash.backup_timeframe');

Artisan::call("queue:prune-failed --hours=$lifetime");

// The successful_jobs table is a CDash specific table, so we have to prune it manually
SuccessfulJob::where('finished_at', '<', Carbon::now()->subHours($lifetime))->delete();
}
}
30 changes: 30 additions & 0 deletions app/Models/AuthToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,23 @@

namespace App\Models;

use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Carbon;

/**
* @property string $hash
* @property int $userid
* @property Carbon $created
* @property Carbon $expires
* @property string $description
* @property int $projectid
* @property string $scope
*
* @method static Builder<AuthToken> expired()
*
* @mixin Builder<AuthToken>
*/
class AuthToken extends Model
{
public const SCOPE_FULL_ACCESS = 'full_access';
Expand All @@ -31,4 +46,19 @@ class AuthToken extends Model
'projectid',
'scope',
];

protected $casts = [
'userid' => 'integer',
'created' => 'datetime',
'expires' => 'datetime',
'projectid' => 'integer',
];

/**
* @param Builder<AuthToken> $query
*/
public function scopeExpired(Builder $query): void
{
$query->where('expires', '<', Carbon::now());
}
}
19 changes: 18 additions & 1 deletion app/Models/SuccessfulJob.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,26 @@
namespace App\Models;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Support\Carbon;

/**
* @property string $filename
* @property Carbon $finished_at
*
* @mixin Builder<SuccessfulJob>
*/
class SuccessfulJob extends Model
{
protected $table = 'successful_jobs';

public $timestamps = false;
protected $fillable = ['filename'];

protected $fillable = [
'filename',
];

protected $casts = [
'finished_at' => 'datetime',
];
}
3 changes: 2 additions & 1 deletion app/Utils/AuthTokenUtil.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use CDash\Model\Project;
use CDash\Model\UserProject;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Config;
use Illuminate\Support\Facades\Gate;
use InvalidArgumentException;
Expand Down Expand Up @@ -242,7 +243,7 @@ public static function hashToken(?string $unhashed_token): string
*/
public static function isTokenExpired(AuthToken $auth_token): bool
{
if (strtotime($auth_token['expires']) < time()) {
if ($auth_token['expires'] < Carbon::now()) {
$auth_token->delete();
return true;
}
Expand Down
13 changes: 0 additions & 13 deletions app/cdash/include/dailyupdates.php
Original file line number Diff line number Diff line change
Expand Up @@ -1049,16 +1049,6 @@ function addDailyChanges(int $projectid): void
cleanBuildEmail();
cleanUserTemp();

// Delete old records from the successful & failed jobs database table.
$dt = new DateTime();
$dt->setTimestamp(time() - (config('cdash.backup_timeframe') * 3600));
DB::table('failed_jobs')
->where('failed_at', '<', $dt)
->delete();
DB::table('successful_jobs')
->where('finished_at', '<', $dt)
->delete();

// If the status of daily update is set to 2 that means we should send an email
$dailyupdate_array = $db->executePreparedSingleRow('
SELECT status
Expand Down Expand Up @@ -1146,9 +1136,6 @@ function addDailyChanges(int $projectid): void
}
}

// Delete expired authentication tokens.
DB::delete('DELETE FROM authtoken WHERE expires < NOW()');

// Delete expired buildgroups and rules.
$current_date = gmdate(FMT_DATETIME);
$datetime = new \DateTime();
Expand Down
6 changes: 6 additions & 0 deletions app/cdash/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ set_tests_properties(/Feature/LdapIntegration PROPERTIES DEPENDS install_2)
add_laravel_test(/Feature/CDashTest)
set_tests_properties(/Feature/CDashTest PROPERTIES DEPENDS install_2)

add_laravel_test(/Feature/Jobs/PruneJobsTest)
set_tests_properties(/Feature/Jobs/PruneJobsTest PROPERTIES DEPENDS install_2)

add_laravel_test(/Feature/Jobs/PruneAuthTokensTest)
set_tests_properties(/Feature/Jobs/PruneAuthTokensTest PROPERTIES DEPENDS install_2)

add_laravel_test(/Feature/LoginAndRegistration)
set_tests_properties(/Feature/LoginAndRegistration PROPERTIES DEPENDS /Feature/CDashTest)

Expand Down
8 changes: 7 additions & 1 deletion docker/docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ php artisan key:check || exit 1

# If the "start-website" argument was provided, start the web server
if [ "$1" = "start-website" ] ; then
echo "Starting Apache..."
echo "Starting background jobs..."
# Output will show up in the logs, but the system will not crash if the schedule process fails.
# In the future, it would be better to do this with a dedicated container which starts on a schedule.
# Bare metal systems should use cron instead (using cron in Docker is problematic).
php artisan schedule:work & # & puts the task in the background.

echo "Starting Apache..."

# Start Apache under the current user, in case the current user isn't www-data. Kubernetes-based systems
# typically run under a random user. We start Apache before running the install scripts so the system can
Expand Down
10 changes: 5 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -14225,11 +14225,6 @@ parameters:
count: 1
path: app/cdash/include/dailyupdates.php

-
message: "#^Parameter \\#1 \\$timestamp of method DateTime\\:\\:setTimestamp\\(\\) expects int, \\(float\\|int\\) given\\.$#"
count: 1
path: app/cdash/include/dailyupdates.php

-
message: "#^Parameter \\#2 \\$offset of function substr expects int, int\\<0, max\\>\\|false given\\.$#"
count: 1
Expand Down Expand Up @@ -29320,6 +29315,11 @@ parameters:
count: 5
path: tests/Feature/GraphQL/ProjectTypeTest.php

-
message: "#^Dynamic call to static method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\<App\\\\Models\\\\SuccessfulJob\\>\\:\\:count\\(\\)\\.$#"
count: 4
path: tests/Feature/Jobs/PruneJobsTest.php

-
message: "#^Call to an undefined method Mockery\\\\Expectation\\:\\:shouldReceive\\(\\)\\.$#"
count: 1
Expand Down
66 changes: 66 additions & 0 deletions tests/Feature/Jobs/PruneAuthTokensTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

namespace Tests\Feature\Jobs;

use App\Jobs\PruneAuthTokens;
use App\Models\AuthToken;
use App\Models\User;
use Illuminate\Support\Carbon;
use Illuminate\Support\Str;
use Tests\TestCase;
use Tests\Traits\CreatesUsers;

class PruneAuthTokensTest extends TestCase
{
use CreatesUsers;

protected User $user;

public function setUp(): void
{
parent::setUp();

$this->user = $this->makeNormalUser();
}

public function tearDown(): void
{
$this->user->delete();

parent::tearDown();
}

public function testExpiredAuthTokenDeleted(): void
{
$hash = Str::uuid()->toString();
AuthToken::create([
'hash' => $hash,
'expires' => Carbon::now()->subMinute(),
'scope' => 'test',
'userid' => $this->user->id,
]);

self::assertNotNull(Authtoken::find($hash));

PruneAuthTokens::dispatch();

self::assertNull(Authtoken::find($hash));
}

public function testValidAuthTokenNotDeleted(): void
{
$hash = Str::uuid()->toString();
AuthToken::create([
'hash' => $hash,
'expires' => Carbon::now()->addMinute(),
'scope' => 'test',
'userid' => $this->user->id,
]);

self::assertNotNull(Authtoken::find($hash));

PruneAuthTokens::dispatch();

self::assertNotNull(Authtoken::find($hash));
}
}
47 changes: 47 additions & 0 deletions tests/Feature/Jobs/PruneJobsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

namespace Tests\Feature\Jobs;

use App\Jobs\PruneJobs;
use App\Models\SuccessfulJob;
use Illuminate\Support\Carbon;
use Illuminate\Support\Str;
use Tests\TestCase;

class PruneJobsTest extends TestCase
{
/**
* Changing the config is difficult since multiple processes are involved.
* Instead, we just rely upon the default value of 48 hours.
*/
public function testExpiredSuccessfulJobDeleted(): void
{
$filename = 'test-filename' . Str::uuid()->toString();
$job = new SuccessfulJob([
'filename' => $filename,
]);
$job->finished_at = Carbon::now()->subHours(1000); // finished_at isn't fillable...
$job->save();

self::assertEquals(1, SuccessfulJob::where('filename', $filename)->count());

PruneJobs::dispatch();

self::assertEquals(0, SuccessfulJob::where('filename', $filename)->count());
}

public function testRecentSuccessfulJobNotDeleted(): void
{
$filename = 'test-filename' . Str::uuid()->toString();
// The timestamp defaults to NOW().
SuccessfulJob::create([
'filename' => $filename,
]);

self::assertEquals(1, SuccessfulJob::where('filename', $filename)->count());

PruneJobs::dispatch();

self::assertEquals(1, SuccessfulJob::where('filename', $filename)->count());
}
}

0 comments on commit d1bf379

Please sign in to comment.