From a779d18c61f8a109534be331b6742b40d1926886 Mon Sep 17 00:00:00 2001
From: Reinhold Kainhofer <reinhold@kainhofer.com>
Date: Thu, 13 Nov 2014 19:09:07 +0100
Subject: [PATCH] Many optimizations and restructurings
Propapate the plugin to the rules
Make many functions protected in the rules
Cache the rules and the match for each method
Evaluate each rule only once
Prepare for scpoding functions and plugin API
---
rules_shipping_advanced.php | 13 +--
rules_shipping_base.php | 207 ++++++++++++++++++++++--------------
2 files changed, 136 insertions(+), 84 deletions(-)
diff --git a/rules_shipping_advanced.php b/rules_shipping_advanced.php
index 246602d..23142a0 100644
--- a/rules_shipping_advanced.php
+++ b/rules_shipping_advanced.php
@@ -56,18 +56,18 @@ class plgVmShipmentRules_Shipping_Advanced extends plgVmShipmentRules_Shipping_B
parent::__construct ($subject, $config);
}
protected function createMethodRule ($r, $countries, $tax) {
- return new ShippingRule_Advanced ($r, $countries, $tax);
+ return new ShippingRule_Advanced ($this, $r, $countries, $tax);
}
/** Allow child classes to add additional variables for the rules
*/
protected function addCustomCartValues (VirtueMartCart $cart, $cart_prices, &$values) {
$values['coupon'] = $cart->couponCode;
- if ($values['zip']) {
- $zip=strtoupper($values['zip']);
- }
// Postal code Check for UK postal codes: Use regexp to determine if ZIP structure matches and also to extract the parts.
// Also handle UK overseas areas/islands that use four-letter outward codes rather than "A{1,2}0{1,2}A{0,1} 0AA"
+ if ($values['zip']) {
+ $zip=strtoupper($values['zip']);
+ }
if (isset($zip) and preg_match('/^\s*(([A-Z]{1,2})(\d{1,2})([A-Z]?)|[A-Z]{4}|GIR)\s*(\d[A-Z]{2})\s*$/', $zip, $match)) {
$values['uk_outward'] = $match[1];
$values['uk_area'] = $match[2];
@@ -104,8 +104,8 @@ class plgVmShipmentRules_Shipping_Advanced extends plgVmShipmentRules_Shipping_B
/** Extend the shipping rules by allowing arbitrary mathematical expressions
*/
class ShippingRule_Advanced extends ShippingRule {
- function __construct ($rule, $countries, $tax_id) {
- parent::__construct ($rule, $countries, $tax_id);
+ function __construct ($method, $rule, $countries, $tax_id) {
+ parent::__construct ($method, $rule, $countries, $tax_id);
}
function tokenize_expression ($expression) {
@@ -407,6 +407,7 @@ class ShippingRule_Advanced extends ShippingRule {
} else {
// Terms without comparisons or assignments are shipping cost expressions
$this->shipping = $res;
+ $this->ruletype = 'shipping';
$this->includes_tax = False;
}
// JFactory::getApplication()->enqueueMessage("<pre>Rule part '$rulepart' parsed into (condition=".print_r($is_condition,1).", assignment=".print_r($is_assignment,1)."): ".print_r($res,1)."</pre>", 'error');
diff --git a/rules_shipping_base.php b/rules_shipping_base.php
index ab8a9a7..1771c26 100644
--- a/rules_shipping_base.php
+++ b/rules_shipping_base.php
@@ -39,11 +39,17 @@ function is_equal($a, $b) {
return $a == $b;
}
}
+
+
/** Shipping costs according to general rules.
* Supported Variables: Weight, ZIP, Amount, Products (1 for each product, even if multiple ordered), Articles
* Assignable variables: Shipping, Name
*/
class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
+ // Store the parsed and possibly evaluated rules for each method (method ID is used as key)
+ protected $rules = array();
+ protected $match = array();
+ var $custom_functions = array ();
/**
* @param object $subject
@@ -234,43 +240,41 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
protected function evaluateMethodRules ($cart, $method, $cart_prices) {
// $method->match will cache the matched rule and the modifiers
- if ($method->evaluated) {
- return $method->match;
+ if (isset($this->match[$method->virtuemart_shipmentmethod_id])) {
+ return $this->match[$method->virtuemart_shipmentmethod_id];
} else {
- $method->evaluated = True;
- $method->match = Null;
// Evaluate all rules and find the matching ones (including modifiers and definitions!)
$result = array("rule"=>Null, "rule_name"=>"", "modifiers_add"=>array(), "modifiers_multiply"=>array());
$cartvals = $this->getCartValues ($cart, $cart->products, $method, $cart_prices);
- foreach ($method->rules as $r) {
+ foreach ($this->rules[$method->virtuemart_shipmentmethod_id] as $r) {
if ($r->matches($cartvals)) {
- switch ($r->getType() {
+ $rtype = $r->getType();
+ switch ($rtype) {
case 'shipping':
case 'shippingwithtax':
case 'noshipping':
$result["rule"] = $r;
- $result["rule_name"] = $r->getRuleName($cartvals);
- break;
- case 'extrashippingcharge':
- $result["modifiers_add"][] = $r;
+ $result["rule_name"] = $r->getRuleName();
break;
- case 'extrashippingmultiplier':
- $result["modifiers_multiply"][] = $r;
+ case 'modifiers_add':
+ case 'modifiers_multiply':
+ $result[$rtype][] = $r;
break;
case 'definition': // A definition has modified the $cartvals, but has no other effects
break;
default:
- $this->printWarning(JText::sprintf('VMSHIPMENT_RULES_UNKNOWN_TYPE', $r->rulestring));
+ $this->printWarning(JText::sprintf('VMSHIPMENT_RULES_UNKNOWN_TYPE', $r->getType(), $r->rulestring));
break;
}
}
if (!is_null($result["rule"])) {
- $method->match = $result;
+ $this->match[$method->virtuemart_shipmentmethod_id] = $result;
return $result; // <- This also breaks out of the foreach loop!
}
}
}
- // None of the rules matched, so return NULL;
+ // None of the rules matched, so return NULL, but keep the evaluated results;
+ $this->match[$method->virtuemart_shipmentmethod_id] = $result;
return NULL;
}
@@ -281,7 +285,7 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
* @return bool
*/
protected function checkConditions ($cart, $method, $cart_prices) {
- if (!isset($method->rules))
+ if (!isset($this->rules[$method->virtuemart_shipmentmethod_id]))
$this->parseMethodRules($method);
$match = $this->evaluateMethodRules ($cart, $method, $cart_prices);
if ($match) {
@@ -308,22 +312,24 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
* @return int
*/
function getCosts (VirtueMartCart $cart, $method, $cart_prices) {
- if (!isset($method->rules)) $this->parseMethodRules($method);
+ if (!isset($this->rules[$method->virtuemart_shipmentmethod_id]))
+ $this->parseMethodRules($method);
$match = $this->evaluateMethodRules ($cart, $method, $cart_prices);
if ($match) {
$r = $match["rule"];
vmdebug('Rule ' . $match["rule_name"] . ' ('.$r->rulestring.') matched.');
$method->tax_id = $r->tax_id;
+ // TODO: Shall we include the name of the modifiers, too?
$method->rule_name = $match["rule_name"];
// Final shipping costs are calculated as:
// Shipping*ExtraShippingMultiplier + ExtraShippingCharge
// with possibly multiple modifiers
- $method->cost = $r->getShippingCosts($cartvals);
+ $method->cost = $r->getShippingCosts();
foreach ($match['modifiers_multiply'] as $modifier) {
- $method->cost *= $modifier->getShippingCosts($cartvals);
+ $method->cost *= $modifier->getShippingCosts();
}
foreach ($match['modifiers_add'] as $modifier) {
- $method->cost += $modifier->getShippingCosts($cartvals);
+ $method->cost += $modifier->getShippingCosts();
}
$method->includes_tax = $r->includes_tax;
return $method->cost;
@@ -454,7 +460,7 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
}
protected function createMethodRule ($r, $countries, $tax) {
- return new ShippingRule(this, $r, $countries, $tax);
+ return new ShippingRule($this, $r, $countries, $tax);
}
private function parseMethodRule ($rulestring, $countries, $tax, &$method) {
@@ -462,12 +468,13 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
foreach ($rules1 as $r) {
// Ignore empty lines
if (empty($r)) continue;
- $method->rules[] = $this->createMethodRule ($r, $countries, $tax);
+ $this->rules[$method->virtuemart_shipmentmethod_id][] = $this->createMethodRule ($r, $countries, $tax);
}
}
protected function parseMethodRules (&$method) {
- if (!isset($method->rules)) $method->rules = array();
+ if (!isset($this->rules[$method->virtuemart_shipmentmethod_id]))
+ $this->rules[$method->virtuemart_shipmentmethod_id] = array();
$this->parseMethodRule ($method->rules1, $method->countries1, $method->tax_id1, $method);
$this->parseMethodRule ($method->rules2, $method->countries2, $method->tax_id2, $method);
$this->parseMethodRule ($method->rules3, $method->countries3, $method->tax_id3, $method);
@@ -639,7 +646,6 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
'discountamount' => 0,
'pricewithouttax' => 0,
);
-// JFactory::getApplication()->enqueueMessage("<pre>Product: ".print_r($products, 1)."</pre>", 'error');
if (!empty($cart_prices)) {
// get prices for the whole cart -> simply user the cart_prices
$data['amount'] = $cart_prices['salesPrice'];
@@ -679,7 +685,6 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
}
protected function getCartValues (VirtueMartCart $cart, $products, $method, $cart_prices) {
-// JFactory::getApplication()->enqueueMessage("<pre>getCartValues, cart=".print_r($cart, 1)."</pre>", 'error');
$address = (($cart->ST == 0 || $cart->STSameAsBT == 1) ? $cart->BT : $cart->ST);
$cartvals = array_merge (
array(
@@ -691,6 +696,7 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
// Add 'skus', 'categories', 'vendors' variables:
$this->getOrderListProperties ($cart, $products),
// Add country / state variables:
+ $this->getOrderAddress ($cart, $address),
$this->getOrderCountryState ($cart, $address),
// Add Total/Min/Max weight and dimension variables:
$this->getOrderWeights ($cart, $products, $method->weight_unit),
@@ -698,27 +704,15 @@ class plgVmShipmentRules_Shipping_Base extends vmPSPlugin {
);
// Let child classes update the $cartvals array, or add new variables
$this->addCustomCartValues($cart, $products, $cart_prices, $cartvals);
+
// Finally, call the triger of vmshipmentrules plugins to let them add/modify variables
JPluginHelper::importPlugin('vmshipmentrules');
$dispatcher = JDispatcher::getInstance();
$dispatcher->trigger('onVmShippingRulesGetCartValues',array(&$cartvals, $cart, $products, $method, $cart_prices));
-
- // Add the whole list of cart value to the values, so we can print them out as a debug statement!
- $cartvals['values_debug'] = print_r($cartvals,1);
- $cartvals['values'] = $cartvals;
+
return $cartvals;
}
- /** Filter the given array of products and return only those that belong to the categories, manufacturers,
- * vendors or products given in the $filter_conditions. The $filter_conditions is an array of the form:
- * array( 'skus'=>array(....), 'categories'=>array(1,2,3,42), 'manufacturers'=>array(77,78,83), 'vendors'=>array(1,2))
- * Notice that giving an empty array for any of the keys means "no restriction" and is exactly the same
- * as leaving out the enty altogether
- */
- function filterProducts($products, $filter_conditions) {
-
-
- }
/**
* Create the table for this plugin if it does not yet exist.
* This functions checks if the called plugin is active one.
@@ -840,8 +834,31 @@ if (class_exists ('ShippingRule')) {
return;
}
+/** Filter the given array of products and return only those that belong to the categories, manufacturers,
+ * vendors or products given in the $filter_conditions. The $filter_conditions is an array of the form:
+ * array( 'skus'=>array(....), 'categories'=>array(1,2,3,42), 'manufacturers'=>array(77,78,83), 'vendors'=>array(1,2))
+ * Notice that giving an empty array for any of the keys means "no restriction" and is exactly the same
+ * as leaving out the enty altogether
+ */
+function filterProducts($products, $filter_conditions) {
+ $result = array();
+ foreach ($products as $p) {
+ if (!empty($filter_conditions['skus']) && !in_array($p->product_sku, $filter_conditions['skus']))
+ continue;
+ if (!empty($filter_conditions['categories']) && count(array_intersect($filter_conditions['categories'], $p->product_categories))==0)
+ continue;
+ if (!empty($filter_conditions['manufacturers']) && count(array_intersect($filter_conditions['manufacturers'], $p->product_manufacturers))==0)
+ continue;
+ if (!empty($filter_conditions['vendors']) && count(array_intersect($filter_conditions['vendors'], $p->product_vendors))==0)
+ continue;
+ $result[] = $p;
+ }
+ return $result;
+}
+
+
class ShippingRule {
- var $method = Null;
+ var $plugin = Null;
var $rulestring = '';
var $name = '';
var $ruletype = '';
@@ -855,7 +872,7 @@ class ShippingRule {
var $tax_id = 0;
var $includes_tax = 0;
- function __construct ($method, $rule, $countries, $tax_id) {
+ function __construct ($plugin, $rule, $countries, $tax_id) {
if (is_array($countries)) {
$this->countries = $countries;
} elseif (!empty($countries)) {
@@ -864,17 +881,17 @@ class ShippingRule {
$this->tax_id = $tax_id;
$this->rulestring = $rule;
$this->parseRule($rule);
- $this->method=$method;
+ $this->plugin=$plugin;
}
- function parseRule($rule) {
+ protected function parseRule($rule) {
$ruleparts=explode(';', $rule);
foreach ($ruleparts as $p) {
$this->parseRulePart($p);
}
}
- function handleAssignment ($var, $value, $rulepart) {
+ protected function handleAssignment ($var, $value, $rulepart) {
switch (strtolower($var)) {
case 'name': $this->name = $value; break;
case 'shipping': $this->shipping = $value; $this->includes_tax = False; $this->ruletype='shipping'; break;
@@ -890,8 +907,7 @@ class ShippingRule {
}
}
-
- function tokenize_expression ($expression) {
+ protected function tokenize_expression ($expression) {
// First, extract all strings, delimited by quotes, then all text operators
// (OR, AND, in; but make sure we don't capture parts of words, so we need to
// use lookbehind/lookahead patterns to exclude OR following another letter
@@ -902,7 +918,7 @@ class ShippingRule {
return $atoms;
}
- function parseRulePart($rulepart) {
+ protected function parseRulePart($rulepart) {
/* In the basic version, we only split at the comparison operators and assume each term on the LHS and RHS is one variable or constant */
/* In the advanced version, all conditions and costs can be given as a full mathematical expression */
/* Both versions create an expression tree, which can be easily evaluated in evaluateTerm */
@@ -923,6 +939,7 @@ class ShippingRule {
$operators = array('<', '<=', '=', '>', '>=', '=>', '=<', '<>', '!=', '==');
if (count($atoms)==1) {
$this->shipping = $this->parseShippingTerm($atoms[0]);
+ $this->ruletype = 'shipping';
} elseif ($atoms[1]=='=') {
$this->handleAssignment ($atoms[0], $atoms[2], $rulepart);
} else {
@@ -940,7 +957,7 @@ class ShippingRule {
}
}
- function parseShippingTerm($expr) {
+ protected function parseShippingTerm($expr) {
/* In the advanced version, shipping cost can be given as a full mathematical expression */
// If the shipping term starts with a double quote, it is a string, so don't turn it into lowercase.
// All other expressions need to be turned into lowercase, because variable names are case-insensitive!
@@ -951,7 +968,7 @@ class ShippingRule {
}
}
- function evaluateComparison ($terms, $vals) {
+ protected function evaluateComparison ($terms, $vals) {
while (count($terms)>2) {
$res = false;
switch ($terms[1]) {
@@ -987,7 +1004,7 @@ class ShippingRule {
return true;
}
- function evaluateListFunction ($function, $args) {
+ protected function evaluateListFunction ($function, $args) {
# First make sure that all arguments are actually lists:
$allarrays = True;
foreach ($args as $a) {
@@ -1016,7 +1033,7 @@ class ShippingRule {
}
}
- function evaluateListContainmentFunction ($function, $args) {
+ protected function evaluateListContainmentFunction ($function, $args) {
# First make sure that the first argument is a list:
if (!is_array($args[0])) {
JFactory::getApplication()->enqueueMessage(JText::sprintf('VMSHIPMENT_RULES_EVALUATE_LISTFUNCTION_CONTAIN_ARGS', $function, $this->rulestring), 'error');
@@ -1099,12 +1116,12 @@ class ShippingRule {
// Functions with variable number of args
switch ($func) {
case "max":
- return max($args); break;
+ return max($args);
case "min":
- return min($args); break;
+ return min($args);
case "list":
case "array":
- return $args; break;
+ return $args;
// List functions:
case "length":
case "complement":
@@ -1114,19 +1131,42 @@ class ShippingRule {
case "join":
case "intersection":
case "list_equal":
- return $this->evaluateListFunction ($func, $args); break;
+ return $this->evaluateListFunction ($func, $args);
case "contains_any":
case "contains_all":
case "contains_only":
case "contains_none":
- return $this->evaluateListContainmentFunction($func, $args); break;
+ return $this->evaluateListContainmentFunction($func, $args);
+
}
+
+ // None of the built-in function
// No known function matches => print an error, return 0
JFactory::getApplication()->enqueueMessage(JText::sprintf('VMSHIPMENT_RULES_EVALUATE_UNKNOWN_FUNCTION', $function, $this->rulestring), 'error');
return 0;
}
-
- function evaluateTerm ($expr, $vals) {
+
+ protected function evaluateVariable ($expr, $vals) {
+ $varname = strtolower($expr);
+ if (array_key_exists(strtolower($expr), $vals)) {
+ return $vals[strtolower($expr)];
+ } elseif ($varname=='values') {
+ return $vals;
+ } elseif ($varname=='values_debug') {
+ return print_r($vals,1);
+ } else {
+ JFactory::getApplication()->enqueueMessage(JText::sprintf('VMSHIPMENT_RULES_EVALUATE_UNKNOWN_VALUE', $expr, $this->rulestring), 'error');
+ return null;
+ }
+ }
+
+ protected function evaluateTerm ($expr, $vals) {
+ // The scoping functions need to be handled differently, because they first need to adjust the cart variables to the filtered product list
+ // before evaluating its first argument. So even though parsing the rules handles scoping functions like any other function, their
+ // evaluation is fundamentally different and is special-cased here:
+ $scoping_functions = array("evaluate_for_categories", "evaluate_for_products", "evaluate_for_vendors", "evaluate_for_manufacturers");
+ $is_scoping = is_array($expr) && ($expr[0]=="FUNCTION") && (count($expr)>1) && in_array($expr[1], $scoping_functions);
+
if (is_null($expr)) {
return $expr;
} elseif (is_numeric ($expr)) {
@@ -1135,16 +1175,14 @@ class ShippingRule {
// Explicit strings are delimited by '...' or "..."
if (($expr[0]=='\'' || $expr[0]=='"') && ($expr[0]==substr($expr,-1)) ) {
return substr($expr,1,-1);
- } elseif (array_key_exists(strtolower($expr), $vals)) {
- return $vals[strtolower($expr)];
} else {
- JFactory::getApplication()->enqueueMessage(JText::sprintf('VMSHIPMENT_RULES_EVALUATE_UNKNOWN_VALUE', $expr, $this->rulestring), 'error');
- return null;
+ return $this->evaluateVariable($expr, $vals);
}
} elseif (is_array($expr)) {
// Operator
$op = array_shift($expr);
$args = array();
+ // First evaluate all operands and only after that apply the function / operator to the already evaluated arguments
$evaluate = true;
if ($op == "FUNCTION") {
$evaluate = false;
@@ -1153,6 +1191,7 @@ class ShippingRule {
$term = $evaluate ? ($this->evaluateTerm($e, $vals)) : $e;
if ($op == 'COMPARISON') {
// For comparisons, we only evaluate every other term (the operators are NOT evaluated!)
+ // The data format for comparisons is: array('COMPARISON', $operand1, '<', $operand2, '<=', ....)
$evaluate = !$evaluate;
}
if ($op == "FUNCTION") {
@@ -1162,6 +1201,7 @@ class ShippingRule {
$args[] = $term;
}
$res = false;
+ // Finally apply the operaton to the evaluated argument values:
switch ($op) {
// Logical operators:
case 'OR': foreach ($args as $a) { $res = ($res || $a); }; break;
@@ -1211,11 +1251,11 @@ class ShippingRule {
}
}
- function calculateShipping ($vals) {
+ protected function calculateShipping ($vals) {
return $this->evaluateTerm($this->shipping, $vals);
}
- function evaluateRule (&$vals) {
+ protected function evaluateRule (&$vals) {
if ($this->evaluated)
return; // Already evaluated
@@ -1237,34 +1277,45 @@ class ShippingRule {
}
// All conditions match
$this->match = True;
+ // Calculate the value (i.e. shipping cost or modifier)
$this->value = $this->calculateShipping($vals);
// For definitions add the variable to the vals
if ($this->ruletype=='definition') {
- $vals[$this->name] = $this->value;
+ $vals[strtolower($this->name)] = $this->value;
}
- }
- function matches(&$vals) {
- $this->evaluateRule($vals);
- return $this->match;
- }
-
- function getRuleName($vals) {
+ // Evaluate the rule name as a translatable string with variables inserted:
// Replace all {variable} tags in the name by the variables from $vals
$matches=array();
$name=JText::_($this->name);
preg_match_all('/{([A-Za-z0-9_]+)}/', $name, $matches);
foreach ($matches[1] as $m) {
- $var=strtolower($m);
- if (isset($vals[$var])) {
- $name = str_replace("{".$m."}", strval($vals[$var]), $name);
+ $val = $this->evaluateVariable($m, $vals);
+ if ($val !== null) {
+ $name = str_replace("{".$m."}", $val, $name);
}
}
- return $name;
+ $this->rulename = $name;
}
-
- function getShippingCosts($vals) {
+
+ function matches(&$vals) {
$this->evaluateRule($vals);
+ return $this->match;
+ }
+
+ function getType() {
+ return $this->ruletype;
+ }
+
+ function getRuleName() {
+ if (!$this->evaluated)
+ vmDebug('WARNING: getRuleName called without prior evaluation of the rule, e.g. by calling rule->matches(...)');
+ return $this->rulename;
+ }
+
+ function getShippingCosts() {
+ if (!$this->evaluated)
+ vmDebug('WARNING: getShippingCosts called without prior evaluation of the rule, e.g. by calling rule->matches(...)');
return $this->value;
}
--
GitLab