Open mhsdesign opened 1 year ago
As discussed the real goal would be to remove our neos site entity at one point and save the domains in the Neos.Neos:Site
node. This would be great and would save us from much trouble. ~But this is now a little to late to start for Neos 9, maybe Neos 10?~
I updated the pr description and we will evaluate how much effort this may take and if its realistic to fix with 9.0
Early thoughts about implementation:
the scoped properties (maybe via https://github.com/neos/neos-development-collection/pull/4779) could be used for the domains and stuff:
'Neos.Neos:Site':
# ..
properties:
domains:
scope: nodeAggregate
type: array<Neos\\Neos\\Domain\\Model\\Domain>
primaryDomain:
scope: nodeAggregate
type: Neos\\Neos\\Domain\\Model\\Domain
siteResourcesPackageKey:
scope: nodeAggregate
type: string
assetCollection:
scope: nodeAggregate
type: Neos\\Media\\Domain\\Model\\AssetCollection
The site config (which might be reworked via https://github.com/neos/neos-development-collection/issues/4582) would still depend on the node name (which must be unique by convention across multiple crs)
How do you find the node with the right domain though if you don't know where to look during routing?
While discussing this topic in Slack we realized that the Node can't be the central authority for sites because there is no central Content Repository.
SiteRepository
an interface without db specific types
Site::getConfiguration()
Settings.yaml
- implementationSite
model, remove all setters
As discussed with @kitsunet the site state (offline / online) might be dropped as this is not an important feature. The assetCollection might be more of a thing to worry but i think we came to the conclusion that one is allowed to configure it via settings and that the concept is not super duper perfect anyway and not fully working?
I played a bit around and the php code of the repository could look like this: (i wasnt totally sure about the nullable or non nullable return types but that might work ^^ - only findSiteBySiteNode is truly non nullable because with a node at hand one truly expects a site!)
interface SiteRepository
{
/**
* Find the site that was specified in the configuration `Neos.Neos.defaultSiteNodeName`
*
* If the configuration is not set it must fall back to the first available site or return null
*
* @throws \Neos\Neos\Domain\Exception if the configuration is invalid and the default site is not found
*/
public function findDefault(): ?Site;
public function findByDomain(Domain $domain): ?Site;
public function findByHost(UriInterface $uriWithHost): ?Site;
public function findAll(): iterable;
public function findOneByNodeName(SiteNodeName $siteNodeName): ?Site;
/**
* Finds a given site by site node.
*
* @throws \Neos\Neos\Domain\Exception in case the passed $siteNode is not a real site node or no site matches this site node
*/
public function findSiteBySiteNode(Node $siteNode): Site;
}
The Site model will be stripped down and become and immutable vo. As the site will be build from the configuration it will already hold all information of SiteConfiguration
and we will get rid of this dto.
final readonly class Site
{
public function __construct(
// from SiteConfiguration
public ContentRepositoryId $contentRepositoryId,
public RoutingConfiguration $routingConfiguration,
// original properties:
/** Name of the site */
public string $name, // todo name is actually bit confusing and doesnt declare that this might be a human readable label. So label or title would fit better.
/** The node name of this site */
public SiteNodeName $nodeName,
/** The key of a package containing the static resources for this site */
public string $siteResourcesPackageKey, // todo maybe just name it $packageKey?
public ?Domains $domains,
// todo how ???
// public AssetCollection $assetCollection,
) {
}
}
final readonly class Domains implements \IteratorAggregate
{
private function __construct(
public Domain $primaryDomain,
private array $rest,
) {
// deduplication logic
}
public static function create(Domain $primaryDomain, Domain ...$rest)
{
return new self($primaryDomain, $rest);
}
public function getIterator(): Traversable
{
yield $this->primaryDomain;
yield from $this->rest;
}
}
final readonly class Domain
{
public function __construct(
public string $hostname,
public ?string $scheme,
public ?int $port
) {
}
}
final readonly class RoutingConfiguration
{
public function __construct(
public string $contentDimensionResolverFactoryClassName,
public array $contentDimensionResolverOptions,
public DimensionSpacePoint $defaultDimensionSpacePoint,
public string $uriPathSuffix,
) {
}
}
migrating things:
all setters will be removed so no mutation via the repository nor the value objects is indented. This has to be manually implemented behind the scenes ;)
// the state will be removed -> if someone needs it this he can implement by returning from the SiteRepository only the online ones. By that we reduce complexity a lot.
getState()
isOnline()
isOffline()
STATE_ONLINE = 1;
STATE_OFFLINE = 2;
// this internal signal will be removed
emitSiteChanged();
// simple properties will persist
getName() -> $site->name
getNodeName() -> $site->nodeName
getSiteResourcesPackageKey() -> $site->siteResourcesPackageKey
getAssetCollection() -> $site->assetCollection
// 9.0 specific configuration stuff
getConfiguration()->contentRepositoryId -> $site->contentRepositoryId
getConfiguration()->contentDimensionResolverFactoryClassName -> $site->routingConfiguration->contentDimensionResolverFactoryClassName
getConfiguration()->contentDimensionResolverOptions -> $site->routingConfiguration->contentDimensionResolverOptions
// contentDimensions.defaultDimensionSpacePoint must be actually required to be set otherwise there is trouble as [] is not always valid
getConfiguration()->defaultDimensionSpacePoint -> $site->routingConfiguration->defaultDimensionSpacePoint
getConfiguration()->uriPathSuffix -> $site->routingConfiguration->uriPathSuffix
// domain related
getDomains() -> $site->domains
hasActiveDomains() -> $site->domains !== null
getActiveDomains() -> iterator_to_array($site->domains)
getFirstActiveDomain() -> $site->domains->primaryDomain
getPrimaryDomain() -> $site->domains->primaryDomain
The above read model would be backed primarily out of the box by a SiteConfigurationSource
which implements the SiteRepository
.
This requires us to adjust how sites are registered in Neos.Neos.sites
.
For example as the configuration is currently only used as enrichment but not as primary source we introduced the wildcard *
. But this when using the settings as site source we are required to explicitly state the nodeName and not use the wildcard *
configuration.
Naturally this would mean having to duplicate all the "basic" options every time. But with the Site Config Presets this will not be necessary https://github.com/neos/neos-development-collection/issues/4582
Currently in 9.0 the configuration might look like:
Neos:
Neos:
sites:
'*':
uriPathSuffix: '.html'
contentRepository: default
contentDimensions:
resolver:
factoryClassName: Neos\Neos\FrontendRouting\DimensionResolution\Resolver\AutoUriPathResolverFactory
But with the Site Config Presets and the SiteConfigurationSource we would write:
Neos:
Neos:
sitePresets:
'default':
uriPathSuffix: '.html'
contentRepository: default
contentDimensions:
resolver:
factoryClassName: Neos\Neos\FrontendRouting\DimensionResolution\Resolver\AutoUriPathResolverFactory
sites:
'my-site-node-name':
preset: 'default'
name: "My Site Titel"
siteResourcesPackageKey: "Neos.Demo"
domains:
primaryDomain:
hostname: "localhost"
scheme: "http"
port: 8080
additionalDomains:
-
hostname: "www.lol.de"
assetCollection: null # todo how to actually reference an assetCollection??? via Persistence_Object_Identifier???
Neos.Neos.sites
configuration with site node aggregate idsalternatively we could already open the door and allow nodeAggregateId's to linking to the node. This is much faster and less quirky ^^. So possibly one would register a node id or enter an absolute nodepath. The downsides of this is that the fetched node has first to be checked if it is of type Neos.Neos:Site
and cannot be blindly trusted. And also the configuration name "some-random-name..." will have no meaning and thus not forbid configuring two sites with the same node... though that might need to be supported with multiple content repositories???. A way out would be to insert the node id or absolute node path into the configuration name.
Neos:
Neos:
sites:
some-random-name-only-for-configuration-readability:
preset: 'default'
node: my-site-node-aggregate-id
# or "legacy" node names via:
# node: <Neos.Neos:Sites>/my-site-node-name
As sites will be defined in the settings, they will have to be created like a content repository needs to be setup. So instead of
flow site:create neosdemo Neos.Demo Neos.Demo:Document.Homepage
it might looks like:
flow site:setup neosdemo Neos.Demo:Document.Homepage
All site and domain command controllers will only allow reading from the new site model. No creation will be possible.
Last Friday we discussed the above idea in the dev meeting:
Following points were made:
Pro (Bastian & Marc Henry):
Con (Martin & Denny):
Jain - in between - (Christian)
Conclusion
As this was discussed super controversially without a clear voting in favour we decided against this.
Having the concept of a site entity (doctrine) and a site node (cr) is not ideal:
We discussed already several ways of improving their relationship:
site
in fusion is a site node, while in php we often mean by$site
the site entity (which is mostly unused in fusion) -> the same discussion came up with the introduction of https://github.com/neos/neos-development-collection/issues/4531 where we discussed to name itNeos.Neos:Site
orNeos.Neos:HomePage
. We decided for Site and declared the site entity rather legacy and some time to be replaced.Neos.Neos.sites
. See https://github.com/neos/neos-development-collection/issues/4470#issuecomment-1890785396