When saving a new file through services file.save, if the file gets renamed to filename_0.ext because filename.ext already exists, the files table ends up with in wrong value.
The version I have now are drupal 6.19 and services 6.x-2.4.
I'm quite sure that this used to work.

After looking into the code, I made the following changes in the file_service.inc

  // Write the file
  $newfilename = $file->filename;
  if (!$newfilename = file_save_data(base64_decode($file->file), $file->filepath .'/'. $file->filename)) {
    return services_error("Could not write file to destination");
  }

  //The filepath that ends up in the node must contain the filename
  $file->filepath .= '/'. $newfilename;

I don't know if it is written in the best possible way, but it works for me.
If anyone has more info on that I would like to hear.

Jean-Paul

CommentFileSizeAuthor
#8 1010544-file-resource-tests-remove-watchdog.patch17.11 KBygerasimov
#6 1010544-file-resource-tests-no-core-patch.patch17.15 KBygerasimov
#5 1010544-file-resource-tests.patch16.96 KBygerasimov
#2 file_service.b1010544.patch3.3 KBbrad.bulger

Comments

miraclegr’s picture

miraclegr commented

Sorry the above code has a bug too. The correct one is

  // Write the file
  $newfilename = $file->filename;
  if (!$newfilename = file_save_data(base64_decode($file->file), $file->filepath .'/'. $file->filename)) {
    return services_error("Could not write file to destination");
  }

  //The filepath that ends up in the node must contain the filename
  $file->filepath = $newfilename;
brad.bulger’s picture

brad.bulger commented
StatusFileSize
new file_service.b1010544.patch3.3 KB

the problem is that the call to file_check_path() is changing $file->filepath to just the directory name, and losing the modified filename. this patch rearranges things somewhat, to be more like file_save_upload(). it's been working for me using it with the Deploy module. it's only a tentative fix, as there is a pending patch to file_save_upload() that does away with some steps, from #30520: file_save_upload() not notifying user when PHP upload limit is exceeded.

marcingy’s picture

marcingy commented
Version: 6.x-2.4 » 7.x-3.x-dev

This needs to be fixed in 7.x first and then backported.

ygerasimov’s picture

ygerasimov commented
Version: 7.x-3.x-dev » 6.x-3.x-dev

@marcingy file resource is not good yet on 6.x-3.x and there is no tests that cover it. 7.x-3.x file_resource is much better. I will take another round to make file resource work on 6.x-3.x.

Moving this issue to 6.x-3.x. Any help will be very appreciated

ygerasimov’s picture

ygerasimov commented
Status: Active » Needs review
StatusFileSize
new 1010544-file-resource-tests.patch16.96 KB

Here is patch that changes file_resource plus adds tests for 6.x-3.x. Please remember to apply patch from #180970: file_check_directory fails to make directories if the parent doesn't already exist in order tests pass as simpletest cannot create files structure without that patch.

Regarding this particular issue (rename file if file with same name exists) there is check in testResourceFileCreate().

ygerasimov’s picture

ygerasimov commented
StatusFileSize
new 1010544-file-resource-tests-no-core-patch.patch17.15 KB

Here is patch that does not require changes in d6 core.

kylebrowning’s picture

kylebrowning commented

Theres a watchdog statement in there, but other than that it looks great. Ill RTBC it when thats removed.

ygerasimov’s picture

ygerasimov commented
StatusFileSize
new 1010544-file-resource-tests-remove-watchdog.patch17.11 KB

Oh... Attached patch without watchdog.

kylebrowning’s picture

kylebrowning commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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