A file entity can be edited and a new file uploaded to 'replace' the original file. This functionality has been added to file_entity module in Media 2.

However, I can't see a way to trigger a derivatives replacement process for the existing, updated entity, using the replacement file. Is this possible at the moment, or would more work be needed on the module, to add the extra hooks? (If more work is needed, I may be able to help with this.)

(Edited after updating file_entity module to latest unstable version, which fixed the other bug I mentioned in passing.)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

slashrsm’s picture

We should probably react on hook_file_update() for this. I am not able to work on this due to lack of time, but I can review and commit patches.

martin_q’s picture

Great. I would be pleased to work on this and propose some patches.

First questions. If the source file is replaced, we must give engines the chance to produce new derivatives. What should be done with them? Specifically, should the filename (or URI) of the new derivative be the same as the derivative it is replacing? (I think yes - so we tell the engine it has to save the new derivative file in place of the old one, rather than saving a new file and deleting the old one.) And if it is a managed file, should the fid (file entity id) be the same as well? So really we would be updating the derivative entity, not replacing it? Or would you prefer a totally new derivative to be created?

EDIT: I've just noticed that media_derivatives_save() is already written for the update case as well as the create case, but the implementation expects that there might be a new URI for the derivative file. So does that mean we should not care about making sure links to derivative files always persist?

slashrsm’s picture

It would probably be the best if we'd allow site admin to configure things like this. Similar to "delete derivatives when source deleted" etc.

martin_q’s picture

It looks like I'm going to have an event 'file_replace' that is triggered on hook_file_update:

  $events['file_replace'] = array(
    'name' => t('File replace derivative event'),
    'description' => t('Derivative event, that will be executed when a file is replaced with a new upload.'),
    'validation_callbacks' => array(),
  );
/**
 * Implements hook_file_update().
 */
function media_derivatives_file_update($file) {
  // Check if this file is one of previously saved temporary files,
  // which has now become a permanent one.
  if (
    $file->status == FILE_STATUS_PERMANENT
    && $tmp_cache = cache_get('media_derivatives_handle_temp', 'cache')
    && isset($tmp_cache->data[$file->fid])
  ) {
    $tmp_data = $tmp_cache->data;

    // Create derivatives for this file.
    media_derivatives_create_all_derivatives($file, 'file_insert');

    // Remove file from temporary files registry.
    unset($tmp_data[$file->fid]);

    // Update temporary files registry.
    if (empty($tmp_data)) {
      cache_clear_all('media_derivatives_handle_temp', 'cache');
    }
    else {
      cache_set('media_derivatives_handle_temp', $tmp_data, 'cache', CACHE_PERMANENT);
    }

  }
  // Check for a replaced source file and invoke updates of its derivatives.
  elseif (!empty($file->replace_upload)) {
    media_derivatives_update_all_derivatives($file, 'file_replace');
  }
}

media_derivatives_start_encode() is the only thing that the scheduler calls and so it will be called for derivatives that are scheduled for updates as well as those scheduled for creation. Right now it assumes that there is no existing derivative; I'm going to change it so that it copes fine with a derivative that exists already, and just updates the record as necessary.

My question now is, in the case where the admin chose for the derivatives to be updated (rather than deleted and a new one created) do you think there should be a new 'hook_media_derivatives_update_derivative' or should the existing 'hook_media_derivatives_create_derivative' be defined so that it is supposed to respond differently to the two events 'file_insert' and 'file_replace'? This will affect how much of media_derivatives_start_encode() needs to be altered to deal separately with updates.

For more reliable backwards compatibility, it would probably better to have the new hook, right? That way, existing behaviour (no update) would stay the same, but anyone is free to implement the new hook so that derivatives are updated.

I guess I'm not completely sure what the purpose of the events is and how else they might be used. What kinds of third-party events do you think might exist in media derivatives modules?

Thanks again for your help!

slashrsm’s picture

New hook is probably the best solution.

Events are just this. Events that happen in the system and we want to react to them. I didn't want to do any assumptions about how will people use media_derivatives. That's why I left it completely open.

martin_q’s picture

Component: User interface » Code
Assigned: Unassigned » martin_q
Category: support » feature
martin_q’s picture

OK, I think I understand then. For now, there will be a one-to-one relationship between the two events ('file_insert' and 'file_update') and the two engine hooks ('hook_media_derivatives_create_derivative' and 'hook_media_derivatives_update_derivative').

The API will then allow other events to be defined, but we expect they would still always trigger either media_derivatives_create_all_derivatives() or media_derivatives_update_all_derivatives() and these in turn would invoke hook_media_derivatives_create_derivative() and hook_media_derivatives_update_derivative() respectively.

If I've understood all that correctly, I can continue! If I'm confused, please let me know!

martin_q’s picture

Maybe media_derivatives_update_all_derivatives() and media_derivatives_create_all_derivatives() should really be the same function, just creating when there is no existing derivative and updating when there is (this is what _update_all_ does anyway). I'll investigate that. It might involve renaming media_derivatives_create_all_derivatives() though, as that would not be the right name... It's OK to rename things while it's still in beta, right?

slashrsm’s picture

I'd be happy if we can leave API as it is even if we're in beta. If it really makes sense to rename it, then we can take a look at that specific case and discuss it. Maybe you can investigate this a bit more and we decide a bit later when we have more arguments for and against it.

Does this sound ok?

martin_q’s picture

Sure thing. That's a good plan.

ahoeben’s picture

Are you using a version with this (comitted) patch?
https://drupal.org/node/1645262

This provides a new "event" named "Field insert or update derivative event". I use it, and for me it works very well when I remove the file from the respective field in the node edit form, upload a new file and submit the node edit form.

martin_q’s picture

Hi ahoeben,

Yes, I do have that patch. I'm dealing with file entities rather than files that are fields on other entities (e.g. nodes). At the moment, there is nothing that deals with file entities that get updated, not even the patch that you mention. But I'm making progress at developing something to cover this situation too.

ahoeben’s picture

My bad, missed the part about file entities

martin_q’s picture

Status: Active » Needs review
FileSize
21.27 KB

Here is the patch.

To assist with documentation for this, I've included an appropriate addition to media_derivatives.api.php file, and tidied it up a little bit at the same time (mainly, converting 'derivate' to 'derivative' throughout).

NB, this patch depends on a function created in the patch at #2120297: Refactoring a few functions - comment #1.

martin_q’s picture

Issue summary: View changes

(Edited after updating file_entity module to latest unstable version, which fixed the other bug I mentioned in passing.)