I think from a performance PoV, it would make more sense to pass the files as a parameter to the hook. Otherwise you're potentially (as there's no caching in getThumbnails) doing multiple iterations over the file system, which is unnecessary.

So for your particular issue, I would advise going down that route. I'll stick up a patch

As for making getThumbnails public (again, explicit rather than implicit), I could see a reason to do that (too). It was made protected because there were no other usages than in the File class heirarchy.

I'm open to changing it with a use case where it's needed though

Just to check... is it thumbnails what you actually want? Or are the URLs actually more useful?

Change 645565 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Add list of thumbnails to LocalFilePurgeThumbnails hook

https://gerrit.wikimedia.org/r/645565

For me, the specific use case is the URL.
onUploadComplete and onLocalFilePurgeThumbnails I get the specific URLs for the thumbnail that are now stale, and send them to my CDN (CloudFlare) with a purge_cache command. (wfExpandUrl( $url ))

From purgeThumbnail

		// Always purge all files from CDN regardless of handler filters
		$urls = [];
		foreach ( $files as $file ) {
			$urls[] = $this->getThumbUrl( $file );
		}
		array_shift( $urls ); // don't purge directory

And from purgeOldThumbnails

		// Purge the CDN
		$urls = [];
		foreach ( $files as $file ) {
			$urls[] = $this->getArchiveThumbUrl( $archiveName, $file );
		}

Would those arrays ($urls) be more useful than the literal result from $this->getThumbnails()?

Yes, that's exactly the sections I'm copying from those functions. I regenerate the $urls[] in my function.

Ok, so we should be passing on the $urls then. The question becomes do we pass both $files and $urls to the hook handlers?

While getArchiveThumbUrl() is cheaper than getThumbnails(), no need to do it twice.

Just to confirm my understanding of what these hooks are providing:
From purgeThumbnails the hook would provide the URLs of the different sizes of thumbnails for the currently "active" file version. This would trigger for every new file upload, as well as moves/deletions.
For example, $urls for this file (https://commons.wikimedia.org/wiki/File:JPG_Test.jpg) would give the following:

• https://upload.wikimedia.org/wikipedia/commons/thumb/2/28/JPG_Test.jpg/191px-JPG_Test.jpg
• https://upload.wikimedia.org/wikipedia/commons/thumb/2/28/JPG_Test.jpg/382px-JPG_Test.jpg
• https://upload.wikimedia.org/wikipedia/commons/thumb/2/28/JPG_Test.jpg/477px-JPG_Test.jpg

From purgeOldThumbnails the hook would provide the URLs of the different sizes of thumbnails for a specified prior version. This would trigger for file deletions and moves, once for each old version of the image.
For example, $urls for this file (https://upload.wikimedia.org/wikipedia/commons/archive/2/28/20201117210124%21JPG_Test.jpg) would give the following:

• https://upload.wikimedia.org/wikipedia/commons/thumb/archive/2/28/20201117210124!JPG_Test.jpg/191px-JPG_Test.jpg
• https://upload.wikimedia.org/wikipedia/commons/thumb/archive/2/28/20201117210124!JPG_Test.jpg/382px-JPG_Test.jpg

If this is correct, $urls is good enough for me, though I can't speak for other consumers of this hook.

Yeah, that would look right to me

Once this is merged, will it be backported to REL1_35?

@Reedy Is there anything required from me to get this merged?

Not really. Just needs someone to CR and merge it...

Change 671153 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_35] Add list of thumbnail urls to LocalFilePurgeThumbnails hook

https://gerrit.wikimedia.org/r/671153

Change 645565 merged by jenkins-bot:
[mediawiki/core@master] Add list of thumbnail urls to LocalFilePurgeThumbnails hook

https://gerrit.wikimedia.org/r/645565

Change 671153 merged by jenkins-bot:
[mediawiki/core@REL1_35] Add list of thumbnail urls to LocalFilePurgeThumbnails hook

https://gerrit.wikimedia.org/r/671153