Get rid of configured baseUri

[bwaidelich]

Since the early days, Flow comes with a setting for the baseUri

But this whole concept has been source of confusion and bugs (see #2084 for example) and doesn't make sense in muliti-domain scenarios (like multi site Neos installations).

The BaseUriProvider that was introduced with #1755 is currently used in the following 7 places in the core packages:

• Neos.Flow:
  • UriBuilder
  • FileSystemTarget
  • WebRedirect
• Neos.Neos:
  • LinkingService #1, LinkingService #2
• Neos.Fusion:
  • BaseUriHelper
• Neos.Form:
  • RedirectFinisher

In all of these cases it should be possible to just fall back to the current ServerRequest (which is either available from the ActionRequest `or can be injected (see #2144)).

For cases where it's not possible to get hold of the active request (e.g. in CLI context) it can be created just like before.
For example for the UriBuilder:

$httpRequest = ServerRequest::fromGlobals();
$actionRequest = ActionRequest::fromHttpRequest($httpRequest);
$uriBuilder = new UriBuilder();
$uriBuilder->setRequest($actionRequest);

[bwaidelich]

BTW: I had a nice discussion with @kitsunet about this and he mentioned another usecase of the baseUri configuration that I didn't have on my radar: If the Flow instance is accessible through a proxy, the public "base URI" can differ from the one of the server request.
But IMO this would be a perfect fit for a corresponding PSR-15 middleware that just replaces the ServerRequest instance.

Also installations that are somewhere below the document root needs some considerations.. I guess that can be done with a middleware as well though

[albe]

has been source of confusion and bugs (see #2084 for example)

I'm not so sure about this. I don't think the concept of baseUri is the culprit here - it's rather the only sane solution to somehow get an expected URI out of the UriBuilder in CLI.

For example for the UriBuilder:

$httpRequest = ServerRequest::fromGlobals();
$actionRequest = ActionRequest::fromHttpRequest($httpRequest);
$uriBuilder = new UriBuilder();
$uriBuilder->setRequest($actionRequest);

I would rather have that be something like:

$baseUri = BaseUriProvider::fromConfiguration(); // or just inject and new Uri($baseUriSetting) or do something fancy if you like
$uriBuilder = new UriBuilder($baseUri);

The fact that the UriBuilder needs an ActionRequest, which needs to wrap an HttpRequest is something we should get rid of. Building an URI is a concern outside of the context of handling an action request that comes through an HTTP interface. Using an existing HttpRequest to build URIs should just be the happy-path fallback solution.
And yes, we also use the ActionRequest for controller name/action name fallback resolving and keeping incoming arguments in the built URIs. Those are optional things though IMO. So the UriBuilder should just work without an ActionRequest (which is also implicated by the fact that it doesn't want the request in the constructor, but only through a setter).

[bwaidelich]

@albe thanks for your valuable feedback once again!

I don't think the concept of baseUri is the culprit here

I think it's at least the culprit for a lot of confusion:
What is a "baseUri" to begin with? And does it make sense to have that globally defined? And, if so, why does it break one of the core scenarios (Neos multi-sites)?

For the record: I don't argue against your requirements. Just to call it "baseUri" and have a global configuration doesn't make sense to me.

I would rather have that be something like:
$baseUri = BaseUriProvider::fromConfiguration();

Sure, the example was simplified. Point is: When you use the UriBuilder directly or indirectly you should be able to pass in the current context, i.e. ServerRequest (and I explain why I think so below)

The fact that the UriBuilder needs an ActionRequest, which needs to wrap an HttpRequest is something we should get rid of.

Yes, I agree with that. The ActionRequest is currently used in order to fall back to the current @package, @controller, ... values when they are not explicitly defined and to merge arguments with the one from the current request.
Moving this out of the UriBuilder into the calling parts would make sense, but be a hell of a breaking change.
But at least the ActionRequest should be optional – as you wrote.

Building an URI is a concern outside of the context of handling an action request that comes through an HTTP interface

Not entirely. With #614 it's possible to generate absolute URLs in order to link between multiple domains (we plan to use this for Neos to get rid of the slow and ugly LinkingService but it can also be useful for Flow applications).
Instead of turning all generated links to be absolute (not a good idea SEO wise) you want them to be host-relative if the current request is already on the target domain.
Furthermore the request host & method is considered in the routing cache so that relative URLs on two different domains don't share the same cache entry.

And, by the way:

(which is also implicated by the fact that it doesn't want the request in the constructor, but only through a setter).

That's still a major flaw IMO that the UriBuilder is mutable. Unfortunately we can't change that without breaking a lot of code, but I'd like to mark all setters deprecated (and provide corresponding ->with*() methods). Otherwise one setActionRequest() changes the behavior of all UriBuilder instances that are invoked afterwards

[albe]

What is a "baseUri" to begin with?

Well, not that it translates 1:1, but maybe https://developer.mozilla.org/en-US/docs/Web/API/Node/baseURI gives a good idea. I think the concept of "baseUri" is pretty well known in the web context generally.

In short:

"The base URL is used to resolve relative URLs"

And does it make sense to have that globally defined?

It depends (tm) :) For the case of a single website, even running on a couple alias domains with a single canonical domain, this would make sense. Also if you have a setup where the original host does not pass down to your application via some obscure proxy. Or, if you have your single-website and just want to render absolute URLs in your e-mail templates that are generated in CLI context. So I'd say: Iff your setup is a single website with one canonical URL, this global setting makes a lot of sense.

And, if so, why does it break one of the core scenarios (Neos multi-sites)?

Because that falls out of the "single canonical domain website" case :) And yes, that is something that still needs to be adressed.

you should be able to pass in the current context

100% agree - just maybe the "context" shouldn't be "ActionRequest linked to an Http request", but rather a set of base- @package, @controller, ... and URI variables?

Maybe s.th. like new UriBuilder(UriBuilderContext::fromActionRequest(...)) for web and new UriBuilder(UriBuilderContext::fromGlobals()) in CLI? (yeah, "contexts", meh...)

you want them to be host-relative if the current request is already on the target domain.

Let me rephrase that a bit: "you want them to be host-relative if the current baseUri (or host, see below) matches the target domain"
The UriBuilder needs a "baseUri" and it can take that from an http request, but not necessarily.

Furthermore the request host & method is considered in the routing cache

Those are just two other variables in the builder "context" and all variables in the context should be taken into account for the cache ofc.

Otherwise one setActionRequest() changes the behavior of all UriBuilder instances that are invoked afterwards

100% again - though that issue is orthogonal to what we discuss here (getting rid of baseUri)

[bwaidelich]

I think the concept of "baseUri" is pretty well known in the web context generally.

I know and I know the documents you linked to. But it's exactly not the same thing that we use it for, i.e. the baseUri of this website is https://github.com/neos/flow-development-collection/issues/2157 according to the spec for example.

But I don't mean to split hairs and I would suggest that we focus on a potential solution since you seem to agree that it has to be addressed in general.

[sorenmalling]

Tipping in, with a usecase of baseUri i haven't thought about.

I did a Flow setup on DigitalOcean Apps, and the reverse proxy stuff of theres, gave my link a url like

project-distribution-ptfcd.ondigitalocean.app

despite my DNS and setup in there system. Adding the baseUri in the Settings.yaml, and using a ENV for the value, solved it.

Where does this response go? 😁

I have never used baseUri before in Flow, but from the documentation i could not read my way through a solution other than adding a baseUri. So as/if we remove it, let's put a task in on upgrading documentation on this topic. I can provide the distribution and environment for creating yaml configuration and test setup for this

[bwaidelich]

I did a Flow setup on DigitalOcean Apps, and the reverse proxy stuff of theres, gave my link a url like [...]

@sorenmalling Thanks for chiming in. Just to make sure: Isn't that what I meant with #2157 (comment) ?

So as/if we remove it, let's put a task in on upgrading documentation on this topic

I would imply that with every new/removed feature. As well as tests, but well... good that you mention it! :)

[sorenmalling]

@sorenmalling Thanks for chiming in. Just to make sure: Isn't that what I meant with #2157 (comment) ?

Absolutely! Consider my comment a +1 on that 👍 (and that I clearly did not read comments before posting)

[mhsdesign]

The baseUri will now be deprecated via: #2744

[mhsdesign]

Now: deprecated via: #3002

[mhsdesign]

Nice i just found out how we do it in the cli rendering for flowpack decoupled content store

https://github.com/Flowpack/Flowpack.DecoupledContentStore/blob/6adb8e04246f5fc9b8471b656db7ddf131474745/Classes/Transfer/Resource/Target/MultisiteFileSystemSymlinkTarget.php#L16-L38

😭 -> so ideally one should not need to go to such lengths and need such global configurations.

[mhsdesign]

Untying the FileSystemTarget from the baseUri is the hardest.

With #3314 i could prevent getPublicStaticResourceUri from crashing in cli, but that only fixes half of the problem.

Sometimes one wants to use a different base uris (hosts) for different renderings for the FileSystemTarget.
That leads to hacks like here, or like so:

    /**
     * @var BaseUriProvider
     * @Flow\Inject(lazy=false)
     */
    protected $baseUriProvider;

    /**
     * @return callable restore the original state
     */
    private function hackTheConfiguredBaseUriOfTheBaseUriProviderSingleton(UriInterface $baseUri): callable
    {
        assert($this->baseUriProvider instanceof BaseUriProvider);

        static $originalConfiguredBaseUri;
        if (!isset($originalConfiguredBaseUri)) {
            $originalConfiguredBaseUri = ObjectAccess::getProperty($this->baseUriProvider, "configuredBaseUri", true);
        }

        ObjectAccess::setProperty($this->baseUriProvider, "configuredBaseUri", (string)$baseUri, true);

        return function () use($originalConfiguredBaseUri) {
            ObjectAccess::setProperty($this->baseUriProvider, "configuredBaseUri", $originalConfiguredBaseUri, true);
        };
    }

So either getPublicStaticResourceUri must always return a host relative path, and the calling site has to prepend the host if necessary, or we need to pass a BaseUri or UriConstraints through the chain which would also breaking.

Non breaking but more hacky would be to have one global place that is allowed to be mutated, which stores the currently set base uri. Instead of introducing a singleton for that i thougth about adding an environment variable FLOW_BASE_URI like shown here.