Skip to content

Comments

[TASK] Upgrade league/flysystem from v1 to v3#5

Open
CybotTM wants to merge 3 commits intomainfrom
feature/flysystem-3
Open

[TASK] Upgrade league/flysystem from v1 to v3#5
CybotTM wants to merge 3 commits intomainfrom
feature/flysystem-3

Conversation

@CybotTM
Copy link
Owner

@CybotTM CybotTM commented Feb 22, 2026

Summary

  • Upgrade league/flysystem from abandoned v1 to v3
  • Migrate code to v3 API (LocalFilesystemAdapter, writeStream, FilesystemException)
  • Upstream phpdocumentor/guides 1.9.5 already supports flysystem v3

Changes

  • composer.json / composer.lock - constraint ^1.1.10 -> ^3.29, pulls in league/flysystem-local
  • CopyResources.php - Adapter\Local -> Local\LocalFilesystemAdapter, remove v1 false-check on readStream
  • TwigExtension.php - League\Flysystem\Exception -> League\Flysystem\FilesystemException
  • IncludeDirective.php - remove @throws \League\Flysystem\FileNotFoundException docblocks
  • SiteSetSettingsDirective.php - remove @throws \League\Flysystem\FileNotFoundException docblock
  • site-set-failure test expectations - v3 error message changed from "Path is outside of the defined root" to "Path traversal detected"

Test plan

  • PHPStan passes (0 errors)
  • Unit tests pass (83/83)
  • Integration tests pass (112/112)
  • Documentation rendering works (CI)

## Summary

- Adds PHP 8.5 (stable since Nov 2025, current: 8.5.3) to the CI test
matrix
- Tests unit and integration suites against PHP 8.5
- No dependency or config changes needed (`platform.php: 8.1.27` ensures
locked install works)

## Changes

- `.github/workflows/main.yaml`: Added `'8.5'` to `matrix.php` in the
`tests` job

## Context

PHP 8.5 has been GA since November 2025. The existing `composer.json`
constraint
(`^8.1`) already allows 8.5. The `config.platform.php: 8.1.27` setting
ensures
`composer install --locked` succeeds regardless of runtime PHP version.

## Test plan

- [ ] CI runs unit tests on PHP 8.5
- [ ] CI runs integration tests on PHP 8.5
- [ ] Existing PHP 8.1-8.4 jobs unaffected
league/flysystem v1 has been abandoned since 2022. Migrate to v3 API:

- Replace Adapter\Local with Local\LocalFilesystemAdapter
- Replace League\Flysystem\Exception with FilesystemException
- Remove readStream false-check (v3 throws instead of returning false)
- Remove outdated @throws FileNotFoundException docblocks
- Update integration test expectations for v3 path traversal message
- Upstream phpdocumentor/guides 1.9.5 already supports flysystem v3
Copilot AI review requested due to automatic review settings February 22, 2026 11:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Upgrades the project from the abandoned league/flysystem v1 to v3, aligning the docs theme + guides integration with the newer Flysystem API and updating integration test fixtures to match new exception messages.

Changes:

  • Bump Flysystem to v3 (and related dependency updates via composer.lock).
  • Migrate theme code to Flysystem v3 classes/exceptions (LocalFilesystemAdapter, FilesystemException).
  • Update integration test expected logs for Flysystem v3 path traversal error wording.

Reviewed changes

Copilot reviewed 6 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
composer.json Updates league/flysystem constraint to v3.
composer.lock Locks Flysystem v3 and related dependency updates (incl. league/flysystem-local).
packages/typo3-docs-theme/src/EventListeners/CopyResources.php Switches local adapter class; updates stream read logic for v3 behavior.
packages/typo3-docs-theme/src/Twig/TwigExtension.php Updates Flysystem exception type used in catch clause.
packages/typo3-docs-theme/src/Directives/IncludeDirective.php Removes obsolete Flysystem v1 @throws FileNotFoundException docblocks.
packages/typo3-docs-theme/src/Directives/SiteSetSettingsDirective.php Removes obsolete Flysystem v1 @throws FileNotFoundException docblock.
tests/Integration/tests/site-set-failure/expected/logs/error.log Adjusts expected error message to “Path traversal detected”.
tests/Integration/tests/site-set-failure/expected/logs/warning.log Adjusts expected warning message to “Path traversal detected”.
.github/workflows/main.yaml Adds a PHP version to the CI test matrix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 45 to 47
foreach ($finder as $file) {
$stream = $source->readStream($file->getRelativePathname());
if ($stream === false) {
$this->logger->warning(sprintf('Cannot read stream from "%s"', $file->getRealPath()));
continue;
}

Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Flysystem v3, readStream() throws a FilesystemException on failure rather than returning false. This loop currently doesn’t catch those exceptions, so a single unreadable file (or a write failure) can abort the entire post-render step; additionally, if an exception is thrown after opening $stream, fclose() may be skipped. Consider wrapping readStream + putStream in try/catch (FilesystemException) and using a finally block to always close the stream while continuing with remaining files.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant