hook_nodeapi() in Ubercart modules

Project: 
Ubercart
Category: 
bug report
Priority: 
critical
Status: 
fixed

This bug occurs in UC Beta 1 and all earlier versions.

hook_nodeapi() is implemented in the modules uc_cart, uc_quote, uc_flatrate, uc_ups, uc_usps, and uc_weightquote, and uc_repeater, as well as many other modules throughout Ubercart.

However, for the named modules, the type of the node is not checked at all inside the hook implementation. At best, this impacts the speed of the site because all these hook_nodeapi() implementations are run in their entirety even for non-Product nodes. At worst, these modules could corrupt the database when they manipulate non-Product nodes.

Many of these implementations have multiple database queries performed whether or not the node in question is a Product!

For example see the following code in uc_quote, in the function uc_quote_nodeapi():

<?php
   
case 'insert':
    case
'update':
     
db_query("DELETE FROM {uc_quote_product_locations} WHERE nid = %d", $node->nid);
     
db_query("INSERT INTO {uc_quote_product_locations} (nid, first_name, last_name, company, street1, street2, city, zone, postal_code, country) VALUES (%d, '%s', '%s', '%s', '%s', '%s', '%s', %d, '%s', %d)",
       
$node->nid, $node->first_name, $node->last_name, $node->company, $node->street1, $node->street2, $node->city, $node->zone, $node->postal_code, $node->country
     
);
?>

In this case, there are two DB queries run every time a node of any type is inserted or updated, with all sorts of extraneous information added to the DB keyed to non-Product nodes! (This can be quickly confirmed by examining your own DB - see all the empty rows in uc_quote_product_locations corresponding to nids for your non-Product nodes like forum posts, book pages, faq, etc.).

I suggest testing $node->type to see if it equals 'product' as the first statement in each of these hook_nodeapi() implementations.

Re: hook_nodeapi() in Ubercart modules

Aye, good idea. I'm going to recommend that we check the node type this way and have Lyle confirm it's correct before I apply it to the cart module:

<?php
 
if (in_array($node->type, array_keys(uc_product_node_info()))){
   
// Run dem queries...
 
}
?>

Re: Re: hook_nodeapi() in Ubercart modules

Yup, that's what I was going to do. Also need to check the module dependencies now to make sure they're all dependent on uc_product.

Update: Committed.