From 46f8500b02dd0572a9e63e1806b947d328773e88 Mon Sep 17 00:00:00 2001 From: Jake Jackson Date: Mon, 18 May 2026 13:14:18 +1000 Subject: [PATCH] fix(queue): restore background processing refactor lost in Phase 1 merge Phase 1 (#1655) carried the bootstrap-level pieces of hot-patch's queue work (GF_Background_Process loader, gfpdf_settings_sanitize queue-toggle hook, 4-arg Controller_Pdf_Queue construction) but the auto-merge took dev's copy of Controller_Pdf_Queue / Queue_Callbacks / Test_Controller_Pdf_Queue. That left the controller's constructor silently dropping its $form dependency and discarded the substantive changes from four hot-patch commits: dc772da0 Update Background Processing Queue (GF 2.9.7/2.9.8 compat) 8a7914dd Convert mini-queues to individual queue items; drop 'unrecoverable' flag; retry from 3 -> 1 389ae6ce Add queue cleanup when Background Processing toggled 3fe1c1e7 Use the correct entry on resend; dedupe identical PDFs This commit re-applies those four commits' intent on top of dev's queue baseline: * Controller_Pdf_Queue - Accept Helper_Form via the 4th constructor arg (matches the call site already in bootstrap.php). - Group $form_async_notifications by [formId][entryId] so resend iteration can re-fetch fresh form/entry data from the DB instead of using stale request-time arrays. - queue_dispatch_resend_notification_tasks() now ignores legacy $form/$entry params, fans out per form+entry pair, and dispatches once at the end. - dispatch_queue is hooked separately on gform_after_submission priority 100 (instead of inline from queue_async_form_submission_tasks), so the resend path can reuse the same dispatch hook. - queue_async_tasks pushes one task per queue item (forwards compat with the new GF library). - Drop 'unrecoverable' => true so a single failure doesn't block other PDFs/notifications. - args carry get_current_user_id() so dev's {user} merge-tag masquerade keeps working. * Queue_Callbacks::create_pdf - Wrap GPDFAPI::create_pdf() with gfpdf_override_pdf_bypass=false so a queued PDF generates even if the request-time bypass filter would normally short-circuit it. * queue_cleanup_task / Queue_Callbacks::cleanup_pdfs stay deprecated per dev's caching-layer + auto-purge direction (6.12). The cleanup queue entries that hot-patch's controller used to push are no longer scheduled, and the tests assert the reduced queue size. Helper_Pdf_Queue.php is identical between dev and hot-patch already (retry budget = 1, etc.) so no changes are needed there. Verified locally: * composer phpcs:lint -- clean * yarn lint:js -- clean * yarn test:php (1119 tests, 4088 assertions) -- all pass Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Controller/Controller_Pdf_Queue.php | 93 ++++++++++++---- src/Statics/Queue_Callbacks.php | 2 + .../Controller/Test_Controller_Pdf_Queue.php | 104 ++++++++++++------ 3 files changed, 144 insertions(+), 55 deletions(-) diff --git a/src/Controller/Controller_Pdf_Queue.php b/src/Controller/Controller_Pdf_Queue.php index 028ef2696..eb511fe4e 100644 --- a/src/Controller/Controller_Pdf_Queue.php +++ b/src/Controller/Controller_Pdf_Queue.php @@ -13,7 +13,7 @@ /** * @package Gravity PDF - * @copyright Copyright (c) 2024, Blue Liquid Designs + * @copyright Copyright (c) 2026, Blue Liquid Designs * @license http://opensource.org/licenses/gpl-2.0.php GNU Public License */ @@ -54,10 +54,18 @@ class Controller_Pdf_Queue extends Helper_Abstract_Controller { */ protected $queue; + /** + * @var Helper_Form + * + * @since 6.13.5 + */ + protected $form; + /** * @var array An array containing the notification objects to be sent using background processing during a request * * @since 6.11.0 + * @since 6.13.5 Group by form ID and Entry ID [ formId => [ entryId => [] ] ] */ protected $form_async_notifications = []; @@ -67,14 +75,17 @@ class Controller_Pdf_Queue extends Helper_Abstract_Controller { * @param Helper_Pdf_Queue $queue * @param Model_PDF $model_pdf * @param LoggerInterface $log Our logger class + * @param Helper_Form $form * * @since 5.0 + * @since 6.13.5 Add $form dependency */ - public function __construct( Helper_Pdf_Queue $queue, Model_PDF $model_pdf, LoggerInterface $log ) { + public function __construct( Helper_Pdf_Queue $queue, Model_PDF $model_pdf, LoggerInterface $log, Helper_Form $form ) { /* Assign our internal variables */ $this->log = $log; $this->model_pdf = $model_pdf; $this->queue = $queue; + $this->form = $form; } /** @@ -87,9 +98,10 @@ public function __construct( Helper_Pdf_Queue $queue, Model_PDF $model_pdf, Logg public function init() { add_filter( 'gform_disable_notification', [ $this, 'maybe_disable_submission_notifications' ], 9999, 5 ); add_action( 'gform_after_submission', [ $this, 'queue_async_form_submission_tasks' ], 5, 2 ); + add_action( 'gform_after_submission', [ $this, 'dispatch_queue' ], 100 ); add_filter( 'gform_disable_resend_notification', [ $this, 'maybe_disable_resend_notifications' ], 10, 4 ); - add_action( 'gform_post_resend_all_notifications', [ $this, 'queue_dispatch_resend_notification_tasks' ], 10, 2 ); + add_action( 'gform_post_resend_all_notifications', [ $this, 'queue_dispatch_resend_notification_tasks' ] ); } /** @@ -161,7 +173,15 @@ public function maybe_disable_resend_notifications( $is_disabled, $notification, protected function should_send_async_notification( $is_disabled, $notification, $form, $entry ) { $send_async_notification = $this->do_we_disable_notification( $is_disabled, $notification, $form, $entry ); if ( $send_async_notification ) { - $this->form_async_notifications[] = $notification; + if ( ! isset( $this->form_async_notifications[ $form['id'] ] ) ) { + $this->form_async_notifications[ $form['id'] ] = []; + } + + if ( ! isset( $this->form_async_notifications[ $form['id'] ][ $entry['id'] ] ) ) { + $this->form_async_notifications[ $form['id'] ][ $entry['id'] ] = []; + } + + $this->form_async_notifications[ $form['id'] ][ $entry['id'] ][] = $notification; } return $send_async_notification; @@ -207,28 +227,54 @@ public function do_we_disable_notification( $is_disabled, $notification, $form, * @param array $form * * @since 5.0 + * @since 6.13.5 Move queue dispatch to self::queue_async_form_submission_dispatch() */ public function queue_async_form_submission_tasks( $entry, $form ) { $this->queue_async_tasks( $form, $entry ); - $this->dispatch_queue(); } /** * Queue and send the notifications after the resend notification process has completed * - * @param array $form - * @param array $entry - * * @since 5.0 + * @since 6.13.5 $form and $entry arguments ignored and now set in self::should_send_async_notification() */ - public function queue_dispatch_resend_notification_tasks( $form, $entry ) { - $this->queue_async_form_submission_tasks( $entry, $form ); + public function queue_dispatch_resend_notification_tasks( $form = null, $entry = null ) { + if ( ! is_null( $entry ) ) { + _doing_it_wrong( __METHOD__, '$entry argument ignored and now set in self::should_send_async_notification()', '6.13.5' ); + } + + /* loop over all form/entries */ + foreach ( $this->form_async_notifications as $form_id => $entry_data ) { + $form = $this->form->get_form( $form_id ); + if ( ! $form ) { + $this->log->error( 'Form not found', $form_id ); + continue; + } + + $entry_ids = array_keys( $entry_data ); + foreach ( $entry_ids as $entry_id ) { + $entry = $this->form->get_entry( $entry_id ); + if ( is_wp_error( $entry ) ) { + $this->log->error( + 'Entry not found', + [ + 'entry_id' => $entry_id, + 'exception' => $entry->get_error_message() , + ] + ); + continue; + } + + $this->queue_async_form_submission_tasks( $entry, $form ); + } + } + + $this->dispatch_queue(); } /** - * Push tasks to the queue for requested PDFs/Notifications - * - * Even if a PDF isn't attached to a notification, it may still need to be generated and saved to disk + * Push tasks to the queue for requested notifications * * @param array $form * @param array $entry @@ -238,10 +284,12 @@ public function queue_dispatch_resend_notification_tasks( $form, $entry ) { * @since 6.11.0 */ public function queue_async_tasks( $form, $entry ) { - $tasks = $this->get_queue_tasks( $entry, $form, $this->form_async_notifications ); + $notifications = $this->form_async_notifications[ $form['id'] ][ $entry['id'] ] ?? []; + $tasks = $this->get_queue_tasks( $entry, $form, $notifications ); - if ( count( $tasks ) > 0 ) { - $this->queue->push_to_queue( $tasks ); + /* Push each task individually for forwards compatibility with new Background Processing update */ + foreach ( $tasks as $task ) { + $this->queue->push_to_queue( [ $task ] ); } } @@ -253,7 +301,7 @@ public function queue_async_tasks( $form, $entry ) { * * @return void * - * @since 6.11.0 + * @since 6.11.0 * @deprecated 6.12.0 Caching layer + auto-purge added */ public function queue_cleanup_task( $form, $entry ) { @@ -332,10 +380,9 @@ protected function queue_pdfs( $notifications, $pdfs, $form, $entry ) { foreach ( $pdfs as $pdf ) { $pdf_queue_data = [ - 'id' => $this->get_queue_id( $form, $entry, $pdf ), - 'func' => '\GFPDF\Statics\Queue_Callbacks::create_pdf', - 'args' => [ $entry['id'], $pdf['id'], get_current_user_id() ], - 'unrecoverable' => true, + 'id' => 'create-pdf-' . $this->get_queue_id( $form, $entry, $pdf ), + 'func' => '\GFPDF\Statics\Queue_Callbacks::create_pdf', + 'args' => [ $entry['id'], $pdf['id'], get_current_user_id() ], ]; /* Check if we need to save the PDF due to a filter */ @@ -380,7 +427,7 @@ protected function queue_notifications( $notifications, $pdfs, $form, $entry ) { foreach ( $pdfs as $pdf ) { if ( $this->model_pdf->maybe_attach_to_notification( $notification, $pdf, $entry, $form ) ) { $queue_data[] = [ - 'id' => $this->get_queue_id( $form, $entry, $pdf ) . '-' . $notification['id'], + 'id' => 'send-notification-' . $this->get_queue_id( $form, $entry, $pdf ) . '-' . $notification['id'], 'func' => '\GFPDF\Statics\Queue_Callbacks::send_notification', 'args' => [ $form['id'], $entry['id'], $notification, get_current_user_id() ], ]; @@ -469,6 +516,6 @@ public function reset_queue() { * @deprecated 6.11 */ public function queue_async_resend_notification_tasks( $notification, $form, $entry ) { - _doing_it_wrong( __METHOD__, 'This method has been removed and no alternative is available.', '6.11' ); + _doing_it_wrong( esc_html( 'queue_async_resend_notification_tasks() was removed in Gravity PDF 6.11' ) ); } } diff --git a/src/Statics/Queue_Callbacks.php b/src/Statics/Queue_Callbacks.php index 482e3e1ec..e80c67d96 100644 --- a/src/Statics/Queue_Callbacks.php +++ b/src/Statics/Queue_Callbacks.php @@ -47,7 +47,9 @@ public static function create_pdf( $entry_id, $pdf_id, $user_id = 0 ) { wp_set_current_user( $user_id ); /* For performance, only generate the PDF if it does not currently exist on disk */ + add_filter( 'gfpdf_override_pdf_bypass', '__return_false', 20 ); $pdf = GPDFAPI::create_pdf( $entry_id, $pdf_id ); + remove_filter( 'gfpdf_override_pdf_bypass', '__return_false', 20 ); /* Reset existing user */ wp_set_current_user( $backup_user_id ); diff --git a/tests/phpunit/unit-tests/Controller/Test_Controller_Pdf_Queue.php b/tests/phpunit/unit-tests/Controller/Test_Controller_Pdf_Queue.php index 90e36521c..dd2b006ce 100644 --- a/tests/phpunit/unit-tests/Controller/Test_Controller_Pdf_Queue.php +++ b/tests/phpunit/unit-tests/Controller/Test_Controller_Pdf_Queue.php @@ -4,6 +4,7 @@ use Exception; use GFPDF\Controller\Controller_Pdf_Queue; +use GFPDF\Helper\Helper_Abstract_Options; use GFPDF\Helper\Helper_Pdf_Queue; use GFPDF\Statics\Cache; use GFPDF\Statics\Queue_Callbacks; @@ -12,7 +13,7 @@ /** * * @package Gravity PDF - * @copyright Copyright (c) 2024, Blue Liquid Designs + * @copyright Copyright (c) 2026, Blue Liquid Designs * @license http://opensource.org/licenses/gpl-2.0.php GNU Public License * @since 5.0 */ @@ -66,7 +67,7 @@ public function set_up() { $this->queue_mock->method( 'save' ) ->willReturn( $this->queue_mock ); - $this->controller = new Controller_Pdf_Queue( $this->queue_mock, $model_pdf, $gfpdf->log ); + $this->controller = new Controller_Pdf_Queue( $this->queue_mock, $model_pdf, $gfpdf->log, $gfpdf->gform ); } /** @@ -116,13 +117,13 @@ public function test_queue_tasks() { } /** - * Test our queue retries once on exception. + * Test our queue attempts to run up to two times when a function throws an exception * * @since 5.0 */ public function test_failed_queue_tasks() { $mock = $this->getMockBuilder( 'stdClass' ) - ->setMethods( [ 'callback' ] ) + ->addMethods( [ 'callback' ] ) ->getMock(); $mock->expects( $this->exactly( 2 ) ) @@ -277,28 +278,25 @@ public function test_maybe_disable_notifications() { */ public function test_queue_async_form_submission_tasks() { $results = $this->create_form_and_entries(); - $entry = $results['entry']; $form = $results['form']; - - /* Set up active and inactive notification */ - $form['notifications']['1254123223'] = [ 'id' => '1254123223', 'isActive' => true, 'event' => 'form_submission' ]; - $form['notifications']['2222222222'] = [ 'id' => '2222222222', 'isActive' => false, 'event' => 'form_submission' ]; - $form['gfpdf_form_settings']['556690c67856b']['notification'][] = '1254123223'; - + $entry = $results['entry']; + $form['notifications']['1254123223'] = $form['notifications']['54bca349732b8']; $form['notifications']['54bca349732b8']['isActive'] = true; + /* Queue multiple entry notifications */ foreach ( $form['notifications'] as $notification ) { $this->controller->maybe_disable_submission_notifications( false, $notification, $form, $entry ); } - $this->controller->queue_async_form_submission_tasks( $entry, $form ); $queue = $this->queue_mock->get_data(); - $this->assertStringContainsString( 'create_pdf', $queue[0][0]['func'] ); - $this->assertStringContainsString( 'create_pdf', $queue[0][1]['func'] ); - $this->assertStringContainsString( 'send_notification', $queue[0][2]['func'] ); - $this->assertStringContainsString( 'send_notification', $queue[0][3]['func'] ); + $this->assertCount( 4, $queue ); + + $this->assertStringContainsString( 'create-pdf-1-1', $queue[0][0]['id'] ); + $this->assertStringContainsString( 'create-pdf-1-1', $queue[1][0]['id'] ); + $this->assertStringContainsString( 'send-notification-1-1', $queue[2][0]['id'] ); + $this->assertStringContainsString( 'send-notification-1-1', $queue[3][0]['id'] ); } /** @@ -308,24 +306,28 @@ public function test_queue_async_form_submission_tasks() { */ public function test_queue_async_resend_notification_tasks() { $results = $this->create_form_and_entries(); - $entry = $results['entry']; $form = $results['form']; $form['notifications']['54bca349732b8']['isActive'] = true; - foreach( $form['notifications'] as $notification ) { - $this->controller->maybe_disable_resend_notifications( false, $notification, $form, $entry ); + foreach( $GLOBALS['GFPDF_Test']->entries['all-form-fields'] as $entry ) { + foreach ( $form['notifications'] as $notification ) { + $this->controller->maybe_disable_submission_notifications( false, $notification, $form, $entry ); + } } - $this->controller->queue_dispatch_resend_notification_tasks( $form, $entry ); + $this->controller->queue_dispatch_resend_notification_tasks(); $queue = $this->queue_mock->get_data(); - $this->assertCount( 3, $queue[0] ); + $this->assertCount( 21, $queue ); - $actions = [ 'create_pdf', 'create_pdf', 'send_notification' ]; - for ( $i = 0; $i < 3; $i++ ) { - $this->assertStringContainsString( $actions[ $i ], $queue[0][ $i ]['func'] ); - } + $this->assertStringContainsString( 'create-pdf-1-1', $queue[0][0]['id'] ); + $this->assertStringContainsString( 'create-pdf-1-1', $queue[1][0]['id'] ); + $this->assertStringContainsString( 'send-notification-1-1', $queue[2][0]['id'] ); + + $this->assertStringContainsString( 'create-pdf-1-7', $queue[18][0]['id'] ); + $this->assertStringContainsString( 'create-pdf-1-7', $queue[19][0]['id'] ); + $this->assertStringContainsString( 'send-notification-1-7', $queue[20][0]['id'] ); } /** @@ -339,13 +341,12 @@ public function test_queue_dispatch_resend_notification_tasks() { ->method( 'dispatch' ) ->willReturn( $this->queue_mock ); - - $this->controller->queue_dispatch_resend_notification_tasks( [ 'id' => 0 ], [ 'id' => 0, 'form_id' => 0 ] ); + $this->controller->queue_dispatch_resend_notification_tasks(); $this->assertSame( 0, $spy->getInvocationCount() ); $this->queue_mock->push_to_queue( 'item' ); - $this->controller->queue_dispatch_resend_notification_tasks( [ 'id' => 0 ], [ 'id' => 0, 'form_id' => 0 ]); + $this->controller->queue_dispatch_resend_notification_tasks(); $this->assertSame( 1, $spy->getInvocationCount() ); } @@ -356,17 +357,17 @@ public function test_queue_dispatch_resend_notification_tasks() { * @since 5.0 */ public function test_cleanup_pdfs() { - $this->setExpectedIncorrectUsage( 'GFPDF\Statics\Queue_Callbacks::cleanup_pdfs'); - $this->setExpectedIncorrectUsage( 'GFPDF\Model\Model_PDF::cleanup_pdf'); + $this->setExpectedIncorrectUsage( 'GFPDF\Statics\Queue_Callbacks::cleanup_pdfs' ); + $this->setExpectedIncorrectUsage( 'GFPDF\Model\Model_PDF::cleanup_pdf' ); $form_class = \GPDFAPI::get_form_class(); $results = $this->create_form_and_entries(); $entry = $results['entry']; - $form = $form_class->get_form( $results['form']['id'] ); /* get from the database so the date created is accurate */ + $form = $form_class->get_form( $results['form']['id'] ); $path = Cache::get_path( $form, $entry, $form['gfpdf_form_settings']['556690c67856b'] ); - $file = "test-{$form['id']}.pdf"; + $file = "test-{$form['id']}.pdf"; wp_mkdir_p( $path ); touch( $path . $file ); @@ -378,4 +379,43 @@ public function test_cleanup_pdfs() { $this->assertFileDoesNotExist( $path . $file ); $this->assertFileDoesNotExist( $path ); } + + /** + * Verify any scheduled queues are cleaned up when the queue setting is toggled + * + * @since 6.12.6 + */ + public function test_queue_cleanup() { + global $gfpdf; + + /* setup page */ + $_POST['option_page'] = 'gfpdf_settings'; + $_POST['_wp_http_referer'] = '/'; + + $queue = new Helper_Pdf_Queue( $gfpdf->log ); + + /* Create queue and verify */ + $queue->push_to_queue( 'item1' )->save(); + $queue->push_to_queue( 'item2' )->save(); + + $this->assertCount( 2, $queue->get_batches() ); + + /* Toggle the settings and verify */ + /** @var Helper_Abstract_Options $options */ + $options = $gfpdf->options; + + $options->update_option( 'background_processing', 'Yes' ); + $options->update_settings( [ 'background_processing' => 'No' ] ); + + $this->assertCount( 0, $queue->get_batches() ); + + /* Add new queue items, update the settings without toggling and verify the queue remains */ + $queue->push_to_queue( 'item1' )->save(); + $queue->push_to_queue( 'item2' )->save(); + + $this->assertCount( 2, $queue->get_batches() ); + + $options->update_settings( [ 'background_processing' => 'No' ] ); + $this->assertCount( 2, $queue->get_batches() ); + } }