File Field Path does not overwrite existing file but adds _0 to filename instead.

I don't necessarily consider this a bug, as I would consider that overwriting my existing files is more damaging.

However, I do believe the behaviour should probably be configurable by the user, so for the moment I will change this issue to a feature request.

Right now my priority is to bugs rather than new features, but I will try to get back to it in the future, otherwise feel free to submit a patch.

This capability is also important to me. Note that -beta3 did replace files (it passed FILE_EXISTS_REPLACE to file_move), and -dev has removed it.

I've attached a patch that provides a configuration option that will allow files to be overwritten. Screenshot also attached.

A much larger development here is the inclusion of tests. I've added a test for this new "replace files" option, and in doing so also created a few utility methods that should make adding additional tests a snap. Now I can sleep at night :)

Expanded tests to check node object. Something not right - working on it.

Tests need to be rewritten completely, and your issue does not cover that scope, so please rewrite your patch to only deal with your issue and not try to re-implement tests.

There is quite a bit of uncommitted code related to the tests, so anything you do in that realm is likely just going to be blown away at a later date,

Look forward to reviewing the updated patch.

Please ignore my previous patch, I've come to the conclusion that it's the wrong approach and that the issue described by the OP is indeed a bug.

To illustrate the problem, I'm going to change the OP's scenario slightly

1. Create a content type with a file field.
2. Configure file field paths to rename file to something that would be easily duplicated. I've tested all of the following, but the last one (although useless) gets the point across:
   1. [file:ffp-name-only-original].[file:ffp-extension-original]
   2. [file:ffp-name-only-original].[file:ffp-extension-original].processed
   3. constant_file_name.ext
3. Attach a test file to a node, which will be processed and saved as constant_file_name.ext
4. Edit that node:
   1. click "Remove" beside your uploaded file
   2. upload the test file again
   3. click "Save"

At this point, the file will be saved as constant_file_name_0.ext, instead of constant_file_name.txt. The issue here is that the system does not know that the user removed the file prior to uploading a new one. During the file_move() operation, Drupal should have recognized this and not incremented the file name (the magic lives in file_field_update()).

The bug is with the way filefield_paths_entity_update() sets the $entity->original property. It needs to be set before we get to the file_move() operation.

Attached patch is pretty straight forward. After we do the _field_load calls, and before process the file paths, we set $entity->original.

A separate attachment is my Simpletests, which tests all of the File Name patterns mentioned above.

I had senario 3 (constant file name). File being created at the end of a batch process. On the initial entity save I got "myfile.ext", on the second save I got "myfile_0.ext", on subsequent saves the file name flipped between these two. Patch in #5 applies cleanly and fixes this issue against 7.x-1.0-beta3.

@ryan_courtnage,

If you do step 3 and 4 without File (Field) Paths installed the exact same behaviour will happen, the new file will have the _0 appended to the file, whereas if you remove, save, then re-add the file for both with and without File (Field) Paths it won't append the _0 to the file.

The reason this happens is that when you remove a file it's not actually removed until after the content is saved, so it is somewhat the expected behaviour.

However, I haven't looked at your proposed patch properly yet, so I'm not entirely ruling out some changes for this functionality yet.

@deciphered

That's a good point - I didn't realize that the stock Drupal file field behaved this way.

The history is in the cck filefield module: http://drupal.org/node/427212, where quicksketch makes his point clear in comment #1. This patch, however, does work in the scenario he describes. If a user uploads file.ext, it's saved as file.ext. If they edit the node, click "Remove" and re-upload file.ext, it is uploaded to the filesystem as file_0.ext. *Only if* the user saves the node, does it get renamed by filefield_paths to file.ext (and file_managed table updated). In other words, if the user abandons the node edit form after clicking 'remove' and uploading again, there is no harm done.

I'm just now running into this but in a different way.

I work at home on my laptop. I have a local install exactly matching that of my live site. When I create a 'work_sample' content, I click on the Choose Image and navigate to the file in the local drupal install which ends up: /sites/all/themes/my_theme/images/samples

So inside that 'samples' folder are all my jpegs. On the live server, I have the exact same structure: /sites/all/themes/my_theme/images/samples and I've already uploaded all the samples. Locally, all works as expected. When I dump the sql file and reimport to the live site, the images get a _0 added.

Since I'm choosing files within the Drupal install and both installs are identical I don't know why the renaming is occurring especially since I'm not actually 'uploading' anything. If I ftp into the server, nothing has changed in the filename. And, before importing the database into the live site, the paths and names are correct.

I thought I could go in phpMyadmin, find the reference and change it there but this new site is on GoDaddy so within phpMyAdmin, browsing tables isn't allowed.

Simple thing though a little more laborious: ftp to server.. add an underscore to images in question. Re-upload image via the Drupal inteface. Fixed. Then, it's easy to just delete all the old files with the underscores.

I tried patch #5 but am still having issues. I have a feed importer adding images to a field that has file field path applied to it. If it comes across a duplicate image it appends_# to the end of the file name and continues on it's way. I'd really just like to skip any duplicates. Any way to do this?

Thanks
Sharon

A straight ugly temporary solution on beta4 is to add FILE_EXISTS_REPLACE to file_move in filefield_paths_filefield_paths_process_file in filefield_path.inc and use a patch with feeds that allow you to setup the file replace and file rename choice, see https://drupal.org/node/1171114

You still need to add $entity->original = $entity; to filefield_paths_entity_update in filefield_paths.module

A better one is to run this hook before in your own module (my_module_filefield_paths_process_file).

I aslo would consider a good thing to set the file behavior to be configurable, for migration purpose at least (not a most wanted then). Rename is the drupal default, feeds could be configurable (not released yet) but this setting is unknow to filefield_path.

Just ran into this issue with Migrate module migration. When I run the migration update (drush mi --all --update) all files flip between the original filename and the _0 filename resulting in 100k image and 200k image style 404s. I'm looking into a good solution.

The patch in #5 fixes the problem although only the first hunk is needed, so the patch needs work to apply cleanly. I'll RTBC if the patch is fixed to apply to the current dev. Thanks!

Scratch the "#5 fix the problem" in #14. There seem to be two separate causes for this issue. I'll try again with the suggestion from #12.

Hmm.. #5 works great on a subset of my import. #5 should be commited, but further testing is needed.

It took me a little while to realize the patches of #2 and #5 were -both- needed. So this patch puts those two patches together in one patch.

Also added changes - after the file_move() call:
1. to replace the file in the entity(node) being edited with the file returned by file_move()
2. and to delete the 'temporary' file from the file_managed table.

would this be possible?

1) Add file example.txt to node and save.

2) Edit node and replace currently stored file with "updated" example.txt.

3) Drupal then renames the old file example-01.txt and uploads "updated" example.txt as "example.txt".

This way it would update links to files such as pdfs that have been manually entered in drupal across the. Users normally don't know that you have to remove the file then save it, then go back in and add the new file.

@hawkdavis: +1 for the idea of at least renaming the *old* file, rather than the new one. Was going to post the same myself. And +1 for making it configurable. I understand a developer's wish to be safe than sorry by not deleting data, but I would think in most cases, you would want the file overwritten. If you want to retain versions, some other solution would be called for, IMHO. Retaining *unwanted* old versions also creates problems when moving sites, uses a lot of space in backups, etc.

@hawkdavis, danbarron - something to consider - system must store a file somewhere, this file must be accessible to different nodes - so all related articles can link to it. If you change a file related to one article you often playing with other nodes (articles) too.

@ryan_courtnage, nice work. Now the thing is, multilingual pages can use same file (with for example different ALT, TITLE attribute) or new one. Let discuss the first option. If you delete/rename a file, nodes in different languages can be fuck up.

Module developers need to test their work with different configurations - ->multilingual<- (i18n, entity translations), multidomain (domain access), shoping (commerce), often marketplaces (only github) pages to see if everything is ok. If module is working well in hard environment, it will for sure, be working fine is simple drupal page - it's just my two cents. Off course it's very hard and time consuming thing...

The patch in #18 works for properly replacing files on upload, but it doesn't properly preserve the new filename (uri is correct though). Attached patch preserves filename for replaced file.

In case it helps though, I have to say that using 7.x-1.0-beta4 in a client's website, I found out the hard way that each file overwrote the previous one if both had the same name. I think it is exactly what the original poster of this thread requests.

Of course, I didn't want this and it seems that the culprit was the default pattern [file:ffp-name-only-original].[file:ffp-extension-original]. I didn't have time to properly read the module code or check the 7.x-1.x-dev version but changing the default pattern to [file:ffp-name-only-original]_[file:timestamp:raw].[file:ffp-extension-original] fixed my problem.

Concluding... Are we sure that the requested feature isn't already implemented? Even if some of us might think of it as a bug...

I need this fix or something like it. I currently have a required file field with constant filename. However, it is impossible to remove and add another file and maintain the constant filename. "_0" is added. I plan to try out the patch.

However, I did have the thought if two independent nodes happen to try to put the same filename in the same folder will this patch over write the file? That result might be unexpected. Most admins / users would assume that a particular file attached to a particular node can only be touched by that node. Of course the scenario would be a terrible configuration, but it is a possibility.

Patch #22 works great for me. Thanks!

In #25 I expressed concern about over writing files related to other nodes, but the feature checkbox warning text is good enough warning for any admins. I also see that there is the beginning logic, currently commented out, to disable the checkbox if the filename is set to remain as the original. I think javascript is needed for that, but given the warning text perhaps it is not needed.

Eager to see this patch applied to dev.

FYI... I manually applied the patch to 7.x-1.0-beta4. again, worked fine.

Got an issue with feeds with that patch. don't know if it's related to the patch provided for feeds, or for filefield path. See my comment on the feeds issue.

I've modified the #22 patch a little bit. The project I'm working on, it is not an option to upload a file with other name then the file's original file name, that's why the #22 patch wasn't good for us. With my modifications the override function works with the original name of the files too. Maybe it will be useful for someone else, too.

@mxr576 an interdiff would be nice to see what you changed.

EDIT wrong person mentioned. Meant mxr576

Hi @joelpittet!

I've fixed a typo in my patch, so I upload it again. Also, here is and interdiff between the #22 and the new patch.

Greetings.

Thank you for the interdiff @mxwright, looks like some nice fixes.

Here's a review of the code:

1. +++ b/filefield_paths.module
   @@ -122,12 +122,23 @@ function filefield_paths_form_alter(&$form, $form_state, $form_id) {
   +        // File exists behavior. Keep disabled if the file name option is the same as the original file, as this can result in
   +        // the uploaded file itself being moved onto itself and then deleted.
   

   80 chcater wrapping for this comment.

2. +++ b/filefield_paths.module
   @@ -122,12 +122,23 @@ function filefield_paths_form_alter(&$form, $form_state, $form_id) {
   +          '#disabled' => FALSE //isset($settings['file_name']) && $settings['file_name']['value'] == '[file:ffp-name-only-original].[file:ffp-extension-original]' ? TRUE : FALSE
   

   Something here looks a bit unfinished.

3. +++ b/filefield_paths.module
   @@ -136,7 +147,7 @@ function filefield_paths_form_alter(&$form, $form_state, $form_id) {
   +          '#weight' => 13
   

   Needs a comma at the end of the array element.

4. +++ b/modules/filefield_paths.inc
   @@ -65,9 +65,18 @@ function filefield_paths_filefield_paths_process_file($type, $entity, $field, $i
   +              $entity->{$instance['field_name']}[$entity->language][0] = (array) $moved_file;
   

   Should this been in a loop instead of a hardcoded 0? Since there may be more than one in a multi field.

Yes, @joelpittet you are right about these, but these problems mostly came from the original patch. If I'll have some time for this, then I'll try to fix these, but feel free anyone to improve this stuff until then.

Ok, things have changed; hook_entity_*() has been replaced with hook_field_storage_pre_*(), and as such it has resolved a lot of issues.

I never reproduced the original issue here, but my understanding was that when you removed a file from an entity then uploaded a new one to the same entity that would result in the same filename that it would suffix the file. I can not reproduce that behaviour.

The behaviour I can reproduce is that if you create a second entity with a file that would result in a the name of an existing file being prefixed, and I'm inclined to think that the prefixing is that correct behaviour in this case as otherwise you'd be overwriting a file that is referenced by another entity.

I'm still happy to consider this patch (once it's re-written), but it appears that it's a feature request rather than a bug.

If I am wrong, please correct me with reproducible steps.

Cheers,
Deciphered.

Despite this being changed to a feature request, I'm using this patch in #30 to resolve the bug where I was getting _0 in the file names.

It may be tricky to reproduce the steps, as I'm using this module to save invoices which are generated for each customer and coming down from the production site access only and saw a bunch being suffixed files before this patch. Though I'll see what what I can do to reproduce the scenario.

An attempt at the fixes I proposed in #32

I am using patch #22 in production to solve what is a bug in my opinion. I have a node type that allows the attachment of one file and I use file field paths to not only set the file location, but the file name. Then when the user edits the node with a goal to attach a different file, without patch #22 the file new file is named *_0, totally ignoring the name I have defined in file field paths, and further the node loses contact with the file.

This is a bug. Do you want me to set the status back to bug?

@webservant316,

I'll treat this like a bug when someone gives me reproducible steps, and not before. The scenario you described does not appear to be a bug, I repeated that test multiple times and it failed to once give me a *_0 file.

It's possible that the issue is related to environment or configuration, which is why you need to provide me with 100% reproducible steps so that I can determine if that is the case.

The best thing you could do would be to provide steps from a fresh install on SimplyTest.me so that I can ensure I am using the same environment as you.

In the meantime, I will write an automated test for the provided scenario, but as I said, I repeated those steps multiple times and not once was I provided with the alleged bug.

Cheers,
Deciphered.

+++ b/filefield_paths.module
@@ -271,6 +283,12 @@ function filefield_paths_field_storage_pre_update($entity_type, $entity) {
+          $entity->original = $entity;

This no longer has any purpose in hook_field_storage_pre_*() as it is already set. This was a workaround from when hook_entity_*() was in use.

Steps:

1. Launch http://ply.st/filefield_paths/7.x-1.x
2. Edit 'Article' content type 'Image' field
3. Change 'File name' to 'file.[file:ffp-extension-original]'
4. Save
5. Create 'Article' with file '01.jpeg'
6. Save content, results in 'field/image/file.jpeg'
7. Edit same node.
8. Remove image.
9. Re-add same file.
10. Save content, results in 'field/image/file.jpeg'
11. Edit same node.
12. Remove image.
13. Add file '02.jpeg'.
14. Save content, results in 'field/image/file.jpeg'

Unless I'm missing something, everything works as expected.

I'll test again on Monday.

@webservant316,

Did you get a chance to test? I know it's extremely hypocritical of me to expect prompt action (and I don't really), but I do want to ensure that if it is a real issue it can be fixed before I consider the stable 1.0 release ready to go (which I'm hoping to do ASAP).

thanks for the bump and sorry for the delay. now hoping for Thursday or Friday. It is on my radar and I am committed to help get this figured out.

@Deciphered I followed your instructions in #41 exactly and sure enough everything works as expected. I tried in multiple ways and it always works.

However, I did the same in my environment and I run into the trouble that started this issue (and that #22 seemed to fix). I don't get it. I tried on both image upload fields and file upload fields, with no luck. I also used fresh copies of the beta4 and dev versions, still no luck. It must be some interaction with another module that is not replicated in the fresh simplytest.me setup.

Ok, I think I found the culprit.

You can recreate the issue simply by checking the box for "Create new revision" under Default options when editing the content type. I found this to be the case both in my environment and in the simplytest.me instance referenced above.

When using the content type after checking the box, each of fields start appending "_0"+ to the filename, before the extension.

the issue must be deeper than that, because I have the issue without 'create new revision' checked. the first theory I was going to test, thur or fri was to see if having a custom, non-default, location for my public files made a difference.

My first report back is that this problem still exists under my configuration and the patch #22 above fixes it.

My setup uses filefield_paths to assign both the location and the filename of the file uploaded. Thus the filename for any file attached to the filefield for a particular node is placed in a directory as follows /filefield/node#/samename.pdf. So when I edit a node with a file already attached, delete the existing file, attach a new file regardless of the name, and hit save, EVERYTHING works. The new file is located at /filefield/node#/samename.pdf..

HOWEVER, if I hit the upload button before hitting save then '_0' is appended to the filename.

Perhaps that is the difference in our testing. Before I set up a minimal install to test this particular sequence perhaps you can test by hitting the upload widget button before save and see if you get the same problem.

I definitely tested hitting the upload button before the save button on SimplyTest.me and my local and was unable to reproduce the issue. Are you using the latest development release when you're testing?

It is possibly environmental, is there anything specific you can tell me about your setup? Is it a fresh install?

I am using 7.x-1.0-beta4. I'll test with the latest dev and report back.

@mxwright,

Your behaviour, the suffix of _# on a revision, is expected behaviour.

When you create a revision of an entity the fields also receive revisions, so the original file remains in the case you where ever wanting to revert to an earlier revision that referenced said file, and overwriting the file instead of renaming would break the revision behaviour.

What you could do is cater for this behaviour by using a revision ID/file ID in the file path/name.

@webservant316,

Hopefully you can report back soon (again, I know, absolutely hypocritical to expect), as at this stage I'm considering issue to be "works as designed".

Upgrading to 7.x-1.0-beta4+27-dev (2015-Aug-31) solved this problem for me. Patch #22 no longer needed. Thanks.

Great, thanks for following up.

I'll use the unpatched version in production and see what happens. (luckily fixable with some shell commands and surgical SQL)

@joelpittet,

If you find it to still be an issue, and its not due to revising, please provide reproducible steps and I'll be happy to reopen and look at again.

After doing lots of research... If upload replace module does not replace the old file with the new file having same name then try below..

Use this module Upload Overwrite

I would recommend in this case to try program Long Path Tool

Ok, I just ran into this issue and it seems that my files are getting renamed to _x when I upload them new.. looking at the node before saving revisions is not checked on and when the node is saved the public:// references the original fiel name not the _0 on top of that.

Im using the latest release build..

:(

@gateway69,

I'm a little confused by the second sentence, can you explain exactly what is happening, each step of the way through the process?

I assume the file is being correctly uploaded to temporary:// on upload, and then when the entity is save the file is being moved to your destination with an _X prefix?
The obvious thing I have to ask; is there already a file with the desired filename in the target destination? Because if so, that is the expected behaviour, otherwise you could potentially overwrite a file referenced by another node or there for another reason, which could be damaging behaviour.

@Deciphered FYI, I've got no more issues on this. I have customer invoices saving up to the server on a cron job and this was happening before but has since ceased. Thank you.

Thanks for the update @joelpittet.

I suspect this issue is done and dusted, but better safe then sorry. Hoping to get the 1.0 release out in a matter of days, but don't want to risk a mass upgrade to a broken module.

Given the lack of information from @gateway69, marking this issue as closed again. If you are going to comment in a closed issue, provide information for the module maintainer to actually look into please.

Rats - this bug is now reintroduced into my website. So again when uploading a file which is supposed to be renamed for filefield_paths to a static name the system stumbles and adds "_0", does not remove the original file, and loses track of the file location.

Not sure what changed, except, Drupal and other modules updates since then.
Drupal core 7.43
File (Field) Paths 7.x-1.0

Solution: I give up on assigning uploaded files to static names.

Re-open the ticket if you want to.

Spent one day to fix this issue. This behavior won't be reproduced on 7.x-1.0 2015nov17. But if you upgrade form alpha8 etc like me, then previous generated xxx_0.ext files is still there, so the old node can't be fixed automatically. I have to sql with database tables file_usage and file_managed to fix them manually.

Per #63 the default behavior is now again adding "_0" instead of replacing the original file. This happens on File (Field) Paths 7.x-1.0 and latest dev. Since the patches were written for older versions, here is a new patch (based on #37) that works with the File (Field) Paths 7.x-1.0. Note that this patch adds a setting "Replace existing files" option that should be checked if you want the "replace" behavior.

Thanks for #65! However, applying #65 is not possible. The following is shown when attempting to apply via git:

Checking patch docroot/sites/all/modules/contrib/filefield_paths/filefield_paths.module...
error: docroot/sites/all/modules/contrib/filefield_paths/filefield_paths.module: No such file or directory
Checking patch docroot/sites/all/modules/contrib/filefield_paths/modules/filefield_paths.inc...
error: docroot/sites/all/modules/contrib/filefield_paths/modules/filefield_paths.inc: No such file or directory

Here is a new patch that fixes this. Please review.

Edit: This patch also appears to fix #1512466: Multivalue field : only the first field is correctly treated.

@NWOM. Yes, I forgot to set the paths. Thanks for correcting the patch. Works for me.

@Izagata: If you've verified it, you can set it to "Reviewed & tested by the community". It only requires one other person to verify the patch in order to set it to that status (other than the uploader).

Also thank you very much for the patch! I've been trying to find a solution to this for awhile now.

Oh my mistake, I completely ignored the test bot. It appears it fails testing for some reason. Back to "Needs Work".

Never mind. Those were failed tests from the patches before.

Is this committed yet?

@gateway: This issue was last active 19 days ago, and the 7.x-1.x-dev's last commit was on 4th of January, 2016.

:( Sad Panda... hmmm.

It appears the recent security update included this patch, even though it wasn't mentioned in the patch notes.

If you're still seeing this issue after updating the most recent version, feel free to reopen. Since the file structure has changed, this patch would need to be rerolled if necessary.

Please merge #75 to fix this issue, that's not there obviously.

Hi all,

tried to patch this, but I'm getting an error:

d_paths_replace-1924686-66.patch
patching file filefield_paths.module
can't find file to patch at input line 44
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/modules/filefield_paths.inc b/modules/filefield_paths.inc
|index bba245174..3ace26b3a 100644
|--- a/modules/filefield_paths.inc
|+++ b/modules/filefield_paths.inc
--------------------------
File to patch: filefield_paths.module
patching file filefield_paths.module
Hunk #1 FAILED at 59.
Hunk #2 FAILED at 99.
2 out of 2 hunks FAILED -- saving rejects to file filefield_paths.module.rej

I used the patch from #75 (filefield_paths_replace-1924686-66.patch)

Is that patch working, or is this still work in progress. I assumed that since the status is fixed the version #75 works.

Am I missing something here?

Cheers.
Paul

Update: I need the "replace" option because I am working on a radiostation website. And I don't want an additional cover art file whenever a song is repeated or another song from the same album plays.

Nevermind.

I edited the two files per hand. Now everything is perfect. :-)

After testing this for some days I found an issue that is also being discussed here:

https://www.drupal.org/project/filefield_paths/issues/2824398
https://www.drupal.org/project/feeds/issues/2512824

Somehow the combination of Feeds and FFP leads to the file status not being set to permanent. This causes a lot or error messages in the logs.

Did not delete temporary file "public://images/username/coverart/Alison Krauss and Union Station – Alison Krauss + Union Station Live.jpg" during garbage collection, because it is in use by the following modules: file.

In post #11 the User SharonD214 suggested to add the possibility to also just skip the file if there is a duplicate already. Would that be possible?

Thanks
Paul

I will not offer any new insights on the issue, however, the latest patch also helped me in my case and I am also using a combination of the message, message_notify and token modules. The patch replaces the files and the tokens are updated as well.
I know that this is more of a "the filefield_paths works well with tokens and messages" rather than a help for this particular issue, but I am using this patch as well and works for me.