file field path set to static path publications
file renamed with [node:title].[file:extension]
cleaned up using path auto
1. initially when I add file and hit upload
origional named file is uploaded to server e.g. test.pdf
then I press save; file test.pdf is renamed correctly to the title of node and origional file extention
e.g. test-publication-6.pdf
2. When I edit the same node and remove content type then upload another file it uploads the origional named file test2.pdf to server (now has test-publication-6.pdf and test2.pdf). When I hit save it removes test-publication-6.pdf and renames test2.pdf to test-publication-06.pdf. This name is incorrect, it should be test-publication-6.pdf.
3. if I repeat step 2. I now have a file with the correct name test-publication-6.pdf.
This bug is causing major inconsitancies with file names.

Comments

Deciphered’s picture

Category:bug» feature
Priority:Major» Normal

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.

ryan_courtnage’s picture

Status:Active» Needs review
StatusFileSize
new10.6 KB
new30.3 KB

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 :)

ryan_courtnage’s picture

Status:Needs review» Needs work

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

Deciphered’s picture

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.

ryan_courtnage’s picture

Status:Needs work» Needs review
StatusFileSize
new20.32 KB
new1.11 KB

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.

Feng-Shui’s picture

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.

Deciphered’s picture

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

ryan_courtnage’s picture

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

thumbslinger’s picture

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.

thumbslinger’s picture

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.

SharonD214’s picture

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

richsky’s picture

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.

eosrei’s picture

Category:Feature request» Bug report
Issue summary:View changes

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.

eosrei’s picture

Status:Needs review» Needs work

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!

eosrei’s picture

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.

eosrei’s picture

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

gmercer’s picture

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.

gmercer’s picture

StatusFileSize
new4.3 KB
hawkdavis’s picture

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.

danbarron’s picture

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

kerios83’s picture

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

duellj’s picture

StatusFileSize
new4.35 KB

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.

mxwright’s picture

Status:Needs work» Needs review
vensires’s picture

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