!!! TASK: Introduce first level content repository projection #5272

mhsdesign · 2024-10-02T14:41:09Z

Replaces the quirky ContentRepositoryReadModelAdapterInterface with a new ContentGraphReadModelInterface that is a 1st level dependency to the ContentRepository.
This simplifies code flow and prevents CR instances without its main projection to be created.

Note

This is a breaking change because it changes the configuration format for the ContentRepositoryRegistry package from

Neos:
  ContentRepositoryRegistry:
    presets:
      'default':
        # ...
        projections:
          'Neos.ContentRepository:ContentGraph':
            catchUpHooks: # ...
          'Neos.Neos:DocumentUriPathProjection': #...

to

Neos:
  ContentRepositoryRegistry:
    presets:
      'default':
        # ...
        contentGraphProjection:
          catchUpHooks: #...
        projections:
          'Neos.Neos:DocumentUriPathProjection': #...

Neos.ContentRepository.Core/Classes/Projection/ProjectionsAndCatchUpHooks.php

bwaidelich

Nice one and it's a good idea to move the countNodes() hack to the higher level interface.
I already added a couple of comments even though this is just a draft yet

Neos.ContentRepository.Core/Classes/ContentRepositoryReadModel.php

Neos.ContentRepository.Core/Classes/ContentRepository.php

Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/CRTestSuiteTrait.php

A ContentRepository cannot exist without this projection

Previously if called with `ProjectionStateInterface::class` it would return the first state which will then not be callable again by its fqn. Now only fqn are allowed. Initially the interface logic was needed for interchangeable cr readmodel projection states

…ound to a workspace but cross content stream Also since its internal and only used for the tests it a good to be hidden in the low level `ContentRepositoryReadModel` next to other low level methods

Neos.ContentRepository.Core/Classes/Projection/ContentRepositoryReadModelProjection.php

Neos.ContentRepository.Core/Classes/ContentRepositoryReadModel.php

bwaidelich

@mhsdesign This is a great start already, but I'm not yet super happy with the split in the CR core.
I have some rough ideas I would like to discuss with you at some point

Neos.ContentRepository.Core/Classes/CommandHandlingDependencies.php

Neos.ContentRepository.Core/Classes/ContentRepository.php

Neos.ContentRepository.Core/Classes/Projection/ProjectionsAndCatchUpHooks.php

…t-level

The name `ContentRepositoryProjectionInterface` doesnt specify what makes it different to `ProjectionInterface` and indeed its a graph that has to be implemented by the adapter

…Graph`

Neos.ContentRepositoryRegistry/Classes/ContentRepositoryRegistry.php

Neos.ContentRepository.Core/Classes/Factory/ProjectionsAndCatchUpHooksFactory.php

bwaidelich

First set of comments, looking great already.
What puzzles me is the distinction between contentRepositoryProjection (in configuration etc) but contentGraphReadModel

Neos.ContentGraph.DoctrineDbalAdapter/src/ContentGraphReadModelAdapter.php

Neos.ContentRepository.Core/Classes/CommandHandlingDependencies.php

bwaidelich · 2024-10-22T07:38:39Z

Neos.ContentRepository.Core/Classes/ContentRepository.php

+        if (in_array(ContentGraphReadModelInterface::class, class_implements($projectionStateClassName), true)) {
+            throw new \InvalidArgumentException(sprintf('Accessing the internal content repository projection state via %s(%s) is not allowed. Please use the API on the content repository instead.', __FUNCTION__, $projectionStateClassName), 1729338679);


nitpick: why not move this check to the top and keep the rest like before?

then it would be more expensive imo as we call class_implements always and here its just to improve the error message ;)

as said above I would just allow it and thrash the check. it's a projectionstate after all, just one with better API to reach it.

We will have to disallow this with the CR Privileges because they enforce the constraints on the CR instance

right, ok fair enough. Then this place is fine for me.

bwaidelich · 2024-10-22T07:39:17Z

Neos.ContentRepository.Core/Classes/ContentRepository.php

+        if (!isset($this->projectionStateCache)) {
+            foreach ($this->projectionsAndCatchUpHooks->projections as $projection) {
+                if ($projection instanceof ContentGraphProjectionInterface) {
+                    continue;
+                }
+                $projectionState = $projection->getState();
+                $this->projectionStateCache[$projectionState::class] = $projectionState;
+            }
+        }


I don't completely get the implications of this, but why do we build up the runtime cache for all projections once any state is retrieved?

because we did so previously almost as well just that we break on the first match, by building it once its more easy to grasp

Neos.ContentRepository.Core/Classes/Projection/ProjectionStatuses.php

kitsunet · 2024-10-22T08:01:14Z

Neos.ContentRepository.Core/Classes/ContentRepository.php

@@ -141,18 +143,24 @@ public function handle(CommandInterface $command): void
     */
    public function projectionState(string $projectionStateClassName): ProjectionStateInterface
    {
+        if (!isset($this->projectionStateCache)) {
+            foreach ($this->projectionsAndCatchUpHooks->projections as $projection) {
+                if ($projection instanceof ContentGraphProjectionInterface) {


So but that means you cannot get the CG projection here anymore? not sure that is good?!

I think that's a good thing (see comment above re PR privileges)

Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/CRTestSuiteTrait.php

Neos.ContentRepositoryRegistry/Classes/ContentRepositoryRegistry.php

…t-level

mhsdesign marked this pull request as draft October 2, 2024 14:41

github-actions bot added the Task label Oct 2, 2024

mhsdesign mentioned this pull request Oct 15, 2024

!!! TASK: Add workspace content stream mapping to content graph projection #5096

Merged

12 tasks

mhsdesign commented Oct 15, 2024

View reviewed changes

Neos.ContentRepository.Core/Classes/Projection/ProjectionsAndCatchUpHooks.php Outdated Show resolved Hide resolved

bwaidelich mentioned this pull request Oct 15, 2024

Workspace aware ContentGraph / Node followup #5038

Closed

15 tasks

bwaidelich reviewed Oct 15, 2024

View reviewed changes

Base automatically changed from task/add-workspace-contentstream-mapping-to-contentgraph to 9.0 October 17, 2024 11:11

github-actions bot added the 9.0 label Oct 17, 2024

mhsdesign added 4 commits October 17, 2024 14:12

FEATURE: Refactor cr readmodel projection to be first level

76f551b

A ContentRepository cannot exist without this projection

TASK: Remove ContentGraphInterface::countNodes as the method is not b…

d20bb65

…ound to a workspace but cross content stream Also since its internal and only used for the tests it a good to be hidden in the low level `ContentRepositoryReadModel` next to other low level methods

TASK: Ignore phpstan error

fb56a07

mhsdesign force-pushed the task/cr-readmodel-first-level branch from f11e713 to fb56a07 Compare October 17, 2024 12:13

mhsdesign marked this pull request as ready for review October 17, 2024 12:14

mhsdesign changed the title ~~TASK: CR readmodel first level~~ TASK: Introduce first level content repository projection Oct 18, 2024

mhsdesign requested review from kitsunet and bwaidelich October 18, 2024 06:57

mhsdesign commented Oct 18, 2024

View reviewed changes

Neos.ContentRepository.Core/Classes/Projection/ContentRepositoryReadModelProjection.php Outdated Show resolved Hide resolved

mhsdesign commented Oct 18, 2024

View reviewed changes

Neos.ContentRepository.Core/Classes/ContentRepositoryReadModel.php Outdated Show resolved Hide resolved

bwaidelich reviewed Oct 18, 2024

View reviewed changes

mhsdesign added 7 commits October 19, 2024 10:57

TASK: Adjust namings

cd4de71

TASK: Introduce dedicated contentRepositoryProjection and factory

c1756bc

TASK: Simplify ProjectionsAndCatchUpHooks

41e3550

Merge remote-tracking branch 'origin/9.0' into task/cr-readmodel-firs…

6df6ba4

…t-level

TASK: Adjust namings (again xD)

b35a28d

The name `ContentRepositoryProjectionInterface` doesnt specify what makes it different to `ProjectionInterface` and indeed its a graph that has to be implemented by the adapter

TASK: Adjust also naming of ContentGraphReadModel

25a6d46

TASK: Move content graph projection interfaces to `Projection\Content…

3dce85d

…Graph`

mhsdesign requested review from bwaidelich, skurfuerst and nezaniel October 21, 2024 20:36

mhsdesign commented Oct 21, 2024

View reviewed changes

Neos.ContentRepositoryRegistry/Classes/ContentRepositoryRegistry.php Outdated Show resolved Hide resolved

mhsdesign commented Oct 21, 2024

View reviewed changes

Neos.ContentRepository.Core/Classes/Factory/ProjectionsAndCatchUpHooksFactory.php Outdated Show resolved Hide resolved

bwaidelich reviewed Oct 22, 2024

View reviewed changes

kitsunet reviewed Oct 22, 2024

View reviewed changes

Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/CRTestSuiteTrait.php Show resolved Hide resolved

kitsunet reviewed Oct 22, 2024

View reviewed changes

Neos.ContentRepositoryRegistry/Classes/ContentRepositoryRegistry.php Outdated Show resolved Hide resolved

kitsunet reviewed Oct 22, 2024

View reviewed changes

Neos.ContentRepositoryRegistry/Classes/ContentRepositoryRegistry.php Outdated Show resolved Hide resolved

mhsdesign and others added 4 commits October 22, 2024 11:31

TASK: Rename contentRepositoryProjection to contentGraphProjection

b2b52cf

TASK: Simplify cr registry projection building and rename factory

17af312

Merge remote-tracking branch 'origin/9.0' into task/cr-readmodel-firs…

e3a2da5

…t-level

Merge branch '9.0' into task/cr-readmodel-first-level

4a4873f

mhsdesign requested review from bwaidelich and kitsunet October 22, 2024 15:25

bwaidelich changed the title ~~TASK: Introduce first level content repository projection~~ !!! TASK: Introduce first level content repository projection Oct 22, 2024

bwaidelich approved these changes Oct 22, 2024

View reviewed changes

Merge branch '9.0' into task/cr-readmodel-first-level

d135476

bwaidelich approved these changes Oct 22, 2024

View reviewed changes

bwaidelich merged commit 7c5ad8c into 9.0 Oct 22, 2024
10 checks passed

bwaidelich deleted the task/cr-readmodel-first-level branch October 22, 2024 17:01

This was referenced Oct 27, 2024

!!! FEATURE: Publishing Version 3 #5301

Merged

!!! FEATURE: Subscription Engine #5321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

!!! TASK: Introduce first level content repository projection #5272

!!! TASK: Introduce first level content repository projection #5272

mhsdesign commented Oct 2, 2024 •

edited by bwaidelich

Loading

bwaidelich left a comment

bwaidelich left a comment

bwaidelich left a comment

bwaidelich Oct 22, 2024

mhsdesign Oct 22, 2024

kitsunet Oct 22, 2024

bwaidelich Oct 22, 2024

kitsunet Oct 22, 2024

bwaidelich Oct 22, 2024

mhsdesign Oct 22, 2024

kitsunet Oct 22, 2024

bwaidelich Oct 22, 2024

		if (in_array(ContentGraphReadModelInterface::class, class_implements($projectionStateClassName), true)) {
		throw new \InvalidArgumentException(sprintf('Accessing the internal content repository projection state via %s(%s) is not allowed. Please use the API on the content repository instead.', __FUNCTION__, $projectionStateClassName), 1729338679);

!!! TASK: Introduce first level content repository projection #5272

!!! TASK: Introduce first level content repository projection #5272

Conversation

mhsdesign commented Oct 2, 2024 • edited by bwaidelich Loading

Note

bwaidelich left a comment

Choose a reason for hiding this comment

bwaidelich left a comment

Choose a reason for hiding this comment

bwaidelich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhsdesign commented Oct 2, 2024 •

edited by bwaidelich

Loading