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

Avoid AOP aspects to tweak core behavior #3216

Open
bwaidelich opened this issue Nov 30, 2020 · 2 comments
Open

Avoid AOP aspects to tweak core behavior #3216

bwaidelich opened this issue Nov 30, 2020 · 2 comments

Comments

@bwaidelich
Copy link
Member

bwaidelich commented Nov 30, 2020

Description

This is an umbrella issue that aims to reduce the amount of Aspect Oriented Programming in the core.

Background

Flow was created as framework for Neos. It seems weird that we have to adjust its behavior via AOP.
But more importantly: Those AOP aspects make debugging harder and weaken our API.

Examples

Some of the most important aspects to get rid of (IMO):

FusionCachingAspect

Caching aspect to speed up \Neos\Fusion\View\FusionView::getMergedFusionObjectTree().
We actually have two of those(!), one in Neos.Neos and one in Neos.Fusion itself.

Last bug: #3191

NodeTypeConfigurationEnrichmentAspect

Swiss army knife aspect that modifies the node type configuration adding i18n labels etc.
Wraps the \Neos\ContentRepository\Domain\Model\NodeType constructor

Partly resolved with #3087
Dedicated Issue: #2867

PluginUriAspect

Rewrites \Neos\Flow\Mvc\Routing\UriBuilder::uriFor() interpreting the internal __node argument.
I don't really understand what it does exactly tbh

SiteRepositoryCachingAspect

Caching aspect to speed up \Neos\Neos\Domain\Repository\SiteRepository::findFirstOnline() and \Neos\Neos\Domain\Repository\DomainRepository::findOneByActiveRequest()

If the cache is really needed (with the new routing it might not be so relevant) this logic should be moved into the corresponding repository (or into the consumer that relies on this to be fast)

NodeIdentityConverterAspect

Wraps \Neos\Flow\Persistence\AbstractPersistenceManager::convertObjectToIdentityArray() in order to make the nodes context path its technical identifier

-> #5069

RouteCacheAspect

Wraps \Neos\Flow\Mvc\Routing\RouterCachingService::extractUuids() and \Neos\Flow\Mvc\Routing\RouterCachingService::generateRouteTags() to tag route caches with the ids and workspace names of the involved nodes.

This should be done in the route part handler (see #3215 #3589)

AugmentationAspect (Neos.Ui) -> #3595

Wraps \Neos\Neos\Service\ContentElementWrappingService::wrapContentObject() adding neos-nodedata script tags.
This should be done via Fusion I suppose

@mhsdesign
Copy link
Member

FusionCachingAspect

is gone with #3839 ;)

@Sebobo
Copy link
Member

Sebobo commented Aug 12, 2024

The neos-nodedata script tags are now gone with neos/neos-ui#3770

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

3 participants