WIP: Replace CatchUpHooks with ContentRepositoryHooks
#4995
Conversation
Resolves: #4992
bwaidelich
changed the title
CatchUpHooks with ContentRepositoryHooks
…2-rework-catch-up-hooks
…2-rework-catch-up-hooks
| $lock->release(); | ||
| } | ||
|
|
||
| /** | ||
| * NOTE: This will NOT trigger hooks! | ||
| * |
There was a problem hiding this comment.
Move to CatchUpService?
Neos.ContentRepository.Core/Classes/EventStore/EventPersister.php
Outdated
Show resolved
Hide resolved
| foreach ($this->projections as $projection) { | ||
| if (!$projection->getCheckpoint()->equals($expectedCheckpoint)) { | ||
| //throw new \RuntimeException(sprintf('Projection %s is at checkpoint %d, but was expected to be at %d', $projection::class, $projection->getCheckpoint()->value, $expectedCheckpoint->value), 1714062281); |
There was a problem hiding this comment.
As discussed with @skurfuerst :
No reason to "stop the world" here, but we should add some result object that contains the projections that were updated/skipped such that we can inform the user immediately
| $event = $this->eventNormalizer->denormalize($eventEnvelope->event); | ||
| $hooks->dispatchBeforeEvent($event, $eventEnvelope); | ||
| foreach ($projectionsToUpdate as $projection) { | ||
| $projection->apply($event, $eventEnvelope); |
There was a problem hiding this comment.
TODO: try/catch -> collect exceptions and throw one combined exception after releasing the lock in line 85
…2-rework-catch-up-hooks
…2-rework-catch-up-hooks
…om/neos/neos-development-collection into feature/4992-rework-catch-up-hooks
| $contentRepository->catchUpProjections(); | ||
|
|
||
| $hooks->dispatchBeforeCatchUp(); | ||
| foreach ($this->eventStore->load($eventsToPublish->streamName)->withMinimumSequenceNumber($expectedCheckpoint->next()) as $eventEnvelope) { |
There was a problem hiding this comment.
This might read more events than were published..
I think we also need to specify the max sequence number!?
There was a problem hiding this comment.
No, I think it's correct this way. But it means that a newly published event might haven been applied in a separate process already
…2-rework-catch-up-hooks
| $hooks->dispatchAfterCatchup(); | ||
| $lock->release(); |
There was a problem hiding this comment.
Maybe those lines should be switched to prevent deadlocks (When the hook produces new events)
| $eventsToPublish->streamName, | ||
| $normalizedEvents, | ||
| $eventsToPublish->expectedVersion | ||
| ); | ||
| $hooks = $this->hooksFactory->build($contentRepository); | ||
| $lock = $this->lockFactory->createLock($contentRepository->id->value . '_catchup'); | ||
| $lock->acquire(true); |
There was a problem hiding this comment.
detect if the lock was required from the same process (i.e. process is owner of the lock) and fail immediately if that's the case à la:
You must not produce events (or send commands) in catch up hooks (except in the afterCatchUp implementation)
See line 80/81
|
This is no longer valid. With the updated #4746 the plan is to basically keep |
Resolves: #4992
WIP because