From 7331036d178ef77ce81218eece126477a243f5d3 Mon Sep 17 00:00:00 2001 From: Christopher Allford Date: Thu, 25 Jun 2020 16:54:17 -0700 Subject: [PATCH] Moved synchronous webhook execution into a shutdown function One of the problems with synchronous webhooks is that they are executed as soon as the related action is. Since we may call an action multiple times in the process of updating something, this causes only the first action to trigger the hook. This differs from asynchronous execution because in that case, the web hook will be executed after the entire request has completed. --- includes/wc-webhook-functions.php | 29 +++++++++++++++++-- .../legacy/unit-tests/webhooks/functions.php | 23 +++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/includes/wc-webhook-functions.php b/includes/wc-webhook-functions.php index 4845889f952..933572520b8 100644 --- a/includes/wc-webhook-functions.php +++ b/includes/wc-webhook-functions.php @@ -8,6 +8,23 @@ defined( 'ABSPATH' ) || exit; +/** + * Process the synchronous web hooks at the end of the request. + * + * @since 4.4.0 + */ +function wc_webhook_execute_synchronous_queue() { + global $wc_queued_sync_webhooks; + if ( empty( $wc_queued_sync_webhooks ) ) { + return; + } + + foreach ( $wc_queued_sync_webhooks as $data ) { + $data['webhook']->deliver( $data['arg'] ); + } +} +register_shutdown_function( 'wc_webhook_execute_synchronous_queue' ); + /** * Process webhook delivery. * @@ -33,8 +50,16 @@ function wc_webhook_process_delivery( $webhook, $arg ) { WC()->queue()->add( 'woocommerce_deliver_webhook_async', $queue_args, 'woocommerce-webhooks' ); } } else { - // Deliver immediately. - $webhook->deliver( $arg ); + // We need to queue the webhook so that it can be ran after the request has finished processing. + // This must be done in order to keep parity with how they are executed asynchronously. + global $wc_queued_sync_webhooks; + if ( ! isset( $wc_queued_sync_webhooks ) ) { + $wc_queued_sync_webhooks = array(); + } + $wc_queued_sync_webhooks[] = array( + 'webhook' => $webhook, + 'arg' => $arg, + ); } } add_action( 'woocommerce_webhook_process_delivery', 'wc_webhook_process_delivery', 10, 2 ); diff --git a/tests/legacy/unit-tests/webhooks/functions.php b/tests/legacy/unit-tests/webhooks/functions.php index ee2cfb220a9..35d418fceeb 100644 --- a/tests/legacy/unit-tests/webhooks/functions.php +++ b/tests/legacy/unit-tests/webhooks/functions.php @@ -238,6 +238,29 @@ class WC_Tests_Webhook_Functions extends WC_Unit_Test_Case { remove_action( 'woocommerce_webhook_process_delivery', array( $this, 'woocommerce_webhook_process_delivery' ), 1, 2 ); } + /** + * Verify that a webhook is queued when intended to be delivered synchronously. This allows us to then execute them + * all in a `register_shutdown_function` after the request has processed. Since async jobs are handled in + * this way, we can be more confident that it is consistent. + */ + public function test_woocommerce_webhook_synchronous_is_queued() { + add_filter( 'woocommerce_webhook_deliver_async', '__return_false' ); + $webhook = wc_get_webhook( $this->create_webhook( 'customer.created' )->get_id() ); + wc_load_webhooks( 'active' ); + add_action( 'woocommerce_webhook_process_delivery', array( $this, 'woocommerce_webhook_process_delivery' ), 1, 2 ); + $customer = WC_Helper_Customer::create_customer( 'test1', 'pw1', 'user1@example.com' ); + + global $wc_queued_sync_webhooks; + $this->assertCount( 1, $wc_queued_sync_webhooks ); + $this->assertEquals( $webhook->get_id(), $wc_queued_sync_webhooks[0]['webhook']->get_id() ); + $this->assertEquals( $customer->get_id(), $wc_queued_sync_webhooks[0]['arg'] ); + + $wc_queued_sync_webhooks = null; + remove_filter( 'woocommerce_webhook_deliver_async', '__return_false' ); + $webhook->delete( true ); + $customer->delete( true ); + } + /** * Helper function to keep track of which webhook (and corresponding arg) has been delivered * within the current request.