Add worker-aware Media app (issues #2, #3)#1
Open
acourtiol wants to merge 1 commit into
Open
Conversation
b1c14e9 to
40a3cc3
Compare
Adds Opengento\Application\App\Media — the missing third worker-mode entry point alongside App\Http (pub/index.php) and App\StaticResource (pub/static.php). Closes the get.php gap that's currently behind two open issues on magento2-frankenphp-base: - opengento#2 "Product Image Cache generation failure" (PLP/PDP image 404s) - #3 "Make all entry points benefit of shared bootstrap" How it works ------------ App\Media implements AppInterface. Per request, it: 1. Snapshots State::$_areaCode (via reflection — see "Why reflection") 2. Clears it 3. Constructs Magento\MediaStorage\App\Media via the worker's shared ObjectManager with request-derived (mediaDirectory, configCacheFile, isAllowed, relativeFileName) arguments — same contract stock pub/get.php uses 4. Delegates to MagentoMedia::launch() (which sets area to GLOBAL internally, materializes the file, returns the FileResponse) 5. In finally{}, restores the original area code so the next /index.php or /static.php request on the same worker sees the area BootstrapPool set up for it The Magento\MediaStorage\App\Media constructor signature is byte-identical between 2.4.8-p4 and 2.4.9-beta1 (verified by diffing both sources), so this layer is dual-version compatible. Why reflection -------------- MagentoMedia::launch() calls State::setAreaCode(AREA_GLOBAL) unconditionally on every invocation. In worker mode the State singleton persists across requests and State::setAreaCode() throws "Area code is already set" on the second call — State doesn't implement ResetAfterRequestInterface (and isn't auto-tracked by AppObjectManager's Resetter since it's resolved via get(), not create()), so the Resetter never clears it between requests. State exposes no public reset API: setAreaCode throws, emulateAreaCode wraps and restores, getAreaCode throws when unset. Reflection on the private _areaCode field is the smallest contained workaround. The save+restore is in a finally{} so a media-app failure can't leave the worker in a broken area state for other endpoints. If/when Magento adds ResetAfterRequestInterface to State (or exposes a clear() method), the reflection here can be deleted. composer.json ------------- Also bumps require constraints to match the "PHP 8.4+, Magento 2.4.8+" support matrix: - php: ^8.3 -> ^8.4 - magento/framework: * -> ^103.0 (covers 2.4.8 + 2.4.9 dev) - magento/module-media-storage: * -> ^100.4 - psr/log: * -> ^3.0
40a3cc3 to
a591ccb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem this addresses
Two open issues on the sibling magento2-frankenphp-base repo:
pub/get.phpisn't replaced by the worker shim, so it runs stock Magentopub/get.phpin classic mode. Under high media load this is either slow (~150–250 ms bootstrap per missing-media request) or fails outright depending on how FrankenPHP routes the request.App\HttpandApp\StaticResourcego throughBootstrapPooland reuse the hotObjectManager. Media doesn't. This PR closes that gap forMagento\MediaStorage\App\Media.Approach
New class:
Opengento\Application\App\Media— sibling toApp\HttpandApp\StaticResource. It's the third worker-mode entry point.Per request, it:
State::$_areaCode(via reflection — see § Why reflection)nullMagento\MediaStorage\App\Mediavia the worker's sharedObjectManagerwith request-derived(mediaDirectory, configCacheFile, isAllowed, relativeFileName)arguments — same contract that stockpub/get.phpbuildsMagentoMedia::launch()(which sets area toGLOBALinternally, materializes the file, returns theFileResponse)finally{}, restores the original area code so the next/index.phpor/static.phprequest on the same worker isn't affectedThe companion PR on
magento2-frankenphp-baseships:pub/get.phpthat does\$frankengento = \Opengento\Application\App\Media::class; include 'worker.php';extra.mapentry so the file lands in the project'spub/directory at install timeWhy reflection<a id="why-reflection">
`MagentoMedia::launch()` unconditionally calls `State::setAreaCode(Area::AREA_GLOBAL)` on every invocation. In worker mode the `State` singleton persists across requests and `State::setAreaCode()` throws "Area code is already set" on the second call.
`State` doesn't implement `ResetAfterRequestInterface` and isn't auto-tracked by `AppObjectManager`'s `Resetter` (it's resolved via `get()` not `create()`), so the resetter never clears `_areaCode` between requests.
`State` also exposes no public reset API:
Reflection on `_areaCode` is the smallest contained workaround. Save+restore lives in a `finally{}` so a media-app failure can't leave the worker in a broken area state for other endpoints.
If/when Magento adds `ResetAfterRequestInterface` to `State` (or exposes a public `clear()`), the reflection here can be deleted.
Open questions / design notes
Stacking
Standalone — doesn't depend on any in-flight PR on this repo. The companion PR at `magento2-frankenphp-base` depends on this being merged (the new `pub/get.php` shim references `Opengento\Application\App\Media`).