Skip to content

[ECP-9944] Add cron to clean up stale adyen_payment_response rows#3297

Merged
shubhamk67 merged 1 commit into
develop-11from
ECP-9944
Apr 23, 2026
Merged

[ECP-9944] Add cron to clean up stale adyen_payment_response rows#3297
shubhamk67 merged 1 commit into
develop-11from
ECP-9944

Conversation

@shubhamk67
Copy link
Copy Markdown
Contributor

Description
This PR introduces an opt-in nightly cron job that removes stale rows from adyen_payment_response. Once the order is finalized or orphaned, it is safe to drop these records.

Fixes #2578

@shubhamk67 shubhamk67 requested a review from a team as a code owner April 22, 2026 17:06
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a PaymentResponseCleanUp cron job to manage the growth of the adyen_payment_response table by deleting records associated with finalized orders or orphaned entries older than a grace period. Key additions include a created_at timestamp in the database schema, configuration toggles, and corresponding unit tests. Feedback suggests improving the architecture by moving database constants to the Resource Model, enhancing the cron job to process multiple batches in a single run for better scalability, and ensuring timezone consistency by using UTC DateTime for date calculations.

Comment on lines +20 to +21
const TABLE_NAME = 'adyen_payment_response';
const TABLE_NAME_ALIAS = 'payment_response';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Database-specific constants such as TABLE_NAME and TABLE_NAME_ALIAS should ideally reside in the Resource Model rather than the Data Interface. The Interface should focus on the data contract, while the Resource Model handles storage details. Moving these to Adyen\Payment\Model\ResourceModel\PaymentResponse would better adhere to the separation of concerns.

Comment on lines +67 to +77
$finalizedRemoved = $this->deleteBatch(
'finalized',
fn (): array => $this->paymentResponseCollectionFactory->create()
->getFinalizedPaymentResponseIds(self::BATCH_SIZE)
);

$orphanRemoved = $this->deleteBatch(
'orphan',
fn (): array => $this->paymentResponseCollectionFactory->create()
->getOrphanPaymentResponseIds(self::ORPHAN_GRACE_DAYS, self::BATCH_SIZE)
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation only processes a single batch of 1000 rows per cron run. For high-volume stores or when clearing a large backlog, this might be insufficient. Consider wrapping the deletion logic in a loop that continues until fewer than BATCH_SIZE rows are returned, or until a maximum number of iterations is reached to prevent timeouts. This would ensure the table is cleaned more effectively without relying solely on the cron scheduler to trigger multiple runs.

*/
public function getOrphanPaymentResponseIds(int $graceDays, int $batchSize): array
{
$dateFrom = date('Y-m-d H:i:s', time() - $graceDays * 24 * 60 * 60);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using date() and time() relies on the PHP server's configured timezone, which may differ from the UTC timezone typically used by the Magento database for timestamp columns. This can result in an incorrect grace period calculation if the server timezone is not UTC. It is recommended to use an explicit UTC DateTime object to ensure the date string matches the database storage.

        $dateFrom = (new \DateTime('now', new \DateTimeZone('UTC')))->sub(new \DateInterval("P{$graceDays}D"))->format('Y-m-d H:i:s');

@sonarqubecloud
Copy link
Copy Markdown

@shubhamk67 shubhamk67 merged commit aa1c0ec into develop-11 Apr 23, 2026
9 of 11 checks passed
@shubhamk67 shubhamk67 deleted the ECP-9944 branch April 23, 2026 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants