Problem/Motivation

The metadata_sanitizer:clean command fatals when using the --field option in environments where file usage includes non-fieldable entities (for example Paragraphs type config entities). In shouldProcessFile(), usage entities are loaded and assumed to implement field APIs. Some loaded entities are config entities (such as ParagraphsType) and do not implement hasField().

Starting metadata cleanup...
 [error]  Error: Call to undefined method Drupal\paragraphs\Entity\ParagraphsType::hasField() in Drupal\metadata_sanitizer\Commands\MetadataSanitizerCommands->shouldProcessFile() (line 175 of /var/www/html/docroot/modules/contrib/metadata_sanitizer/src/Commands/MetadataSanitizerCommands.php) #0 /var/www/html/docroot/modules/contrib/metadata_sanitizer/src/Commands/MetadataSanitizerCommands.php(108): Drupal\metadata_sanitizer\Commands\MetadataSanitizerCommands->shouldProcessFile()

Steps to reproduce

1. A paragraph with an image field (field_media_image)
2. run drush msc --field=field_media_image --extensions=png,gif,jpg,jpeg

Proposed resolution

Before calling hasField(), ensure the loaded entity is fieldable, for example by checking instanceof FieldableEntityInterface.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork metadata_sanitizer-3593884

Command icon 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:

About issue forks

Comments

nicholass created an issue. See original summary.

nicholass opened merge request !16

nicholass’s picture

nicholass
Primary language English
Location FL
commented

Tested this on a few different image fields in paragraphs and is working.

nicholass’s picture

nicholass
Primary language English
Location FL
commented
Status: Active » Needs review
nicholass’s picture

nicholass
Primary language English
Location FL
commented
Issue summary: View changes
zarabatana’s picture

zarabatana commented
Status: Needs review » Needs work

Thanks, the functional fix looks correct to me.

The issue is caused by usage entities that are not fieldable, and the MR adds an instanceof FieldableEntityInterface check before calling hasField(), which matches the proposed resolution.

However, I think this should stay Needs work for now because the PHPStan job is still failing.

The remaining failures look like CI/PHPStan cleanup: some obsolete unmatched ignore patterns, plus one current error where $target_id is accessed directly on FieldItemInterface.

Once PHPStan is green, I think this can be moved to RTBC.

nicholass’s picture

nicholass
Primary language English
Location FL
commented

@zarabatana - Trying out the new VScode agents interface, I didn't expect it to commit but it did seem to accurately fix the issue, so I patched our main website and I am now able to sanitize past files in paragraph fields, and is handling bad files who's formats don't match the extensions (that's great!).

Cleaned metadata on: /sites/default/files/gallery/migrated/yo___________ian-clu.jpg
Cleaned metadata on:/sites/default/files/gallery/migrated/cros___________sey.jpg
[error] Failed to strip metadata from/sites/default/files/gallery/migrated/cro___________ign.jpg: Warning: No writable tags set from /sites/default/files/gallery/migrated/cro___________ign.jpg
Error: Not a valid JPG (looks more like a PNG) - /sites/default/files/gallery/migrated/cro___________ign.jpg

Failed to clean metadata on: /sites/default/files/gallery/migrated/cro___________ign.jpg
Cleaned metadata on: /sites/default/files/gallery/migrated/cro___________ouet.jpg

It will take me a while to fully test on all our fields and files, but looks good so far!

zarabatana’s picture

zarabatana commented
Status: Needs work » Reviewed & tested by the community

Thanks @nicholass, this looks good to me now.

I reviewed the latest diff and the fix is focused on the original issue: it skips non-fieldable usage entities before calling hasField(), which prevents the Drush fatal error when file usage includes config entities such as Paragraphs types.

The PHPStan issues have also been addressed by replacing direct $target_id access with getValue() and removing the obsolete baseline ignores.

The added regression test covers the non-fieldable config entity case, and the pipeline is green.

Moving this to RTBC and merging.

zarabatana’s picture

zarabatana commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

zarabatana’s picture

zarabatana commented
Status: Fixed » Closed (fixed)
nicholass’s picture

nicholass
Primary language English
Location FL
commented

Thanks - I had no issues on our site so glad its been merged! - Thanks again for being a very responsive maintainer!