feat(scheduling): Sync Static Global Clock with Clock Service (Quick Solution)#618
Conversation
…clock handling. Add `StaticGlobalClockTest` to ensure correct clock behavior.
| $this->assertInstanceOf(Clock::class, $globalClock); | ||
| $this->assertEquals('2025-08-11 16:00:00', $globalClock->now()->format('Y-m-d H:i:s')); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you provide test case with delay message using new clock strategy?
So similar test to this, but by changing time using clock
There was a problem hiding this comment.
Added this separate test: DelayedMessageAgainstGlobalClockTest
| return self::$globalClock ??= self::defaultClock(); | ||
| } | ||
|
|
||
| public static function resetGlobalClock(): void |
There was a problem hiding this comment.
| public static function resetGlobalClock(): void | |
| public static function resetToNativeClock(): void |
There was a problem hiding this comment.
suggestion applied
- Add `PlaceOrder`, `OrderWasPlaced`, `OrderService`, and `NotificationService` test fixtures. - Improve `Clock` initialization to ensure `globalClock` is only set when unset. - Rename `resetGlobalClock` to `resetToNativeClock` for better clarity. - Add `StaticPsrClock` implementation for static clock testing. - Create `DelayedMessageAgainstGlobalClockTest` to validate delay handling against clock changes. - Update and refactor existing clock-related tests for consistency.
| { | ||
| $ecotoneTestSupport = EcotoneLite::bootstrapFlowTesting( | ||
| [EcotoneClockInterface::class, OrderService::class, NotificationService::class, CustomNotifier::class], | ||
| [$clock = new Clock(new StaticPsrClock('2025-08-11 16:00:00')), new OrderService(), new NotificationService(), $notifier = new CustomNotifier()], |
There was a problem hiding this comment.
@dgafka just out of curiosity, can you explain why instance of EcotoneClockInterface on this test is reinstantiated? It's the reason why I added this if condition in Clock::__constructor();
if (!self::$globalClock) {
self::$globalClock = $this->clock ? $this : self::defaultClock();
}
There was a problem hiding this comment.
It's registered in container as Ecotone\Messaging\Scheduling\Clock instead of PSR\Clock.
This will be replaced by RegisterSingletonMessagingServices, as PSR Clock is actual entrypoint, Ecotone's Clock is internal Service.
I will fix it in follow up
Automatically synchronize
Clock::$globalClockwith the injected clock instance, eliminating the need for manualClock::set()calls in test environments.Why is this change proposed?
Problem
When using a custom clock (e.g., a static clock for testing) with
\Ecotone\Messaging\Scheduling\Clock, the global clock remains unsynchronized. This causes incorrect timestamps in message headers created viaMessageHeaders::createMessageHeadersWith().Currently, the workaround requires calling
Clock::set()explicitly in each test case, which leaks internal implementation details into application tests.Solution
Set
$globalClockautomatically when aClockinstance is constructed:If an internal clock is provided → use it as the global clock
Otherwise → default to NativeClock
This allows applications to define their own
PsrClockInterfaceimplementation without manual global clock management.Cleaner Solution
A cleaner long-term solution would be removing the deprecated
Clock::get()method entirely and replace it withClockinstance. However, this creates a cascade of changes, particularly aroundDelayableQueueChannelwhich callsMessageBuilder::fromMessage($message)without access to aClockinstance.Description of Changes
Ecotone\Messaging\Scheduling\Clock: Auto-set$globalClockon construction; addedresetGlobalClock()for test teardownTest\Ecotone\Messaging\Unit\Scheduling\StaticGlobalClockTest: Verify expected behaviorPull Request Contribution Terms