Cleanup on Uninstall

Created on 15 June 2023, over 1 year ago
Updated 26 July 2023, over 1 year ago

Problem/Motivation

Field storage definitions for enabled algorithms are not removed when the module is uninstalled. This can lead to errors during file uploads when the module is uninstalled.

Steps to reproduce

  1. Enable and configure the filehash module with at least one algorithm
  2. Uninstall the filehash module
  3. Attempt to upload a file to the site

Proposed resolution

uninstall algorithm field definitions when the module is uninstalled

Remaining tasks

User interface changes

API changes

Data model changes

✨ Feature request
Status

Closed: cannot reproduce

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States saltwaterskin

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

  • Issue created by @saltwaterskin
  • πŸ‡ΊπŸ‡ΈUnited States saltwaterskin

    Patch for removing field definitions

  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

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

  • πŸ‡ΊπŸ‡ΈUnited States saltwaterskin

    @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.

  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

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

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.2 & MySQL 8
    last update over 1 year ago
    13 pass
  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    @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?

  • πŸ‡ΊπŸ‡ΈUnited States saltwaterskin

    @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.

  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    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.

  • Status changed to Postponed: needs info over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    Postponing until we have more info on what triggers this issue

  • Status changed to Closed: cannot reproduce over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    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?

Production build 0.71.5 2024