Skip to content

[ECP-9941] Fix L2/L3 enhanced scheme data validation#3307

Merged
shubhamk67 merged 1 commit into
mainfrom
ECP-9941
May 27, 2026
Merged

[ECP-9941] Fix L2/L3 enhanced scheme data validation#3307
shubhamk67 merged 1 commit into
mainfrom
ECP-9941

Conversation

@shubhamk67

Copy link
Copy Markdown
Contributor

Description
This PR fixes L2/L3 enhanced scheme data validation errors by introducing a dedicated validator.

@shubhamk67 shubhamk67 added the Fix Indicates a bug fix label May 26, 2026
@shubhamk67 shubhamk67 requested a review from a team as a code owner May 26, 2026 16:38

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

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.

Code Review

This pull request introduces a new Level23DataValidator class to sanitize and validate Level 2/3 transaction data, refactoring AdditionalDataLevel23DataBuilder to delegate validation tasks to this new class. It also adds comprehensive unit tests for the validator. The code review feedback highlights several critical issues: a potential fatal error due to a missing null check on the shipping address, a regression where casting quantities to integers truncates fractional values, a regex bug that rejects non-ASCII product descriptions, and an opportunity to improve sanitization by using trim() instead of ltrim().

Comment on lines +94 to 119
$postalCode = $this->level23DataValidator->sanitizePostalCode(
(string) $order->getShippingAddress()->getPostcode()
);
if ($postalCode !== null) {
$additionalDataLevel23[self::ENHANCED_SCHEME_DATA_PREFIX . '.destinationPostalCode'] =
$postalCode;
}

$additionalDataLevel23[self::ENHANCED_SCHEME_DATA_PREFIX . '.destinationCountryCode'] =
$order->getShippingAddress()->getCountryId();
$alpha3CountryCode = $this->level23DataValidator->convertCountryCodeToAlpha3(
(string) $order->getShippingAddress()->getCountryId()
);
if ($alpha3CountryCode !== null) {
$additionalDataLevel23[self::ENHANCED_SCHEME_DATA_PREFIX . '.destinationCountryCode'] =
$alpha3CountryCode;
}

if (!empty($order->getShippingAddress()->getRegionCode())) {
$additionalDataLevel23[self::ENHANCED_SCHEME_DATA_PREFIX . '.destinationStateProvinceCode'] =
$order->getShippingAddress()->getRegionCode();
$regionCode = $order->getShippingAddress()->getRegionCode();
if (!empty($regionCode)) {
$sanitizedRegionCode = $this->level23DataValidator->sanitizeStateProvinceCode(
(string) $regionCode
);
if ($sanitizedRegionCode !== null) {
$additionalDataLevel23[self::ENHANCED_SCHEME_DATA_PREFIX . '.destinationStateProvinceCode'] =
$sanitizedRegionCode;
}
}

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.

high

The $order->getShippingAddress() method can return null (for example, if the shipping address is missing or deleted). Calling methods like getPostcode(), getCountryId(), or getRegionCode() directly on a potentially null object will trigger a fatal error.

Please add a null check for the shipping address before accessing its properties.

                $shippingAddress = $order->getShippingAddress();
                if ($shippingAddress) {
                    $postalCode = $this->level23DataValidator->sanitizePostalCode(
                        (string) $shippingAddress->getPostcode()
                    );
                    if ($postalCode !== null) {
                        $additionalDataLevel23[self::ENHANCED_SCHEME_DATA_PREFIX . '.destinationPostalCode'] =
                            $postalCode;
                    }

                    $alpha3CountryCode = $this->level23DataValidator->convertCountryCodeToAlpha3(
                        (string) $shippingAddress->getCountryId()
                    );
                    if ($alpha3CountryCode !== null) {
                        $additionalDataLevel23[self::ENHANCED_SCHEME_DATA_PREFIX . '.destinationCountryCode'] =
                            $alpha3CountryCode;
                    }

                    $regionCode = $shippingAddress->getRegionCode();
                    if (!empty($regionCode)) {
                        $sanitizedRegionCode = $this->level23DataValidator->sanitizeStateProvinceCode(
                            (string) $regionCode
                        );
                        if ($sanitizedRegionCode !== null) {
                            $additionalDataLevel23[self::ENHANCED_SCHEME_DATA_PREFIX . '.destinationStateProvinceCode'] =
                                $sanitizedRegionCode;
                        }
                    }
                }

// `totalAmount` should be a non-zero numeric value.
if ($orderItem->getRowTotal() === 0) {
$validationResult = false;
$quantity = (int) $item->getQtyOrdered();

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.

high

Casting $item->getQtyOrdered() to (int) truncates fractional quantities (e.g., 1.5 becomes 1). This is a regression from the original implementation which preserved fractional quantities as strings. It will lead to incorrect total amount calculations and mismatched order data sent to Adyen.

Please cast the quantity to (float) instead of (int).

        $quantity = (float) $item->getQtyOrdered();

Comment on lines +178 to +181
public function calculateLineItemTotalAmount(int $quantity, int $unitPrice, int $discountAmount): int
{
return ($quantity * $unitPrice) - $discountAmount;
}

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.

high

To support fractional quantities correctly, the $quantity parameter should be typed as float instead of int. The calculation should also round the resulting total to the nearest minor unit (integer) before returning.

    public function calculateLineItemTotalAmount(float $quantity, int $unitPrice, int $discountAmount): int
    {
        return (int) round(($quantity * $unitPrice) - $discountAmount);
    }

Comment on lines +255 to +258
private function isAllSpecialCharacters(string $value): bool
{
return $value !== '' && preg_match('/^[^a-zA-Z0-9]+$/', $value) === 1;
}

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.

high

The regex /^[^a-zA-Z0-9]+$/ matches any string consisting entirely of non-ASCII alphanumeric characters. This means purely non-ASCII product descriptions (e.g., in Chinese, Japanese, Cyrillic, Arabic, etc.) will be incorrectly identified as consisting entirely of "special characters" and rejected.

To support internationalization and multibyte characters, use the u (UTF-8) modifier along with Unicode character properties \p{L} (letters) and \p{N} (numbers).

    private function isAllSpecialCharacters(string $value):
    {
        return $value !== '' && preg_match('/^[\p{L}\p{N}]+$/u', $value) === 1;
    }

Comment on lines +221 to +225
private function trimAndTruncate(string $value, int $maxLength): string
{
$value = ltrim($value);
return mb_substr($value, 0, $maxLength);
}

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 ltrim only removes leading spaces, leaving trailing spaces intact. Trailing spaces are generally undesirable in sanitized fields (like postal codes, product codes, or customer references) and can cause validation issues or waste character limits.

Please use trim() instead of ltrim() to clean up both ends of the string.

    private function trimAndTruncate(string $value, int $maxLength): string
    {
        $value = trim($value);
        return mb_substr($value, 0, $maxLength);
    }

Comment on lines +194 to +202
public function testCalculateLineItemTotalAmount(
int $quantity,
int $unitPrice,
int $discountAmount,
int $expected
): void {
$result = $this->validator->calculateLineItemTotalAmount($quantity, $unitPrice, $discountAmount);
$this->assertSame($expected, $result);
}

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

Update the parameter type of $quantity to float to match the updated signature of calculateLineItemTotalAmount.

    public function testCalculateLineItemTotalAmount(
        float $quantity,
        int $unitPrice,
        int $discountAmount,
        int $expected
    ): void {
        $result = $this->validator->calculateLineItemTotalAmount($quantity, $unitPrice, $discountAmount);
        $this->assertSame($expected, $result);
    }

@sonarqubecloud

Copy link
Copy Markdown

@shubhamk67 shubhamk67 merged commit ed877ff into main May 27, 2026
9 of 10 checks passed
@shubhamk67 shubhamk67 deleted the ECP-9941 branch May 27, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Indicates a bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants