Eager load a message's reply and files so a strict application can re…

CHANGELOG.md

@@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.4.1]
+
+### Fixed
+
+- The thread lazy loaded each message's reply and its files while rendering: one
+  query per message for the quote, another for the attachments. In an application
+  running `Model::preventLazyLoading()` -- the posture of a careful one -- that is
+  not a slow thread but a thrown `LazyLoadingViolationException`, and the chat
+  would not render at all. Both are eager loaded now, and so is the message and
+  conversation the attachment download authorises against.
+- `tests/TestCase.php` forbids lazy loading, so the suite fails on this class of
+  bug rather than leaving a consumer to find it. It was a consumer that found this
+  one.
+
 ## [0.4.0]
 
 ### Added

src/Http/DownloadAttachment.php

@@ -26,7 +26,10 @@ public function __construct(private Attachments $attachments) {}
     public function __invoke(int $attachment): Response|StreamedResponse
     {
         $user = Filum::user();
-        $file = Attachment::query()->find($attachment);
+        // The message and its conversation are what authorization turns on, so
+        // they are asked for by name rather than reached for afterwards -- an
+        // application running preventLazyLoading would refuse the reach.
+        $file = Attachment::query()->with('message.conversation')->find($attachment);
 
         if (! Filum::authorized($user) || ! $user instanceof Authenticatable || ! $file instanceof Attachment) {
             abort(404);

src/Messages/Messages.php

@@ -47,13 +47,12 @@ public function __construct(
      * outage costs a delay, not a message -- and the announcement is allowed to
      * fail quietly because the reconciliation poll will find what it missed.
      *
+     * @param  list<array{disk: string, path: string, name: string, mime: string, size: int}>  $attachments
+     *
      * @throws NotAParticipant when the sender is not in the conversation.
      * @throws NotAGroup when the conversation is a group and groups are switched off.
      * @throws RateLimited when the sender is sending too fast.
      */
-    /**
-     * @param  list<array{disk: string, path: string, name: string, mime: string, size: int}>  $attachments
-     */
     public function send(
         Conversation $conversation,
         Authenticatable $sender,
@@ -217,7 +216,14 @@ private function announce(Message $message): void
      */
     public function page(Conversation $conversation, ?int $from = null): Collection
     {
+        // Eager loaded because the ledger reads both for every line it draws.
+        // Left lazy this is one query per message for the quote and another for
+        // its files -- and in an application running preventLazyLoading, which is
+        // the posture of a careful one, it is not a slow thread but a thrown
+        // exception. Filum learned that from a consumer rather than from its own
+        // suite; tests/TestCase.php now forbids lazy loading so it cannot recur.
         $query = Message::query()
+            ->with(['replyTo', 'attachments'])
             ->where('conversation_id', $conversation->id)
             ->orderByDesc('id');
 
@@ -278,6 +284,7 @@ public function since(Conversation $conversation, int $after): Collection
     {
         /** @var Collection<int, Message> $messages */
         $messages = Message::query()
+            ->with(['replyTo', 'attachments'])
             ->where('conversation_id', $conversation->id)
             ->where('id', '>', $after)
             ->orderBy('id')

tests/TestCase.php

@@ -8,6 +8,7 @@
 use Heyosseus\Filum\FilumServiceProvider;
 use Heyosseus\Filum\Tests\Fixtures\User;
 use Illuminate\Contracts\Config\Repository;
+use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Schema\Blueprint;
 use Illuminate\Support\Facades\Schema;
 use Orchestra\Testbench\TestCase as Orchestra;
@@ -22,6 +23,13 @@ protected function setUp(): void
 
         // The gate is static, so one test's callback must not leak into the next.
         Filum::flush();
+
+        // Applications that take performance seriously turn this on, and a
+        // package that lazy loads inside their views does not merely run slowly
+        // there -- it throws. Filum found that out from a consumer rather than
+        // from its own suite once; enabling it here is how it does not happen
+        // twice.
+        Model::preventLazyLoading();
     }
 
     #[Override]