From f2f3e0fbe3823111af5596e3bd7c77176fe8c547 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 29 Jul 2026 22:32:18 +0600 Subject: [PATCH 1/2] refactor: remove deprecated OrderService and Customer pass-throughs Drop handleOrderSave/removeOrder and Customer::getId wrappers after confirming zero in-repo callers; core uses msOrder::save/remove and Customer::getOrCreate directly. Closes #357 --- .../src/Controllers/Customer/Customer.php | 11 --- .../src/Services/Customer/RegisterService.php | 2 +- .../src/Services/Order/OrderService.php | 32 -------- .../DeprecatedPassThroughRemovedTest.php | 80 +++++++++++++++++++ 4 files changed, 81 insertions(+), 44 deletions(-) create mode 100644 core/components/minishop3/tests/DeprecatedPassThroughRemovedTest.php diff --git a/core/components/minishop3/src/Controllers/Customer/Customer.php b/core/components/minishop3/src/Controllers/Customer/Customer.php index dd76b3fc..123e6868 100644 --- a/core/components/minishop3/src/Controllers/Customer/Customer.php +++ b/core/components/minishop3/src/Controllers/Customer/Customer.php @@ -307,17 +307,6 @@ public function validate(string $key, mixed $value): mixed return $response['data']['value']; } - /** - * Get customer ID for order (legacy method) - * - * @deprecated Use getOrCreate() instead - * @return int Customer ID or 0 if not found/created - */ - public function getId(): int - { - return $this->getOrCreate(); - } - public function create(array $customerData): msCustomer|null { // Allow plugins to modify data before creation diff --git a/core/components/minishop3/src/Services/Customer/RegisterService.php b/core/components/minishop3/src/Services/Customer/RegisterService.php index 1c98c1a9..38cb8a05 100644 --- a/core/components/minishop3/src/Services/Customer/RegisterService.php +++ b/core/components/minishop3/src/Services/Customer/RegisterService.php @@ -67,7 +67,7 @@ public function setEmailVerification(EmailVerificationService $service): void * * Used in: * - Register API processor (explicit user registration) - * - Customer::getId() (automatic registration during checkout) + * - Customer::getOrCreate() (automatic registration during checkout) * * @param array $data Customer data: * - email (required) diff --git a/core/components/minishop3/src/Services/Order/OrderService.php b/core/components/minishop3/src/Services/Order/OrderService.php index cc722446..42250edf 100644 --- a/core/components/minishop3/src/Services/Order/OrderService.php +++ b/core/components/minishop3/src/Services/Order/OrderService.php @@ -118,38 +118,6 @@ public function updateProducts(msOrder $order): bool return $order->save(); } - /** - * Handle order save with events - * - * @deprecated Logic moved to msOrder::save(), this method kept for backward compatibility - * - * @param msOrder $order - * @param bool|null $cacheFlag - * @return bool - */ - public function handleOrderSave(msOrder $order, ?bool $cacheFlag = null): bool - { - // Simply delegate call to msOrder::save() - // It already contains all event logic - return $order->save($cacheFlag); - } - - /** - * Delete order with events - * - * @deprecated Logic moved to msOrder::remove(), this method kept for backward compatibility - * - * @param msOrder $order - * @param array $ancestors - * @return bool Deletion result - */ - public function removeOrder(msOrder $order, array $ancestors = []): bool - { - // Simply delegate call to msOrder::remove() - // It already contains all event logic - return $order->remove($ancestors); - } - /** * Get order statistics * diff --git a/core/components/minishop3/tests/DeprecatedPassThroughRemovedTest.php b/core/components/minishop3/tests/DeprecatedPassThroughRemovedTest.php new file mode 100644 index 00000000..4550cc0e --- /dev/null +++ b/core/components/minishop3/tests/DeprecatedPassThroughRemovedTest.php @@ -0,0 +1,80 @@ + [ + 'function handleOrderSave', + 'function removeOrder', + ], + 'src/Controllers/Customer/Customer.php' => [ + 'function getId', + ], +]; + +foreach ($checks as $relativePath => $forbiddenSnippets) { + $path = $root . '/' . $relativePath; + $src = file_get_contents($path); + if ($src === false || $src === '') { + $fail("unable to read {$relativePath}"); + } + + foreach ($forbiddenSnippets as $snippet) { + if (str_contains($src, $snippet)) { + $fail("{$relativePath} must not contain deprecated pass-through {$snippet}"); + } + } +} + +$coreSrc = $root . '/src'; +$iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($coreSrc, FilesystemIterator::SKIP_DOTS) +); + +foreach ($iterator as $fileInfo) { + if (!$fileInfo->isFile() || $fileInfo->getExtension() !== 'php') { + continue; + } + + $path = $fileInfo->getPathname(); + $relative = str_replace($root . '/', '', $path); + $src = file_get_contents($path); + if ($src === false) { + $fail("unable to read {$relative}"); + } + + if (str_contains($src, 'handleOrderSave(')) { + $fail("core must not call handleOrderSave(); use msOrder::save() (found in {$relative})"); + } + + if (str_contains($src, 'removeOrder(')) { + $fail("core must not call removeOrder(); use msOrder::remove() (found in {$relative})"); + } + + if ( + str_contains($relative, 'Controllers/Customer/Customer.php') + || str_contains($relative, 'Services/Customer/RegisterService.php') + ) { + continue; + } + + if (preg_match('/\$this->ms3->customer->getId\s*\(/', $src)) { + $fail("core must not call customer->getId(); use getOrCreate() (found in {$relative})"); + } +} + +fwrite(STDOUT, "OK DeprecatedPassThroughRemovedTest\n"); +exit(0); From 5a186f8db76c680b7cb17933f0058616ca3698c1 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 29 Jul 2026 23:04:06 +0600 Subject: [PATCH 2/2] test(regression): convert deprecated passthrough test to PHPUnit Move tests/DeprecatedPassThroughRemovedTest.php into tests/Unit/Regression/ as a PHPUnit class and narrow assertions to method_exists + ReflectionClass::hasMethod on OrderService and Customer, dropping the full-tree grep that was prone to false positives. --- .../DeprecatedPassThroughRemovedTest.php | 80 ------------------- .../DeprecatedPassThroughRemovedTest.php | 48 +++++++++++ 2 files changed, 48 insertions(+), 80 deletions(-) delete mode 100644 core/components/minishop3/tests/DeprecatedPassThroughRemovedTest.php create mode 100644 core/components/minishop3/tests/Unit/Regression/DeprecatedPassThroughRemovedTest.php diff --git a/core/components/minishop3/tests/DeprecatedPassThroughRemovedTest.php b/core/components/minishop3/tests/DeprecatedPassThroughRemovedTest.php deleted file mode 100644 index 4550cc0e..00000000 --- a/core/components/minishop3/tests/DeprecatedPassThroughRemovedTest.php +++ /dev/null @@ -1,80 +0,0 @@ - [ - 'function handleOrderSave', - 'function removeOrder', - ], - 'src/Controllers/Customer/Customer.php' => [ - 'function getId', - ], -]; - -foreach ($checks as $relativePath => $forbiddenSnippets) { - $path = $root . '/' . $relativePath; - $src = file_get_contents($path); - if ($src === false || $src === '') { - $fail("unable to read {$relativePath}"); - } - - foreach ($forbiddenSnippets as $snippet) { - if (str_contains($src, $snippet)) { - $fail("{$relativePath} must not contain deprecated pass-through {$snippet}"); - } - } -} - -$coreSrc = $root . '/src'; -$iterator = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($coreSrc, FilesystemIterator::SKIP_DOTS) -); - -foreach ($iterator as $fileInfo) { - if (!$fileInfo->isFile() || $fileInfo->getExtension() !== 'php') { - continue; - } - - $path = $fileInfo->getPathname(); - $relative = str_replace($root . '/', '', $path); - $src = file_get_contents($path); - if ($src === false) { - $fail("unable to read {$relative}"); - } - - if (str_contains($src, 'handleOrderSave(')) { - $fail("core must not call handleOrderSave(); use msOrder::save() (found in {$relative})"); - } - - if (str_contains($src, 'removeOrder(')) { - $fail("core must not call removeOrder(); use msOrder::remove() (found in {$relative})"); - } - - if ( - str_contains($relative, 'Controllers/Customer/Customer.php') - || str_contains($relative, 'Services/Customer/RegisterService.php') - ) { - continue; - } - - if (preg_match('/\$this->ms3->customer->getId\s*\(/', $src)) { - $fail("core must not call customer->getId(); use getOrCreate() (found in {$relative})"); - } -} - -fwrite(STDOUT, "OK DeprecatedPassThroughRemovedTest\n"); -exit(0); diff --git a/core/components/minishop3/tests/Unit/Regression/DeprecatedPassThroughRemovedTest.php b/core/components/minishop3/tests/Unit/Regression/DeprecatedPassThroughRemovedTest.php new file mode 100644 index 00000000..23496274 --- /dev/null +++ b/core/components/minishop3/tests/Unit/Regression/DeprecatedPassThroughRemovedTest.php @@ -0,0 +1,48 @@ + + */ + public static function removedPassThroughsProvider(): iterable + { + yield 'OrderService::handleOrderSave' => [OrderService::class, 'handleOrderSave']; + yield 'OrderService::removeOrder' => [OrderService::class, 'removeOrder']; + yield 'Customer::getId' => [Customer::class, 'getId']; + } + + #[DataProvider('removedPassThroughsProvider')] + public function testDeprecatedPassThroughIsRemoved(string $class, string $method): void + { + self::assertFalse( + method_exists($class, $method), + sprintf('%s::%s() must remain removed (deprecated pass-through).', $class, $method) + ); + + $reflection = new \ReflectionClass($class); + self::assertFalse( + $reflection->hasMethod($method), + sprintf('%s::%s() must not be redeclared (checked via reflection).', $class, $method) + ); + } +}