render_get_rules() doesn't send plugin specific rules

On further study, it appears that the real issue occurs in render_get_rules() of the render.module.

  static $rules = array();
  
  if (!empty($rules)) {
    return $rules;
  }

  if (isset($plugin)) {
    $result = db_query("SELECT * FROM {render} WHERE plugin = '%s' ORDER BY weight", check_plain($plugin));
  }
  else {
    $result = db_query('SELECT * FROM {render} ORDER BY weight');
  }

I'm guessing that it's returning the $rules without running the query that selects only rules by $plugin. Maybe this would be a better solution? :

  static $rules = array();
  
  if (isset($plugin)) {
    $result = db_query("SELECT * FROM {render} WHERE plugin = '%s' ORDER BY weight", check_plain($plugin));
  }
  else if (empty($rules)) {
    $result = db_query('SELECT * FROM {render} ORDER BY weight');
  }
  else {
    return $rules;
  }

So I've figured out the problem, but am not sure about the solution. Declaring $rules as static inside render_get_rules() essentially sends all the rules to each plugin.

That would seem simple enough to fix, but the real problem is that render_get_rules() also sets the sitewide 'render_plugins' variable by recursing through each rule and creating an array of each plugin name, which I'm assuming is why you've set the rules static in the first place.

So, the only solution I came up with would be to declare $active_plugins static instead. Let me know what you think.

I'm not sure I understand. If by rule cache you mean the names of each plugin, then that is being stored in the static $active_plugins. If you mean something else, then it's probably beyond the scope of what I understand is happening in the function.

By my understanding the function is performing 3 separate tasks:

• if $plugin is set, return only the rules for that specific plugin.
• if $plugin isn't set, return all rules.
• use set_variable to store all the plugin types.

In my testing, all the above tasks are being accomplished with the patch. Let me know what I'm missing.

Ahh, ok, I think I understand. You are setting $rules static so that there will be minimal database calls, correct? If that's the case, then yes I can understand why the above patch would be a bad idea.

I've attached a new patch that puts each plugin's rules in $rules['plugin_name'], and also stores all rules in $rules['all']. It will return $rules['all'] unless a specific plugin's rules are requested. Let me know what you think.

The only thing missing was the early-return I outlined in #5. If you implement it the same way as it was before, we don't need extra indentation.

Well, I see that, but the problem I noticed with that method is that it's not actually storing all the rules from the initial database call, so the early return is kind of useless since the static $active_plugins variable is only storing the plugins' name, not the parameters of the each rule.

Encapsulating the database call within if (isset($rules)) means we only need one instance of this:

  if ($plugin == NULL) {
    return $rules['all'];
  }
  return $rules[$plugin];

otherwise we'd need it up at the top with the if (!isset($rules)) { and at the bottom after the database call. So that's why everything's getting the extra indentation.

Furthermore, we don't need an additional 'all' array containing all rules from all plugins, because we can iterate over the whole array where needed.

I agree that we can iterate elsewhere, but I was trying to come up with a solution that only modified the code within the render_get_rules() function.

btw: Please read http://drupal.org/patch/create (at least use -up arguments, please).

I'm sorry, I was being lazy just viewing the unsaved changes within my IDE.

Let me know how you want to proceed.

Sorry, I don't know why I wasn't sent an e-mail on #11: I didn't realize you had responded with a new patch. I don't have time to patch and test this week, but from just reading the patch, I think it looks like a good idea.

I'll test next week and get back to you..

thanks!

That patch (#11) gave me all sorts of problems. None of my rules were coming up, when I cleared the cache only one file was created, named _rules.js, there were some foreach errors as if some arrays weren't getting populated, and another error saying that the plugins had no javascript creation function.

I really think #8 is the way to go, I've been using it on a site since I first posted it and it's been working with no problems. I've re-rolled and attached it.

Thoughts?

Hi sun,

I'm sorry I haven't replied sooner. I don't have time right now to test this patch.

Thanks..