Skip to content

Commit

Permalink
[FIX] form crm saving lost concurrent update
Browse files Browse the repository at this point in the history
  • Loading branch information
cyrillbolliger committed Nov 3, 2023
1 parent 26a37e2 commit ac61e84
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 18 deletions.
30 changes: 20 additions & 10 deletions wordpress/wp-content/themes/les-verts/lib/form/include/CrmSaver.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,13 @@ public static function save_to_crm( bool $force = false ) {
Util::debug_log( "submissionId={$item->get_submission_id()} msg=No data. Discarding entry." );

// remove item from queue
$queue->filter( static function ( $q_item ) use ( $item ) {
return $item->get_submission_id() !== $q_item->get_submission_id();
} );
try {
$queue->filter( static function ( $q_item ) use ( $item ) {
return $item->get_submission_id() !== $q_item->get_submission_id();
} );
} catch ( Exception $e ) {
// do nothing, well remove it the next time
}
}

// if we have exceeded the max attempts,
Expand Down Expand Up @@ -156,15 +160,19 @@ public static function save_to_crm( bool $force = false ) {
// on success: remove item from queue
Util::debug_log( "submissionId={$item->get_submission_id()} msg=Saved successfully. CRM id: $crm_id" );

$queue->filter( static function ( $q_item ) use ( $item ) {
if ( $item->get_submission_id() === $q_item->get_submission_id() ) {
Util::debug_log( "submissionId={$item->get_submission_id()} msg=Remove from queue." );
try {
$queue->filter( static function ( $q_item ) use ( $item ) {
if ( $item->get_submission_id() === $q_item->get_submission_id() ) {
Util::debug_log( "submissionId={$item->get_submission_id()} msg=Remove from queue." );

return false; // remove from queue
}
return false; // remove from queue
}

return true;
} );
return true;
} );
} catch ( Exception $e ) {
// do nothing, we'll remove it the next time
}
}
}

Expand Down Expand Up @@ -270,6 +278,8 @@ private static function send_permanent_error_notification( $submission, $err_mes

/**
* Add the submission to saving queue
*
* @throws Exception
*/
public function queue() {
$data = $this->get_data();
Expand Down
62 changes: 58 additions & 4 deletions wordpress/wp-content/themes/les-verts/lib/form/include/Mailer.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ public static function get_queue(): QueueDao {

/**
* Add the mails for the submission to the sending queue
*
* @throws Exception
*/
public function queue_mails() {
$this->queue_confirmation();
Expand All @@ -69,6 +71,8 @@ public function queue_mails() {

/**
* Queue the confirmation mail, if confirmation is enabled and we have a destination address
*
* @throws Exception
*/
private function queue_confirmation() {
$to = $this->submission->meta_get_linked_email();
Expand All @@ -91,6 +95,8 @@ private function queue_confirmation() {

/**
* Queue the notification mail, if notification is enabled
*
* @throws Exception
*/
private function queue_notification() {
if ( $this->form->has_notification() ) {
Expand Down Expand Up @@ -176,8 +182,14 @@ public static function send_mails() {
$queue = self::get_queue();
$error = 0;

/** @var Mail $mail */
$mail = $queue->pop();
try {
/** @var Mail $mail */
$mail = $queue->pop();
} catch ( Exception $e ) {
// just wait for the next run
return;
}

while ( ! empty( $mail ) ) {
$sent = $mail->send();

Expand All @@ -186,7 +198,11 @@ public static function send_mails() {

// requeue mail on error if number of retries is not exceeded
if ( $mail->get_sending_attempts() < self::SENDING_RETRIES ) {
$queue->push( $mail );
try {
$queue->push( $mail );
} catch ( Exception $e ) {
self::send_error_notification( $mail, $e );
}
$error ++;
}

Expand All @@ -196,7 +212,45 @@ public static function send_mails() {
}
}

$mail = $queue->pop();
try {
$mail = $queue->pop();
} catch ( Exception $e ) {
// just wait for the next run
return;
}
}
}

private static function send_error_notification( Mail $mail, Exception $e ) {
$domain = Util::get_domain();

$subject = sprintf(
__( '%s: ERROR while sending mail', THEME_DOMAIN ),
$domain
);

$message = sprintf(
__(
"Hi Admin of %s\n\n" .
"There was an ERROR while sending the following mail and we were unable to save it back to the sending queue." .
"YOU MUST SEND IT MANUALLY. The mail:\n\n---\n\n" .
"to: %s\n" .
"subject: %s\n" .
"body: %s\n\n---\n\n" .
"More details in the error message below.\n\n" .
"Have a nice day.\n" .
"Your Website - %s\n\n" .
"Error message:\n%s",
THEME_DOMAIN
),
$domain,
$mail->get_to(),
$mail->get_subject(),
$mail->get_body(),
$domain,
$e->getMessage(),
);

Util::send_mail_to_admin( $subject, $message );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,22 @@

namespace SUPT;

use Exception;

/**
* Data queue. Based on WordPress' options api.
*
* @package SUPT
*/
class QueueDao {
/**
* Time to wait to acquire a lock.
*
* Should be long enough to survive the sending time of a mail, but
* be short enough to not run into PHPs max_execution_time timeout.
*/
const LOCK_ACQUIRE_TIMEOUT_SECONDS = 15;

/**
* The key under witch the queue is stored
*
Expand All @@ -29,6 +39,8 @@ public function __construct( $key ) {
* Add item to the end of the queue
*
* @param mixed $item
*
* @throws Exception
*/
public function push( $item ) {
$this->lock();
Expand All @@ -44,6 +56,8 @@ public function push( $item ) {
* Return the first item from the queue and remove it
*
* @return mixed|null null if queue is empty
*
* @throws Exception
*/
public function pop() {
$this->lock();
Expand Down Expand Up @@ -102,6 +116,8 @@ public function clear() {
* Remove item with given index from queue
*
* @param int $index
*
* @throws Exception
*/
public function remove( int $index ) {
$this->lock();
Expand All @@ -120,6 +136,8 @@ public function remove( int $index ) {
* the item, true will keep it.
*
* @return int The number of elements removed
*
* @throws Exception
*/
public function filter( callable $callback ): int {
$this->lock();
Expand All @@ -134,13 +152,34 @@ public function filter( callable $callback ): int {
return $lenBefore - $lenAfter;
}

/**
* @throws Exception
*/
private function lock() {
global $wpdb;
$wpdb->query( "LOCK TABLES $wpdb->options WRITE" );
$lock = $this->get_lock_name();
$timeout = self::LOCK_ACQUIRE_TIMEOUT_SECONDS;
$result = $wpdb->get_var( "SELECT GET_LOCK('$lock', $timeout);" );
if ( $result !== '1' ) {
throw new Exception( "Failed to acquire DB lock \"$lock\": $result" );
}
}

/**
* @throws Exception
*/
private function unlock() {
global $wpdb;
$wpdb->query( "UNLOCK TABLES" );
$lock = $this->get_lock_name();
$result = $wpdb->get_var( "SELECT RELEASE_LOCK('$lock');" );
if ( $result !== '1' ) {
throw new Exception( "Failed to release DB lock \"$lock\": $result" );
}
}

private function get_lock_name(): string {
global $wpdb;

return DB_NAME . '.' . $wpdb->postmeta . '.' . $this->key;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

namespace SUPT;

use Exception;
use WP_CLI;
use function WP_CLI\Utils\format_items;

Expand Down Expand Up @@ -158,6 +159,7 @@ public function store( $args, $assoc_args ) {
*
* [--all]
* : Delete all crm data in the queue.
* @throws Exception
*/
public function delete( $args, $assoc_args ) {
$queue = CrmSaver::get_queue();
Expand All @@ -173,7 +175,11 @@ public function delete( $args, $assoc_args ) {
/** @var CrmQueueItem $item */
foreach ( $queue->get_all() as $index => $item ) {
if ( $id === $item->get_submission_id() ) {
$queue->remove( $index );
try {
$queue->remove( $index );
} catch ( Exception $e ) {
WP_CLI::error( "Failed to delete item. {$e->getMessage()}" );
}

return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

namespace SUPT;

use Exception;
use WP_CLI;
use function WP_CLI\Utils\format_items;

Expand Down Expand Up @@ -134,7 +135,11 @@ public function delete( $args, $assoc_args ) {
return;
}

$queue->remove( absint( $args[0] ) );
try {
$queue->remove( absint( $args[0] ) );
} catch ( Exception $e ) {
WP_CLI::error( "Failed to delete item. {$e->getMessage()}" );
}
}
}

Expand Down

0 comments on commit ac61e84

Please sign in to comment.