fix(#3494240): skip the redirect when a file leaves the staging location

CHANGELOG.md

@@ -26,6 +26,12 @@ release is dated by its Drupal.org release node.
   was the wrapper's absolute URL encoded into one path segment, a row that
   could never match. A file on a private field that moved after a title
   change with active updating on left one behind.
+- [#3494240](https://www.drupal.org/i/3494240): Stopped creating a redirect
+  on an entity's first save, and when a file leaves the staging location on a
+  later save. Nothing could link to either path, and with "Create Redirect"
+  on every new upload left a redirect behind. A bare scheme root such as
+  `public://` is not treated as a staging location, because it would match
+  every file on the scheme.
 
 ## 8.x-1.0-rc2 - 2026-09-07
 

src/Hook/FileFieldPathsProcessFileLegacy.php

@@ -4,6 +4,7 @@
 
 namespace Drupal\filefield_paths\Hook;
 
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Component\Utility\DeprecationHelper;
 use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\Core\Extension\ModuleHandlerInterface;
@@ -30,6 +31,7 @@
 final readonly class FileFieldPathsProcessFileLegacy {
 
   public function __construct(
+    private ConfigFactoryInterface $configFactory,
     private FileSystemInterface $fileSystem,
     private FileRepositoryInterface $fileRepository,
     private StreamWrapperManagerInterface $streamWrapperManager,
@@ -165,10 +167,14 @@ public function fileFieldPathsProcessFile(ContentEntityInterface $entity, FileFi
       }
       $this->processOutcome->recordUpdated($file->id());
 
-      // Create redirect from old location.
+      // Create redirect from old location, unless the entity is new or the
+      // file is only now leaving the staging area. Nothing can link to a
+      // path that existed only between the upload and the save.
       if (
         !empty($settings['redirect']) && $settings['active_updating'] &&
-        $this->moduleHandler->moduleExists('redirect')
+        $original_entity instanceof ContentEntityInterface &&
+        $this->moduleHandler->moduleExists('redirect') &&
+        !$this->isStagedUpload($file->getFileUri(), $settings)
       ) {
         $redirect = $this->getRedirect();
         $redirect->createRedirect($file->getFileUri(), $new_file->getFileUri(), $file->language());
@@ -238,4 +244,34 @@ private function sanitizeDestination(string $destination, string $source_uri, st
     return implode('/', $segments) . '/' . $event->getFilename();
   }
 
+  /**
+   * Checks whether a file is still at the upload staging location.
+   *
+   * @param string $uri
+   *   The file URI before the move.
+   * @param array $settings
+   *   The File (Field) Paths settings for the field.
+   *
+   * @return bool
+   *   TRUE if the file has not left the staging location yet.
+   */
+  private function isStagedUpload(string $uri, array $settings): bool {
+    $temp_location = $settings['temp_location'] ?? NULL;
+    if (empty($temp_location)) {
+      $temp_location = $this->configFactory
+        ->get('filefield_paths.settings')
+        ->get('temp_location');
+    }
+    if (!is_string($temp_location) || $temp_location === '') {
+      return FALSE;
+    }
+    // A bare scheme root such as "public://" is not a staging directory, and
+    // the settings form accepts one. Used as a prefix it would match every
+    // file on that scheme and stop redirects being created at all.
+    if ((string) $this->streamWrapperManager::getTarget($temp_location) === '') {
+      return FALSE;
+    }
+    return str_starts_with($uri, rtrim($temp_location, '/') . '/');
+  }
+
 }

tests/src/Kernel/FileFieldPathsProcessFileLegacyTest.php

@@ -158,6 +158,7 @@ protected function getService(): FileFieldPathsProcessFileLegacy {
    */
   protected function constructService(?FileSystemInterface $fileSystem = NULL, ?FileRepositoryInterface $fileRepository = NULL): FileFieldPathsProcessFileLegacy {
     return new FileFieldPathsProcessFileLegacy(
+      $this->container->get('config.factory'),
       $fileSystem ?? $this->container->get('file_system'),
       $fileRepository ?? $this->container->get('file.repository'),
       $this->container->get('stream_wrapper_manager'),

tests/src/Kernel/StagingRedirectTest.php

@@ -0,0 +1,219 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\filefield_paths\Kernel;
+
+use Drupal\Core\Field\FieldStorageDefinitionInterface;
+use Drupal\KernelTests\KernelTestBase;
+use Drupal\entity_test\Entity\EntityTest;
+use Drupal\field\Entity\FieldConfig;
+use Drupal\field\Entity\FieldStorageConfig;
+use Drupal\file\Entity\File;
+use PHPUnit\Framework\Attributes\Group;
+use PHPUnit\Framework\Attributes\RunTestsInSeparateProcesses;
+
+/**
+ * Tests which moves earn a redirect.
+ *
+ * @group filefield_paths
+ * @covers \Drupal\filefield_paths\Hook\FileFieldPathsProcessFileLegacy
+ */
+#[Group('filefield_paths')]
+#[RunTestsInSeparateProcesses]
+class StagingRedirectTest extends KernelTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array<string>
+   */
+  protected static $modules = [
+    'system',
+    'user',
+    'field',
+    'file',
+    'path_alias',
+    'redirect',
+    'link',
+    'entity_test',
+    'filefield_paths',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+    $this->installEntitySchema('user');
+    $this->installEntitySchema('file');
+    $this->installEntitySchema('entity_test');
+    $this->installEntitySchema('redirect');
+    $this->installEntitySchema('path_alias');
+    $this->installSchema('file', ['file_usage']);
+    $this->installConfig(['filefield_paths']);
+    // Pin the staging location. The install default moved to
+    // temporary://filefield_paths in 8.x-1.0-rc2, and the files these tests
+    // attach must sit inside the staging location whatever the default is.
+    $this->config('filefield_paths.settings')->set('temp_location', 'public://filefield_paths')->save();
+    $this->config('redirect.settings')->set('default_status_code', 301)->save();
+
+    FieldStorageConfig::create([
+      'field_name' => 'field_file',
+      'entity_type' => 'entity_test',
+      'type' => 'file',
+      'cardinality' => FieldStorageDefinitionInterface::CARDINALITY_UNLIMITED,
+      'settings' => ['uri_scheme' => 'public'],
+    ])->save();
+
+    $options = ['slashes' => FALSE, 'pathauto' => FALSE, 'transliterate' => FALSE];
+    $field = FieldConfig::create([
+      'entity_type' => 'entity_test',
+      'field_name' => 'field_file',
+      'bundle' => 'entity_test',
+    ]);
+    $field->setThirdPartySetting('filefield_paths', 'enabled', TRUE);
+    $field->setThirdPartySetting('filefield_paths', 'file_path', [
+      'value' => 'sorted',
+      'options' => $options,
+    ]);
+    $field->setThirdPartySetting('filefield_paths', 'file_name', [
+      'value' => '',
+      'options' => $options,
+    ]);
+    $field->setThirdPartySetting('filefield_paths', 'active_updating', TRUE);
+    $field->setThirdPartySetting('filefield_paths', 'redirect', TRUE);
+    $field->setThirdPartySetting('filefield_paths', 'retroactive_update', FALSE);
+    $field->save();
+  }
+
+  /**
+   * Saves a new entity with a file at the given URI.
+   *
+   * @param string $uri
+   *   Where the file sits before the entity is saved.
+   */
+  private function createEntityWithFileAt(string $uri): void {
+    EntityTest::create([
+      'name' => 'test',
+      'field_file' => [['target_id' => $this->createFileAt($uri)->id()]],
+    ])->save();
+  }
+
+  /**
+   * Saves an entity without a file, then saves it again with one.
+   *
+   * @param string $uri
+   *   Where the file sits before the second save.
+   */
+  private function updateEntityWithFileAt(string $uri): void {
+    $entity = EntityTest::create(['name' => 'test']);
+    $entity->save();
+    $entity = EntityTest::load($entity->id());
+    $entity->set('field_file', [['target_id' => $this->createFileAt($uri)->id()]]);
+    $entity->save();
+  }
+
+  /**
+   * Writes a file to disk and saves a permanent file entity for it.
+   */
+  private function createFileAt(string $uri): File {
+    $file_system = $this->container->get('file_system');
+    $directory = $file_system->dirname($uri);
+    $file_system->prepareDirectory($directory, $file_system::CREATE_DIRECTORY);
+    file_put_contents($uri, 'contents');
+    $file = File::create(['uri' => $uri]);
+    $file->setPermanent();
+    $file->save();
+    return $file;
+  }
+
+  /**
+   * Counts the redirects that exist.
+   */
+  private function redirectCount(): int {
+    return count($this->container->get('entity_type.manager')
+      ->getStorage('redirect')
+      ->loadMultiple());
+  }
+
+  /**
+   * A new entity earns no redirect, wherever its file came from.
+   *
+   * Nothing can link to a file's path before the entity's first save.
+   *
+   * @see https://www.drupal.org/i/3494240
+   */
+  public function testNoRedirectWhenTheEntityIsNew(): void {
+    $this->createEntityWithFileAt('public://published/example.txt');
+
+    $this->assertFileExists('public://sorted/example.txt');
+    $this->assertSame(0, $this->redirectCount(), 'A new entity should not leave a redirect behind.');
+  }
+
+  /**
+   * A file leaving the staging area earns no redirect.
+   *
+   * The staging path only ever existed between the upload and the save, so
+   * nothing can be linking to it, even when the entity already existed.
+   *
+   * @see https://www.drupal.org/i/3494240
+   */
+  public function testNoRedirectWhenTheFileComesFromStaging(): void {
+    $this->updateEntityWithFileAt('public://filefield_paths/example.txt');
+
+    $this->assertFileExists('public://sorted/example.txt');
+    $this->assertSame(0, $this->redirectCount(), 'A staged upload should not leave a redirect behind.');
+  }
+
+  /**
+   * A file that was already published still earns a redirect.
+   *
+   * @see https://www.drupal.org/i/3494240
+   */
+  public function testRedirectWhenThePublishedFileMovesOnUpdate(): void {
+    $this->updateEntityWithFileAt('public://published/example.txt');
+
+    $this->assertFileExists('public://sorted/example.txt');
+    $this->assertSame(1, $this->redirectCount(), 'A published file that moves should leave a redirect.');
+  }
+
+  /**
+   * A bare scheme root is not a staging location.
+   *
+   * The settings form accepts "public://" with no directory. Used as a
+   * staging prefix it would match every file on the scheme and quietly stop
+   * redirects being created at all.
+   *
+   * @see https://www.drupal.org/i/3494240
+   */
+  public function testBareSchemeRootIsNotTreatedAsStaging(): void {
+    $this->config('filefield_paths.settings')
+      ->set('temp_location', 'public://')
+      ->save();
+
+    $this->updateEntityWithFileAt('public://published/example.txt');
+
+    $this->assertFileExists('public://sorted/example.txt');
+    $this->assertSame(1, $this->redirectCount(), 'A bare scheme root must not suppress redirects.');
+  }
+
+  /**
+   * The field's own staging location wins over the global one.
+   *
+   * @see https://www.drupal.org/i/3494240
+   */
+  public function testFieldStagingLocationTakesPrecedence(): void {
+    $field = FieldConfig::loadByName('entity_test', 'entity_test', 'field_file');
+    \assert($field instanceof FieldConfig);
+    $field->setThirdPartySetting('filefield_paths', 'temp_location', 'public://custom_stage');
+    $field->save();
+    $this->container->get('entity_field.manager')->clearCachedFieldDefinitions();
+
+    $this->updateEntityWithFileAt('public://custom_stage/example.txt');
+
+    $this->assertFileExists('public://sorted/example.txt');
+    $this->assertSame(0, $this->redirectCount(), 'The field level staging location should suppress the redirect.');
+  }
+
+}