fix(media-uploads): null private_url for missing Dropbox files and backoff for pending upload retries

app/Services/FileSystem/AbstractFileDownloadStrategy.php

@@ -85,7 +85,7 @@ public function getUrl(string $relativeFileName, bool $useTemporaryUrl = false,
 
         $key = sprintf("%s/%s_%b_%s", $this->getDriver(), $relativeFileName, $useTemporaryUrl, $ttl);
         $res = Cache::get($key);
-        if(!empty($res) && $res != '#' && !$avoidCache) {
+        if(!empty($res) && !$avoidCache) {
             Log::debug
             (
                 sprintf
@@ -100,12 +100,12 @@ public function getUrl(string $relativeFileName, bool $useTemporaryUrl = false,
         // @see https://laravel.com/docs/8.x/filesystem#temporary-urls
         $res = $useTemporaryUrl ? Storage::disk($this->getDriver())->temporaryUrl($relativeFileName, now()->addMinutes($ttl))
             : Storage::disk($this->getDriver())->url($relativeFileName);
-        if(!empty($res) && $res != '#') {
+        if(!empty($res)) {
             $ttl = $useTemporaryUrl ? ($ttl - 1) : intval(Config::get('cache_api_response.file_url_lifetime', 3600));
             Log::debug(sprintf("AbstractFileDownloadStrategy::getUrl adding key %s res %s ttl %s to cache", $key, $res, $ttl));
             Cache::add($key, $res, $ttl);
         }
-        return $res;
+        return empty($res) ? null : $res;
     }
 
     /**

app/Services/FileSystem/Dropbox/DropboxAdapter.php

@@ -24,6 +24,10 @@ final class DropboxAdapter extends BaseDropboxAdapter
     /*
      * This method returns a public URL for the given path in Dropbox.
      * this is overloaded to retrieve a URL with the preview mode enabled.
+     * Returns an empty string when no shared link can be produced (e.g. the file is not in Dropbox);
+     * the parent declares getUrl(): string, so null is not allowed here by PHP return-type covariance.
+     * AbstractFileDownloadStrategy::getUrl() normalizes it to null, so serializers expose
+     * private_url = null and clients fall back to public_url.
      */
     public function getUrl(string $path): string
     {
@@ -65,6 +69,6 @@ public function getUrl(string $path): string
         catch (Exception $ex){
             Log::error($ex);
         }
-        return '#';
+        return '';
     }
 }

app/Services/Model/ISummitService.php

@@ -552,10 +552,13 @@ public function regenerateTemporalUrlsForMediaUploads(int $summit_id):void;
      * Process pending media uploads from the PendingMediaUpload table.
      * 429 rate-limit handling is now transparently handled by RetryAfterDropboxClient.
      *
-     * @param int $max_retries Maximum retry attempts per upload across cron runs (default 3)
+     * Attempts are spaced with exponential backoff (5, 10, 20, 40 ... minutes since the last
+     * failed attempt); when the budget is exhausted the row is marked Error and logged at error level.
+     *
+     * @param int $max_retries Maximum retry attempts per upload across cron runs (default 5)
      * @return array Stats array: ['processed' => int, 'errors' => int]
      */
-    public function processPendingMediaUploads(int $max_retries = 3): array;
+    public function processPendingMediaUploads(int $max_retries = 5): array;
 
     /**
      * @param Summit $summit

app/Services/Model/Imp/SummitService.php

@@ -128,6 +128,11 @@
 final class SummitService
     extends AbstractPublishService implements ISummitService
 {
+    /**
+     * Base wait (minutes) between attempts of a pending media upload; doubles per attempt: 5, 10, 20, 40 ...
+     */
+    const PendingMediaUploadRetryBaseMinutes = 5;
+
     /**
      * @var ISummitEventRepository
      */
@@ -4389,7 +4394,7 @@ public function validateBadge(Summit $summit, string $badge_qr_code): SummitAtte
      * @param int $max_retries
      * @return array
      */
-    public function processPendingMediaUploads(int $max_retries = 3): array
+    public function processPendingMediaUploads(int $max_retries = 5): array
     {
         Log::debug(sprintf(
             "SummitService::processPendingMediaUploads max_retries %s",
@@ -4422,14 +4427,28 @@ public function processPendingMediaUploads(int $max_retries = 3): array
                 $upload_id = $pending_upload->getId();
                 Log::debug(sprintf("SummitService::processPendingMediaUploads processing upload ID %s", $upload_id));
 
+                $attempts = $pending_upload->getAttempts();
+
                 // Check retry limit
-                if ($pending_upload->getAttempts() >= $max_retries) {
+                if ($attempts >= $max_retries) {
                     $this->tx_service->transaction(function () use ($pending_upload) {
                         $pending_upload->setStatus(PendingMediaUpload::STATUS_ERROR);
                         $pending_upload->setErrorMessage('Max retries exceeded');
                     });
                     $stats['errors']++;
-                    Log::warning(sprintf("SummitService::processPendingMediaUploads upload ID %s exceeded max retries", $upload_id));
+                    // Permanent Error transition: error level so it survives LOG_LEVEL=error in production
+                    Log::error(sprintf("SummitService::processPendingMediaUploads upload ID %s exceeded max retries (%s), marked as Error", $upload_id, $max_retries));
+                    continue;
+                }
+
+                // Exponential backoff between attempts (5, 10, 20, 40 ... minutes since the last failed attempt)
+                if ($attempts > 0 && !self::isPendingMediaUploadRetryDue($pending_upload, $attempts)) {
+                    Log::debug(sprintf(
+                        "SummitService::processPendingMediaUploads upload ID %s skipped, backoff not elapsed (attempts %s, retry %s minutes after the last attempt)",
+                        $upload_id,
+                        $attempts,
+                        self::getPendingMediaUploadRetryDelayMinutes($attempts)
+                    ));
                     continue;
                 }
 
@@ -4529,22 +4548,30 @@ public function processPendingMediaUploads(int $max_retries = 3): array
                 } catch (\Exception $ex) {
                     // On failure, status remains at whatever partial state was reached
                     // Set error message and mark as ERROR only if max retries exhausted
-                    $this->tx_service->transaction(function () use ($pending_upload, $ex, $max_retries) {
+                    $attempts = $pending_upload->getAttempts();
+                    $exhausted = $attempts >= $max_retries;
+                    $this->tx_service->transaction(function () use ($pending_upload, $ex, $exhausted) {
                         $pending_upload->setErrorMessage($ex->getMessage());
-                        if ($pending_upload->getAttempts() >= $max_retries) {
+                        if ($exhausted) {
                             $pending_upload->setStatus(PendingMediaUpload::STATUS_ERROR);
                         }
                         // else: leave at current status (Pending, PublicStorageUploaded, or PrivateStorageUploaded) for retry
                     });
 
                     $stats['errors']++;
-                    Log::warning(sprintf(
+                    $message = sprintf(
                         "SummitService::processPendingMediaUploads upload ID %s failed (attempt %s/%s): %s",
                         $upload_id,
-                        $pending_upload->getAttempts(),
+                        $attempts,
                         $max_retries,
                         $ex->getMessage()
-                    ));
+                    );
+                    // Permanent Error transition: error level so it survives LOG_LEVEL=error in production
+                    if ($exhausted) {
+                        Log::error($message . ' - max retries exhausted, marked as Error');
+                    } else {
+                        Log::warning($message);
+                    }
                 }
             }
 
@@ -4573,4 +4600,33 @@ public function processPendingMediaUploads(int $max_retries = 3): array
 
         return $stats;
     }
+
+    /**
+     * Exponential backoff between attempts of a pending media upload: a row that already failed
+     * $attempts times is due again PendingMediaUploadRetryBaseMinutes * 2^(attempts - 1) minutes
+     * after its LastEdited (the time of the last failed attempt): 5, 10, 20, 40 ...
+     * @param PendingMediaUpload $pending_upload
+     * @param int $attempts
+     * @return bool
+     */
+    private static function isPendingMediaUploadRetryDue(PendingMediaUpload $pending_upload, int $attempts): bool
+    {
+        // LastEdited is stored as DefaultTimeZone wall-clock and hydrated by Doctrine in the app
+        // timezone (UTC), so the raw getLastEdited() is off by the zone offset; getLastEditedUTC()
+        // re-interprets it correctly. Compare instants in UTC.
+        $last_edited = $pending_upload->getLastEditedUTC();
+        if (is_null($last_edited)) return true;
+        $next_retry_at = (clone $last_edited)->modify(sprintf('+%d minutes', self::getPendingMediaUploadRetryDelayMinutes($attempts)));
+        $now = new \DateTime('now', new \DateTimeZone('UTC'));
+        return $next_retry_at <= $now;
+    }
+
+    /**
+     * @param int $attempts failed attempts so far (>= 1)
+     * @return int minutes to wait after the last attempt: 5, 10, 20, 40 ...
+     */
+    private static function getPendingMediaUploadRetryDelayMinutes(int $attempts): int
+    {
+        return self::PendingMediaUploadRetryBaseMinutes * (2 ** max($attempts - 1, 0));
+    }
 }

tests/Unit/Services/DropboxAdapterGetUrlTest.php

@@ -0,0 +1,100 @@
+<?php namespace Tests\Unit\Services;
+/**
+ * Copyright 2026 OpenStack Foundation
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+use App\Services\FileSystem\Dropbox\DropboxAdapter;
+use GuzzleHttp\Psr7\Response;
+use Illuminate\Container\Container;
+use Illuminate\Support\Facades\Facade;
+use Mockery;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+use Spatie\Dropbox\Client as DropboxClient;
+use Spatie\Dropbox\Exceptions\BadRequest;
+
+/**
+ * Class DropboxAdapterGetUrlTest
+ *
+ * Unit tests for {@see DropboxAdapter::getUrl()}: the value handed to
+ * PresentationMediaUpload serializers as `private_url`.
+ *
+ * @package Tests\Unit\Services
+ */
+class DropboxAdapterGetUrlTest extends TestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+        // Minimal facade application so Log:: calls in the SUT resolve
+        // without a full Laravel app (same setup as ProcessPendingMediaUploadsTest).
+        Facade::clearResolvedInstances();
+        $app = new Container();
+        $app->singleton('log', fn() => new NullLogger());
+        Container::setInstance($app);
+        Facade::setFacadeApplication($app);
+    }
+
+    protected function tearDown(): void
+    {
+        Facade::setFacadeApplication(null);
+        Facade::clearResolvedInstances();
+        Container::setInstance(null);
+        Mockery::close();
+        parent::tearDown();
+    }
+
+    /**
+     * The file never reached Dropbox (e.g. the pending upload failed), so
+     * sharing/create_shared_link_with_settings answers path/not_found.
+     * The adapter must report "no link" as an empty string (the parent declares getUrl(): string,
+     * so null is not allowed), never as a placeholder like "#"
+     * that clients would treat as a real URL. AbstractFileDownloadStrategy turns it into null.
+     */
+    public function testGetUrlReturnsEmptyStringWhenSharedLinkCannotBeCreated(): void
+    {
+        $path = 'PresentationMediaUploads/Private/73/9121/missing.pptx';
+
+        $client = Mockery::mock(DropboxClient::class);
+        $client->shouldReceive('createSharedLinkWithSettings')
+            ->once()
+            ->with($path)
+            ->andThrow(new BadRequest(new Response(409, [], json_encode([
+                'error_summary' => 'path/not_found/',
+                'error' => ['.tag' => 'path', 'path' => ['.tag' => 'not_found']],
+            ]))));
+        $client->shouldNotReceive('listSharedLinks');
+
+        $adapter = new DropboxAdapter($client);
+
+        $this->assertSame('', $adapter->getUrl($path));
+    }
+
+    /**
+     * Anti-regression: when Dropbox creates the shared link, its URL is returned as-is.
+     */
+    public function testGetUrlReturnsSharedLinkUrlOnSuccess(): void
+    {
+        $path = 'PresentationMediaUploads/Private/73/9082/deck.pptx';
+        $url = 'https://www.dropbox.com/scl/fi/abc123/deck.pptx?dl=0';
+
+        $client = Mockery::mock(DropboxClient::class);
+        $client->shouldReceive('createSharedLinkWithSettings')
+            ->once()
+            ->with($path)
+            ->andReturn(['url' => $url]);
+
+        $adapter = new DropboxAdapter($client);
+
+        $this->assertSame($url, $adapter->getUrl($path));
+    }
+}

tests/Unit/Services/FileDownloadStrategyGetUrlTest.php

@@ -0,0 +1,106 @@
+<?php namespace Tests\Unit\Services;
+/**
+ * Copyright 2026 OpenStack Foundation
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+use App\Services\FileSystem\Dropbox\DropboxStorageFileDownloadStrategy;
+use Illuminate\Container\Container;
+use Illuminate\Support\Facades\Facade;
+use Mockery;
+use Mockery\MockInterface;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+
+/**
+ * Class FileDownloadStrategyGetUrlTest
+ *
+ * Unit tests for {@see \App\Services\FileSystem\AbstractFileDownloadStrategy::getUrl()}
+ * through the Dropbox strategy: the value serializers expose as `private_url`.
+ *
+ * @package Tests\Unit\Services
+ */
+class FileDownloadStrategyGetUrlTest extends TestCase
+{
+    /** @var MockInterface */
+    private $cache;
+
+    /** @var MockInterface */
+    private $filesystem;
+
+    /** @var MockInterface */
+    private $config;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        // Minimal facade application: Log -> NullLogger, Cache / Storage / Config -> mocks.
+        Facade::clearResolvedInstances();
+        $app = new Container();
+        $app->singleton('log', fn() => new NullLogger());
+        $this->cache = Mockery::mock();
+        $this->filesystem = Mockery::mock();
+        $this->config = Mockery::mock();
+        $app->instance('cache', $this->cache);
+        $app->instance('filesystem', $this->filesystem);
+        $app->instance('config', $this->config);
+        Container::setInstance($app);
+        Facade::setFacadeApplication($app);
+    }
+
+    protected function tearDown(): void
+    {
+        Facade::setFacadeApplication(null);
+        Facade::clearResolvedInstances();
+        Container::setInstance(null);
+        Mockery::close();
+        parent::tearDown();
+    }
+
+    /**
+     * The Dropbox adapter reports "no link" as an empty string (its parent forbids null).
+     * The strategy must hand serializers null, and must not cache the miss.
+     */
+    public function testGetUrlReturnsNullAndDoesNotCacheWhenDiskHasNoUrl(): void
+    {
+        $path = 'PresentationMediaUploads/Private/73/9121/missing.pptx';
+
+        $disk = Mockery::mock();
+        $disk->shouldReceive('url')->once()->with($path)->andReturn('');
+        $this->filesystem->shouldReceive('disk')->with('dropbox')->andReturn($disk);
+        $this->cache->shouldReceive('get')->once()->andReturn(null);
+        $this->cache->shouldNotReceive('add');
+
+        $strategy = new DropboxStorageFileDownloadStrategy();
+
+        $this->assertNull($strategy->getUrl($path));
+    }
+
+    /**
+     * Anti-regression: a real URL is returned and cached for the configured lifetime.
+     */
+    public function testGetUrlReturnsAndCachesDiskUrl(): void
+    {
+        $path = 'PresentationMediaUploads/Private/73/9082/deck.pptx';
+        $url = 'https://www.dropbox.com/scl/fi/abc123/deck.pptx?dl=0';
+
+        $disk = Mockery::mock();
+        $disk->shouldReceive('url')->once()->with($path)->andReturn($url);
+        $this->filesystem->shouldReceive('disk')->with('dropbox')->andReturn($disk);
+        $this->cache->shouldReceive('get')->once()->andReturn(null);
+        $this->config->shouldReceive('get')->with('cache_api_response.file_url_lifetime', 3600)->andReturn(3600);
+        $this->cache->shouldReceive('add')->once()->with(Mockery::type('string'), $url, 3600);
+
+        $strategy = new DropboxStorageFileDownloadStrategy();
+
+        $this->assertSame($url, $strategy->getUrl($path));
+    }
+}