diff --git a/includes/commerce.controller.inc b/includes/commerce.controller.inc index 1cdd605..30de2d6 100644 --- a/includes/commerce.controller.inc +++ b/includes/commerce.controller.inc @@ -21,14 +21,65 @@ 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); + } + } + + /** + * Overrides DrupalDefaultEntityController::load(). + */ + public function load($ids = array(), $conditions = array()) { + // If locking has been required, then bypass the internal cache for any + // entities that are not already locked. + if (!empty($conditions['_lock']) && $this->requiresLocking()) { + foreach (array_diff($ids, $this->lockedEntities) as $id) { + unset($this->entityCache[$id]); + } + } + elseif (isset($conditions['_lock']) && !$conditions['_lock']) { + // Remove the pseudo-condition _lock if it is FALSE. + unset($conditions['_lock']); + } + return parent::load($ids, $conditions); + } + + /** * Override of DrupalDefaultEntityController::buildQuery(). * * Handle pessimistic locking. */ protected function buildQuery($ids, $conditions = array(), $revision_id = FALSE) { + $lock = !empty($conditions['_lock']); + unset($conditions['_lock']); + $query = parent::buildQuery($ids, $conditions, $revision_id); - if (isset($this->entityInfo['locking mode']) && $this->entityInfo['locking mode'] == 'pessimistic') { + if ($lock && $this->requiresLocking()) { // 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. @@ -48,41 +99,6 @@ class DrupalCommerceEntityController extends DrupalDefaultEntityController imple return $query; } - 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]); - } - } - 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); - } - } - } - /** * (Internal use) Invokes a hook on behalf of the entity. * @@ -124,6 +140,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 +207,22 @@ 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 (!empty($entity->{$this->idKey}) && !$this->isLocked($entity)) { + $args = array( + '@entity_type' => $this->entityType, + '@id' => $entity->{$this->idKey}, + ); + // Debug locking issues. + // @todo remove this when #2240427 is finally solved + if (function_exists('dpm')) { + dpm(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 6)); + } + throw new Exception(format_string('Cannot save unlocked @entity_type entity #@id', $args)); + } + } + if (!isset($transaction)) { $transaction = db_transaction(); $started_transaction = TRUE; @@ -237,9 +287,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 ced58af..25974ec 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 @@ -590,7 +592,7 @@ function commerce_cart_theme() { */ function commerce_cart_user_login(&$edit, $account) { // Get the user's anonymous shopping cart order if it exists. - if ($order = commerce_cart_order_load()) { + if ($order = commerce_cart_order_load($account->uid, TRUE)) { // Convert it to an authenticated cart. commerce_cart_order_convert($order, $account); } @@ -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, 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,20 +804,27 @@ 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); // If a valid cart order ID exists for the user, return it now. if (!empty($order_id)) { - return commerce_order_load($order_id); + return commerce_order_load($order_id, $lock); } return FALSE; @@ -1003,7 +1017,8 @@ function commerce_cart_commerce_entity_access_condition_commerce_order_alter(&$c * Converts an anonymous shopping cart order to an authenticated cart. * * @param $order - * The anonymous order to convert to an authenticated cart. + * The anonymous order to convert to an authenticated cart. The order must be + * locked (see commerce_order_load()). * @param $account * The user account the order will belong to. * @@ -1298,11 +1313,13 @@ 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)) { $order = commerce_cart_order_new($uid); + // Reload the order from the database, to enable locking. + $order = commerce_order_load($order->order_id, TRUE); $order->data['last_cart_refresh'] = REQUEST_TIME; } @@ -1453,16 +1470,12 @@ function commerce_cart_product_add_by_id($product_id, $quantity = 1, $combine = * The shopping cart order to delete from. * @param $line_item_id * The ID of the product line item to delete from the order. - * @param $skip_save - * TRUE to skip saving the order after deleting the line item; used when the - * order would otherwise be saved or to delete multiple product line items - * from the order and then save. * * @return * The order with the matching product line item deleted from the line item * reference field. */ -function commerce_cart_order_product_line_item_delete($order, $line_item_id, $skip_save = FALSE) { +function commerce_cart_order_product_line_item_delete($order, $line_item_id) { $line_item = commerce_line_item_load($line_item_id); // Check to ensure the line item exists and is a product line item. @@ -1480,13 +1493,10 @@ function commerce_cart_order_product_line_item_delete($order, $line_item_id, $sk // Invoke the product removal event with the line item about to be deleted. rules_invoke_all('commerce_cart_product_remove', $order, $product, $line_item->quantity, $line_item); - // Delete the actual line item. + // Delete the actual line item. This will in turn save the order, because the + // reference will be deleted. commerce_line_item_delete($line_item->line_item_id); - if (!$skip_save) { - commerce_order_save($order); - } - return $order; } diff --git a/modules/cart/includes/commerce_cart.admin.inc b/modules/cart/includes/commerce_cart.admin.inc index bccf451..88d069c 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..bd41fa9 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'); @@ -936,6 +938,7 @@ function commerce_checkout_order_can_checkout($order) { * Completes the checkout process for the given order. */ function commerce_checkout_complete($order) { + $order = commerce_order_load($order->order_id, TRUE); rules_invoke_all('commerce_checkout_complete', $order); } 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..a22fafe 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(); @@ -295,6 +295,7 @@ function commerce_checkout_form_validate($form, &$form_state) { // Save the updated order object and reset the order in the form cache to // ensure rebuilt forms use the updated order. + $order = commerce_order_load($order->order_id, TRUE); commerce_order_save($order); $form_state['build_info']['args'][0] = $order; @@ -312,7 +313,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 +349,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 +368,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..ff0fbd9 100644 --- a/modules/checkout/tests/commerce_checkout.test +++ b/modules/checkout/tests/commerce_checkout.test @@ -68,7 +68,7 @@ class CommerceCheckoutTestProcess extends CommerceBaseTestCase { // Logout to test the checkout process with anonymous user. $this->drupalLogout(); - // Override user variable to get the enviroment fully set. + // Override user variable to get the environment fully set. global $user; $user = user_load(0); @@ -76,9 +76,7 @@ class CommerceCheckoutTestProcess extends CommerceBaseTestCase { $this->drupalPost('node/' . $this->product_node->nid, array(), t('Add to cart')); // 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(); + $this->order = reset(commerce_order_load_multiple(array(), array('uid' => $user->uid, 'status' => 'cart'), TRUE, TRUE)); } /** @@ -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.')); @@ -178,7 +174,7 @@ class CommerceCheckoutTestProcess extends CommerceBaseTestCase { $this->drupalPost(NULL, array(), t('Continue to next step')); // Reload the order directly from db to update status. - $order = commerce_order_load_multiple(array($this->order->order_id), array(), TRUE); + $order = commerce_order_load_multiple(array($this->order->order_id), array(), TRUE, TRUE); // Order status should be pending when completing checkout process. $this->assertEqual(reset($order)->status, 'pending', t('Order status is \'Pending\' after completing checkout')); @@ -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.')); @@ -334,7 +328,7 @@ class CommerceCheckoutTestProcess extends CommerceBaseTestCase { $this->drupalPost(NULL, array(), t('Continue to next step')); // Reload the order directly from db to check status. - $order = commerce_order_load_multiple(array($this->order->order_id), array(), TRUE); + $order = commerce_order_load_multiple(array($this->order->order_id), array(), TRUE, TRUE); // Order status should be pending when completing checkout process. $this->assertEqual(reset($order)->status, 'pending', t('Order status is \'Pending\' after completing checkout.')); diff --git a/modules/customer/commerce_customer.module b/modules/customer/commerce_customer.module index 649ff2a..6231219 100644 --- a/modules/customer/commerce_customer.module +++ b/modules/customer/commerce_customer.module @@ -740,7 +740,7 @@ function commerce_customer_commerce_customer_profile_delete($profile) { // Loop over results for each type of entity returned. foreach ($result as $entity_type => $data) { // Load the entities of the current type. - $entities = entity_load($entity_type, array_keys($data)); + $entities = entity_load($entity_type, array_keys($data), array('_lock' => TRUE)); // Loop over each entity and remove the reference to the deleted profile. foreach ($entities as $entity_id => $entity) { diff --git a/modules/line_item/commerce_line_item.module b/modules/line_item/commerce_line_item.module index b5eee98..47211e3 100644 --- a/modules/line_item/commerce_line_item.module +++ b/modules/line_item/commerce_line_item.module @@ -656,7 +656,7 @@ function commerce_line_item_delete_references($line_item) { // Loop over results for each type of entity returned. foreach ($result as $entity_type => $data) { // Load the entities of the current type. - $entities = entity_load($entity_type, array_keys($data)); + $entities = entity_load($entity_type, array_keys($data), array('_lock'=> TRUE)); // Loop over each entity and remove the reference to the deleted line item. foreach ($entities as $entity_id => $entity) { 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 43db91b..47cb739 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/order/tests/commerce_order.test b/modules/order/tests/commerce_order.test index 1164634..b5eec85 100644 --- a/modules/order/tests/commerce_order.test +++ b/modules/order/tests/commerce_order.test @@ -45,7 +45,7 @@ class CommerceOrderCRUDTestCase extends CommerceBaseTestCase { $this->assertIdentical($return, SAVED_NEW, 'commerce_order_save() successfully saved the new order.'); // Ensure commerce_order_load() loaded the saved order. - $loaded_order = commerce_order_load($new_order->order_id); + $loaded_order = commerce_order_load($new_order->order_id, TRUE); foreach ($fields as $field) { $this->assertEqual($loaded_order->{$field}, $new_order->{$field}, 'The ' . check_plain($field) . ' value loaded by commerce_order_load() matches the value saved by commerce_order_save()'); } @@ -77,7 +77,7 @@ class CommerceOrderCRUDTestCase extends CommerceBaseTestCase { $saved_orders[$order->order_id] = $order->order_number; } - $loaded_orders = commerce_order_load_multiple(array_keys($saved_orders)); + $loaded_orders = commerce_order_load_multiple(array_keys($saved_orders), array(), TRUE, TRUE); $this->assertEqual(count($saved_orders), count($loaded_orders), 'commerce_order_load_multiple() loaded the proper number of the orders.'); foreach ($loaded_orders as $loaded_order) { $this->assertEqual($loaded_order->order_number, $saved_orders[$loaded_order->order_id], 'commerce_order_load_multiple() successfully loaded a order.'); diff --git a/modules/order/tests/commerce_order_ui.test b/modules/order/tests/commerce_order_ui.test index 969fed6..135e34a 100644 --- a/modules/order/tests/commerce_order_ui.test +++ b/modules/order/tests/commerce_order_ui.test @@ -63,9 +63,6 @@ class CommerceOrderUIAdminTest extends CommerceBaseTestCase { $orders = commerce_order_load_multiple(array(), array('uid' => $this->store_customer->uid)); $this->order = reset($orders); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); - // Enable an additional currency. $this->enableCurrencies(array('EUR')); } @@ -152,9 +149,6 @@ class CommerceOrderUIAdminTest extends CommerceBaseTestCase { // Reload the order directly from db. $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 if the product has been added to the order. foreach (entity_metadata_wrapper('commerce_order', reset($order))->commerce_line_items as $delta => $line_item_wrapper) { if ($line_item_wrapper->type->value() == 'product') { @@ -190,9 +184,6 @@ class CommerceOrderUIAdminTest extends CommerceBaseTestCase { $order = reset($orders); $order_wrapper = entity_metadata_wrapper('commerce_order', $order); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); - // Also wrap the product to access easier to its price. $product_wrapper = entity_metadata_wrapper('commerce_product', $this->product); @@ -264,8 +255,6 @@ class CommerceOrderUIAdminTest extends CommerceBaseTestCase { // Check if the links for editing the order are present. $links = menu_contextual_links('commerce-order', 'admin/commerce/orders', array($this->order->order_id)); - // Reset the cache as we don't want to keep the lock. - entity_get_controller('commerce_order')->resetCache(); $this->assertRaw((theme('links', array('links' => $links, 'attributes' => array('class' => array('links', 'inline', 'operations'))))), t('Links for orders are present')); $this->drupalGet('admin/commerce/orders/'. $this->order->order_id . '/view'); diff --git a/modules/payment/commerce_payment.module b/modules/payment/commerce_payment.module index 4b5f166..1503f85 100644 --- a/modules/payment/commerce_payment.module +++ b/modules/payment/commerce_payment.module @@ -334,6 +334,11 @@ function commerce_payment_redirect_pane_next_page($order, $log = '') { $order_status = commerce_order_status_load($order->status); if ($order_status['state'] == 'checkout' && $order_status['checkout_page'] == 'payment') { + // Reload the order with locking. + if (!entity_get_controller('commerce_order')->isLocked($order)) { + $order = commerce_order_load($order->order_id, TRUE); + } + $payment_page = commerce_checkout_page_load($order_status['checkout_page']); $next_page = $payment_page['next_page']; @@ -448,7 +453,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)); diff --git a/modules/product_reference/commerce_product_reference.module b/modules/product_reference/commerce_product_reference.module index c9f6ca6..5fe1a1f 100644 --- a/modules/product_reference/commerce_product_reference.module +++ b/modules/product_reference/commerce_product_reference.module @@ -289,7 +289,7 @@ function commerce_product_reference_commerce_product_delete($product) { // Loop over results for each type of entity returned. foreach ($result as $entity_type => $data) { // Load the entities of the current type. - $entities = entity_load($entity_type, array_keys($data)); + $entities = entity_load($entity_type, array_keys($data), array('_lock' => TRUE)); // Loop over each entity and remove the reference to the deleted product. foreach ($entities as $entity_id => $entity) { diff --git a/tests/commerce_base.test b/tests/commerce_base.test index d0c9f00..bbb9032 100644 --- a/tests/commerce_base.test +++ b/tests/commerce_base.test @@ -367,12 +367,12 @@ abstract class CommerceBaseTestCase extends DrupalWebTestCase { foreach($products as $product_id => $quantity) { if ($product = commerce_product_load($product_id)) { $line_item = commerce_product_line_item_new($product, $quantity); - $line_item = commerce_cart_product_add($uid, $line_item); + commerce_cart_product_add($uid, $line_item); } } // Load the order for returning it. - $order = commerce_cart_order_load($uid); + $order = commerce_cart_order_load($uid, TRUE); if (!empty($customer_profile_id)) { $order->commerce_customer_billing[LANGUAGE_NONE][0]['profile_id'] = $customer_profile_id;