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

Cleanup duplicate php class names across project #4341

Open
mhsdesign opened this issue Jun 18, 2023 · 3 comments
Open

Cleanup duplicate php class names across project #4341

mhsdesign opened this issue Jun 18, 2023 · 3 comments
Labels
9.0 Backlog A category for things that are okay to be rather 'hidden' I: Discussion‌‌

Comments

@mhsdesign
Copy link
Member

mhsdesign commented Jun 18, 2023

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 -> #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

-> #4528

NodeAddress ✅

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

-> #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

-> #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

-> #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 mhsdesign changed the title Cleanup duplicate classNames across project Cleanup duplicate php class names across project Sep 7, 2023
@mhsdesign
Copy link
Member Author

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 added a commit that referenced this issue Sep 18, 2023
This will make it easier to import the correct Neos CR Node - which you will want to use 100% of the time instead.

Resolves partially #4341
mhsdesign added a commit that referenced this issue Sep 18, 2023
This will make it easier to import the correct Neos CR WorkspaceName - which one will want to use more likely.

Resolves partially #4341
@mhsdesign
Copy link
Member Author

mhsdesign commented Sep 18, 2023

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 #4530

mhsdesign added a commit that referenced this issue Sep 18, 2023
@mhsdesign mhsdesign moved this from Todo to Under Review 👀 in Neos 9.0 Release Board Sep 19, 2023
@mhsdesign mhsdesign added the EPIC label Sep 25, 2023
neos-bot pushed a commit to neos/neos that referenced this issue Oct 26, 2023
@mhsdesign mhsdesign moved this from Under Review 👀 to Low Priority ↘ in Neos 9.0 Release Board Feb 8, 2024
@bwaidelich bwaidelich removed the EPIC label Apr 28, 2024
@bwaidelich
Copy link
Member

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

@mhsdesign mhsdesign added Backlog A category for things that are okay to be rather 'hidden' and removed Task labels Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9.0 Backlog A category for things that are okay to be rather 'hidden' I: Discussion‌‌
Projects
None yet
Development

No branches or pull requests

2 participants