-
-
Notifications
You must be signed in to change notification settings - Fork 188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FEATURE: Dynamic resource uri host (baseUri) for CLI #3334
Comments
I'd rather allow for a custom "base URI provider" anyone can implement if needed. Similar to https://github.com/flownative/flow-google-cloudstorage?tab=readme-ov-file#dynamic-custom-base-uri |
Okay @kitsunet argued before that we dont want to make the explicit passing of the base uri a concern of the resource targets as some of the targets absolutely not require this information like cloud storage where the host is determined by the cloud. Now a recent change to one of the flow cloud packages, to the google cloud storage shows that this might not entirely be true and a feature was introduced to access flows base uri: flownative/flow-google-cloudstorage#49 to be able to implement something like this targets:
googlePersistentResourcesTarget:
target: 'Flownative\Google\CloudStorage\GcsTarget'
targetOptions:
bucket: 'flownativecom.flownative.cloud'
persistentResourceUris:
pattern: '{flowBaseUri}assets/{sha1}/{filename}' which probably relies on the web server redirecting internally all calls to $this->resourceManager->getPublicPersistentResourceUri($resource, new Uri('http://localhost')); Wdyt? @kdambekalns |
@kdambekalns uupps comments are in a funny order ^^ the problem of a custom tunnel through space is that its still a tunnel and does not get the desired information from the outside but attempts to figure it out itself. In #3354 i drafted out a way to make uri building mostly side effect free in terms of gathering required context - that requires the passing of important information explicitly from one part to the other. A custom base uri provider php implementation must still be able to access something like the request - the possible actual one or a stubbed one which might be used to render fusion from cli. For that to be done without the current potholes of the tunnel through space the resource manager must receive that instead which is really odd: |
To me one method that can be asked and figures out things on it's own is fine. Even when passing information down, where do you get the needed information from? So far the problem really exists only for CLI and the odd case of "multisite, but the URI is not the one we request". In both cases the developer is the only one who knows what would be correct – and probably could centralize that knowledge. Background: The use case for the change to the Google Cloud Storage target was that absolute URIs were needed, due to some process that turned HTML into PDF and could not deal with URLs like |
well in fusion that is with the latest changes more simple. Normally fusion contains by convention a So instead of using the tunnel through space here https://github.com/neos/neos-development-collection/blob/cc70fedd9ac9ed21139f658a7746689dd76475ce/Neos.Fusion/Classes/FusionObjects/ResourceUriImplementation.php#L138 we could write: $possibleRequest = $this->runtime->fusionGlobals->get('request');
if ($possibleRequest instanceof ActionRequest) {
$baseUri = RequestInformationHelper::generateBaseUri($possibleRequest->getHttpRequest());
} else {
// or fallback to baseUri or any other convention we find
// $baseUri = $this->runtime->fusionGlobals->get('baseUri');
throw new \RuntimeException();
}
return $this->resourceManager->getPublicPackageResourceUri($package, $path, $baseUri); which will avoid any obscured errors from three miles away. I opened a non breaking pr: #3445 and found imo a solution that shouldnt trouble us really that much - just an extra interface which allows to unlock the base uri functionality: /**
* Allows resource uris to be built with and absolute uri
*
* For targets implementing this interface, setAbsoluteBaseUri() must be invoked getting a resource uri
*
* - {@see TargetInterface::getPublicStaticResourceUri}
* - {@see TargetInterface::getPublicPersistentResourceUri}
*/
interface AbsoluteBaseUriAwareTarget
{
public function setAbsoluteBaseUri(UriInterface $baseUri): void;
} |
I suspect it is mostly causing issues when people have CLI commands, and I doubt the majority of those uses Fusion… And how is a global Fusion variable holding the request better than a global PHP prototype? |
Building a resource uri with the default configuration will crash on CLI:
The entry point ResourceManager::getPublicPackageResourceUri
calls TargetInterface::getPublicStaticResourceUri
which will use the configured target like localWebDirectoryStaticResourcesTarget
which in case of the FileSystemTarget and FileSystemSymlinkTarget
will call the BaseUriProvider::getConfiguredBaseUriOrFallbackToCurrentRequest
which uses
$bootstrap->getActiveRequestHandler->getHttpRequest
which is not possible on CLI.Possible workarounds include:
localWebDirectoryStaticResourcesTarget.targetOptions.baseUri
tohttp://localhost:8081/_Resources/Static/Packages/
Neos.Flow.http.baseUri
tohttp://localhost:8081
getResourcesBaseUri
https://github.com/Flowpack/Flowpack.DecoupledContentStore/blob/6adb8e04246f5fc9b8471b656db7ddf131474745/Classes/Transfer/Resource/Target/MultisiteFileSystemSymlinkTarget.php#L16-L38To actually fix this issue we should pass the current base uri to the target:
The methods are currently only used by the resource manager Neos & Flow.
And there we could allow a backwards compatible nullable parameter which will default to the current request:
(i left out the demonstration for
ResourceManager::getPublicPersistentResourceUriByHash
as its unused)Meaning the resource manager can be used with fixed base uris
or when leaving this parameter out, the previous default behaviour is used:
a little more breaking would be do always enforce to specify the baseUri which would definitely be correct.
In Fusion we could start inside our prototypes to call
getPublicPersistentResourceUri
with the base uri of the currentrequest and thus make the life a little easier ;)
Alternative fixes
BaseUriProvider::applyBaseUriToClosure($newBaseUri, fn () => $resourcemanger->...(...))
FLOW_BASE_URI
see FEATURE: Make base URI configuration obsolete #3002getPublicStaticResourceUri
from crashing in cli con… #3314The text was updated successfully, but these errors were encountered: