| Project: | Domain Access |
| Version: | 6.x-2.4 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
I'm a developer of the http://drupal.org/project/icanlocalize module and I'm trying to get it to work nicely with the domain access module.
In our module, I'm creating nodes and saving them with the node_save function.
During a node save the domain_get_node_domains function can get called before the domain access module has saved its data to the database for a new node. This means the $lookup value gets initialized to the default domain even thou the database is getting set to the correct value later in the node_save process.
If the node is then re-loaded it is missing the correct domain access settings because it comes from the $lookup variable and if you re-save the node, the in-correct domain access is saved to the database.
I've changed the weight of our module to ensure that the nodeapi hook gets called after the domain access module. The problem still occurs because the menu module is trying to access the new node before domain access has saved it's data to the database.
Is it possible to change the caching in the domain_get_node_domains so that the cached value is cleared when a new value is written to the database. This would solve the problem for me.
Possibly change the $lookup variable from a static variable to a global variable. What do you think?
Comments
#1
I am a user of both icanlocalize and domain access. subscribing.
#2
Normally, internal static functions pass resets. This one doesn't have one, because I've never seen the need. Normally, node saves invoke new page loads, so the static is reset on submit.
But I understand this is happening because you are acting during hook_nodeapi() after the node is saved, but the $node->domains data is old due to the static.
This is a problem because we don't update {domain_access} until after node_save() is over, because that's when hook_node_access_records() runs. See http://api.drupal.org/api/function/node_save/6
So the static isn't necessarily the problem. The problem is the order in which data storage is performed. Allowing a reset of the static won't fix the problem during hook_nodeapi(). We would have to move the storage function (_domain_store_grants()), and to do that, we'd have to move the grants logic out of domain_node_access_records() into hook_nodeapi().
All of which I am very reluctant to do in D6.
You probably need to set the $node->domains value manually before running node_save(), if you aren't already.
#3
For me the problem is fixed if I remove the static variable in the code and add the lookup to the $_domain global variable
ie. change:
<?php
static $lookup = array();
if (isset($lookup[$nid])) {
return $lookup[$nid];
}
?>
to:
<?php
global $_domain;
if (isset($_domain['lookup'])) {
$lookup = $_domain['lookup'];
} else {
$lookup = array();
}
if (isset($lookup[$nid])) {
return $lookup[$nid];
}
?>
Then save it to the global at the end of the function:
<?php$_domain['lookup'] = $lookup;
?>
And then in _domain_store_grants we delete the lookup node from the global.
<?phpglobal $_domain;
if (isset($_domain['lookup'])) {
unset($_domain['lookup'][$nid]);
}
?>
This may not be the best solution but it ensures that the domain_get_node_domains function does return what's stored in the database.
#4
@brucepearson — can you try and see if you can set $node->domains manually, as agentrickard suggests?
@agentrickard — could there be any other implications on the functionality of DA, if I apply the changes suggested in #3 above?
Thank you both for your work towards resolving this.
#5
the $node->domains is set when I call the node_save function.
I've attached a patch for the code described in #3.
#6
I think carrying around all that data in a global is a bad idea. I'd prefer to simply remove the static.
But let me understand: The issue is that you're saving the same node multiple times during the same request?
#7
Yes, the node may be saved multiple times during the request. The request is either a cron job or an xml-rpc call.
Our module deals with translating nodes. Generally on the first pass we save a new translation or multiple translations and then on the second pass we go through the created nodes and adjust various things.
An example of the second pass is to correct links to other translated content. We can't do this until we have saved all the nodes.
#8
Can you point me to where in your module code this happens? I'd like to understand what we're trying to fix.
#9
Here's a patch that allows the static to be reset and forcibly resets it after node grants are saved. Might work.
Other options:
1) Move the reset call to hook_nodeapi() $op = 'presave'. But this would likely just reset the value to what it had been.
2) Unset the value on reset.
3) Call the function from your module.
I think #2 might work.
#10
Here's patch #2. This should reset the $node->domains lookup on presave, which means the next time it gets called, it will look for new data.
#11
Probably the best patch. But does it work?
#12
Thanks for your work on this. Unfortunately the patch doesn't work as it is. I can get it to work by resetting the lookup during the _domain_store_grants function instead of the 'presave'.
<?php
function _domain_store_grants($nid, $grants = array()) {
if ($nid > 0 && !empty($grants)) {
db_query("DELETE FROM {domain_access} WHERE nid = %d", $nid);
foreach ($grants as $grant) {
db_query("INSERT INTO {domain_access} (nid, gid, realm) VALUES (%d, %d, '%s')", $nid, $grant['gid'], $grant['realm']);
}
// Reset the static lookup for this node.
domain_get_node_domains($nid, TRUE);
}
// Ensure that our default grant is present.
domain_set_default_grant();
}
?>
I think it makes more sense to put it here because this is when the lookup table for that node is no longer valid.
#13
Works for me. I had it there in one iteration (#9). Can you re-roll it?
#14
Here's the patch.
#15
The patch in #14 works for me. Thanks for the patch.
#16
Committed to HEAD and 6--2. Thanks for the debug reports.
#17
Automatically closed -- issue fixed for 2 weeks with no activity.