Problem/Motivation
The function signature for clearstatcache is clearstatcache(bool $clear_realpath_cache = false, string $filename = ''): void but in FileFieldRevisionTest::testRevisions it is called 4 times with only a string file URI.
Since these filename strings would be truthy that would be equivalent of passing TRUE for the $clear_realpath_cache argument, meaning we are clearing the entire stat cache and realpath cache 4 times!
Moreover, there is a todo comment above these calls that suggests the calls should not be necessary:
// TODO: This seems like a bug in File API. Clearing the stat cache should
// not be necessary here. The file really is deleted, but stream wrappers
// doesn't seem to think so unless we clear the PHP file stat() cache.
The test still passes after removing these calls, so that confirms they are not necessary.
Steps to reproduce
Proposed resolution
Remove calls to clearstatcache
Merge request link
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3417935
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mstrelan commentedComment #4
acbramley commentedPipeline is green, test still makes sense without these lines (e.g the comment above the user deletion talks about checking the file, which is done below these removed lines). Don't see a reason to hold this up.
Comment #5
mstrelan commentedFWIW the TODO comment and clearstatcache call was first added in the file contrib module 14 years ago and moved to core in #391330: File Field for Core.
It was then updated to include the file URI in #1748880: Only clear the stat cache for the files we care about.
Comment #6
mstrelan commentedComment #9
catchCommitted/pushed to 11.x and cherry-picked to 10.2.x, thanks! Amazing what you find sometimes.