Cleanup on Uninstall

Patch for removing field definitions

@saltwaterskin I was under the impression that these get cleaned up on the next cron run after uninstalling. Is that not happening?

@mfb Running cron does not appear to clean these up for me no. I neglected to add the use statement to the patch I uploaded.

Well that's no good! We should add test coverage

@saltwaterskin I tried to reproduce this issue but was unable to.

\Drupal::entityDefinitionUpdateManager()->getFieldStorageDefinition('sha512', 'file'); returns NULL after uninstall.

Are you seeing this issue on a bare bones install of Drupal core, or only with some other contrib modules installed?

@mfb No, I have not tested this on a barebones install of drupal core. And my use case may not match a typical use of the module. Here is the scenario I am using that causes the issue.

Module installed, enabled and configured on the development, staging, and production environments. During local development we do not want the module installed to prevent errors around missing files. So we use config split so that the module is disabled locally after syncing configuration. So it may be a combination of config-split and disabling the module through configuration import. Or it could be any number of other contributed or custom modules that are causing the issues for me.

If you are not able to reproduce the issues that I am facing it is likely that the differences between my setup and a barebones Drupal install are causing the issues and that this is not a problem that needs to be resolved by the module.

I see. Well as long as we can write a test that reproduces the issue (which could involve contrib modules, custom test modules, etc.), then we could try to resolve it.

Likewise we could address using the module on local dev environments. I'm not clear what the issue is there - you should only get a warning if the file is missing and hash is also missing from the database.

Postponing until we have more info on what triggers this issue

Closing for now, but feel free to re-open if there's a feature request here. For example, see https://git.drupalcode.org/project/filehash/-/blob/2.x/src/FileHash.php#... - adding a setting to toggle the auto-hash behavior might help with your issue re: local dev instances?

Patch is still needed. I'm testing Drupal 11.1.1 wanting to isolate a problem so I wanted to uninstall filehash and needed this patch.

It needs a reroll for 3.x

@joseph.olstad I wasn't able to reproduce this issue; please write a failing test or update the steps to reproduce

@mfb

Yes actually I think this may have been due to our custom module that relies on filehash, I forgot about it and should put filehash as a dependency.

So ya, might not need this patch at all.

Then sounds like this can be closed? Feel free to re-open if you have more info