Replace Existing Files

The patch looks simple, but for some reason it behaves quite weird. I tried to use the replace option and it seems to work fine on the first save, but when the file should be actually replaced on the second upload+save, the path gets stuck to the temporary directory like https://example.lndo.site/sites/default/files/filefield_paths/filename.pdf when saving the entity. And there is no file in the path, so the temporary directory gets deleted like it should. Leaving the link to the file pointing at nothing.

Also I can see at /admin/content/files that a new file entity is created every time I replace the file and save the parent entity (using the same file all the time to make sure the replace is happening). And the path is the same as above for each new file entity.

Comment Removed: Sorry, I thought we were using this patch; we are using a custom patch. But it is failing in a similar way, so hopefully we can share our solution soon.

Thanks for your work on this. We also needed to have the ability to replace files rather than rename. We're trying to setup revision handling of files in media and this got us quite a bit closer. I noticed that the parameter expected for file_move was a little different so I updated that. Additionally we only really want redirects to be created for new revisions. I would think that would be an expected behavior for most sites so its added in here too. This also solved an issue of redirects not saving.

Turns out that the core file_move function skips saving the updated URI for a file when using the Replace setting. So after the file is moved I added another check for that makes sure the URI saved to the file is the same as the new destination.
Not sure the full implication of this or whether this is the 'right' way to go. Our use case has to do with moderation. We want the editors to be able to upload a file which get saved to a specific path. Then upload an identically named file as a draft which gets saved to a different folder using a custom token. Then when they publish that new one it moves the new file to the previous folder and replaces the existing file.
This is working correctly for me now. I'm not seeing orphaned files on my local and the file URI is updated correctly.

Thanks for the patch. Please provide steps to reproduce how to use the new feature. Or provide the test which uses the introduced functionality.

The way we're using this is to facilitate draft preview of new files on Media entities. So we have a token in the path for the draft version of the file. This is triggered by the moderation workflow on the Media entity. i.e. Draft, Needs Review, Published.

Our Steps to reproduce the functionality are;

Field Settings:

1. Enable the new Replace checkbox.
2. Add a token in the path settings based on the revision status of the Media item.

Workflow:

1. Create a Media entity and upload a file (we're doing images and pdfs).
2. Publish the Media entity and it should be in the directory path outlined by the path settings. i.e. /file/goes/here
3. Edit that same Media entity, and change the file uploaded. Use a new file named the same as the original.
4. Save the Media entity in Draft mode. The new file is uploaded and placed along the same path with an additional draft folder. i.e. file/goes/here/draft
5. Now the Media item should have a published version and a draft version each with an identically named file, in different directories.
6. Edit the Media entity and publish the draft version. The new file should be moved into the non draft directory and overwrite/replace the previously published image file. The name should be the same, no _0 added to the file name.

Previously the file would move into the non draft directory but get its name changed. This new feature provides the replace setting in the field formatter and does the actual replace. Per my last patch it also makes sure the File URI gets updated since core doesn't do that for file_move.

Rerolling #6 for the latest dev.

Rerolling #10 for the latest dev.

There are a few contrib modules which allow the upload replace option configured. It might be cleaner in the first instance to allow other modules to set this option.

This allows the replace behaviour to be set in hook_filefield_paths_process_file(). For example usage, see the dev version of File Upload Options: https://git.drupalcode.org/project/file_upload_options/-/blob/8.x-1.x/fi...

Rerolling #11 for the latest dev to remove the deprecated function.

IMHO I'm not really sure I agree with having to use yet another module when this one already handles the files and this patch is pretty straightforward.

#13:

IMHO I'm not really sure I agree with having to use yet another module when this one already handles the files and this patch is pretty straightforward.

I tend to agree. It would be good if core handled this eventually.

Regarding the patch, the new config option needs to be added to the schema.

Also, why not have a select or radios where you can choose how to handle existing files with the same name? EXISTS_RENAME, EXISTS_REPLACE or EXISTS_ERROR (prevent upload).

It would be good if core handled this eventually.

Yeah that would be nice :)

The patch applies fine and works but yeah, I need to do some work on it. I hadn't thought about providing for the EXISTS_ERROR option. I'll try to do some work on this later in the week and switch that over to a select list.

The patch applies fine and works

Can you tell me the version of the module and which patch makes this work? None of the combinations I have tried seem to be working for me.

Thanks!

@ekorotkin I'm using composer to install this for Drupal 8 so I have this "drupal/filefield_paths": "1.x-dev" for composer and I'm also applying my latest patch from #13. Also, you can see exactly how we're using in #9. Its a sort of draft/preview workflow using tokens and this module.

Another update to make sure the FileSystemInterface class is defined.
I'm not sure about the automated errors on the last patch. I don't know if they are related to this patch, but I'm not familiar with the automated test.

I couldn't apply the patch with composer, neither to the dev nor alpha version.

Hmm... ok let me check it. I created with PHPStorm in the project. It does apply cleanly for me via composer so I'll have to see whats going on.
Probably based it off the wrong version.

Here's a reroll from 1.0.0-beta5.

#22 It's work for me but I use in combination with File Upload Options Module
https://www.drupal.org/project/file_upload_options

Tested #22 using the image field type (not media) and it works. No need for file_upload_options module in my case.

#22 applied cleanly and works great after configuring each field to with the "replace" checkbox. Much appreciated.

Tested #22 on the latest dev without file_upload_options module and it's been working great. Setting to needs NR because it wasn't before. Can this be RTBC?

Please see the remaining work to do in #3069511-015: Replace Existing Files, which I've also placed into the OP to help guide this issue to its conclusion.

Can we get these resolved or answered, so we can bring a maintainer back here for review and sign-off?

Unassigning as well? This looks to be stuck in 'Assigned' since its inception.

I've tested the Replace Existing Files option with patch #22 and the file is successfully replaced. However, the filename in the field preview didn't get updated when using filefield_paths naming tokens. The filename in the preview is stuck on the original uploaded filename.

Steps to reproduce

1. Install filefield_paths
2. Apply patch #22
3. Select - Skip field - under Name field mapping at /admin/structure/media/manage/document
4. Drag Name textfield to the top to enable the field at /admin/structure/media/manage/document/form-display
5. Check Enable File (Field) Paths? at /admin/structure/media/manage/document/fields/media.document.field_media_document
6. Enter [media:name:value].[file:ffp-extension-original] in File name
7. Enable Replace Existing Files
8. Enable Active updating
9. Create a document media entity and upload a file
10. Rename the file by changing the name of the media entity

#22 no longer applies to the beta6 security update. Can someone please reroll this?

I will reroll the patch.

Attaching re-rolled patch.

Attaching new re-rolled patch with codding standard fixes as well.

Please create a separate issue for Coding Standards fixes, as the sustained patch has now bloated in size and that's not in scope for this issue.

Removing patch #35 and leaving #32 as the one in-review?

#32 fails on Drupal 10:

Error: Call to undefined function file_move() in filefield_paths_filefield_paths_process_file() (line 117 of /filefield_paths/filefield_paths.inc).

Needs work for:

• Drupal 10 compatibility
• Tests that are failing
• Tests for this feature

I'll try to repair the tests and see if I can add a new test for this feature as well.

I've worked some on this feature. I made the following changes:

• Added config schema for the new "replace" setting.
• Fixed a D10 compatibility issue: replaced file_move() with \Drupal::service('file.repository')->move().
• Added some test coverage. The tests ensure that files don't get duplicated on the file system.

However, this still needs some work. While a file gets replaced on the file system, we do get two file entities pointing to the same item on the file system. That is not good. We would still end up with a lot of duplicate file entities. Even worse, if one of the duplicated file entities gets removed, the physical file gets removed as well, resulting into other file entities pointing to no longer existing items on the file system.

So that definitely needs to get fixed.

My client hasn't given a go to work further on this in the nearby future, so unassigning for now.

This feature is EXACTLY what I've been looking for. Thank you for the work on this.
Just tested with great success for file replacement. But, like previously stated by @chrisck in comment #29, the generic file format for display on the field is showing the uploaded file name, but the url is linking to the tokenized file name and newly uploaded, different file. Which is really great!

I am running Drupal 9.5.9

File (Field) Paths - 1.0-beta6

Used the patch from comment #39

Hoping to be able to help in getting new work tested and moved a bit forward. My small team is going to be at Drupalcon Pittsburgh this upcoming week and maybe there are others that want to join together to get this issue worked out.

Backtracking on my earlier comment, I'm not sure Filefield Paths is the right place to determine what should happen when a file already exists. It's a great module for specifying the desired location for a file, but it isn't responsible for initiating the upload.

For example, Feeds allows you to specify whether to replace an existing file or rename the new file. This choice isn't respected when using Filefield Paths.

DropzoneJS also has its own logic and I expect other modules will as well.

It's tricky because each module has its own configuration.

Currently having issues where our content uploaders have two tabs open and have an attached document, and try to attach another document on the other tab (same node) it appends an underscore. This makes sense because the temp folder already has that file, but would be great if there was some type of warning. It looks like this would need to be changed at the file widget level though, and require something similar to the media entity file replace.

One problem I see with this occurs when used with multiple file field field:

In HTML5 you can drag & drop a list of files into a multiple file input element. When re-uploading a file with the same name the user might expect that the file is overwritten, which also happens with this patch. But in the file widget the file is listed twice and in the field value two references are stored to the same managed file entity. Manually removing one of the entities is save but still it would be better if the files list would be clever enough to filter for duplicates.

I’ve applied the patch mentioned in #39 and noticed the following issues:

When a media file is moved from one location to another, an incorrect redirect is being created. Specifically, the redirect has the same source and destination URLs.

Additionally, I observed that when a media entity is created, a redirect is being generated from the temporary location to the permanent one. This redirect is unnecessary and should be avoided.

Adding a new patch that addresses both of these issues.

I am also looking for this, using the "File (Field) Paths" module in a migration, and it seems like _0.jpg files are created when I run migrate:import --update, so I need to rollback, to not get a lot of duplicate files.

Rerolling

The patch #45 has been rerolled for "^1.0@beta" - 1.0.0-rc1 (Drupal 11.2.10)

Corrected previous patch #48

After retesting I want to point out the problem @megachriz reported in #39:

While a file gets replaced on the file system, we do get two file entities pointing to the same item on the file system. That is not good. We would still end up with a lot of duplicate file entities. Even worse, if one of the duplicated file entities gets removed, the physical file gets removed as well, resulting into other file entities pointing to no longer existing items on the file system.

So that definitely needs to get fixed.

I can confirm this happens with this patch, and as stated, when removing the seemingly unused file entry, the actual file itself is also removed while it's still referenced in the database

Rerolled #49 for 1.0.0-rc1 (Drupal 11.2/11.3), with the test additions removed.

This version also addresses the duplicate-file-entity problem raised in #39 and #50: when "replace" is enabled and a managed file entity already exists at the destination URI, its URI is freed to a temporary path before the move, so fileRepository->move() lets the uploading file's own entity take ownership of the destination URI instead of the move reusing the old entity's record. That keeps a single file entity pointing at the destination, avoiding the case where removing one of the duplicate entities deletes the physical file out from under the other still-referencing entity.

Uses the FileExists enum (FileExists::Replace / FileExists::Rename) instead of the deprecated file_move()/FileSystemInterface constants.

Patch attached: filefield_paths-replace-existing-files-3069511-rc1.patch