TASK: Cleanup projection catch-up trigger extensibility

[mhsdesign]

Extracted from #4988

Followup for #5049

We decided to run the content repository in sync mode. This was done first via a little hack. This pr now removes the left-over interfaces of the catchup triggering and hardcodes the catchup to be fully synchronous.

Upgrade instructions

Review instructions

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[mhsdesign]

having to pass the full cr along here seems wrong but is the only way if the EventPersister wants to be a separate class and not inlined in the ContentRepository

Previously the dependency problem was not really visible as we used the contentRepositoryRegistry to get the content repository in a ProjectionCatchUpTriggerInterface but there was no dependency flow set up then as well.

[bwaidelich]

+1 by reading, thanks!

[kitsunet]

This is the weirdest API, why would we have this here? CommandHandlingDependencies::publishEvents makes no sense to me.

[mhsdesign]

the problem is that for publishing events we need a catchup afterwards and the catchup hook extensibility require the content repository to be build.

The CommandHandlingDependencies are a purely internal thing and the handle() method on it already is a code smell due to the workspace command handler "doing to much" besides just publishing events for one command as we issue sub commands in the middle of handling another command.

For publishing an event stream, we need low level access to publish all events which was previously done via $this->eventPersister->publishEvents($events).
This leveraged the odd dependency flow of refetching the content repository from the Neos registry (see comment above) but now we need to pass the content repository for the said reasons explicitly.

Instead of exposing the content repository from $commandHandlingDependencies and using it like $this->eventPersister->publishEvents($events, $commandHandlingDependencies->contentRepository) i propose to keep the contentRepository instance private and introduce the new method: $commandHandlingDependencies->publishEvents($events)

[kitsunet]

I'd rather expose the CR here instead of adding this method, but maybe an abstraction around projection catchup is not bad, I do like the current api better than what happens in this change.

[mhsdesign]

we discussed having a factory like so, but the catchup logic is still placed ON the content repository and we would have to move the codeblock to the event persister ...

final readonly class CatchUpHookFactoryBuilder
{
    public function __construct(
        private ContentRepository $contentRepository
    ) {
    }

    public function build(CatchUpHookFactoryInterface $catchUpHookFactory): CatchUpHookInterface
    {
        return $catchUpHookFactory->build($this->contentRepository);
    }
}

Also introducing a read site content repository or just passing the content repository read model to the catchup hooks would certainly be promising as it would prevent recursive handling via handle but nothing for now.

[mhsdesign]

haha @kitsunet with #5301 on the rise i guess i can revert d3119b8 as well have so much logic on the poor object that an additional method persistEvents wouldn't hurt :D

I just experimented with returning the events instead of handling them in between, which is much cleaner imo and would allow us to actually get rid of that thing :D https://neos-project.slack.com/archives/C0L41E268/p1729546247272549

[kitsunet]

I still like it better this way 😆

[kitsunet]

so this seems to be the single use of this weird API, why don't we keep the EventPersister here?

[kitsunet]

I don't like the persistEvents api addition, that shouldn't be there IMHO.

[kitsunet]

I like it like this