Skip to content

Commit 0bf7cc0

Browse files
committed
fix(editor): keep working when the realtime channel never delivers
1 parent 2e47bf7 commit 0bf7cc0

8 files changed

Lines changed: 105 additions & 3 deletions

File tree

app/Http/Controllers/Api/SessionValidationController.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ public function __invoke(Request $request, string $sessionId, EditorSessionManag
5050
);
5151
}
5252

53+
// A session that needed no confirmation has no window yet; the first
54+
// one to arrive takes it.
55+
$sessions->adoptWindow($session, $verificationId);
56+
5357
if (! $session->matchesConfirmedVerification($verificationId)) {
5458
return $this->refuse(
5559
$sessionId,

app/Models/Server.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ class Server extends Model implements Authenticatable
3232
'token_hash',
3333
];
3434

35+
/** Until a plugin reports a live channel, its requests are queued. */
36+
protected $attributes = [
37+
'uses_polling' => true,
38+
];
39+
3540
/**
3641
* A server is considered reachable while it has sent a heartbeat inside this window.
3742
*/

app/Services/EditorSessionManager.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,33 @@ public function confirm(string $verificationId, string $confirmedBy): ?EditorSes
8686
});
8787
}
8888

89+
/**
90+
* Adopts a window as the confirmed one when no confirmation was required.
91+
*
92+
* Such a session is created already confirmed but with no window attached,
93+
* so without this the first window to validate is turned away as if another
94+
* had got there first.
95+
*/
96+
public function adoptWindow(EditorSession $session, string $verificationId): bool
97+
{
98+
if ($session->require_confirmation || $session->confirmed_verification_id !== null) {
99+
return false;
100+
}
101+
102+
return DB::transaction(function () use ($session, $verificationId): bool {
103+
$locked = EditorSession::whereKey($session->getKey())->lockForUpdate()->first();
104+
105+
if ($locked === null || $locked->confirmed_verification_id !== null || $locked->require_confirmation) {
106+
return false;
107+
}
108+
109+
$locked->update(['confirmed_verification_id' => $verificationId]);
110+
$session->setAttribute('confirmed_verification_id', $verificationId);
111+
112+
return true;
113+
});
114+
}
115+
89116
/**
90117
* Claims the session for the first browser window that validates it.
91118
*/

database/factories/ServerFactory.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ public function definition(): array
3232
];
3333
}
3434

35+
/** A server whose plugin holds a live channel, so requests are broadcast. */
36+
public function onChannel(): static
37+
{
38+
return $this->state(['uses_polling' => false]);
39+
}
40+
3541
public function offline(): static
3642
{
3743
return $this->state(['last_seen_at' => now()->subHour()]);
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
use Illuminate\Database\Migrations\Migration;
4+
use Illuminate\Database\Schema\Blueprint;
5+
use Illuminate\Support\Facades\DB;
6+
use Illuminate\Support\Facades\Schema;
7+
8+
return new class extends Migration
9+
{
10+
/**
11+
* A server is assumed to be collecting requests over HTTP until it proves
12+
* it holds a live channel. A queued request can always be picked up later,
13+
* while one broadcast to an empty channel is simply lost.
14+
*/
15+
public function up(): void
16+
{
17+
Schema::table('servers', function (Blueprint $table) {
18+
$table->boolean('uses_polling')->default(true)->change();
19+
});
20+
21+
DB::table('servers')->update(['uses_polling' => true]);
22+
}
23+
24+
public function down(): void
25+
{
26+
Schema::table('servers', function (Blueprint $table) {
27+
$table->boolean('uses_polling')->default(false)->change();
28+
});
29+
}
30+
};

tests/Feature/Http/Controllers/Api/MenuControllerTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use App\Events\MenuChanged;
66
use App\Events\RpcRequested;
77
use App\Models\EditorSession;
8+
use App\Models\Server;
89
use App\Services\RpcBridge;
910
use Illuminate\Foundation\Testing\RefreshDatabase;
1011
use Illuminate\Support\Facades\Event;
@@ -93,7 +94,9 @@ public function test_returns_422_for_an_unknown_platform(): void
9394

9495
private function editorSession(): EditorSession
9596
{
96-
return EditorSession::factory()->create(['confirmed' => true, 'consumed' => true, 'active' => false]);
97+
return EditorSession::factory()
98+
->for(Server::factory()->onChannel())
99+
->create(['confirmed' => true, 'consumed' => true, 'active' => false]);
97100
}
98101

99102
/**

tests/Feature/Http/Controllers/Api/SessionValidationControllerTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,33 @@ public function test_signs_the_window_in_so_it_can_reach_the_menu_endpoints(): v
3737
$this->assertAuthenticatedAs($session, 'editor');
3838
}
3939

40+
public function test_opens_a_session_that_needed_no_confirmation(): void
41+
{
42+
// The plugin marks these confirmed on creation but attaches no window.
43+
$session = EditorSession::factory()->create(['require_confirmation' => false, 'confirmed' => true]);
44+
$verificationId = (string) Str::uuid();
45+
$session->verifications()->create(['verification_id' => $verificationId]);
46+
47+
$this->getJson($this->url($session->session_id, $verificationId))
48+
->assertOk()
49+
->assertJson(['valid' => true, 'message' => 'Session validated']);
50+
}
51+
52+
public function test_refuses_a_second_window_on_a_session_that_needed_no_confirmation(): void
53+
{
54+
$session = EditorSession::factory()->create(['require_confirmation' => false, 'confirmed' => true]);
55+
$first = (string) Str::uuid();
56+
$second = (string) Str::uuid();
57+
$session->verifications()->create(['verification_id' => $first]);
58+
$session->verifications()->create(['verification_id' => $second]);
59+
60+
$this->getJson($this->url($session->session_id, $first))->assertJson(['valid' => true]);
61+
62+
$this->getJson($this->url($session->session_id, $second))
63+
->assertOk()
64+
->assertJson(['valid' => false, 'message' => 'Session already validated from another window']);
65+
}
66+
4067
public function test_reserves_the_session_so_a_second_window_is_refused(): void
4168
{
4269
$verificationId = (string) Str::uuid();

tests/Feature/Services/RpcBridgeTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class RpcBridgeTest extends TestCase
1818
public function test_returns_the_payload_the_plugin_posts_back(): void
1919
{
2020
$bridge = app(RpcBridge::class);
21-
$server = Server::factory()->create();
21+
$server = Server::factory()->onChannel()->create();
2222

2323
// The plugin answers on a separate request; a listener stands in for it.
2424
Event::listen(RpcRequested::class, fn (RpcRequested $event) => $bridge->resolve($event->requestId, [
@@ -36,7 +36,7 @@ public function test_returns_the_payload_the_plugin_posts_back(): void
3636
public function test_broadcasts_the_request_on_the_private_channel_of_the_server(): void
3737
{
3838
$bridge = app(RpcBridge::class);
39-
$server = Server::factory()->create();
39+
$server = Server::factory()->onChannel()->create();
4040
$captured = null;
4141

4242
Event::listen(RpcRequested::class, function (RpcRequested $event) use ($bridge, &$captured): void {

0 commit comments

Comments
 (0)