Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BUG: Neos_Fusion_ParsePartials cache not flushed for moved files because realpath fails #4415

Open
mhsdesign opened this issue Jul 26, 2023 · 2 comments
Open

BUG: Neos_Fusion_ParsePartials cache not flushed for moved files because realpath fails #4415

mhsdesign opened this issue Jul 26, 2023 · 2 comments
Labels
Bug

Comments

@mhsdesign
Copy link
Member

mhsdesign commented Jul 26, 2023 •
edited

From Neos 8.3.0 .. 8.3.9 we didnt handle this case by just throwing an error.

Exception in line 53 of /Users/marchenryschultz/Code/core/Neos.NeosIo/Packages/Application/Neos.Fusion/Classes/Core/Cache/ParserCacheFlusher.php: Couldn't resolve realpath for: '/Users/marchenryschultz/Code/core/Neos.NeosIo/Packages/Sites/Neos.NeosIo/Resources/Private/Fusion/Content/BrandLogo/BrandLogo.fusion'

21 Neos\Fusion\Core\Cache\ParserCacheFlusher::getCacheIdentifierForFile("/Users/marchenryschultz/Code/core/Neos.NeosIo/Pack…/Private/Fusion/Content/BrandLogo/BrandLogo.fusion")
20 Neos\Fusion\Core\Cache\ParserCacheFlusher::flushPartialCacheOnFileChanges("Fusion_Files", array|107|, "Neos\Flow\Monitor\FileMonitor::filesHaveChanged")
19 call_user_func_array(array|2|, array|3|)
18 Neos\Flow\SignalSlot\Dispatcher::dispatch("Neos\Flow\Monitor\FileMonitor", "filesHaveChanged", array|2|)
17 Neos\Flow\Monitor\FileMonitor_Original::emitFilesHaveChanged("Fusion_Files", array|107|)
16 Neos\Flow\Monitor\FileMonitor_Original::detectChanges()
15 Neos\Fusion\Package::Neos\Fusion\{closure}(Neos\Flow\Core\Booting\Step, "runtime", "Neos\Flow\Core\Booting\Sequence::afterInvokeStep")
14 Closure::__invoke(Neos\Flow\Core\Booting\Step, "runtime", "Neos\Flow\Core\Booting\Sequence::afterInvokeStep")
13 call_user_func_array(array|2|, array|3|)
12 Neos\Flow\SignalSlot\Dispatcher::dispatch("Neos\Flow\Core\Booting\Sequence", "afterInvokeStep", array|2|)
11 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
10 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
9 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
8 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
7 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
6 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
5 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
4 Neos\Flow\Core\Booting\Sequence::invoke(Neos\Flow\Core\Bootstrap)
3 Neos\Flow\Http\RequestHandler::boot()
2 Neos\Flow\Http\RequestHandler::handleRequest()
1 Neos\Flow\Core\Bootstrap::run()

With Neos 8.3.10 #4838 tried to fix this, but accidentally completely broke caching.

Instead the current hotfix is to silently ignore removed fusion files and not trigger cache invalidator for them.
That would lead to a growing cache and maybe odd behaviour in some cases, but that seems a better compromise.
Related issue of flows php caching not being flushed for delted classes: neos/flow-development-collection#3303

for some reason it also happened for js files???

yes because we listen to everything: #4606 (comment)

Exception in line 53 of /Users/marchenryschultz/Code/core/Neos.NeosIo/Packages/Application/Neos.Fusion/Classes/Core/Cache/ParserCacheFlusher.php: Couldn't resolve realpath for: '/Users/marchenryschultz/Code/core/Neos.NeosIo/Packages/Sites/Neos.NeosIo/Resources/Private/Fusion/Content/Tabs/Tabs.js'

21 Neos\Fusion\Core\Cache\ParserCacheFlusher::getCacheIdentifierForFile("/Users/marchenryschultz/Code/core/Neos.NeosIo/Pack…osIo/Resources/Private/Fusion/Content/Tabs/Tabs.js")
20 Neos\Fusion\Core\Cache\ParserCacheFlusher::flushPartialCacheOnFileChanges("Fusion_Files", array|2|, "Neos\Flow\Monitor\FileMonitor::filesHaveChanged")
19 call_user_func_array(array|2|, array|3|)
18 Neos\Flow\SignalSlot\Dispatcher::dispatch("Neos\Flow\Monitor\FileMonitor", "filesHaveChanged", array|2|)
17 Neos\Flow\Monitor\FileMonitor_Original::emitFilesHaveChanged("Fusion_Files", array|2|)
16 Neos\Flow\Monitor\FileMonitor_Original::detectChanges()
15 Neos\Fusion\Package::Neos\Fusion\{closure}(Neos\Flow\Core\Booting\Step, "runtime", "Neos\Flow\Core\Booting\Sequence::afterInvokeStep")
14 Closure::__invoke(Neos\Flow\Core\Booting\Step, "runtime", "Neos\Flow\Core\Booting\Sequence::afterInvokeStep")
13 call_user_func_array(array|2|, array|3|)
12 Neos\Flow\SignalSlot\Dispatcher::dispatch("Neos\Flow\Core\Booting\Sequence", "afterInvokeStep", array|2|)
11 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
10 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
9 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
8 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
7 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
6 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
5 Neos\Flow\Core\Booting\Sequence::invokeStep(Neos\Flow\Core\Booting\Step, Neos\Flow\Core\Bootstrap)
4 Neos\Flow\Core\Booting\Sequence::invoke(Neos\Flow\Core\Bootstrap)
3 Neos\Flow\Http\RequestHandler::boot()
2 Neos\Flow\Http\RequestHandler::handleRequest()
1 Neos\Flow\Core\Bootstrap::run()

It seems that when deleting files we dont correctly handle them.

option A: flush all if it couldn't resolve?
option B: assume its already a full path and still calculate the cache identifier.
@mhsdesign mhsdesign added the Bug label Aug 22, 2023
mhsdesign added a commit that referenced this issue Sep 14, 2023
@mhsdesign
BUGFIX: 4415 Fusion error on dev cache invalidation
43b92bf 
Resolves #4415
@mhsdesign mhsdesign mentioned this issue Sep 14, 2023
Closed
6 tasks
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Jan 16, 2024
@mhsdesign
BUGFIX: Fusion avoid error on cache invalidation while developing
c675950 
Replaces neos#4509
Resolves neos#4415

After deleting a fusion file like `BrandLogo.fusion` one will face the error after booting flow and thus triggering the file monitor and its listeners: (even like a simple `flow help`)

```
Couldn't resolve realpath for: '/absolutePath/Code/core/Neos.NeosIo/Packages/Sites/Neos.NeosIo/Resources/Private/Fusion/Content/BrandLogo/BrandLogo.fusion'
```

This is caused as `realpath` returns false if the file was deleted, and we were to eager validating this. But as flows file monitor already returns absolute paths we can skip the realpath calculation here and move it to the `ParserCache::cacheForFusionFile`.
Initially the call to `realpath` was made in a single place to avoid making to many assumptions about the form flow returned file paths.
@mhsdesign mhsdesign mentioned this issue Jan 16, 2024
Merged
6 tasks
mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this issue Jan 16, 2024
@mhsdesign
BUGFIX: Fusion avoid error on cache invalidation while developing
5377b86 
Replaces neos#4509
Resolves neos#4415

After deleting a fusion file like `BrandLogo.fusion` one will face the error after booting flow and thus triggering the file monitor and its listeners: (even like a simple `flow help`)

```
Couldn't resolve realpath for: '/absolutePath/Code/core/Neos.NeosIo/Packages/Sites/Neos.NeosIo/Resources/Private/Fusion/Content/BrandLogo/BrandLogo.fusion'
```

This is caused as `realpath` returns false if the file was deleted, and we were to eager validating this. But as flows file monitor already returns absolute paths we can skip the realpath calculation here and move it to the `ParserCache::cacheForFusionFile`.
Initially the call to `realpath` was made in a single place to avoid making to many assumptions about the form flow returned file paths.
@mhsdesign mhsdesign mentioned this issue Jan 18, 2024
Closed
@mhsdesign mhsdesign mentioned this issue Feb 28, 2024
Closed
@mhsdesign mhsdesign reopened this Feb 28, 2024
@mhsdesign mhsdesign changed the title BUG: ParserCacheIdentifierTrait resolve realpath doesnt work after file move. BUG: Neos_Fusion_ParsePartials cache not flushed for moved files because realpath fails Feb 28, 2024
@mhsdesign mhsdesign mentioned this issue Feb 28, 2024
Merged
6 tasks
@mhsdesign
Copy link
Member Author

The issue was reopened because, by reverting the fix #4838 (because it caused that bug)
we trigger this original problem to resurface (but silently this time, no one will notice the cache flodding :D)

See also reminder in the code:

neos-development-collection/Neos.Fusion/Classes/Core/Cache/ParserCacheFlusher.php

Lines 58 to 62 in c161df2

if ($status === ChangeDetectionStrategyInterface::STATUS_DELETED) {
// Ignoring removed files means we cannot flush removed files, but this is a compromise for now.
// See https://github.com/neos/neos-development-collection/issues/4415 as reminder that flushing is disabled for deleted files
continue;
}

The decision is further documented here

@mhsdesign
Copy link
Member Author

Funnily enough the rather hacky fix in #4509 actually worked (which we decided against because we wanted to do things better)

I have this as patch since September 23 applied in a bigger project without any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@mhsdesign