When media folder term is being removed, the hook_taxonomy_term_remove is called. But it is called AFTER db query execution, so db actually doesn't contain data about hierarchy and other things. Also, we can't rely on cache, which is persists.

Media Browser Plus implementation of hook_taxonomy_term_remove function stack:

media_browser_plus_taxonomy_term_delete
media_browser_plus_construct_dir_path
taxonomy_get_parents_all
taxonomy_get_parents

The last one contains db query, based on taxonomy_term_data and taxonomy_term_hierarchy tables.

So paths, created by media_browser_plus_construct_dir_path are incorrect:

before delete:
media/shou_programmy/vedushchie
generated path inside hook:
media/vedushchie

before delete:
media/uslugi/vizazh/nyu-york_testovaya_galereya
generated path inside hook:
media/nyu-york_testovaya_galereya

As result physical folders are never being deleted.

In my cron routine, which removes media folder terms with no files, i'm invoking hook_taxonomy_term_remove before actual term delete

function MYMODULE_cron() {
  $vocabulary = taxonomy_vocabulary_machine_name_load('media_folders');
  $terms = entity_load('taxonomy_term', FALSE, array('vid' => $vocabulary->vid));
  $root = variable_get('media_browser_plus_root_folder_tid');
  unset($terms[$root]);
  foreach ($terms as $term) {
    $children = taxonomy_get_children($term->tid, $vocabulary->vid);
    if ($children) continue;
    $file_query = new EntityFieldQuery();
    $files = $file_query
      ->entityCondition('entity_type', 'file')
      ->fieldCondition('field_folder', 'tid', $term->tid)
      ->execute();
    if ($files || (isset($files['file']) && $files['file'])) continue;
    module_invoke('media_browser_plus', 'taxonomy_term_delete', $term);
    taxonomy_term_delete($term->tid);
  }
}

Comments

player259’s picture

Title: Delete term related folder » Delete term related physical folder
das-peter’s picture

Status: Active » Fixed

Wow, that's a nasty one.
The issue is indeed that hook_taxonomy_term_delete() is invoked after all data are removed from the db. I actually think this is core bug and we should patch to locations where this happens - so far I've found comment and taxonomy to do so. Nodes and user seem to invoke the hook before actually removing the data.

The reason we experience the issue only occasionally is that the static caches are still valid, at least if they were build before the hook is invoked. This is the case for all taxonomy terms with children.

For now I've introduced an new form submit handler for the taxonomy term deletion confirm form. The submit handler ensures the static caches are built so that we can built the appropriate path.
Further I've created a test to specifically test this scenario.

And while dealing with this issue I found another one related to folder term creation and fixed that too.
Commit: http://drupalcode.org/project/media_browser_plus.git/commit/18fc2c10b526...

player259’s picture

If I want to delete term programmatically via taxonomy_term_delete(), I should build static cache manually, right?

Code snippet will be look like this

taxonomy_get_parents($term->tid);
taxonomy_get_parents_all($term->tid);
taxonomy_term_delete($term->tid);
das-peter’s picture

If I want to delete term programmatically via taxonomy_term_delete(), I should build static cache manually, right?

That's correct.

Status: Fixed » Closed (fixed)

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

jmev’s picture

Status: Closed (fixed) » Active
Issue tags: +osx, +directory, +files, +taxonomy

player259, thanks for posting that. I wasn't able to get the folder delete even with this. I'm using a Mac and whenever there's a .DS_Store file in the directory, it can't be removed. I added an unlink of my own in my module that solves this for that specific file, but any chance there's a better way? If not, I'm thinking this check could/should be done in the module itself, in this manner (replacing line 695 of):

          if (!@drupal_rmdir($folder_path)) {
            if(basename($folder_path == '.DS_Store'){
              drupal_unlink($folder_path);
            }else{
              drupal_set_message(t('Unable to delete the folder (!path) on the disk', array('!path' => $folder_path)), 'error');
            }
          }

Thoughts?

das-peter’s picture

Status: Active » Closed (fixed)

@jmev Please create an own issue for your specific problem. This issue here is closed and had nothing to do with (OS specific) leftover files.