diff --git a/includes/commerce.controller.inc b/includes/commerce.controller.inc index 1cdd605..226e259 100644 --- a/includes/commerce.controller.inc +++ b/includes/commerce.controller.inc @@ -21,6 +21,36 @@ class DrupalCommerceEntityController extends DrupalDefaultEntityController imple protected $lockedEntities = array(); /** + * Determines whether the provided entity is locked. + * + * @param $entity + * The entity to check. + */ + public function isLocked($entity) { + return isset($this->lockedEntities[$entity->{$this->idKey}]); + } + + /** + * Determines whether the current entity type requires pessimistic locking. + */ + protected function requiresLocking() { + return isset($this->entityInfo['locking mode']) && $this->entityInfo['locking mode'] == 'pessimistic'; + } + + /** + * Checks the list of tracked locked entities, and if it's empty, commits + * the transaction in order to remove the acquired locks. + * + * The transaction is not necessarily committed immediately. Drupal will + * commit it as soon as possible given the state of the transaction stack. + */ + protected function releaseLock() { + if ($this->requiresLocking() && empty($this->lockedEntities)) { + unset($this->controllerTransaction); + } + } + + /** * Override of DrupalDefaultEntityController::buildQuery(). * * Handle pessimistic locking. @@ -28,7 +58,7 @@ class DrupalCommerceEntityController extends DrupalDefaultEntityController imple protected function buildQuery($ids, $conditions = array(), $revision_id = FALSE) { $query = parent::buildQuery($ids, $conditions, $revision_id); - if (isset($this->entityInfo['locking mode']) && $this->entityInfo['locking mode'] == 'pessimistic') { + if ($this->requiresLocking() && !empty($conditions['_lock'])) { // In pessimistic locking mode, we issue the load query with a FOR UPDATE // clause. This will block all other load queries to the loaded objects // but requires us to start a transaction. @@ -51,35 +81,22 @@ class DrupalCommerceEntityController extends DrupalDefaultEntityController imple public function resetCache(array $ids = NULL) { parent::resetCache($ids); - // Maintain the list of locked entities, so that the releaseLock() method - // can know when it's time to commit the transaction. - if (!empty($this->lockedEntities)) { - if (isset($ids)) { - foreach ($ids as $id) { - unset($this->lockedEntities[$id]); + if ($this->requiresLocking()) { + // Maintain the list of locked entities, so that the releaseLock() method + // can know when it's time to commit the transaction. + if (!empty($this->lockedEntities)) { + if (isset($ids)) { + foreach ($ids as $id) { + unset($this->lockedEntities[$id]); + } + } + else { + $this->lockedEntities = array(); } } - else { - $this->lockedEntities = array(); - } - } - - // Try to release the lock, if possible. - $this->releaseLock(); - } - /** - * Checks the list of tracked locked entities, and if it's empty, commits - * the transaction in order to remove the acquired locks. - * - * The transaction is not necessarily committed immediately. Drupal will - * commit it as soon as possible given the state of the transaction stack. - */ - protected function releaseLock() { - if (isset($this->entityInfo['locking mode']) && $this->entityInfo['locking mode'] == 'pessimistic') { - if (empty($this->lockedEntities)) { - unset($this->controllerTransaction); - } + // Try to release the lock, if possible. + $this->releaseLock(); } } @@ -124,6 +141,24 @@ class DrupalCommerceEntityController extends DrupalDefaultEntityController imple return; } + if ($this->requiresLocking()) { + // Forbid the deletion of unlocked entities. + foreach ($entities as $entity) { + $unlocked_entities = array(); + if (!isset($this->lockedEntities[$entity->{$this->idKey}])) { + $unlocked_ids[] = $entity->{$this->idKey}; + } + + if ($unlocked_ids) { + $args = array( + '@entity_type' => $this->entityType, + '@ids' => implode(', ', $unlocked_ids), + ); + throw new Exception(format_string('Cannot delete unlocked @entity_type entities: @ids.', $args)); + } + } + } + if (!isset($transaction)) { $transaction = db_transaction(); $started_transaction = TRUE; @@ -173,6 +208,17 @@ class DrupalCommerceEntityController extends DrupalDefaultEntityController imple * SAVED_NEW or SAVED_UPDATED depending on the operation performed. */ public function save($entity, DatabaseTransaction $transaction = NULL) { + if ($this->requiresLocking()) { + // Forbid the saving of unlocked entities. + if (!isset($this->lockedEntities[$entity->{$this->idKey}])) { + $args = array( + '@entity_type' => $this->entityType, + '@id' => $entity->{$this->idKey}, + ); + throw new Exception(format_string('Cannot delete unlocked @entity_type entity #@id.', $args)); + } + } + if (!isset($transaction)) { $transaction = db_transaction(); $started_transaction = TRUE; @@ -237,9 +283,11 @@ class DrupalCommerceEntityController extends DrupalDefaultEntityController imple // newly saved entity. $this->entityCache[$entity->{$this->idKey}] = $entity; - // Maintain the list of locked entities and release the lock if possible. - unset($this->lockedEntities[$entity->{$this->idKey}]); - $this->releaseLock(); + if ($this->requiresLocking()) { + // Maintain the list of locked entities and release the lock if possible. + unset($this->lockedEntities[$entity->{$this->idKey}]); + $this->releaseLock(); + } $this->invoke($op, $entity); diff --git a/modules/cart/commerce_cart.module b/modules/cart/commerce_cart.module index cb71b70..79a819f 100644 --- a/modules/cart/commerce_cart.module +++ b/modules/cart/commerce_cart.module @@ -50,6 +50,8 @@ function commerce_cart_menu() { // their own local action menu items if needed. if (module_exists('commerce_order_ui')) { $items['admin/commerce/orders/%commerce_order/edit/refresh'] = array( + // Lock the loaded order. + 'load arguments' => array(TRUE), 'title' => 'Apply pricing rules', 'description' => 'Executes the cart order refresh used to apply all current pricing rules on the front end.', 'page callback' => 'drupal_get_form', @@ -286,7 +288,7 @@ function commerce_cart_form_alter(&$form, &$form_state, $form_id) { */ function commerce_cart_checkout_form_cancel_submit($form, &$form_state) { // Update the order to the cart status. - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); $form_state['order'] = commerce_order_status_update($order, 'cart', TRUE); // Skip saving in the status update and manually save here to force a save @@ -309,7 +311,7 @@ function commerce_cart_checkout_form_cancel_submit($form, &$form_state) { */ function commerce_cart_line_item_views_form_submit($form, &$form_state) { // Reset the status of the order to cart. - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); $form_state['order'] = commerce_order_status_update($order, 'cart', TRUE); // Skip saving in the status update and manually save here to force a save @@ -616,7 +618,7 @@ function commerce_cart_user_update(&$edit, $account, $category) { $result = $query->execute(); if (!empty($result['commerce_order'])) { - foreach (commerce_order_load_multiple(array_keys($result['commerce_order'])) as $order) { + foreach (commerce_order_load_multiple(array_keys($result['commerce_order']), array(), FALSE, TRUE) as $order) { if ($order->mail != $account->mail) { $order->mail = $account->mail; commerce_order_save($order); @@ -688,6 +690,11 @@ function commerce_cart_block_view($delta) { function commerce_cart_order_can_refresh($order) { global $user; + // Unlocked orders can't be modified. + if (!commerce_order_is_locked($order)) { + return FALSE; + } + // Force the shopping cart refresh on /cart and /checkout/* paths if enabled. if (variable_get('commerce_cart_refresh_force', TRUE) && (current_path() == 'cart' || strpos(current_path(), 'checkout/') === 0)) { @@ -797,14 +804,21 @@ function theme_commerce_cart_empty_page() { /** * Loads the shopping cart order for the specified user. * + * By default, the returned order is unlocked and can only be used for + * reading purposes. Orders meant to be saved or deleted must be locked, + * which means all other attempts to load the same order will be blocked + * until the order is saved or deleted. + * * @param $uid * The uid of the customer whose cart to load. If left 0, attempts to load * an anonymous order from the session. + * @param $lock + * Whether to lock the loaded order. * * @return * The fully loaded shopping cart order or FALSE if nonexistent. */ -function commerce_cart_order_load($uid = 0) { +function commerce_cart_order_load($uid = 0, $lock = FALSE) { // Retrieve the order ID for the specified user's current shopping cart. $order_id = commerce_cart_order_id($uid); @@ -1298,7 +1312,7 @@ function commerce_cart_product_add($uid, $line_item, $combine = TRUE) { } // First attempt to load the customer's shopping cart order. - $order = commerce_cart_order_load($uid); + $order = commerce_cart_order_load($uid, TRUE); // If no order existed, create one now. if (empty($order)) { diff --git a/modules/cart/includes/commerce_cart.admin.inc b/modules/cart/includes/commerce_cart.admin.inc index bccf451..082de67 100644 --- a/modules/cart/includes/commerce_cart.admin.inc +++ b/modules/cart/includes/commerce_cart.admin.inc @@ -45,7 +45,7 @@ function commerce_cart_order_refresh_form($form, &$form_state, $order) { * Form submit callback for commerce_cart_order_refresh_form(). */ function commerce_cart_order_refresh_form_submit($form, &$form_state) { - if ($order = commerce_order_load($form_state['values']['order_id'])) { + if ($order = commerce_order_load($form_state['values']['order_id']), TRUE) { commerce_cart_order_refresh($order); drupal_set_message(t('Pricing rules have been applied and the order updated.')); $form_state['redirect'] = 'admin/commerce/orders/' . $order->order_id . '/edit'; diff --git a/modules/cart/tests/commerce_cart.test b/modules/cart/tests/commerce_cart.test index 723fa1f..ddc559c 100644 --- a/modules/cart/tests/commerce_cart.test +++ b/modules/cart/tests/commerce_cart.test @@ -510,8 +510,6 @@ class CommerceCartTestCaseAnonymousToAuthenticated extends CommerceCartTestCase // Get the order just created. $order_anonymous = reset(commerce_order_load_multiple(array(), array('uid' => $user->uid, 'status' => 'cart'), TRUE)); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); // Access to the cart and check if the product is in it. $this->drupalGet($this->getCommerceUrl('cart')); @@ -530,8 +528,6 @@ class CommerceCartTestCaseAnonymousToAuthenticated extends CommerceCartTestCase // Get the order for user just logged in. $order_authenticated = reset(commerce_order_load_multiple(array(), array('uid' => $this->store_customer->uid, 'status' => 'cart'), TRUE)); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); // Access to the cart and check if the product is in it. $this->drupalGet($this->getCommerceUrl('cart')); diff --git a/modules/checkout/commerce_checkout.module b/modules/checkout/commerce_checkout.module index 1f390fa..33ebd0d 100644 --- a/modules/checkout/commerce_checkout.module +++ b/modules/checkout/commerce_checkout.module @@ -87,6 +87,8 @@ function commerce_checkout_menu() { // defining their own local action menu items if needed. if (module_exists('commerce_order_ui')) { $items['admin/commerce/orders/%commerce_order/edit/checkout'] = array( + // Lock the loaded order. + 'load arguments' => array(TRUE), 'title' => 'Simulate checkout completion', 'description' => 'Directly invokes the checkout completion rules on the order.', 'page callback' => 'drupal_get_form', @@ -263,7 +265,7 @@ function commerce_checkout_form_alter(&$form, &$form_state, $form_id) { * Submit handler used to redirect to the checkout page. */ function commerce_checkout_line_item_views_form_submit($form, &$form_state) { - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); // Set the order status to the first checkout page's status. $order_state = commerce_order_state_load('checkout'); diff --git a/modules/checkout/includes/commerce_checkout.admin.inc b/modules/checkout/includes/commerce_checkout.admin.inc index 3c17859..925f9ea 100644 --- a/modules/checkout/includes/commerce_checkout.admin.inc +++ b/modules/checkout/includes/commerce_checkout.admin.inc @@ -438,7 +438,7 @@ function commerce_checkout_complete_form($form, &$form_state, $order) { * Form submit callback for commerce_checkout_complete_form(). */ function commerce_checkout_complete_form_submit($form, &$form_state) { - if ($order = commerce_order_load($form_state['values']['order_id'])) { + if ($order = commerce_order_load($form_state['values']['order_id'], TRUE)) { commerce_checkout_complete($order); drupal_set_message(t('Checkout completion rules have been executed for the order.')); $form_state['redirect'] = 'admin/commerce/orders/' . $order->order_id . '/edit'; diff --git a/modules/checkout/includes/commerce_checkout.pages.inc b/modules/checkout/includes/commerce_checkout.pages.inc index 6bbd013..8237994 100644 --- a/modules/checkout/includes/commerce_checkout.pages.inc +++ b/modules/checkout/includes/commerce_checkout.pages.inc @@ -232,7 +232,7 @@ function commerce_checkout_form_validate($form, &$form_state) { $checkout_page = $form_state['checkout_page']; // Load a fresh copy of the order stored in the form. - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); // Catch and clear already pushed messages. $previous_messages = drupal_get_messages(); @@ -312,7 +312,7 @@ function commerce_checkout_form_submit($form, &$form_state) { $checkout_page = $form_state['checkout_page']; // Load a fresh copy of the order stored in the form. - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); // If we are going to redirect with checkout pane messages stored in the form // state, they will not be displayed on a subsequent form build like normal. @@ -348,7 +348,7 @@ function commerce_checkout_form_submit($form, &$form_state) { function commerce_checkout_form_back_submit($form, &$form_state) { // If there is a previous page... if ($previous_page = commerce_checkout_page_load($form_state['checkout_page']['prev_page'])) { - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); // Move the form back to that page. if ($previous_page['prev_page']) { @@ -367,7 +367,7 @@ function commerce_checkout_form_back_submit($form, &$form_state) { * Special submit handler for the cancel button to avoid processing orders. */ function commerce_checkout_form_cancel_submit($form, &$form_state) { - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); // Set the order status back to the first checkout page's status. $order_state = commerce_order_state_load('checkout'); diff --git a/modules/checkout/tests/commerce_checkout.test b/modules/checkout/tests/commerce_checkout.test index 20a99a2..7e9b69e 100644 --- a/modules/checkout/tests/commerce_checkout.test +++ b/modules/checkout/tests/commerce_checkout.test @@ -77,8 +77,6 @@ class CommerceCheckoutTestProcess extends CommerceBaseTestCase { // Get the order for the anonymous user. $this->order = reset(commerce_order_load_multiple(array(), array('uid' => $user->uid, 'status' => 'cart'), TRUE)); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); } /** @@ -162,8 +160,6 @@ class CommerceCheckoutTestProcess extends CommerceBaseTestCase { // Load the order to check the status. $order = commerce_order_load_multiple(array($this->order->order_id), array(), TRUE); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); // At this point we should be in Checkout Review. $this->assertEqual(reset($order)->status, 'checkout_review', t('Order status is \'Checkout Review\' in the review phase.')); @@ -324,8 +320,6 @@ class CommerceCheckoutTestProcess extends CommerceBaseTestCase { // Load the order to check the status. $order = commerce_order_load_multiple(array($this->order->order_id), array(), TRUE); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); // At this point we should be in Checkout Review. $this->assertEqual(reset($order)->status, 'checkout_review', t('Order status is \'Checkout Review\' in the review phase.')); diff --git a/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_delete.inc b/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_delete.inc index ccd4b0e..cbe3da8 100644 --- a/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_delete.inc +++ b/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_delete.inc @@ -53,7 +53,7 @@ class commerce_line_item_handler_field_edit_delete extends views_handler_field { } function views_form_submit($form, &$form_state) { - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); $field_name = $this->options['id']; foreach (element_children($form[$field_name]) as $row_id) { diff --git a/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_quantity.inc b/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_quantity.inc index be9fe55..c078527 100644 --- a/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_quantity.inc +++ b/modules/line_item/includes/views/handlers/commerce_line_item_handler_field_edit_quantity.inc @@ -102,7 +102,7 @@ class commerce_line_item_handler_field_edit_quantity extends views_handler_field // Process the deletes first. foreach ($deleted_line_items as $line_item_id) { - $order = commerce_order_load($form_state['order']->order_id); + $order = commerce_order_load($form_state['order']->order_id, TRUE); commerce_cart_order_product_line_item_delete($order, $line_item_id); } diff --git a/modules/order/commerce_order.install b/modules/order/commerce_order.install index 4aa42ec..ed14103 100644 --- a/modules/order/commerce_order.install +++ b/modules/order/commerce_order.install @@ -340,7 +340,7 @@ function commerce_order_update_7102(&$sandbox) { // Loop over the orders, loading and resaving each one. foreach ($orders as $order) { - $order = commerce_order_load($order->order_id); + $order = commerce_order_load($order->order_id, TRUE); // Save the order as a new revision with an update log message. $order->revision = TRUE; diff --git a/modules/order/commerce_order.module b/modules/order/commerce_order.module index 7bbe0e8..7c54326 100644 --- a/modules/order/commerce_order.module +++ b/modules/order/commerce_order.module @@ -746,24 +746,43 @@ function commerce_order_save($order) { /** * Loads an order by ID. + * + * By default, the returned order is unlocked and can only be used for + * reading purposes. Orders meant to be saved or deleted must be locked, + * which means all other attempts to load the same order will be blocked + * until the order is saved or deleted. + * + * @param $lock + * Whether to lock the loaded order. */ -function commerce_order_load($order_id) { - $orders = commerce_order_load_multiple(array($order_id), array()); +function commerce_order_load($order_id, $lock = FALSE) { + $orders = commerce_order_load_multiple(array($order_id), array(), FALSE, $lock); return $orders ? reset($orders) : FALSE; } /** * Loads an order by number. + * + * By default, the returned order is unlocked and can only be used for + * reading purposes. Orders meant to be saved or deleted must be locked, + * which means all other attempts to load the same order will be blocked + * until the order is saved or deleted. + * + * @param $lock + * Whether to lock the loaded order. */ -function commerce_order_load_by_number($order_number) { - $orders = commerce_order_load_multiple(array(), array('order_number' => $order_number)); +function commerce_order_load_by_number($order_number, $lock = FALSE) { + $orders = commerce_order_load_multiple(array(), array('order_number' => $order_number), FALSE, $lock); return $orders ? reset($orders) : FALSE; } /** * Loads multiple orders by ID or based on a set of matching conditions. * - * @see entity_load() + * By default, the returned orders are unlocked and can only be used for + * reading purposes. Orders meant to be saved or deleted must be locked, + * which means all other attempts to load the same order will be blocked + * until the order is saved or deleted. * * @param $order_ids * An array of order IDs. @@ -774,15 +793,31 @@ function commerce_order_load_by_number($order_number) { * the $order_ids array assuming the revision_id is valid for the order_id. * @param $reset * Whether to reset the internal order loading cache. + * @param $lock + * Whether to lock the loaded order. * * @return * An array of order objects indexed by order_id. */ -function commerce_order_load_multiple($order_ids = array(), $conditions = array(), $reset = FALSE) { +function commerce_order_load_multiple($order_ids = array(), $conditions = array(), $reset = FALSE, $lock = FALSE) { + $conditions['_lock'] = $lock; return entity_load('commerce_order', $order_ids, $conditions, $reset); } /** + * Determines whether or not the given order object is locked. + * + * @param $order + * A fully loaded order object. + * + * @return + * Boolean indicating whether or not the order object is locked. + */ +function commerce_order_is_locked($order) { + return entity_get_controller('commerce_order')->isLocked($order); +} + +/** * Determines whether or not the given order object represents the latest * revision of the order. * diff --git a/modules/order/commerce_order_ui.module b/modules/order/commerce_order_ui.module index ce4eaaa..cf7e658 100644 --- a/modules/order/commerce_order_ui.module +++ b/modules/order/commerce_order_ui.module @@ -48,6 +48,8 @@ function commerce_order_ui_menu() { 'context' => MENU_CONTEXT_PAGE | MENU_CONTEXT_INLINE, ); $items['admin/commerce/orders/%commerce_order/edit'] = array( + // Lock the loaded order. + 'load arguments' => array(TRUE), 'title' => 'Edit', 'page callback' => 'commerce_order_ui_order_form_wrapper', 'page arguments' => array(3), @@ -59,6 +61,8 @@ function commerce_order_ui_menu() { 'file' => 'includes/commerce_order_ui.orders.inc', ); $items['admin/commerce/orders/%commerce_order/delete'] = array( + // Lock the loaded order. + 'load arguments' => array(TRUE), 'title' => 'Delete', 'page callback' => 'commerce_order_ui_order_delete_form_wrapper', 'page arguments' => array(3), diff --git a/modules/order/includes/commerce_order.forms.inc b/modules/order/includes/commerce_order.forms.inc index 4ed313d..993ce3f 100644 --- a/modules/order/includes/commerce_order.forms.inc +++ b/modules/order/includes/commerce_order.forms.inc @@ -219,7 +219,7 @@ function commerce_order_order_form_submit($form, &$form_state) { // If the user is editing an order, load a fresh copy to merge changes to. if ($form_state['commerce_order']->order_id) { - $form_state['commerce_order'] = commerce_order_load($form_state['commerce_order']->order_id); + $form_state['commerce_order'] = commerce_order_load($form_state['commerce_order']->order_id, TRUE); } // Merge changes into the order object in the form state so it is accessible diff --git a/modules/payment/commerce_payment.module b/modules/payment/commerce_payment.module index 4b5f166..88813f2 100644 --- a/modules/payment/commerce_payment.module +++ b/modules/payment/commerce_payment.module @@ -448,7 +448,7 @@ function commerce_payment_commerce_payment_transaction_insert($transaction) { // If the inserted transaction was marked as a success and placed against a // valid order... if ($transaction->status == COMMERCE_PAYMENT_STATUS_SUCCESS && - $order = commerce_order_load($transaction->order_id)) { + $order = commerce_order_load($transaction->order_id, TRUE)) { // Then check to make sure the event hasn't been invoked for this order. if (empty($order->data['commerce_payment_order_paid_in_full_invoked'])) { // Check the order balance and invoke the event. diff --git a/modules/payment/tests/commerce_payment_ui.test b/modules/payment/tests/commerce_payment_ui.test index b59e657..1c73dfb 100644 --- a/modules/payment/tests/commerce_payment_ui.test +++ b/modules/payment/tests/commerce_payment_ui.test @@ -144,8 +144,6 @@ class CommercePaymentUITest extends CommerceBaseTestCase { // Reload the order. $order = commerce_order_load_multiple(array($this->order->order_id), array(), TRUE); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); // Check order balance, it should be half of total now. $new_balance = commerce_payment_order_balance(reset($order));