ENH Provide hook for updating absoluteBaseURL

[GuySartorelli]

One use case for this that I've run into a few times is for sending emails using a queued job from the CLI.

Calling setBody on an email uses Director::absoluteURL which ultimately uses Director::baseURL to make email links absolute. This can often result in the host being the server name rather than the actual website host name.

Providing this as an extension makes it easy to configure for a variety of possible situations that could result in needing dynamic control of the absolute base URL - such as using subsites.

[michalkleiner]

It feels wrong adding the same hook/extension point in three different places. Can it be consolidated into one?

[GuySartorelli]

It feels wrong adding the same hook/extension point in three different places. Can it be consolidated into one?

I didn't want to change too much of the structure of absoluteURL - and I think it's important for this the $parent to be updatable the same way that the result of absoluteBaseURL is updatable.

The only clean ways to consolidate them that I can think of are:

1. Consolidate within absoluteURL by moving the hook to just before $parent is used, and omit the hook from absoluteBaseURL - but then calling absoluteBaseURL directly won't have the updated base
2. Move all of the logic that gets different $parent values out of absoluteURL and into absoluteBaseURL while keeping the original default behaviour of absoluteBaseURL in tact.

Out of the above I prefer 2 since it means calls to absoluteBaseURL have the updated base, but that's a bigger change than I had originally set out to make.

@michalkleiner Let me know if either of the above appeal to you, or if you can think of some alternative that hasn't occurred to me.

[emteknetnz]

Yeah this PR seems a bit wrong in its current state. We certainly should not have the same hook in 3 different places.

I'm not sure why the 2 hooks in absoluteUrl() are there?

absoluteBaseURL() calls absoluteUrl(), meaning the hook would get called twice? (once in absoluteUrl(), then a second time in absoluteBaseURL())

Seems like we should retain the changes in absoluteBaseURL() and revert the absoluteUrl()?

[GuySartorelli]

Ultimately, if a developer wants to convert a relative URL to an absolute URL, they will call absoluteUrl(), which will use either self::protocolAndHost() or dirname($_SERVER['REQUEST_URI'] . 'x') as the "base" of the absolute URL. The intention of this PR is to allow developers to define, through an Extention class, an alternative "base".

If a developer want just the absolute base URL, it should be likewise updated - but as you correctly pointed out this ultimately will call absoluteUrl() anyway so we can remove the call in absoluteBaseUrl itself.

I think that having looked at this with fresh eyes the best way to do this is to put a single hook after the $parent variable has been determined and pass in that variable to be set. That variable does, afterall, effectively represent the absolute base URL.

silverstripe-framework/src/Control/Director.php

Lines 453 to 472 in f9cceef

	// Absolute urls without protocol are added
	// E.g. //google.com -> http://google.com
	if (strpos($url, '//') === 0) {
	return self::protocol() . substr($url, 2);
	}
	// Determine method for mapping the parent to this relative url
	if ($relativeParent === self::ROOT || self::is_root_relative_url($url)) {
	// Root relative urls always should be evaluated relative to the root
	$parent = self::protocolAndHost();
	} elseif ($relativeParent === self::REQUEST) {
	// Request relative urls rely on the REQUEST_URI param (old default behaviour)
	if (!isset($_SERVER['REQUEST_URI'])) {
	return false;
	}
	$parent = dirname($_SERVER['REQUEST_URI'] . 'x');
	} else {
	// Default to respecting site base_url
	$parent = self::absoluteBaseURL();
	}

If @emteknetnz and @michalkleiner are happy with this approach, I'll make the change.

Edit: We'll probably need one before line 456 as well - or else refactor the way/timing of that return statement to avoid duplicate hook calls.

[emteknetnz]

Yes, just adding a single extension hook inside absoluteURL() static::singleton()->extend('updateAbsoluteURLParent', $parent); seems like it should be sufficient

I don't think we need to do anything with the line 456 since that's for external linls

[GuySartorelli]

@emteknetnz I've made the change. That's much nicer, I'm glad you didn't like my first approach 😁

[emteknetnz]

This PR ended up indirectly causing a lot of regressions - I've created a PR to revert it #10291