fix(#3269636): use stream wrapper URL resolution for private:// redirect paths

[Decipher]

Summary

Adds Kernel test coverage for src/Redirect.php, which currently has 0%
coverage. Includes a test that intentionally fails, reproducing the bug
reported in #3269636
("Incorrect redirect from public to private folder"). This is deliberately
a test-only, red MR — the fix will follow as a separate MR once this is
reviewed.

Closes #

Background

Drupal.org issue #3269636 reports that moving a file from public:// to
private:// creates a redirect pointing at the raw private filesystem path
(e.g. /sites/default/PRIVATE/folder/file) instead of the web-accessible
download route (/system/files/folder/file). The issue had been Postponed
[NMI] since 2022 pending re-confirmation against rc1.

Root cause, confirmed while writing this test: Redirect::getPath() calls
$wrapper->getDirectoryPath() for any LocalStream-based wrapper. For
public:// this happens to be web-accessible, but
PrivateStream::getDirectoryPath() returns Settings::get('file_private_path')
— the raw filesystem base path — rather than going through
PrivateStream::getExternalUrl(), which builds the
system.private_file_download route.

While building this test, also found and documented (but did not fix) a
second, previously unreported bug: the dedup pre-check in
createRedirect() hashes the destination path, but the redirect
entity's own preSave() always recomputes its stored hash from the
source path. The two never match, so calling createRedirect() twice
with identical arguments throws an uncaught EntityStorageException
instead of being silently skipped. Tracked as a @todo-style note in the
test for now; no Drupal.org issue filed yet.

Changes

File	Type	Purpose
tests/src/Kernel/RedirectTest.php	New	Kernel coverage for Redirect::createRedirect() / getPath(): public:// happy path (passing), duplicate-call behaviour (passing, documents the dedup bug), private:// destination path (failing — reproduces #3269636)

Test plan

• DRUPAL_VERSION=10 make test-unit passes (11/11)
• DRUPAL_VERSION=10 make test-kernel — 2 pass, 1 intentionally fails (testCreateRedirectForPrivateSchemeUsesWebAccessiblePath), confirming the reproduction
• DRUPAL_VERSION=10 make lint — PHPCS clean (PHPStan hits a pre-existing environment memory-limit issue unrelated to this change, reproduced on 8.x-1.x HEAD as well)
• Follow-up MR: fix Redirect::getPath() to resolve a web-accessible URL for non-directly-servable schemes; this test should then turn green

Summary by CodeRabbit

• Tests
  • Added/updated kernel tests to validate redirect behavior for both public:// and private:// file paths, including handling duplicate redirect creation without errors.
• Bug Fixes
  • Corrected how redirect target paths are resolved for public:// vs non-public schemes, ensuring the resulting redirect URL uses the appropriate, user-facing path format.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 25448e99-e8f5-4e22-8cc1-c4aa6290ed40

📥 Commits

Reviewing files that changed from the base of the PR and between 646d619 and 36b96df.

📒 Files selected for processing (2)

• src/Redirect.php
• tests/src/Kernel/RedirectTest.php

---

📝 Walkthrough

Walkthrough

Updates the Redirect service to differentiate stream scheme handling: public:// URIs resolve to directory paths relative to the Drupal root, while other schemes use the stream wrapper's external URL. Tests now verify duplicate redirect creation does not throw and private-scheme redirects resolve without conditional skipping.

Changes

Redirect path resolution and test behavior updates

Layer / File(s)	Summary
Path resolution logic for stream schemes src/Redirect.php	getPath() docblock clarifies public:// path handling relative to Drupal root and external URL behavior for other schemes. Implementation adds public-scheme branch returning directory path plus target; otherwise returns wrapper external URL.
Test updates for duplicate creation and private-scheme behavior tests/src/Kernel/RedirectTest.php	testCreateRedirectTwiceWithSameArgumentsDoesNotThrow() now asserts the second createRedirect() call does not throw and verifies only one entity is stored (gated by issueRedirectDedupFixed). Removed issue3269636Fixed flag and its conditional skip from private-scheme test, allowing it to run unconditionally and assert /system/files/... redirect URL.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐇 Schemes diverge—the rabbit makes a choice,
Public paths sing, external URLs voice,
Duplicates once thrown, now gently rest,
Private files redirect their finest best,
The tests run free, no flags need confess! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title references issue #3269636 and describes a fix for stream wrapper URL resolution for private:// redirect paths, which directly aligns with the main objective of addressing the incorrect redirect behavior from public to private folders documented in the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/3269636-private_redirect_path

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.79%. Comparing base (b1c6eac) to head (36b96df).

Additional details and impacted files

@@             Coverage Diff             @@
##           8.x-1.x       #6      +/-   ##
===========================================
+ Coverage    79.72%   79.79%   +0.06%     
===========================================
  Files           17       17              
  Lines          666      668       +2     
===========================================
+ Hits           531      533       +2     
  Misses         135      135              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.