From 9bd4047f3c9130efba327797fc55e017ebb8e5c5 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Wed, 4 Sep 2024 20:47:03 +0800 Subject: [PATCH 01/10] performance improvements --- src/services/Carts.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/Carts.php b/src/services/Carts.php index 72af206fac..4bfbcb0a5b 100644 --- a/src/services/Carts.php +++ b/src/services/Carts.php @@ -99,7 +99,7 @@ public function init() /** * Get the current cart for this session. * - * @param bool $forceSave Force the cart to save when requesting it. + * @param bool $forceSave Force the cart to save if it has no ID. * @throws ElementNotFoundException * @throws Exception * @throws Throwable @@ -163,7 +163,7 @@ public function getCart(bool $forceSave = false): Order $hasSomethingChangedOnCart = ($hasIpChanged || $hasOrderLanguageChanged || $hasUserChanged || $hasPaymentCurrencyChanged || $hasOrderSiteIdChanged); // If the cart has already been saved (has an ID), then only save if something else changed. - if (($this->_cart->id && $hasSomethingChangedOnCart) || $forceSave) { + if (($this->_cart->id && $hasSomethingChangedOnCart) || (!$this->_cart->id && $forceSave)) { Craft::$app->getElements()->saveElement($this->_cart, false); } From dcd49a12b1845b281980819d87dafb1fb028c2c4 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Tue, 10 Sep 2024 15:42:44 +0800 Subject: [PATCH 02/10] Reapply "Performance improvement order condition on shipping" This reverts commit b92e1be1c8930abe84e7245b11d19178665e71a1. --- src/models/ShippingRule.php | 8 ++++---- src/services/ShippingMethods.php | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/models/ShippingRule.php b/src/models/ShippingRule.php index b6c9388b74..78dc9aa8c6 100644 --- a/src/models/ShippingRule.php +++ b/src/models/ShippingRule.php @@ -229,8 +229,9 @@ function($attribute) { if (!$order) { $order = new Order(); } + $orderAsArray = Plugin::getInstance()->getShippingMethods()->getSerializedOrderForMatchingRules($order); $orderConditionParams = [ - 'order' => $order->toArray([], ['lineItems.snapshot', 'shippingAddress', 'billingAddress']), + 'order' => $orderAsArray, ]; if (!Plugin::getInstance()->getFormulas()->validateConditionSyntax($this->{$attribute}, $orderConditionParams)) { $this->addError($attribute, Craft::t('commerce', 'Invalid order condition syntax.')); @@ -273,10 +274,9 @@ public function matchOrder(Order $order): bool $lineItems = $order->getLineItems(); if ($this->orderConditionFormula) { - $fieldsAsArray = $order->getSerializedFieldValues(); - $orderAsArray = $order->toArray([], ['lineItems.snapshot', 'shippingAddress', 'billingAddress']); + $orderAsArray = Plugin::getInstance()->getShippingMethods()->getSerializedOrderForMatchingRules($order); $orderConditionParams = [ - 'order' => array_merge($orderAsArray, $fieldsAsArray), + 'order' => $orderAsArray, ]; if (!Plugin::getInstance()->getFormulas()->evaluateCondition($this->orderConditionFormula, $orderConditionParams, 'Evaluate Shipping Rule Order Condition Formula')) { return false; diff --git a/src/services/ShippingMethods.php b/src/services/ShippingMethods.php index 77d2aa294e..dbb880dad6 100644 --- a/src/services/ShippingMethods.php +++ b/src/services/ShippingMethods.php @@ -59,6 +59,11 @@ class ShippingMethods extends Component */ private ?array $_allShippingMethods = null; + /** + * @var array + */ + private array $_serializedOrdersByNumber = []; + /** * Returns the Commerce managed shipping methods stored in the database. * @@ -141,9 +146,31 @@ public function getMatchingShippingMethods(Order $order): array $shippingMethods[$method->getHandle()] = $method; // Keep the key being the handle of the method for front-end use. } + // Clear the memoized data so next time we watch to match rules, we get fresh data. + $this->_serializedOrdersByNumber = []; + return $shippingMethods; } + /** + * Creates an order as an array for matching rules. + * We do this centrally here so that we can clear the memoized data centrally. + * + * @param Order $order + * @return array + */ + public function getSerializedOrderForMatchingRules(Order $order): array + { + if (isset($this->_serializedOrdersByNumber[$order->number])) { + return $this->_serializedOrdersByNumber[$order->number]; + } + + $fieldsAsArray = $order->getSerializedFieldValues(); + $orderAsArray = $order->toArray([], ['lineItems.snapshot', 'shippingAddress', 'billingAddress']); + $this->_serializedOrdersByNumber[$order->number] = array_merge($orderAsArray, $fieldsAsArray); + return $this->_serializedOrdersByNumber[$order->number]; + } + /** * Get a matching shipping rule for Order and shipping method. * From 165d875a106451efa06b6aadc096ce79b2c95f2e Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Tue, 10 Sep 2024 16:08:42 +0800 Subject: [PATCH 03/10] Improve cart performance --- CHANGELOG-WIP.md | 2 ++ src/controllers/CartController.php | 8 ++++---- src/services/Carts.php | 7 ++++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGELOG-WIP.md b/CHANGELOG-WIP.md index b89120d813..bf484b752a 100644 --- a/CHANGELOG-WIP.md +++ b/CHANGELOG-WIP.md @@ -1,3 +1,5 @@ # Release Notes for Craft Commerce 4.7 (WIP) ## Unreleased + +- Improved the performance of shipping rule matching when an order condition formula is used. ([3653](https://github.com/craftcms/commerce/pull/3653)) \ No newline at end of file diff --git a/src/controllers/CartController.php b/src/controllers/CartController.php index c4ec3d6a69..8fe3dce194 100644 --- a/src/controllers/CartController.php +++ b/src/controllers/CartController.php @@ -135,7 +135,7 @@ public function actionUpdateCart(): ?Response // Get the cart from the request or from the session. // When we are about to update the cart, we consider it a real cart at this point, and want to actually create it in the DB. - $this->_cart = $this->_getCart(true); + $this->_cart = $this->_getCart(saveIfNew: true); // Can clear line items when updating the cart $clearLineItems = $this->request->getParam('clearLineItems'); @@ -572,7 +572,7 @@ private function _returnCart(): ?Response * @throws NotFoundHttpException * @throws Throwable */ - private function _getCart(bool $forceSave = false): Order + private function _getCart(bool $forceSave = false, bool $saveIfNew = false): Order { $orderNumber = $this->request->getBodyParam('number'); @@ -590,7 +590,7 @@ private function _getCart(bool $forceSave = false): Order $requestForceSave = (bool)$this->request->getBodyParam('forceSave'); $doForceSave = ($requestForceSave || $forceSave); - return Plugin::getInstance()->getCarts()->getCart($doForceSave); + return Plugin::getInstance()->getCarts()->getCart(forceSave: $doForceSave, saveIfNew: $saveIfNew); } /** @@ -685,7 +685,7 @@ private function _setAddresses(): void $this->_cart->sourceBillingAddressId = $billingAddressId; /** @var Address $cartBillingAddress */ - $cartBillingAddress = Craft::$app->getElements()->duplicateElement($userBillingAddress, [ + $cartBillingAddress = Craft::$app->getElements()->duplicateElement($userBillingAddress, [ 'owner' => $this->_cart, ]); $this->_cart->setBillingAddress($cartBillingAddress); diff --git a/src/services/Carts.php b/src/services/Carts.php index 4bfbcb0a5b..3975b945cf 100644 --- a/src/services/Carts.php +++ b/src/services/Carts.php @@ -99,12 +99,13 @@ public function init() /** * Get the current cart for this session. * - * @param bool $forceSave Force the cart to save if it has no ID. + * @param bool $forceSave Force the cart. + * @param bool $saveIfNew Only saves the cart if it has no ID. * @throws ElementNotFoundException * @throws Exception * @throws Throwable */ - public function getCart(bool $forceSave = false): Order + public function getCart(bool $forceSave = false, bool $saveIfNew = false): Order { $this->_getCartCount++; //useful when debugging $currentUser = Craft::$app->getUser()->getIdentity(); @@ -163,7 +164,7 @@ public function getCart(bool $forceSave = false): Order $hasSomethingChangedOnCart = ($hasIpChanged || $hasOrderLanguageChanged || $hasUserChanged || $hasPaymentCurrencyChanged || $hasOrderSiteIdChanged); // If the cart has already been saved (has an ID), then only save if something else changed. - if (($this->_cart->id && $hasSomethingChangedOnCart) || (!$this->_cart->id && $forceSave)) { + if (($this->_cart->id && $hasSomethingChangedOnCart) || $forceSave || (!$this->_cart->id && $saveIfNew)) { Craft::$app->getElements()->saveElement($this->_cart, false); } From ba3e1f81e9d5f485309107133a5edff4a2ea45b4 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Tue, 10 Sep 2024 16:09:15 +0800 Subject: [PATCH 04/10] Release notes --- CHANGELOG-WIP.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG-WIP.md b/CHANGELOG-WIP.md index bf484b752a..2d8482999f 100644 --- a/CHANGELOG-WIP.md +++ b/CHANGELOG-WIP.md @@ -2,4 +2,5 @@ ## Unreleased +- Improved the performance of adding to the cart. - Improved the performance of shipping rule matching when an order condition formula is used. ([3653](https://github.com/craftcms/commerce/pull/3653)) \ No newline at end of file From 20323dc256e076b02c150d32d7eeb72bacdfad8f Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Tue, 10 Sep 2024 16:32:30 +0800 Subject: [PATCH 05/10] Save if new in cart --- src/controllers/CartController.php | 12 +++++++++--- src/services/Carts.php | 5 ++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/controllers/CartController.php b/src/controllers/CartController.php index 8fe3dce194..1ec49fed5a 100644 --- a/src/controllers/CartController.php +++ b/src/controllers/CartController.php @@ -135,7 +135,7 @@ public function actionUpdateCart(): ?Response // Get the cart from the request or from the session. // When we are about to update the cart, we consider it a real cart at this point, and want to actually create it in the DB. - $this->_cart = $this->_getCart(saveIfNew: true); + $this->_cart = $this->_getCart(); // Can clear line items when updating the cart $clearLineItems = $this->request->getParam('clearLineItems'); @@ -572,7 +572,7 @@ private function _returnCart(): ?Response * @throws NotFoundHttpException * @throws Throwable */ - private function _getCart(bool $forceSave = false, bool $saveIfNew = false): Order + private function _getCart(bool $forceSave = false): Order { $orderNumber = $this->request->getBodyParam('number'); @@ -590,7 +590,13 @@ private function _getCart(bool $forceSave = false, bool $saveIfNew = false): Ord $requestForceSave = (bool)$this->request->getBodyParam('forceSave'); $doForceSave = ($requestForceSave || $forceSave); - return Plugin::getInstance()->getCarts()->getCart(forceSave: $doForceSave, saveIfNew: $saveIfNew); + $cart = Plugin::getInstance()->getCarts()->getCart(forceSave: $doForceSave); + + if (!$cart->id) { + Craft::$app->getElements()->saveElement($this->_cart, false); + } + + return $cart; } /** diff --git a/src/services/Carts.php b/src/services/Carts.php index 3975b945cf..f67cdcabaa 100644 --- a/src/services/Carts.php +++ b/src/services/Carts.php @@ -100,12 +100,11 @@ public function init() * Get the current cart for this session. * * @param bool $forceSave Force the cart. - * @param bool $saveIfNew Only saves the cart if it has no ID. * @throws ElementNotFoundException * @throws Exception * @throws Throwable */ - public function getCart(bool $forceSave = false, bool $saveIfNew = false): Order + public function getCart(bool $forceSave = false): Order { $this->_getCartCount++; //useful when debugging $currentUser = Craft::$app->getUser()->getIdentity(); @@ -164,7 +163,7 @@ public function getCart(bool $forceSave = false, bool $saveIfNew = false): Order $hasSomethingChangedOnCart = ($hasIpChanged || $hasOrderLanguageChanged || $hasUserChanged || $hasPaymentCurrencyChanged || $hasOrderSiteIdChanged); // If the cart has already been saved (has an ID), then only save if something else changed. - if (($this->_cart->id && $hasSomethingChangedOnCart) || $forceSave || (!$this->_cart->id && $saveIfNew)) { + if (($this->_cart->id && $hasSomethingChangedOnCart) || $forceSave) { Craft::$app->getElements()->saveElement($this->_cart, false); } From c9f40f2bce6a88ae9db3ddcc4329e23183b873a7 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Tue, 10 Sep 2024 16:33:48 +0800 Subject: [PATCH 06/10] no need to save if it was forced saved --- src/controllers/CartController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controllers/CartController.php b/src/controllers/CartController.php index 1ec49fed5a..b2944517b9 100644 --- a/src/controllers/CartController.php +++ b/src/controllers/CartController.php @@ -590,9 +590,9 @@ private function _getCart(bool $forceSave = false): Order $requestForceSave = (bool)$this->request->getBodyParam('forceSave'); $doForceSave = ($requestForceSave || $forceSave); - $cart = Plugin::getInstance()->getCarts()->getCart(forceSave: $doForceSave); + $cart = Plugin::getInstance()->getCarts()->getCart($doForceSave); - if (!$cart->id) { + if (!$cart->id && !$doForceSave) { Craft::$app->getElements()->saveElement($this->_cart, false); } From 5daaf25af15ecf7dc1bfbaf921aad7d1f4e1376c Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Tue, 10 Sep 2024 16:34:39 +0800 Subject: [PATCH 07/10] No need --- src/controllers/CartController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/CartController.php b/src/controllers/CartController.php index b2944517b9..6102232404 100644 --- a/src/controllers/CartController.php +++ b/src/controllers/CartController.php @@ -592,7 +592,7 @@ private function _getCart(bool $forceSave = false): Order $cart = Plugin::getInstance()->getCarts()->getCart($doForceSave); - if (!$cart->id && !$doForceSave) { + if (!$cart->id) { Craft::$app->getElements()->saveElement($this->_cart, false); } From f2d2c1021c88c2ddf0b7acb2ef5a20b72fb9ee52 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Mon, 30 Sep 2024 22:35:18 +0800 Subject: [PATCH 08/10] Fix php error --- src/controllers/CartController.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/controllers/CartController.php b/src/controllers/CartController.php index 6102232404..2c6b805254 100644 --- a/src/controllers/CartController.php +++ b/src/controllers/CartController.php @@ -580,7 +580,7 @@ private function _getCart(bool $forceSave = false): Order // Get the cart from the order number $cart = Order::find()->number($orderNumber)->isCompleted(false)->one(); - if (!$cart) { + if ($cart === null) { throw new NotFoundHttpException('Cart not found'); } @@ -590,13 +590,13 @@ private function _getCart(bool $forceSave = false): Order $requestForceSave = (bool)$this->request->getBodyParam('forceSave'); $doForceSave = ($requestForceSave || $forceSave); - $cart = Plugin::getInstance()->getCarts()->getCart($doForceSave); + $this->_cart = Plugin::getInstance()->getCarts()->getCart($doForceSave); - if (!$cart->id) { + if (!$this->_cart->id) { Craft::$app->getElements()->saveElement($this->_cart, false); } - return $cart; + return $this->_cart; } /** From 9bfe91a26bd27faf40ddd59cf90e52be6e4a3cf4 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Wed, 9 Oct 2024 17:25:26 +0800 Subject: [PATCH 09/10] Test --- src/controllers/CartController.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/controllers/CartController.php b/src/controllers/CartController.php index 2c6b805254..62a22edaab 100644 --- a/src/controllers/CartController.php +++ b/src/controllers/CartController.php @@ -592,10 +592,6 @@ private function _getCart(bool $forceSave = false): Order $this->_cart = Plugin::getInstance()->getCarts()->getCart($doForceSave); - if (!$this->_cart->id) { - Craft::$app->getElements()->saveElement($this->_cart, false); - } - return $this->_cart; } From 25402393f70d13b577c4ec1947676dec7617eef8 Mon Sep 17 00:00:00 2001 From: Luke Holder Date: Wed, 9 Oct 2024 17:43:53 +0800 Subject: [PATCH 10/10] Add since tag --- src/services/ShippingMethods.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/ShippingMethods.php b/src/services/ShippingMethods.php index dbb880dad6..397ca760ff 100644 --- a/src/services/ShippingMethods.php +++ b/src/services/ShippingMethods.php @@ -158,6 +158,7 @@ public function getMatchingShippingMethods(Order $order): array * * @param Order $order * @return array + * @since 4.7.0 */ public function getSerializedOrderForMatchingRules(Order $order): array {