Parameter 1 to upload_replace_file_update() expected to be a reference, value given in module_invoke_all()

Same problem

I also have the same problem

I'm also getting this message.

+1 on this. I too am receiving this warning. Also getting it for upload_replace_file_delete() as well.

Likewise, though I'm in D6

I'm pretty sure this is because hook_file_update and hook_file_delete are not supposed to take a referenced argument. The & needs to be removed from the function definition of these hook implementations.

+1

I get this same problem on D6, but only when using the workflow module to publish the most current pending revision. If I don't use the workflow module, and just click "Publish This" in the Revisions tab it works as it is suppose to and I don't get the trailing _0. It would be nice to be able to use the workflow module with this module.

For Drupal 7 you have to change the access to the file object and also the database requests:

/**
 * @file
 * A module file for providing functionality to replace files on upload
 * Typical Drupal behavior is to rename files on upload to <filename>_0.<ext>
 * This module modifies that behavior.
 *
 * 1.0 - Behavior is as follows: when a user is replacing a file (remove and save).
 *   if the file to be removed is <filename>.<ext> and the file to be saved is
 *   <filename>_<[0-9]+>.<ext> then the file will be removed and the new file will
 *   be renamed to <filename>.<ext>
 */


/**
 * Implementation of hook_file_update()
 */
function upload_replace_file_update($new_file) {
  if (!$new_file->fid) {
    //Nothing to do if no fileid
    return;
  }
  $desired_destination = preg_replace('/_[0-9]+\.(.*)$/', '.$1', $new_file->uri);
  $db_path = db_select('file_managed', 'f')
    ->fields('f', array('uri'))
    ->condition('fid', $new_file->fid)
    ->execute()
    ->fetchAssoc();
  if ($db_path['uri'] != $new_file->uri) {
    //this happens when a reversion is being reverted
    $next_good_uri = file_destination($desired_destination, FILE_EXISTS_RENAME);
    db_update('file_managed')
      ->fields(array('uri' => $next_good_uri))
      ->condition('fid', $new_file->fid)
      ->execute();
    $new_file->uri = $desired_destination;
  }
  else {
    //If the filename has be modified by adding a _0 value, or
    //on certain situations the uri will not match the uri in the db, such as
    //when reverting a revision.  When reverting a revision change the filename as well
    if (!strpos($new_file->uri, $new_file->filename)) {
      //the filename is not in the uri, so drupal must have added a "_0" before the extension
      //find the file that is blocking this file from keeping the correct path
      $result = db_select('file_managed', 'f')
        ->fields('f')
        ->condition('uri', $desired_destination)
        ->execute();
      //@todo only one result is handled, should allow for multiple results
      foreach ($result as $file) {
        $is_blocked = TRUE;
        $blocking_file = $file;
        $tmp_destination = file_directory_temp()."/test_-".$blocking_file->fid."_-".$blocking_file->filename."";
      }

      $old_destination = $db_path['uri'];
      //Swap the files
      if ($is_blocked) {
        //move the blocking file to a temparary location
        if (!file_unmanaged_move($desired_destination, $tmp_destination)) {
          drupal_set_message(t('The file %old could not be moved to %new', array('%old' => $desired_destination, '%new' => $tmp_destination)), 'error');
          return;
        }
        //DRUPAL 7 no longer changes the source uri during move
        //move blocking file was successful, update the DB
        db_update('file_managed')
          ->fields(array('uri' => $tmp_destination))
          ->condition('fid', $blocking_file->fid)
          ->execute();
      }


      //move the newfile to the prefered location
      if (!file_unmanaged_move($old_destination, $desired_destination)) {
        drupal_set_message(t('The file %old could not be moved to %new', array('%old' => $old_destination, '%new' => $desired_destination)), 'error');
        return;
      }
      //move newfile was successful, update the DB
      db_update('file_managed')
        ->fields(array('uri' => $desired_destination))
        ->condition('fid', $new_file->fid)
        ->execute();
      $new_file->uri = $desired_destination;//set the newfile's path to the correct path


      if ($is_blocked) {
        //move the older file from temp to the new _0 location
        if (!file_unmanaged_move($tmp_destination, $old_destination)) {
          drupal_set_message(t('The file %old could not be moved to %new', array('%old' => $tmp_destination, '%new' => $old_destination)), 'error');
          return;
        }
        //move blocking file was successful, update the DB with the actual location after file copy, so we use tmp_destination as it was updated during the move
        db_update('file_managed')
          ->fields(array('uri' => $old_destination))
          ->condition('fid', $blocking_file->fid)
          ->execute();
      }
    }
  }
  //Have to clear the cache because the revision data is cached somewhere
  /*
   * Find the nids where this file is used
  $query = "SELECT DISTINCT id FROM {file_usage} WHERE fid=%d";
  $result = db_query($query, $new_file->fid);
  while($data = db_fetch_object($result)) {
    cache_clear_all("node:$data->id");
  }
  */
  //This is inefficent, but how can we determine what nodes use this file?
 // cache_clear_all('*', 'cache_content', TRUE);
}

/**
 * HOOK_file_delete, update the uri in the file object before deleting as we may have altered it above
 * @param object $new_file
 */
/*
function upload_replace_file_delete($file) {
  $file->uri = db_result(db_query("SELECT uri FROM {file_managed} WHERE fid = %d", $file->fid));
}
*/

I'm confused: Is the revised code in #9 proposed as a patch? Is anyone with commit privs planning to commit it? Thanks.

Just replace the code in the file "sites/all/modules/upload_replace/upload_replace.module" with this new code.
(-> I think the original code was made for Drupal 6 - and just copied.)

replaced the .module file with the code from #9...works fine.

***drupal 7.12 this is not working anymore for me ...not sure what changed.

old files are getting renamed..but new file is not being uploaded.

Code from #9 works for me. Here it is in a patch. I think this can go in.

So, does #8/#13 work in v6 and/or will it be rolled in to an beta or updated version?

+1

I'm still using D6 and have this same problem. Module doesn't even do its job.

Many thanks for this module. I can verify that the code from #9 works nicely. Would be great to see this rolled into the next version.

I've installed 7.x-1.x-dev with patch 13 applied in a fresh Drupal 7.17 install. While the error is gone, images are not replaced or renamed.

I am using 7.x-1.0-beta1, patch #13, Drupal 7.16.
When replacing a file, the old files get renamed "_0", but there is no new file on the server!

Step by step:
1. Create a new node, uploading a file "xyz.txt" to a filefield, saving the node: the file "xyz.txt" is on the server, everything is okay.
2. Editing the same node, deleting the file and uploading the new file "xyz.txt", saving the node:
The Database tells me, that the uri for the fid is still "xyz.txt"
The old file gets renamed to "xyz_0.txt".
The new file ... is gone?

Would be great, if this module would work! :-)

@ everyone saying the new file is gone: the delete hook needs to be active. Otherwise when you remove & replace the file, Drupal will delete your NEW file instead of the OLD file.

Since it doesn't look like patches are going in, here is the code for upload_replace.module. Replace everything in upload_replace.module with the code below:

<?php
/**
 * @file
 * A module file for providing functionality to replace files on upload
 * Typical Drupal behavior is to rename files on upload to <filename>_0.<ext>
 * This module modifies that behavior.
 *
 * 1.0 - Behavior is as follows: when a user is replacing a file (remove and save).
 *   if the file to be removed is <filename>.<ext> and the file to be saved is
 *   <filename>_<[0-9]+>.<ext> then the file will be removed and the new file will
 *   be renamed to <filename>.<ext>
 */

/**
 * Implements hook_file_update()
 */
function upload_replace_file_update($new_file) {
  if (!$new_file->fid) {
    //Nothing to do if no fileid
    return;
  }
  
  $desired_destination = preg_replace('/_[0-9]+\.(.*)$/', '.$1', $new_file->uri);
  $db_path = db_select('file_managed', 'f')
    ->fields('f', array('uri'))
    ->condition('fid', $new_file->fid)
    ->execute()
    ->fetchAssoc();
  if ($db_path['uri'] != $new_file->uri) {
    //This happens when a reversion is being reverted.
    $next_good_uri = file_destination($desired_destination, FILE_EXISTS_RENAME);
    db_update('file_managed')
      ->fields(array('uri' => $next_good_uri))
      ->condition('fid', $new_file->fid)
      ->execute();
    $new_file->uri = $desired_destination;
  }
  else {
    //If the filename has be modified by adding a _0 value, or
    //on certain situations the uri will not match the uri in the db, such as
    //when reverting a revision. When reverting a revision change the filename as well.
    if (!strpos($new_file->uri, $new_file->filename)) {
      //The filename is not in the uri, so drupal must have added a "_0" before the extension.
      //Find the file that is blocking this file from keeping the correct path.
      $result = db_select('file_managed', 'f')
        ->fields('f')
        ->condition('uri', $desired_destination)
        ->execute();
      //@todo only one result is handled, should allow for multiple results
      foreach ($result as $file) {
        $is_blocked = TRUE;
        $blocking_file = $file;
        $tmp_destination = file_directory_temp()."/test_-".$blocking_file->fid."_-".$blocking_file->filename."";
      }
      $old_destination = $db_path['uri'];
      
      //Swap the files.
      if ($is_blocked) {
        //Move the blocking file to a temparary location.
        if (!file_unmanaged_move($desired_destination, $tmp_destination)) {
          drupal_set_message(t('The file %old could not be moved to %new', array('%old' => $desired_destination, '%new' => $tmp_destination)), 'error');
          return;
        }
        //Move blocking file was successful, update the DB
        db_update('file_managed')
          ->fields(array('uri' => $tmp_destination))
          ->condition('fid', $blocking_file->fid)
          ->execute();
      }

      //Move the newfile to the prefered location
      if (!file_unmanaged_move($old_destination, $desired_destination)) {
        drupal_set_message(t('The file %old could not be moved to %new', array('%old' => $old_destination, '%new' => $desired_destination)), 'error');
        return;
      }
      //Move newfile was successful, update the DB
      db_update('file_managed')
        ->fields(array('uri' => $desired_destination))
        ->condition('fid', $new_file->fid)
        ->execute();
      //Set the newfile's path to the correct path
      $new_file->uri = $desired_destination;


      if ($is_blocked) {
        //Move the older file from temp to the new _X location
        if (!file_unmanaged_move($tmp_destination, $old_destination)) {
          drupal_set_message(t('The file %old could not be moved to %new', array('%old' => $tmp_destination, '%new' => $old_destination)), 'error');
          return;
        }
        //Move blocking file was successful, update the DB with the actual location after file copy, so we use tmp_destination as it was updated during the move
        db_update('file_managed')
          ->fields(array('uri' => $old_destination))
          ->condition('fid', $blocking_file->fid)
          ->execute();
      }
    }
  }
  //Have to clear the cache because the revision data is cached somewhere
  /*
   * Find the nids where this file is used
  $query = "SELECT DISTINCT id FROM {file_usage} WHERE fid=%d";
  $result = db_query($query, $new_file->fid);
  while($data = db_fetch_object($result)) {
    cache_clear_all("node:$data->id");
  }
  */
  //This is inefficent, but how can we determine what nodes use this file?
 // cache_clear_all('*', 'cache_content', TRUE);
}

/**
 * Implements hook_file_delete()
 *
 * Update the uri in the file object before deleting as we may have altered it above.
 */
function upload_replace_file_delete($file) {
  $file->uri = db_query("SELECT uri FROM {file_managed} WHERE fid = :fid", array(':fid' => $file->fid))->fetchField();
}

Didn't test it really well ... but it seems to work now! :-))
Thanks for this work!

#20 did it for me.

It really is a shame that beta1 even exists in it's state. The code at #20 should be released ASAP.

Thanks, it seems to work now, but I get this Notice:

Undefined variable: is_blocked in upload_replace_file_update() (line 58 ...

#20 Works for me too :)

Hi,

While testing i run into the following problem:
#20 does not work with node revisions on, when viewing the node it self it displays the correct image but in a view for example it displays the previous image...

I do you use File (Field) Paths for renaming the file so it has the same name name as the title...perhaps that does affect this module?

Kind regards,
Henk

Seconding #23's problem. It DOES seem to work, however.

Attached a patch that worked for me. I took code from comment 20 and did some formatting refinements and modified hook_file_delete.

Patch in #28 solved the problem for me, thanks.

Patch #28 seems to have solved this issue for me as well. Thanks minnur and entendu.

Spoke too soon. It worked on my test site, but after patching the module on the production site, I received this new error message after several attempts:

Warning: Parameter 1 to upload_replace_file_update() expected to be a reference, value given in module_invoke_all() (line 895 of drupal\includes\module.inc).

#20 did work for me.

Fixed some typos and formatting issues in #20. And it is working fine so RTBC.

Can we get this applied to the Dev version?

dev version + patch in #32 worked for me

dev version + patch #32 does not work for me.

It renames files in a "funky" (unwanted) style.

Correct name of uploaded file:

lgogdownloader_2.12-1_amd64.deb

File name after uploading using dev + patch #32:

lgogdownloader.12-1_amd64_1.deb

dev version + patch in #32 works fine.

Alright this doesn't work at all. I've tried the latest stable 7 version and dev version with and without the patch - sometimes I get the error, other times I don't, but I ALWAYS get _0, _1 etc added to files. I really need this feature in a site, are there any other ways to achieve this?

Managed to fix the problem:

Change:
function upload_replace_file_update(&$new_file)

To:
function upload_replace_file_update($new_file)

This prevents the error and now it also seems to be working as expected :-S

@webbymatt is that with or without the patch?

@webbymatt please help is that with or without the patch?.
how to fix this, is it to the dev version

@ginosuave @ZaDeveloper - @webbymatt's change is already in the patch, so if you've already patched you don't need to implement his change.

Problem with the patch in #32:

On a site using the Media module, when I attempted to place an externally hosted video (e.g, YouTube or Vimeo) through a media field or in a WYSIWYG body field, Upload Replace with this patch was intercepting the save process, causing Media to treat the videos as regular files that needed saving to the file system rather than links to pull from the video provider's site. This results in the following error:

File youtube://v/LqbYVr5jBVk could not be copied, because the destination directory is not configured correctly.

And when you attempt to replace a video that received the error above, you get:

File /path/to/temp/directory/test_-1934_-Tiny_Birthday_For_A_Tiny_Hedgehog_Ep._2 could not be deleted because it is not a valid URI. This may be caused by improper use of file_delete() or a missing stream wrapper.

As a result of this conflict, the connection to YouTube/Vimeo/etc. is not properly made and neither videos nor video thumbnails display. Disabling Upload Replace or reverting the patch resolves the problem.

@tedbow was helping me track this down and reports:

The problem was coming from this:

/**
 * Implements hook_file_update().
 */
function upload_replace_file_update($new_file) {

The property $new_file->uri equals "youtube://v/ANhd4EhwVtI"

They could possibly check what the uri starts with and if it is unknown (I am not sure what it would be for an actual file) then skip it.

It took me ages to figure out that Upload Replace was causing this Media error, so we certainly don't want to commit this patch without resolving this.

Something like this?

directly after the function definition:

  // check scheme to not interfere with external files
  $scheme = file_uri_scheme($new_file->uri);
  if ( !($scheme == 'public' || $scheme == 'private') ) {
    // return if $scheme is not (public or private)
    // then it is some other scheme like youtube or s3 etc.
    return;
  }

Hi,

I've added @grahamtk's code in #43 to the patch in #32. This resolves the issue on comment #42 for us when using media_vimeo.

Latest patch (#44) on the dev version worked nicely for me. Thanks much!

Thanks for your work with this module :) Great job!
Patch 44 works well, but this module still does not work with the Feeds module.

Please see separate issue:
https://www.drupal.org/node/2139685#comment-9931630

We need to get this change committed so that we can integrate other changes into it, for example the issue with image styles described here: https://www.drupal.org/node/2037295

Other patches cannot be applied easily once the patch in comment #44 is applied.

This patch is a re-roll of what is in patch 44, but it also includes the image style fix from here: https://www.drupal.org/node/2037295

Since this module is not being maintained I wasn't sure the proper way to integrate both of those fixes together. Sorry for the long patch file name, but I was trying to make it clear what it does :-)

The diff between #44 and the patch in #48 is simply the addition of these lines:

  //Have to clear the cache because the revision data is cached somewhere
  $query = "SELECT DISTINCT uri FROM {file_managed} WHERE fid=:fid";
  $uri = db_query($query, array(':fid' => $new_file->fid))->fetchField();
  image_path_flush($uri);

Changing status to needs review

Confirming patch #48 works for us. Thanks!
Bumping to critical since in it's current state, the module is pretty much useless.
Since maintainer is unresponsive on this 5 year old issue, maybe creating a new module is the way to go.

Thanks, #48 worked for me!

#48 is a bad patch file, on account of converting from Unix to Mac EOL formats (i.e. it changes the entire file, instead of just the modified lines).

When you fix that, the interdiff described by #49 isn't quite accurate either (although the differences are only comments).

I've re-rolled #48 using the correct EOL style, and amended its merge of the secondary patch from #2037295: Image Cache not cleared - code commented out (lines 107-128) so that it better fits the described change. I changed the header comment for that latter change, because the existing one wasn't appropriate. I've provided the interdiff with the patch from #44.

This post doesn't constitute any kind of review. I've merely established that #48 was used to patch this module in a code base I've acquired, and I needed a patch file I could apply cleanly to recreate that version of the module.

See #53 for details. These are the exact same files, but I had used the wrong comment number in the filenames.

re: #51 - The correct way to go is to take over maintainership of this module. There is a process for this when the current maintainer is to busy too respond.

In addition there was a request to take over maintainership at #2195237: Offering to maintain Upload File Replace with no response so if anyone would be considering making a new module they should just go through the process here https://www.drupal.org/node/251466 (Dealing with unsupported (abandoned) projects).

re: #42 I think we should get this patch merged and open a new issue for yours and make it a blocker for the next beta release, but at least get this into the dev branch.

If I have time I will apply to be co-maintainer of this module and get this merged.

Until then I am going to mark this RTBC to get it merged when one of us gets commit access, the patch is working better than beta1 for pretty much everyone so it won't be a regression and will actually work.

Some more coding and documentation standards cleanup, no functional changes from #54. We have been using #20 for over 1.5 years in production so far with the addition of #2691847: Image Style Derivative not recreated which is similar to #2037295: Image Cache not cleared - code commented out (lines 107-128).

I also am having an issue with $file->filename not getting updated on the old file. I am going to roll a patch up for that and make a new issue as to not complicate this one further.

I made a new patch for adding the filename here => #2751549: Filename not getting updated for replaced file

Out of curiosity, why was this comment added?

+ // @todo Should we clear the other styles too?

Assuming "other styles" means the derivatives of other images, I can't fathom why that is being considered.

edit: Actually, I can see that comment being removed in your patches in https://www.drupal.org/node/2037295 so its addition here seems unintentional?

It is worth noting that the upload_replace_file_delete() currently doesn't do anything, not does it in HEAD (commented out there). It just does a select:

function upload_replace_file_delete($file) {
  $file->uri = db_select('file_managed', 'f')
    ->fields('f', array('uri'))
    ->condition('f.fid', $file->fid)
    ->execute()
    ->fetchField();
}

And hook_file_delete is not passed by reference so the URI does not get updated as the code suggests. This should be removed for now or fixed because it causes a useless DB query on every file deletion.

#59 - Correct, it was referencing the old URL's style. The patch in #2037295: Image Cache not cleared - code commented out (lines 107-128) takes care of that.

Okay, ran into another issue with filenames not getting displayed until after cache is cleared. It appears we really should try to be using file_save() instead of all these manual DB queries. file_save() calls entity_get_controller('file')->resetCache(array($fid)); at the end so I just added that for now since I don't have time to refactor it.

Here is a patch that actually includes the resetCache and #2037295: Image Cache not cleared - code commented out (lines 107-128) as well. I also removed this line that doesn't do anything, $new_file->uri = $desired_destination; Sorry I don't have time to cleanly separate these, someone feel free to do so.

Also, #5 in #2037295: Image Cache not cleared - code commented out (lines 107-128) is bad, I had the wrong variable name $original_path vs. $old_destination. This latest patch is updated.

Argh, I also have #2751549: Filename not getting updated for replaced file applied in this patch.

https://www.drupal.org/node/2751549#comment-11308699

Oh my, completely forgot about objects being passed by reference, my patch in #62 started breaking stuff and then I remembered.

Here is a patch with the 'line that doesn't do anything' in #62 added back and also the hook_file_delete() implementation uncommented back. Also had to split out the file cache reset, was throwing missing property errors in some cases.

Also interdiffing against #57 and hiding #62.

Hey all,

I asked the original maintainer to help co-maintain this project and they gave me commit access.

I have committed #57!!

New patch works great, but introduces a dependency on the image module. See: https://www.drupal.org/node/2758119

@jay-dee-ess Thanks for that new report and making a new issue!