Problem/Motivation

When a media type is set to Download and sync assets, and the asset filename from the DAM has changed since the asset was last synced to Drupal, when the sync runs the new file is downloaded, and the existing file entity is updated, but the old file remains on the filesystem (and is no longer associated with a Drupal file entity).

Steps to reproduce

  1. Configure a media type to Download and sync assets
  2. Sync an asset of that media type to Drupal, noting the file name
  3. Upload a new version of the file to the DAM, using a different file name
  4. Re-sync the asset
  5. In the Drupal file listing (/admin/content/files), you will see the same file entity updated with the new file name
  6. Examining the filesystem directly, you will see both the old file and the new file

This case should be handled in \Drupal\acquia_dam\AssetFileEntityHelper::updateExistingFile().

CommentFileSizeAuthor
#3 acquia_dam-3569995-3-files-not-cleaned-up.patch1.02 KBbecw

Issue fork acquia_dam-3569995

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

becw created an issue. See original summary.

becw’s picture

becw commented
Issue summary: View changes

I originally reported this issue in acquia_dam 1.1.7, and half of the issue is still present as of acquia_dam 1.1.14.

Previously, both the new file and the old file remained both on the filesystem and as a Drupal file entity. Now, the existing Drupal file entity is updated with the new file, but the old file is still present on the filesystem (and no longer linked to a Drupal file entity).

I've updated the issue description to reflect the behavior as of acquia_dam 1.1.14.

becw’s picture

becw commented
Status: Active » Needs review
StatusFileSize
new acquia_dam-3569995-3-files-not-cleaned-up.patch1.02 KB

I've attached a patch that fixes the issue in my testing.

rajeshreeputra made their first commit to this issue’s fork.

rajeshreeputra opened merge request !306

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented

AssetFileEntityHelper changes:
Modified updateExistingFile() to delete all files in the directory before writing the new file, using new deleteDirectoryContents() helper handles the directory cleanup with a \UnexpectedValueException catch for the edge cases if any like directory does not present.
This not only handles the filename-change orphan scenario but also cleans up any pre-existing orphaned files from before this fix was deployed.

Code coverage:
New test testAssetFileReDownloadFilenameChange() in MediaIntegrationTest:

  • Uses asset a56fb261-8ad5-4e0d-8323-0e8a3659ed38 with old version ed84ab8e-a822-471d-b2f4-21b7353e4d0c (filename bobsCarsAgileExercisesOld.pdf) and new version e37312f5-e761-4ec4-92f2-8f0407f57bc7 (filename bobsCarsAgileExercisesNew.pdf)
  • Creates media entity → triggers initial download of old file
  • Creates a manually orphaned file in the same directory to simulate orphaned file left from previous sync
  • Switches fixture to new version data → triggers re-download
  • Asserts orphaned file and old file are cleaned up, new file exists on disk
  • Covers downloadFile, updateExistingFile, and deleteDirectoryContents

  • rajeshreeputra committed c9547fc4 on 1.1.x 
    Resolve #3569995 "Fix: previous versions of assets not deleted when...
rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Status: Needs review » Fixed

MR merged!

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.

  • rajeshreeputra committed c9547fc4 on 1.1.x-webhook 
    Resolve #3569995 "Fix: previous versions of assets not deleted when...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.