Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
 openhands
|
461d6e7fa8 |
@@ -42,6 +42,13 @@ class economic_invoice_draft
|
||||
*/
|
||||
protected float $conversion_rate = 1.0;
|
||||
|
||||
/**
|
||||
* Whether pre-flight validation runs inside addLines() before sending to e-conomic.
|
||||
* Defense in depth — even after sanitization, a final check catches anything that slips through.
|
||||
* @var bool $preflight_enabled
|
||||
*/
|
||||
protected bool $preflight_enabled = true;
|
||||
|
||||
|
||||
/**
|
||||
* Construct a new Economic draft invoice object
|
||||
@@ -116,9 +123,142 @@ class economic_invoice_draft
|
||||
*/
|
||||
public function addLines(): void
|
||||
{
|
||||
if ($this->preflight_enabled) {
|
||||
$this->preflightValidate($this->draft_lines, null);
|
||||
}
|
||||
$this->flushLinesInBatches();
|
||||
}
|
||||
|
||||
/**
|
||||
* Pre-flight validation: defense in depth before sending to e-conomic.
|
||||
* Validates each line against 5 rules and throws RuntimeException on the first violation.
|
||||
*
|
||||
* Rules (in order, per line):
|
||||
* 1. description — must be non-empty after trim()
|
||||
* 2. description — must be <= 250 chars
|
||||
* 3. productNumber (if present in product.productNumber) — must match /^[A-Za-z0-9._-]{1,50}$/
|
||||
* 4. quantity — must be a positive number (> 0)
|
||||
* 5. unitNetPrice — must be a number (>= 0)
|
||||
*
|
||||
* @param array<int,array<string,mixed>> $lines
|
||||
* @param int|null $orderId Optional order id for log context
|
||||
* @throws \RuntimeException on any rule violation
|
||||
*/
|
||||
public function preflightValidate(array $lines, ?int $orderId = null): void
|
||||
{
|
||||
foreach ($lines as $i => $line) {
|
||||
if (!is_array($line)) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'line is not an array',
|
||||
$line
|
||||
);
|
||||
}
|
||||
|
||||
// Rule 1 + 2: description
|
||||
$description = $line['description'] ?? null;
|
||||
if ($description === null) {
|
||||
$description = '';
|
||||
}
|
||||
if (!is_scalar($description)) {
|
||||
$description = (string)$description;
|
||||
} else {
|
||||
$description = (string)$description;
|
||||
}
|
||||
$descriptionTrimmed = trim($description);
|
||||
if ($descriptionTrimmed === '') {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'description is empty',
|
||||
$description
|
||||
);
|
||||
}
|
||||
if (mb_strlen($descriptionTrimmed) > 250) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'description exceeds 250 chars (length=' . mb_strlen($descriptionTrimmed) . ')',
|
||||
$description
|
||||
);
|
||||
}
|
||||
|
||||
// Rule 3: productNumber (only if present in product.productNumber)
|
||||
if (isset($line['product']) && is_array($line['product']) && array_key_exists('productNumber', $line['product'])) {
|
||||
$productNumber = $line['product']['productNumber'];
|
||||
if ($productNumber === null) {
|
||||
$productNumber = '';
|
||||
} else {
|
||||
$productNumber = (string)$productNumber;
|
||||
}
|
||||
if (!preg_match('/^[A-Za-z0-9._-]{1,50}$/', $productNumber)) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'productNumber does not match /^[A-Za-z0-9._-]{1,50}$/',
|
||||
$productNumber
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Rule 4: quantity — only required if present in the line (text lines omit it)
|
||||
if (array_key_exists('quantity', $line)) {
|
||||
$quantity = $line['quantity'];
|
||||
if (!is_numeric($quantity) || (float)$quantity <= 0) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'quantity is not a positive number',
|
||||
$quantity
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Rule 5: unitNetPrice — only required if present in the line
|
||||
if (array_key_exists('unitNetPrice', $line)) {
|
||||
$unitNetPrice = $line['unitNetPrice'];
|
||||
if (!is_numeric($unitNetPrice) || (float)$unitNetPrice < 0) {
|
||||
$this->logAndThrow(
|
||||
$i,
|
||||
$orderId,
|
||||
'unitNetPrice is not a number >= 0',
|
||||
$unitNetPrice
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Log the offending line and throw a RuntimeException.
|
||||
*/
|
||||
private function logAndThrow(int $lineIndex, ?int $orderId, string $rule, mixed $value): never
|
||||
{
|
||||
$valueTruncated = is_scalar($value) ? (string)$value : json_encode($value);
|
||||
if ($valueTruncated === false) {
|
||||
$valueTruncated = '[unserializable]';
|
||||
}
|
||||
if (mb_strlen($valueTruncated) > 200) {
|
||||
$valueTruncated = mb_substr($valueTruncated, 0, 200) . '...';
|
||||
}
|
||||
$orderContext = $orderId === null ? 'order=n/a' : 'order=' . $orderId;
|
||||
error_log(sprintf(
|
||||
'[preflight] validation failed: %s | line=%d | %s | value=%s',
|
||||
$rule,
|
||||
$lineIndex,
|
||||
$orderContext,
|
||||
$valueTruncated
|
||||
));
|
||||
$orderPart = $orderId === null ? '' : ' (order ' . $orderId . ')';
|
||||
throw new \RuntimeException(sprintf(
|
||||
'Preflight validation failed for line %d: %s%s',
|
||||
$lineIndex,
|
||||
$rule,
|
||||
$orderPart
|
||||
));
|
||||
}
|
||||
|
||||
/**
|
||||
* Add queued draft lines using chunked requests.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,379 @@
|
||||
<?php
|
||||
|
||||
namespace tests\Unit\Economic;
|
||||
|
||||
use helpers\economic_invoice_draft;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use RuntimeException;
|
||||
|
||||
require_once __DIR__ . '/../../../modules/economic/helpers/economic_invoice_draft.php';
|
||||
|
||||
/**
|
||||
* Unit tests for the pre-flight validation in economic_invoice_draft.
|
||||
*
|
||||
* These tests build a draft instance via the skip_fetch path (so we never
|
||||
* hit the e-conomic API), call preflightValidate() directly, and assert
|
||||
* that each rule is enforced.
|
||||
*/
|
||||
class EconomicInvoiceDraftPreflightTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* Build a draft instance in skip_fetch mode. We never hit the network.
|
||||
*/
|
||||
private function makeDraft(bool $preflight = true): economic_invoice_draft
|
||||
{
|
||||
$draft = new economic_invoice_draft(12345, 'DKK', true);
|
||||
// Make the preflight_enabled flag mutable for the disabled test.
|
||||
$reflection = new \ReflectionClass($draft);
|
||||
$prop = $reflection->getProperty('preflight_enabled');
|
||||
$prop->setAccessible(true);
|
||||
$prop->setValue($draft, $preflight);
|
||||
return $draft;
|
||||
}
|
||||
|
||||
private function callPreflight(economic_invoice_draft $draft, array $lines, ?int $orderId = null): void
|
||||
{
|
||||
$draft->preflightValidate($lines, $orderId);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 1: description must be non-empty after trim()
|
||||
// ========================================================================
|
||||
|
||||
public function testEmptyDescriptionThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/description is empty/');
|
||||
$this->callPreflight($draft, [
|
||||
['description' => ''],
|
||||
], 42);
|
||||
}
|
||||
|
||||
public function testWhitespaceOnlyDescriptionThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/description is empty/');
|
||||
$this->callPreflight($draft, [
|
||||
['description' => " \t \n "],
|
||||
], 99);
|
||||
}
|
||||
|
||||
public function testMissingDescriptionThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/description is empty/');
|
||||
$this->callPreflight($draft, [
|
||||
['quantity' => 1, 'unitNetPrice' => 10.0],
|
||||
], 1);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 2: description must be <= 250 chars
|
||||
// ========================================================================
|
||||
|
||||
public function testTooLongDescriptionThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/exceeds 250 chars/');
|
||||
$long = str_repeat('a', 251);
|
||||
$this->callPreflight($draft, [
|
||||
['description' => $long],
|
||||
], 7);
|
||||
}
|
||||
|
||||
public function testDescriptionAt250Passes(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
// 250 chars — should pass (not throw)
|
||||
$this->callPreflight($draft, [
|
||||
['description' => str_repeat('b', 250)],
|
||||
], 8);
|
||||
$this->assertTrue(true); // no exception means success
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 3: productNumber must match /^[A-Za-z0-9._-]{1,50}$/
|
||||
// ========================================================================
|
||||
|
||||
public function testInvalidProductNumberThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/productNumber does not match/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'PROD/01'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 11);
|
||||
}
|
||||
|
||||
public function testProductNumberTooLongThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/productNumber does not match/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => str_repeat('a', 51)],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 12);
|
||||
}
|
||||
|
||||
public function testValidProductNumberPasses(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'PROD-01.0_v2'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 13);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 4: quantity must be a positive number (> 0)
|
||||
// ========================================================================
|
||||
|
||||
public function testZeroQuantityThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/quantity is not a positive number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 0,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 21);
|
||||
}
|
||||
|
||||
public function testNegativeQuantityThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/quantity is not a positive number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => -1.5,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 22);
|
||||
}
|
||||
|
||||
public function testNonNumericQuantityThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/quantity is not a positive number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 'NaN-ish',
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 23);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Rule 5: unitNetPrice must be a number (>= 0)
|
||||
// ========================================================================
|
||||
|
||||
public function testNegativeUnitPriceThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/unitNetPrice is not a number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => -5.0,
|
||||
],
|
||||
], 31);
|
||||
}
|
||||
|
||||
public function testNonNumericUnitPriceThrows(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->expectException(RuntimeException::class);
|
||||
$this->expectExceptionMessageMatches('/unitNetPrice is not a number/');
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 'free',
|
||||
],
|
||||
], 32);
|
||||
}
|
||||
|
||||
public function testZeroUnitPricePasses(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
// 0 is allowed — it's "a number >= 0"
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 0,
|
||||
],
|
||||
], 33);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Happy path: a fully-valid line passes
|
||||
// ========================================================================
|
||||
|
||||
public function testValidLinePasses(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$this->callPreflight($draft, [
|
||||
[
|
||||
'description' => 'A normal product line',
|
||||
'product' => ['productNumber' => 'WASH-01'],
|
||||
'quantity' => 2,
|
||||
'unitNetPrice' => 99.5,
|
||||
],
|
||||
[
|
||||
'description' => 'A text-only line',
|
||||
],
|
||||
[
|
||||
'description' => 'Discount',
|
||||
'product' => ['productNumber' => 'TotDiscount'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 0.0,
|
||||
],
|
||||
], 100);
|
||||
$this->assertTrue(true);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Disabled preflight: bad lines must NOT throw
|
||||
// ========================================================================
|
||||
|
||||
public function testPreflightCanBeDisabled(): void
|
||||
{
|
||||
// The preflight_enabled flag gates addLines() (i.e. it controls whether
|
||||
// preflightValidate() runs before flushLinesInBatches). It does NOT
|
||||
// affect direct calls to preflightValidate(). So this test verifies:
|
||||
// 1. The default value is true.
|
||||
// 2. The flag is mutable to false.
|
||||
// 3. addLines() is wired to short-circuit preflight when the flag is false.
|
||||
$draft = $this->makeDraft(true);
|
||||
|
||||
// (1) Default: preflight is enabled
|
||||
$ref = new \ReflectionClass($draft);
|
||||
$prop = $ref->getProperty('preflight_enabled');
|
||||
$prop->setAccessible(true);
|
||||
$this->assertTrue($prop->getValue($draft), 'preflight_enabled should default to true');
|
||||
|
||||
// (2) Mutable
|
||||
$prop->setValue($draft, false);
|
||||
$this->assertFalse($prop->getValue($draft));
|
||||
|
||||
// (3) Disabled: addLines() must NOT call preflightValidate().
|
||||
// We assert this indirectly: queue a guaranteed-invalid line and then
|
||||
// catch the exception that flushLinesInBatches() would raise when it
|
||||
// tries to send to e-conomic. If preflight were enabled we'd get
|
||||
// RuntimeException("description is empty") first.
|
||||
$this->expectException(\Throwable::class);
|
||||
$reflection = new \ReflectionClass($draft);
|
||||
$linesProp = $reflection->getProperty('draft_lines');
|
||||
$linesProp->setAccessible(true);
|
||||
$linesProp->setValue($draft, [
|
||||
['description' => ''], // would fail rule 1 if preflight ran
|
||||
]);
|
||||
$draft->addLines();
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Multiple errors: report the first one
|
||||
// ========================================================================
|
||||
|
||||
public function testMultipleErrorsReportsFirst(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$caught = null;
|
||||
try {
|
||||
$this->callPreflight($draft, [
|
||||
// First line is fine
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'P1'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
// Second line: empty description (rule 1)
|
||||
['description' => ''],
|
||||
// Third line: would also fail, but we should never get here
|
||||
[
|
||||
'description' => 'OK',
|
||||
'product' => ['productNumber' => 'BAD/CHAR'],
|
||||
'quantity' => 1,
|
||||
'unitNetPrice' => 10.0,
|
||||
],
|
||||
], 300);
|
||||
} catch (RuntimeException $e) {
|
||||
$caught = $e;
|
||||
}
|
||||
$this->assertNotNull($caught, 'Expected a RuntimeException to be thrown');
|
||||
$this->assertStringContainsString('line 1', $caught->getMessage());
|
||||
$this->assertStringContainsString('description is empty', $caught->getMessage());
|
||||
$this->assertStringContainsString('order 300', $caught->getMessage());
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Exception message includes order id when provided
|
||||
// ========================================================================
|
||||
|
||||
public function testExceptionMessageIncludesOrderId(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$caught = null;
|
||||
try {
|
||||
$this->callPreflight($draft, [
|
||||
['description' => ''],
|
||||
], 4242);
|
||||
} catch (RuntimeException $e) {
|
||||
$caught = $e;
|
||||
}
|
||||
$this->assertNotNull($caught);
|
||||
$this->assertStringContainsString('order 4242', $caught->getMessage());
|
||||
}
|
||||
|
||||
public function testExceptionMessageOmitsOrderWhenNull(): void
|
||||
{
|
||||
$draft = $this->makeDraft();
|
||||
$caught = null;
|
||||
try {
|
||||
$this->callPreflight($draft, [
|
||||
['description' => ''],
|
||||
], null);
|
||||
} catch (RuntimeException $e) {
|
||||
$caught = $e;
|
||||
}
|
||||
$this->assertNotNull($caught);
|
||||
$this->assertStringNotContainsString('order', $caught->getMessage());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user