di

di copied to clipboard

This PR adds possibility to invalidate the DI container using given callback.

• feature
• not aware of any BC break (but causes an error with cached container on package update)

Currently, you can make DependencyChecker track specific files (and classes or methods). Using callback, we could invalidate the container, for instance:

• when there is a new file (like a presenter with routing annotation - this is actually real use case),
• when something of environment changes.

I'm not sure about proper implementation - open to discussion. What concerns me is isExpired() method; it used to call calculateHash() only when a file was changed, but now it gets called always when there is a callback as well (which causes tracked files to be loaded and inspected unnecessarily). An optimization would require a separate hash for callbacks.

EDIT: This actually probably breaks when nette/di updates and there's a cached container regardless of the VERSION advance. Could be fixed using optional parameters in isExpired() signature, if that's an issue.

Usage

Dependency is supposed to be defined like this:

Array(string & callable $callback, mixed ...$arg)

First argument must be a callable and string, which limits the callback to a function or a static method. Remaining arguments are stored within the callback and are always passed to it (I can explain what's that good for if it's not obvious).

$compiler->addDependencies([

    // track for PHP version change
    ['phpversion'],

    // track for an extension
    ['extension_loaded', 'sockets'],

    // stupid simple file watcher
    ['glob', 'src/*/*Presenter.php'],

    // custom callback
    [[MyClass:class, 'getInstallationHash'], 123],

]);

I would consider sytanx for static method call as [MyClass::class, 'method']. It is easier to maintain in IDE. Sure, uglier version [MyClass::class . '::method'] would work ;)

Yup, here's what I have been thinking:

• the array syntax is mostly used with an object as a first argument - we'd have to check and reject such callback,
• if we use the array syntax, how arguments will be passed (can't think of an intuitive way)?

I don't really mind anyway, if you want, I'll edit the PR.

Probably [[MyClass::class, 'method'], 'arg', 'arg'] but save your time and don't edit it now. It's just note.

(Also note that support for MyClass::class . '::method' MAY be removed in PHP 8)

I've allowed the callback to be specified using array syntax.

Something about performace - would it be OK to use part of hash for files and other part for callbacks? I think there's no need for entirely separated hash.

I think Callback::isStatic() may help https://api.nette.org/2.4/Nette.Utils.Callback.html

Something about performace - would it be OK to use part of hash for files and other part for callbacks? I think there's no need for entirely separated hash.

It must be separated. Hashes for callbacks will be generated for each request and it should not influence generating hashes for files.

Updates:

• files and callbacks are checked individually using single concatenated hash,
• no failure on package update with cached container (but I don't like the solution and considering 3.0 is not out yet, we could just leave it crash?),
• using Nette\Utils\Callback::isStatic().

Now I've been thinking about this - in practice, it'd be useful to be able alter cached data without rebuilding the container. For example, if a class file changes, but there's no interesting annotation in it, I don't want to discard the container, but next time, I would have to load and inspect the file again.

Not sure if done properly, but here's a patch which excludes parameters from hashing and when they change (using reference), they'll get stored without rebuilding, much like class files do.

That makes sense, but it must be done a little differently. I'll try write more later.