search

neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
260 stars 220 forks source link

Cleanup duplicate php class names across project #4341

Open mhsdesign opened 1 year ago

mhsdesign commented 1 year ago

Lately i was using the new ESCR WorkspaceFinder and passed at first the wrong WorkspaceName into it (because we have this thing twice in our code base)

That got me thinking, if we have also other classNames that are duplicates and could lead to confusion. So i searched all duplicate base file names in this dev-collection and found the following. I excluded the results that i dont think are relevant.

Related: #4777

FusionView -> https://github.com/neos/neos-development-collection/issues/4476

its always hard to explain the difference in slack. And the fact that they have the same name doesnt make it easier. The Neos.Neos view should be named NodeFusionView or sth

./Neos.Neos/Classes/View/FusionView.php
./Neos.Fusion/Classes/View/FusionView.php

Node ✅

lets rename the second (internal) one

./Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Node.php
./Neos.Fusion.Afx/Classes/Parser/Expression/Node.php

-> https://github.com/neos/neos-development-collection/pull/4528

NodeAddress ✅

./Neos.Neos/Classes/AssetUsage/Dto/NodeAddress.php
./Neos.Neos/Classes/FrontendRouting/NodeAddress.php

-> https://github.com/neos/neos-development-collection/pull/4540

User ✅

i would prefer, if the User utility has an Utility suffix We even only use it aliased as UserUtility

./Neos.Neos/Classes/Domain/Model/User.php
./Neos.Neos/Classes/Utility/User.php

-> https://github.com/neos/neos-development-collection/pull/4530

UserService

its confusing that we have this service twice

A comment from late 2015 suggest that \Neos\Neos\Service\UserService is "legacy", see commit

The methods getters of this class are accessible via the "context.userInformation" variable in security policies and thus are implicitly considered to be part of the public API. This UserService should be replaced by \Neos\Neos\Domain\Service\UserService in the long run.

./Neos.Neos/Classes/Service/UserService.php
./Neos.Neos/Classes/Domain/Service/UserService.php

NodeAggregateIds

why? Lets check if we can just use the shared model of the core

./Neos.ContentRepository.Core/Classes/SharedModel/Node/NodeAggregateIds.php
./Neos.ContentGraph.PostgreSQLAdapter/src/Domain/Projection/NodeAggregateIds.php

WorkspaceName ✅

why? Is the Neos.Neos WorkspaceName legacy? Can it be removed?

./Neos.ContentRepository.Core/Classes/SharedModel/Workspace/WorkspaceName.php
./Neos.Neos/Classes/Domain/Model/WorkspaceName.php

-> https://github.com/neos/neos-development-collection/pull/4534

CrCommandController and WorkspaceCommandController

why are those command controllers separated? Shouldn't they belong to one controller?

./Neos.ContentRepositoryRegistry/Classes/Command/CrCommandController.php
./Neos.ContentRepository.LegacyNodeMigration/Classes/Command/CrCommandController.php
./Neos.Neos/Classes/Command/CrCommandController.php

./Neos.ContentRepositoryRegistry/Classes/Command/WorkspaceCommandController.php
./Neos.Neos/Classes/Command/WorkspaceCommandController.php

Exception

those exceptions should have more distinct names

./Neos.Media/Classes/Exception.php
./Neos.Neos/Classes/Controller/Exception.php
./Neos.Neos/Classes/Routing/Exception.php
./Neos.Neos/Classes/Exception.php
./Neos.Neos/Classes/Domain/Exception.php
./Neos.Fusion/Classes/Exception.php
mhsdesign commented 9 months ago

We discussed here https://neos-project.slack.com/archives/C3MCBK6S2/p1694081919618779 that we want to rename the Neos WorkspaceName:

I'd favor renaming it to NeosWorkspaceName as it contains valid domain logic for Neos

mhsdesign commented 9 months ago

I would like to deprecate the User utility instead of renaming it to UserUtility. The helper contained minimal logic, which is now mostly moved to NeosWorkspaceName.

https://github.com/neos/neos-development-collection/blob/8336726355e6f70e5a71b8df5df8cf7396c03c55/Neos.Neos/Classes/Utility/User.php

Removing or renaming the UserUtility might be a little unwise, as its also used in the Neos.Demo!

See pr https://github.com/neos/neos-development-collection/pull/4530

bwaidelich commented 2 months ago

I took the freedom to make this an issue of the DX epic (#4777)