From dbd2e1772801e31013e337fc819264f151fe0a67 Mon Sep 17 00:00:00 2001
From: Anil Kulkarni <anil@terminal.space>
Date: Thu, 13 Apr 2023 01:44:15 -0700
Subject: [PATCH] Ensure the cloud url is used when publishing a status to
 activitypub Posting is done in two steps - first the media is uploaded and
 that begins a processing pipeline. When the media is finally processed, the
 media pipeline sets the cdn_url to the final image (in the cloud if
 configured). At any time, the user can click post. This begins a separate
 NewStatusPipeline to deliver the status over ActivityPub. This causes a race.
 If the NewStatusPipeline runs before the media finishes processing, then the
 status is delivered with the un-processed (and un-optimized images). If
 pixelfed is set to use cloud storage, then the ActivityPub message also
 incorrectly uses the local media.

This commit fixes the race condition by waiting for all the media to be processed before sending the message over ActivityPub.
A new publish_delayed column is set per-post, indicating whether the
status should be delayed until all media is processed.

Then, the NewStatusPipeline is called twice per post -
When the media finishes processing and when the status is posted.

Both times, the pipeline will check to see if it's valid to post
If so, the pipeline will try to set publish_delayed to false
and only if it is the first time doing so, will it publish the post
---
 app/Jobs/StatusPipeline/NewStatusPipeline.php | 18 ++++++++++++++++++
 app/Services/MediaStorageService.php          | 18 ++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/app/Jobs/StatusPipeline/NewStatusPipeline.php b/app/Jobs/StatusPipeline/NewStatusPipeline.php
index 8ccb2926f..4896aaf89 100644
--- a/app/Jobs/StatusPipeline/NewStatusPipeline.php
+++ b/app/Jobs/StatusPipeline/NewStatusPipeline.php
@@ -2,6 +2,7 @@
 
 namespace App\Jobs\StatusPipeline;
 
+use App\Media;
 use App\Status;
 use Cache;
 use Illuminate\Bus\Queueable;
@@ -44,6 +45,23 @@ class NewStatusPipeline implements ShouldQueue
      */
     public function handle()
     {
+        if (!Status::where('id', $this->status->id)->exists()) {
+            // The status has already been deleted by the time the job is running
+            // Don't publish the status, and just no-op
+            return;
+        }
+        if (config_cache('pixelfed.cloud_storage') && !config('pixelfed.media_fast_process')) {
+            $still_processing = Media::whereStatusId($this->status->id)
+                ->whereNull('cdn_url')
+                ->exists();
+            if ($still_processing) {
+                // The media items in the status are still being processed.
+                // We can't publish the status to ActivityPub because the final remote URL is not
+                // yet known. Instead, do nothing here. The media pipeline will re-call the NewStatusPipeline
+                // once all media items are finished processing
+                return;
+            }
+        }
         StatusEntityLexer::dispatch($this->status);
     }
 }
diff --git a/app/Services/MediaStorageService.php b/app/Services/MediaStorageService.php
index 3b5884811..8a07cc7ec 100644
--- a/app/Services/MediaStorageService.php
+++ b/app/Services/MediaStorageService.php
@@ -4,7 +4,9 @@ namespace App\Services;
 
 use App\Jobs\AvatarPipeline\AvatarStorageCleanup;
 use App\Jobs\MediaPipeline\MediaDeletePipeline;
+use App\Jobs\StatusPipeline\NewStatusPipeline;
 use App\Media;
+use App\Status;
 use App\Util\ActivityPub\Helpers;
 use GuzzleHttp\Client;
 use GuzzleHttp\Exception\RequestException;
@@ -79,6 +81,22 @@ class MediaStorageService
         } else {
             (new self)->localToCloud($media);
         }
+
+        if ($media->status_id && config_cache('pixelfed.cloud_storage') && !config('pixelfed.media_fast_process')) {
+            $still_processing = Media::whereStatusId($media->status_id)
+                ->whereNull('cdn_url')
+                ->exists();
+            if (!$still_processing) {
+                // In this configuration, publishing the status is delayed until the media uploads
+                // Since all media have been processed, we can kick the NewStatusPipeline job
+                // N.B. there's a timing condition with multiple MediaStorageService workers matching this if statement
+                // However, it's acceptable to publish the same status multiple times to ActivityPub
+                $status = Status::where('id', $media->status_id)->first(); // This could be null if the status was deleted
+                if ($status) {
+                    NewStatusPipeline::dispatch($status);
+                }
+            }
+        }
     }
 
     protected function localToCloud($media)