Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ VATSIM_API_KEY=
VATSIM_API_BASE=https://apiv2.vatsim.dev/v2/

VATSIM_NET_WEBHOOK_KEY=
VATSIM_NET_BOOKINGS_URL=https://atc-bookings.vatsim.net/api
VATSIM_NET_BOOKINGS_KEY=

#VATSIM_DATA_FEED=

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

declare(strict_types=1);

namespace App\Jobs\Booking;

use App\Jobs\Concerns\LogsJobFailure;
use App\Jobs\Job;
use App\Models\Booking;
use App\Services\Bookings\VatsimNetBookingSyncService;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;

class SyncToVatsimNet extends Job implements ShouldQueue
{
use Dispatchable, InteractsWithQueue, LogsJobFailure, SerializesModels;

public function __construct(
private readonly int $bookingId,
private readonly bool $deleted = false,
private readonly ?int $remoteId = null,
) {}

public function handle(VatsimNetBookingSyncService $service): void
{
if ($this->deleted) {
$service->delete($this->remoteId);

return;
}

$booking = Booking::find($this->bookingId);

if ($booking === null) {
return;
}

$service->sync($booking);
}

protected function logJobContext(): array
{
return [
'booking_id' => $this->bookingId,
'deleted' => $this->deleted,
];
}
}
75 changes: 75 additions & 0 deletions app/Libraries/VatsimNetBookings.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);

namespace App\Libraries;

use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Log;

class VatsimNetBookings
{
private string $url;

private string $key;

public function __construct()
{
$this->url = (string) config('services.vatsim-net.bookings.url');
$this->key = (string) config('services.vatsim-net.bookings.key');
}

public function create(array $payload): int
{
$response = $this->client()->post('booking', $payload);

$this->logFailure('create', $response->status(), $response->body());

$response->throw();

$id = (int) $response->json('id');

if ($id <= 0) {
throw new \RuntimeException('VATSIM.net booking create returned an invalid id.');
}

return $id;
}

public function update(int $remoteId, array $payload): void
{
$response = $this->client()->put("booking/{$remoteId}", $payload);

$this->logFailure('update', $response->status(), $response->body());

$response->throw();
}

public function delete(int $remoteId): void
{
$response = $this->client()->delete("booking/{$remoteId}");

$this->logFailure('delete', $response->status(), $response->body());

$response->throw();
}

private function client()
{
return Http::baseUrl($this->url)
->withToken($this->key)
->acceptJson()
->asJson();
}
Comment on lines +57 to +63

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[performance · medium]
This HTTP client makes outbound calls to VATSIM.net from a queued worker (SyncToVatsimNet) without any timeout. Guzzle's default timeout/connect_timeout is 0 (wait indefinitely), so a stalled or unresponsive VATSIM.net endpoint can hold a Horizon worker open for an unbounded amount of time. Add explicit ->timeout(...) / ->connectTimeout(...) (e.g. 15s) when building the pending request.


private function logFailure(string $method, int $status, string $body): void
{
if ($status >= 400) {
Log::warning('VATSIM.net booking request failed', [
'method' => $method,
'status' => $status,
'body' => $body,
]);
}
}
}
5 changes: 5 additions & 0 deletions app/Models/Booking.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@

use App\Models\Atc\Position;
use App\Models\Mship\Account;
use App\Observers\BookingObserver;
use Carbon\Carbon;
use Illuminate\Database\Eloquent\Attributes\ObservedBy;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\MorphTo;

#[ObservedBy([BookingObserver::class])]
class Booking extends Model
{
use HasFactory;
Expand All @@ -36,11 +39,13 @@ class Booking extends Model
'bookable_type',
'bookable_id',
'cts_booking_id',
'vatsim_net_booking_id',
];

protected $casts = [
'starts_at' => 'datetime',
'ends_at' => 'datetime',
'vatsim_net_booking_id' => 'integer',
];

public function position(): BelongsTo
Expand Down
57 changes: 57 additions & 0 deletions app/Observers/BookingObserver.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php

declare(strict_types=1);

namespace App\Observers;

use App\Jobs\Booking\SyncToVatsimNet;
use App\Models\Booking;

class BookingObserver
{
private const RELEVANT_FIELDS = [
Comment on lines +10 to +12

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[bug · high]
This observer dispatches the queue job from created/updated/deleted, but it does not implement ShouldHandleEventsAfterCommit. Booking creation and deletion are wrapped in DB transactions (e.g. MentoringSessionsService::acceptSession() -> createCoreBooking(), and BookingService::cancelCtsBooking() -> DB::connection('cts')->transaction(...)). Two consequences follow:

  1. On a sync queue driver (tests/dev), the job runs before the transaction commits, so Booking::find($this->bookingId) in the job returns null and the sync is silently skipped (created bookings never reach VATSIM.net).
  2. If the enclosing transaction later rolls back, the job has already been enqueued and will create/update a remote booking for a record that never persisted.

Other observers in this project (e.g. RosterObserver, TrainingPlaceObserver) already implement ShouldHandleEventsAfterCommit for exactly this reason. Add implements ShouldHandleEventsAfterCommit to defer dispatching until after commit.

'position_id',
'member_id',
'starts_at',
'ends_at',
'type',
'cts_booking_id',
];

public function created(Booking $booking): void
{
$this->dispatch($booking);
}

public function updated(Booking $booking): void
{
if ($booking->wasChanged(self::RELEVANT_FIELDS)) {
$this->dispatch($booking);
}
}

public function deleted(Booking $booking): void
{
$this->dispatch($booking, deleted: true);
}

private function dispatch(Booking $booking, bool $deleted = false): void
{
if ($this->shouldSkip($booking)) {
return;
}

$remoteId = $booking->vatsim_net_booking_id !== null ? (int) $booking->vatsim_net_booking_id : null;

SyncToVatsimNet::dispatch($booking->getKey(), $deleted, $remoteId);
}

private function shouldSkip(Booking $booking): bool
{
if ((string) config('services.vatsim-net.bookings.key') === '') {
return true;
}

return $booking->type === Booking::TYPE_EVENT;
}
}
118 changes: 118 additions & 0 deletions app/Services/Bookings/VatsimNetBookingSyncService.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
<?php

declare(strict_types=1);

namespace App\Services\Bookings;

use App\Libraries\VatsimNetBookings;
use App\Models\Booking;
use App\Models\Cts\ExamBooking;
use App\Models\Cts\Session;

class VatsimNetBookingSyncService
{
public function __construct(
private readonly VatsimNetBookings $bookings,
) {}

public function sync(Booking $booking): void
{
$payload = $this->payload($booking);

if ($payload === null) {
return;
}

if ($booking->vatsim_net_booking_id !== null) {
$this->bookings->update((int) $booking->vatsim_net_booking_id, $payload);

return;
}

$remoteId = $this->bookings->create($payload);

$booking->updateQuietly(['vatsim_net_booking_id' => $remoteId]);
}

public function delete(?int $remoteId): void
{
if ($remoteId === null) {
return;
}

$this->bookings->delete($remoteId);
}

private function payload(Booking $booking): ?array
{
if ($booking->type === Booking::TYPE_EVENT) {
return null;
}

$position = $booking->position;

if ($position === null || $position->isVirtual()) {
return null;
}

$cid = $this->resolveControllerCid($booking);

if ($cid === null) {
return null;
}

return [
'callsign' => $booking->ctsBooking?->position ?? $position->callsign,
'cid' => $cid,
'type' => $this->mapType($booking->type),
'start' => $booking->starts_at->format('Y-m-d H:i:s'),
'end' => $booking->ends_at->format('Y-m-d H:i:s'),
];
}

private function resolveControllerCid(Booking $booking): ?int
{
return match ($booking->type) {
Booking::TYPE_STANDARD => $booking->member_id !== null ? (int) $booking->member_id : null,
Booking::TYPE_EXAM => $this->resolveExamCid($booking),
Booking::TYPE_MENTORING => $this->resolveMentoringCid($booking),
default => null,
};
}

private function resolveExamCid(Booking $booking): ?int
{
$exam = $booking->bookable;

if (! $exam instanceof ExamBooking) {
return null;
}

$account = $exam->loadMissing('examiners.primaryExaminer')->examiners?->primaryExaminer?->account;

return $account?->id;
}

private function resolveMentoringCid(Booking $booking): ?int
{
$session = $booking->bookable;

if (! $session instanceof Session) {
return null;
}

$account = $session->loadMissing('mentor')->mentor?->account;

return $account?->id;
}

private function mapType(string $type): string
{
return match ($type) {
Booking::TYPE_STANDARD => 'booking',
Booking::TYPE_EXAM => 'exam',
Booking::TYPE_MENTORING => 'mentoring',
default => 'booking',
};
}
}
4 changes: 4 additions & 0 deletions config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@
'secret' => env('VATSIM_OAUTH_SECRET'),
'scopes' => explode(',', env('VATSIM_OAUTH_SCOPES', '')),
],
'bookings' => [
'url' => env('VATSIM_NET_BOOKINGS_URL', 'https://atc-bookings.vatsim.net/api'),
'key' => env('VATSIM_NET_BOOKINGS_KEY', ''),
],
],

'gander-oceanic' => [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
public function up(): void
{
Schema::table('bookings', function (Blueprint $table) {
$table->unsignedBigInteger('vatsim_net_booking_id')->nullable()->after('cts_booking_id')->index();
});
}

public function down(): void
{
Schema::table('bookings', function (Blueprint $table) {
$table->dropColumn('vatsim_net_booking_id');
});
}
};
Loading
Loading