flow-development-collection icon indicating copy to clipboard operation
flow-development-collection copied to clipboard

BUGFIX: Stabilise file monitor

Open mhsdesign opened this issue 1 year ago • 3 comments

While trying to fix https://github.com/neos/flow-development-collection/issues/3303 i noticed that my file monitor had stale caches.

I deleted a file long time, but it was still present in Cache/Data/Flow_Monitor/Flow_ClassFiles_filesAndModificationTimes. That should not be the case as it should have been flushed. Maybe the file monitor even noticed the change and other parts of flow were aware of it but just the deletion in the file didnt work.

Either way it seems to be flaky and expensive to have two caches with similar data. This change in theorie makes the Flow_ClassFiles_directoriesAndFiles cache obsolete by introducing a new method to that on the ModificationTimeStrategy.

The idea was, if i loop already through all the folders we can just diff it to the previously saved modification stamps and see whats left and check for file existence.

That seems to work well. And in one hickup in flow, that i could not further reproduce, even better than the current strategy which didnt detect a removal.

to test i just dumped a bit around ;D

if ($this->identifier === 'Flow_ClassFiles' && str_ends_with($path, '/Packages/Neos/Neos.Neos/Classes/')) {
    // previously considered files for deletion
    var_dump($this->directoriesAndFiles[$path]);
    // more relivable?
    var_dump($deletedFiles);
}

TODOS;

  • why is the filemonitor proxied???
  • do people implement their own strategy? at least this commit suggests that: https://github.com/kitsunet/flow-development-collection/commit/b54f80393d15ac93086b15b9902b677dcd332b0f

mhsdesign avatar Feb 04 '24 15:02 mhsdesign

I discussed this shortly with @kitsunet this seems a more stable way. Separating the FileMonitor from its ChangeDetectionStrategyInterface is a doomed task and probably doesn't even work currently, that means no one can implement this. So we dont need to make sure that things are backwards compatible.

mhsdesign avatar Feb 06 '24 19:02 mhsdesign

IMHO we should internal or deprecate the interface though, to make this clear?

kitsunet avatar Mar 28 '24 14:03 kitsunet

Yes id say we have ONE internal ChangeDetectionStrategyInterface which will behave like proposed and remove all these funny strategies.

If you still agree - at least thats what we talked about once see https://github.com/neos/flow-development-collection/pull/3304#issuecomment-1930630031 - please let me know then ill refactor this ;)

mhsdesign avatar Mar 28 '24 20:03 mhsdesign