LocalFile::maybeUpgradeRow() decides whether the file metadata in the database for a particular file requires updating and re-deriving by testing whether $this->media_type === null; it appears from what I can see that this branch of the if statement is meant to be ran against a row loaded from the database where null would only mean that the value in the database is null.

https://github.com/wikimedia/mediawiki/commit/b814245d9f3bb8e2406fea880f9aedd514454c6c appears to have altered the order of operations (to fix T179760) and the row is no longer loaded from the database before maybeUpgradeRow() is called.

maybeUpgradeRow() is now called from purgeCache(), however it does not appear that anything prior to this call actually loads the object from the database. Given that there is no default for $media_type, we always land on the first branch of the if statement and the row is always upgraded, even when it does not require upgrading. MediaHandler::isFileMetadataValid() does not appear to be consulted at all during the process.

It appears that from the above commit that $this->maybeUpgradeRow() was previously called as the last statement of loadFromRow() which mean that it was previously impossible to reach maybeUpgradeRow() on an unloaded object, however after that change, a file is now judged on whether it needs to be upgraded without actually loading the file at all.

I stumbled on this issue primarily after digging into file moving. LocalFile::move() is the clearest example of this and it appears that every call of this will try maybeUpgradeRow() and will always upgrade the row because it never loaded the data from the DB.

Problems

The first issue with the above is it appears to create unnecessary work. upgradeRow() eventually calls loadFromFile(), fetches the file and then runs all of the work afterwards to invariably write the exact same data back to the DB.

Reading the row at the start of the process and then deciding whether the row needs to be updated based on the result avoids having to fetch the file from the backend etc.

The other issue is that isFileMetadataValid() is skipped during the above process. The null branch returns before $this->getHandler() is called so MediaHandler::isFileMetadataValid() is never called. A custom handler which returns METADATA_GOOD with the intention of not having the row upgraded on purpose is ignored and loadFromFile() upgrades its row anyway.

For a handler whose row is not derivable from the file stored on disk has the metadata destroyed with no way tor recover it.

This doesn't appear to be an issue with things like WikiFilePage::doPurge() because they appear to hold a loaded file.