While working on #2037405: Reacting to file replacement, I spotted some scope for refactoring, some of which is needed for my patch in that issue. The patch for this issue does the following:

(a) refactors media_derivatives_file_load() to use a single db_merge() rather than db_select() plus PHP logic that decides whether to do db_insert() or db_update().

(b) renames _media_derivatives_delete_derivatives() to _media_derivatives_recursively_delete_derivatives() for clarity. The two parts for deleting managed and unmanaged files are abstracted out into separate functions _media_derivatives_delete_managed() and _media_derivatives_delete_unmanaged().

(c) _media_derivatives_delete_unmanaged() is also used by a streamlined media_derivatives_delete_unmanaged(), though I note that this latter function is not actually used anywhere in the module itself.

(d) _media_derivatives_delete_managed() will also be used by media_derivatives_delete_derivative() which I had to create for #2037405: Reacting to file replacement.

(e) Removes confusing reuse of the same variable name from media_derivatives_inform_file_object(). The arguments are changed from ($file, $derivative) to ($file, $derivative_mdid) because $derivative here is not an object but just an mdid value. The second, unrelated, use of $derivative is renamed to $derivative_query, because here it is a query object and we call its fetchField() function.

CommentFileSizeAuthor
#1 media_derivatives.code_.2120297-1.patch10.13 KBmartin_q
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin_q’s picture

Status: Active » Needs review
FileSize
10.13 KB

Patch attached. These changes are all just suggestions, though #2037405: Reacting to file replacement depends on _media_derivatives_delete_managed().