Skip to content
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

Experiment: Replace SiteEntity with SiteNodeAggregate #4780

Closed
wants to merge 2 commits into from

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Nov 27, 2023

WIP

Related: #4470

only works for one site right now at time - no real domain support.

For migration use

flow site:setProperties neosdemo --siteResourcesPackageKey Neos.Demo --name "Neos Demo"

@github-actions github-actions bot added the 9.0 label Nov 27, 2023
Comment on lines -56 to -61
/**
* @var Site
* @ORM\ManyToOne(inversedBy="domains")
* @Flow\Validate(type="NotEmpty")
*/
protected $site;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how should the relation ship of

domain entity -> site node aggregate

work? Currently in this wip the domain wouldnt know its site directly but one has to fetch all site node aggregates and look into them

Comment on lines +64 to +106
public function findAll(): iterable
{
$cr = $this->contentRepositoryRegistry->get(ContentRepositoryId::fromString('default'));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how can we find all site node aggregates efficiently? Loop over all crs registered and fetch the node aggregates und the Neos.Neos:Sites node?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that seems SUPER expensive for something that will happen on every request. (we could cache it of course).

If anything, I would see a custom projection for it?

@mhsdesign mhsdesign force-pushed the task/mergeSiteNodeAndSiteEntity branch from a407cb0 to b415aa0 Compare November 27, 2023 20:15
@mhsdesign mhsdesign changed the title WIP Merge SiteNode And site entity Experiment: Replace SiteEntity with SiteNodeAggregate Jan 14, 2024
@mhsdesign mhsdesign force-pushed the task/mergeSiteNodeAndSiteEntity branch from 806c0e2 to 5baa0b6 Compare January 14, 2024 10:03
@mhsdesign mhsdesign force-pushed the task/mergeSiteNodeAndSiteEntity branch from 5baa0b6 to ce4c272 Compare January 14, 2024 12:08
@mhsdesign mhsdesign added 9.1 and removed 9.0 labels Feb 7, 2024
@mhsdesign
Copy link
Member Author

Closing this experiment as written here

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.

That could be worked around by caching or projections or other stuff but it might not be worth the hassle. At least definitely not for now.

Also working with node aggregates is not easy at all, #4830 as we just want to use aggregate scoped properties but still have to issue the set properties command on a fake (random) dsp

For the neos 9 release we will keep the site entity and domain entity as is.

@mhsdesign mhsdesign closed this Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants