[Clock] Access global system clock

[ro0NL]

Description

In our entities we create a lot of datetime values, typically:

public function __construct() {
    $this->createdAt = new \DateTime();
}

Upgrading this to using a clock requires currently a refactoring of the domain layer, either using an entity listener to inject datetime values during flush, or pass it from the outside (in this case a new constructor argument).

Using an entity listener might break all kinds of domain logic that expect the datetime value to be set.

Passing it from the outside is plain boring for what is initializing a typed value (datetime datatype), and also impacts the domain layer signature wise.

To overcome i propose a static accesible system clock, which should make upgrading more easy.

static Clock::setSystemClock(self $clock): void
static Clock::getSysyemClock(): self

function sf/now(): \DateTimeImmutable;

$this->createtAt = sf/now();

This might be frowned upon, but it's really what happens today already. Constructing a datatype should not require a service layer on high level IMHO.

This is comparable to string datatype u('...')->toString().

Example

No response

[denis-korchagin95]

The another way it's provide the createdAt date to the entity.

Just like that:

public __construct(DateTimeImmutable $createdAt) {
     $this->createdAt = $createdAt;
}

In this variant, your entity not depends on any kind of global function and also they are more testable now.

[ro0NL]

Im not fond of it of, as it changes the semantic of what is currently a static concern.

But if our ecosystem decides this is the way, then OK :)

I'll keep my new \DateTime() until proven otherwise :)

[nicolas-grekas]

Doing time injection via the global scope (ie a function or a static method) is already doable with Chronos and the likes. The only difference if we provide something similar would be the vendor you're coupling your code to.

I'm not sure coupling to the Symfony vendor is something that fits our philosophy.

Instead, I can foresee different ways:

• using an entity factory - maybe the corresponding repository?
• passing either the DateTimeImmutable or a ClockInterface to the constructor.
• or considering an entity listener, aka let the ORM set the time pre-persist (and add a dedicated attribute to make this easy?)

Would anyone have another idea that doesn't involve the global state?

[fmata]

Entity listener is convenient (we use a trait and an interface to flag our entities) with one downside: the data added is accessible only after persisting or flushing, so getter on entities must return null to be consistent.

Passing the time to the constructor seems to be the right way but we don't do that because most of the time there is already many parameters and time is almost mandatory for all entities.

The global state with a static service/function is the least worst idea, but if we could find another one I will be very happy too :)

[yivi]

Time and date feel naturally like global state concerns.

In my case I just use a domain wrappers for this kind of thing, which in turn can use anything (Symfony Clock, Chronos, Brick/Date-Time, native clock access) to provide the date / time objects as needed. My domain wrapper can be statically manipulated, which allows for happier testing.

Letting the class consumer choose the datetime (passing it via constructor) feels dirty, would require additional checks and validations in many contexts.

Using an entity listener would allow the entity to exist in an invalid state (no createdAt value), and would further couple the domain to a vender, and a "heavier" vendor at that. A vendor that just encapsulates primitive types is an easier sale than coupling to the persistance layer, IMO.

Using an entity factory and only allowing the entities be created that way would likely be neater way... but would be a bit more work.

Just using my $this->createdAt = DomainClock::now() seems to be the simpler solution right now, IMO.

[nicolas-grekas]

Using an entity listener would allow the entity to exist in an invalid state (no createdAt value)

It doesn't have to be an invalid state: the domain could very well capture the "not persisted yet" state this way.

DomainClock::now() seems to be the simpler solution right now

LGTM also.

This resolves to: should we add Clock::now() to the component? I've no strong opinion on this personally.

[nicolas-grekas]

Let's conclude on code. See #48642

[yivi]

This resolves to: should we add Clock::now() to the component? I've no strong opinion on this personally.

Personally, I don't feel it's necessary, since it can easily resolved at the app/domain level for this kind of use case.

I'm lucky that I've been wrapping new DateTimeImmutable in DomainClock::now() calls for a while, so this finds me better prepared.

But the createdAt = new DTI is a fairly common idiom. Also understanding that to date/time feels as "global" as they can be, I can understand how many users will come up looking for a ready-made solution. Maybe discussing it will make better patterns become apparent.