Merge pull request #25183 from om4james/webhook-de-duplication

Fix for Duplicate Webhook deliveries
This commit is contained in:
Peter Fabian 2020-02-05 18:38:15 +01:00 committed by GitHub
commit 5ba2cdafa5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 70 additions and 1 deletions

View File

@ -22,6 +22,14 @@ require_once 'legacy/class-wc-legacy-webhook.php';
*/
class WC_Webhook extends WC_Legacy_Webhook {
/**
* Store which object IDs this webhook has processed (ie scheduled to be delivered)
* within the current page request.
*
* @var array
*/
protected $processed = array();
/**
* Stores webhook data.
*
@ -105,6 +113,9 @@ class WC_Webhook extends WC_Legacy_Webhook {
return;
}
// Mark this $arg as processed to ensure it doesn't get processed again within the current request.
$this->processed[] = $arg;
/**
* Process webhook delivery.
*
@ -125,7 +136,7 @@ class WC_Webhook extends WC_Legacy_Webhook {
* @return bool True if webhook should be delivered, false otherwise.
*/
private function should_deliver( $arg ) {
$should_deliver = $this->is_active() && $this->is_valid_topic() && $this->is_valid_action( $arg ) && $this->is_valid_resource( $arg );
$should_deliver = $this->is_active() && $this->is_valid_topic() && $this->is_valid_action( $arg ) && $this->is_valid_resource( $arg ) && ! $this->is_already_processed( $arg );
/**
* Let other plugins intercept deliver for some messages queue like rabbit/zeromq.
@ -282,6 +293,19 @@ class WC_Webhook extends WC_Legacy_Webhook {
return true;
}
/**
* Checks if the specified resource has already been queued for delivery within the current request.
*
* Helps avoid duplication of data being sent for topics that have more than one hook defined.
*
* @param mixed $arg First hook argument.
*
* @return bool
*/
protected function is_already_processed( $arg ) {
return false !== array_search( $arg, $this->processed, true );
}
/**
* Deliver the webhook payload using wp_safe_remote_request().
*

View File

@ -11,6 +11,12 @@
*/
class WC_Tests_Webhook_Functions extends WC_Unit_Test_Case {
/**
* Temporarily store webhook delivery counters.
* @var array
*/
protected $delivery_counter = array();
/**
* Data provider for test_wc_is_webhook_valid_topic.
*
@ -207,6 +213,45 @@ class WC_Tests_Webhook_Functions extends WC_Unit_Test_Case {
$this->assertFalse( wc_load_webhooks( $status, 1 ) );
}
/**
* Verify that a webhook that has multiple hooks defined (in WC_Webhook::get_topic_hooks()),
* is only delivered once.
*
* This example uses Customer Created (which has 3 hooks defined), to verify that creating a customer
* will only deliver the payload once per webhook.
*/
public function test_woocommerce_webhook_is_delivered_only_once() {
$webhook1 = wc_get_webhook( $this->create_webhook( 'customer.created' )->get_id() );
$webhook2 = 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 );
$customer1 = WC_Helper_Customer::create_customer( 'test1', 'pw1', 'user1@example.com' );
$customer2 = WC_Helper_Customer::create_customer( 'test2', 'pw2', 'user2@example.com' );
$this->assertEquals( 1, $this->delivery_counter[ $webhook1->get_id() . $customer1->get_id() ] );
$this->assertEquals( 1, $this->delivery_counter[ $webhook2->get_id() . $customer1->get_id() ] );
$this->assertEquals( 1, $this->delivery_counter[ $webhook1->get_id() . $customer2->get_id() ] );
$this->assertEquals( 1, $this->delivery_counter[ $webhook2->get_id() . $customer2->get_id() ] );
$webhook1->delete( true );
$webhook2->delete( true );
$customer1->delete( true );
$customer2->delete( true );
remove_action( 'woocommerce_webhook_process_delivery', array( $this, 'woocommerce_webhook_process_delivery' ), 1, 2 );
}
/**
* Helper function to keep track of which webhook (and corresponding arg) has been delivered
* within the current request.
*
* @param WC_Webhook $webhook Webhook that is processing delivery.
* @param mixed $arg Webhook arg (usually resource ID).
*/
public function woocommerce_webhook_process_delivery( $webhook, $arg ) {
if ( ! isset( $this->delivery_counter[ $webhook->get_id() . $arg ] ) ) {
$this->delivery_counter[ $webhook->get_id() . $arg ] = 0;
}
$this->delivery_counter[ $webhook->get_id() . $arg ] ++;
}
/**
* Create and save a webhook for testing.
*