I am attempting to make the best hook_uninstall ever :D Sorry in advance for the long post.
As is the case with a lot of modules, mine makes many variables per node type it is enabled for. It would be tiresome and inefficient to delete every one of these variables with variable_del.
A common solution that appears to have its roots in Drupal 6 is db_query("DELETE FROM {variable} WHERE name LIKE 'mymodule_%'");
Or the Drupal 7 converted version, which is technically incorrect, being db_delete('variable')->condition('name', 'mymodule_%', 'LIKE')->execute();
Or the even better and correct D7 version db_delete('variable')->condition('name', db_like('mymodule_') . '%', 'LIKE')->execute();
This doesn't fully replicate variable_del though, as variable_del also flushes the cache and removes the variables from the global $conf array. Clearing the cache is simple, just add the same bit of code cache_clear_all('variables', 'cache_bootstrap');
Clearing the variables from the $conf array is a bit trickier, and there are a few methods that I can see.
- We could do a db_select before the db_delete to obtain a list of keys to remove from the $conf array. This involves 2 db queries, so probably not the most efficient (unless there is a way to combine the select and delete into 1 that I am not aware of?)
- Search through the $conf array for keys that start with 'mymodule_', much the same as the db like query. I tested a few different methods for this, which gave varying results (timed with microtime() and displayed as difference*1000).
-
$conf = array_intersect_key($conf, array_flip(preg_grep("/^mymodule_.*/", array_keys($conf), PREG_GREP_INVERT))); // Result 0.41699409484863 -
foreach($conf as $key => $value) { if (substr($key, 0, strlen('mymodule_')) === 'mymodule_') { unset($conf[$key]); } } // Result 1.2187957763672 -
foreach($conf as $key => $value) { if (0 === strpos($key, 'mymodule_')) { unset($conf[$key]); } } // Result 0.69212913513184 -
foreach($conf as $key => $value) { if (preg_match("/^mymodule_.*/", $key) === 1) { unset($conf[$key]); } } // Result 1.2669563293457
So 2.1 is the fastest, but it does a lot. 2.3 is 50% slower, but is a lot easier to understand.
-
So, here is my final code (using 2.1). My questions to those smarter than me are: will it correctly modify the global $conf (or does it need to be @$conf of something?), is it the best method, and do you have any other comments?
global $conf;
$module_string = "mymodule_";
db_delete('variable')->condition('name', db_like($module_string) . '%', 'LIKE')->execute();
cache_clear_all('variables', 'cache_bootstrap');
$conf = array_intersect_key($conf, array_flip(preg_grep("/^{$module_string}.*/", array_keys($conf), PREG_GREP_INVERT)));
Comments
Personally, I would use
Personally, I would use variable_del, and set up a few loops to handle the relevant elements. It's not good practice to bypass the APIs when they exist.
Contact me to contract me for D7 -> D10/11 migrations.
Or anything really. I'm friendly, and open to work or conversation.
And also ..
remember that variables can hold arrays as well. So it's not necessary to have 1 variable for 1 value.
True, but my module hooks
True, but my module hooks into the node type edit form, which saves each setting field you add to the form as an individual setting, suffixed with the node type. See function node_type_form_submit. So my 10xsettings per node type could easily be 40 settings. Doing a variable_del 40 times is a lot of database calls.
Doing a variable_del 40 times
Sure, but it's a page that will only ever be run once on a given server. The number of database calls per page on pages that are regularly accessed is something that should always be considered, but for a page like this, run by an admin one time, it's better to prioritize the APIs over the page execution time. You want to keep your system clean, and by bypassing the APIs, you may be orphaning elements in your system that are deleted by the API, but missed by you.
Contact me to contract me for D7 -> D10/11 migrations.
Or anything really. I'm friendly, and open to work or conversation.
OK, you've twisted my arm
OK, you've twisted my arm enough!
What would be the best way to populate a list of variables?
Use Api
Hey @neRok, @jaypan is right. If you have apis and wrappers, then use them. Otherwise what is the use of CMS.
Why not just few lines using
Why not just few lines using the API correctly?
I found next way
I found next way
That's somewhat dangerous -
That's somewhat dangerous - if any other modules happen to use forward_ as the start of their variable, you will delete those as well.
Contact me to contract me for D7 -> D10/11 migrations.
Or anything really. I'm friendly, and open to work or conversation.
It's bad practice naming
It's bad practice naming variables not started from module name. From coding standards:
Somewhat dangerous maybe only if will be other module started from "forward_". For example, forward_node, etc.
It's bad practice naming
I don't disagree, but not everyone realizes/follows best standards, and as you say, another module may also start with forward.
Contact me to contract me for D7 -> D10/11 migrations.
Or anything really. I'm friendly, and open to work or conversation.