refactor(di): OptionSync/Loader + ManagerOrderCostRecalculator in ServiceRegistry

core/components/minishop3/config/ms3.services.example.php

@@ -193,9 +193,15 @@
      * 'ms3_category_service'        - Category operations
      * 'ms3_category_option_service' - Category options
      *
-     * Product Options:
+     * Product Options (override via ms3.services.php / ms3.services.d/):
      * --------------
-     * 'ms3_option_service'          - EAV options system
+     * 'ms3_option_service'          - EAV options facade
+     * 'ms3_option_loader'           - load option values / admin fields
+     * 'ms3_option_sync'             - save/sync product option values
+     *
+     * Order manager cost:
+     * -------------------
+     * 'ms3_manager_order_cost_recalculator' - manager order totals recalc
      *
      * Utilities:
      * --------

core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php

@@ -330,9 +330,6 @@ public function recalculateCost(array $params = []): array
             return Response::error('Order not found', HttpStatus::NOT_FOUND)->getData();
         }
 
-        /** @var MiniShop3 $ms3 */
-        $ms3 = $this->modx->services->get('ms3');
-
         $modeIn = strtolower(trim((string)($params['mode'] ?? ManagerOrderCostRecalculator::MODE_AUTO)));
         $allowedModes = [
             ManagerOrderCostRecalculator::MODE_AUTO,
@@ -348,7 +345,7 @@ public function recalculateCost(array $params = []): array
             $options['manual_delivery_cost'] = $params['manual_delivery_cost'];
         }
 
-        $recalculator = new ManagerOrderCostRecalculator($this->modx, $ms3);
+        $recalculator = $this->modx->services->get('ms3_manager_order_cost_recalculator');
         $result = $recalculator->recalculate($order, $options);
 
         if (empty($result['success'])) {

core/components/minishop3/src/ServiceRegistry.php

@@ -35,6 +35,66 @@ class ServiceRegistry
     /** @var modX */
     protected modX $modx;
 
+    /**
+     * Controllers requiring only a MiniShop3 instance: __construct(MiniShop3 $ms3).
+     */
+    public const CONTROLLERS_WITH_MS3_ONLY = [
+        'ms3_cart',
+        'ms3_order',
+        'ms3_customer',
+    ];
+
+    /**
+     * Services requiring both modX and MiniShop3: __construct(modX $modx, MiniShop3 $ms3).
+     */
+    public const SERVICES_WITH_MODX_AND_MS3 = [
+        'ms3_order_draft_manager',
+        'ms3_order_cost_calculator',
+        'ms3_manager_order_cost_recalculator',
+        'ms3_order_user_resolver',
+        'ms3_order_log',
+        'ms3_cart_item_manager',
+        'ms3_customer_address_manager',
+    ];
+
+    /**
+     * Services with complex dependencies resolved via DI
+     * (see {@see registerServiceWithDependencies()}).
+     */
+    public const SERVICES_WITH_DEPENDENCIES = [
+        'ms3_option_service',
+        'ms3_order_field_manager',
+        'ms3_order_address_manager',
+        'ms3_order_submit_handler',
+        'ms3_order_status',
+        'ms3_order_finalize',
+    ];
+
+    /**
+     * DI keys that {@see registerServiceWithDependencies()} resolves for each
+     * service. Exposed so tests can assert every referenced dependency is a
+     * registered service key (#363).
+     */
+    public const SERVICE_DEPENDENCIES = [
+        'ms3_option_service' => [
+            'ms3_option_loader',
+            'ms3_option_sync',
+            'ms3_category_option_service',
+        ],
+        'ms3_order_field_manager' => ['ms3_order_draft_manager'],
+        'ms3_order_address_manager' => ['ms3_order_draft_manager', 'ms3_order_field_manager'],
+        'ms3_order_submit_handler' => [
+            'ms3_order_draft_manager',
+            'ms3_order_cost_calculator',
+            'ms3_order_field_manager',
+            'ms3_order_address_manager',
+            'ms3_order_user_resolver',
+            'ms3_order_number_generator',
+        ],
+        'ms3_order_finalize' => ['ms3_order_number_generator'],
+        'ms3_order_status' => ['ms3_order_log'],
+    ];
+
     /**
      * Default services (built into component)
      *
@@ -93,6 +153,10 @@ class ServiceRegistry
             'class' => \MiniShop3\Services\Order\OrderCostCalculator::class,
             'interface' => null,
         ],
+        'ms3_manager_order_cost_recalculator' => [
+            'class' => \MiniShop3\Services\Order\ManagerOrderCostRecalculator::class,
+            'interface' => null,
+        ],
         'ms3_order_field_manager' => [
             'class' => \MiniShop3\Services\Order\OrderFieldManager::class,
             'interface' => null,
@@ -150,6 +214,14 @@ class ServiceRegistry
             'class' => \MiniShop3\Services\Option\OptionService::class,
             'interface' => null,
         ],
+        'ms3_option_loader' => [
+            'class' => \MiniShop3\Services\Option\OptionLoaderService::class,
+            'interface' => null,
+        ],
+        'ms3_option_sync' => [
+            'class' => \MiniShop3\Services\Option\OptionSyncService::class,
+            'interface' => null,
+        ],
         'ms3_cart' => [
             'class' => \MiniShop3\Controllers\Cart\Cart::class,
             'interface' => null,
@@ -446,39 +518,17 @@ protected function registerService(string $serviceKey, array $config): bool
 
         $modx = $this->modx;
 
-        // Controllers requiring only MiniShop3 instance: __construct(MiniShop3 $ms3)
-        $controllersWithMs3Only = ['ms3_cart', 'ms3_order', 'ms3_customer'];
-
-        // Services requiring both modX and MiniShop3: __construct(modX $modx, MiniShop3 $ms3)
-        $servicesWithModxAndMs3 = [
-            'ms3_order_draft_manager',
-            'ms3_order_cost_calculator',
-            'ms3_order_user_resolver',
-            'ms3_order_log',
-            'ms3_cart_item_manager',
-            'ms3_customer_address_manager',
-        ];
-
-        // Services with complex dependencies (resolved via DI)
-        $servicesWithDependencies = [
-            'ms3_order_field_manager',
-            'ms3_order_address_manager',
-            'ms3_order_submit_handler',
-            'ms3_order_status',
-            'ms3_order_finalize',
-        ];
-
-        if (in_array($serviceKey, $controllersWithMs3Only)) {
+        if (in_array($serviceKey, self::CONTROLLERS_WITH_MS3_ONLY, true)) {
             $this->modx->services->add($serviceKey, function () use ($validatedClass, $modx) {
                 $ms3 = $modx->getService('MiniShop3', \MiniShop3\MiniShop3::class);
                 return new $validatedClass($ms3);
             });
-        } elseif (in_array($serviceKey, $servicesWithModxAndMs3)) {
+        } elseif (in_array($serviceKey, self::SERVICES_WITH_MODX_AND_MS3, true)) {
             $this->modx->services->add($serviceKey, function () use ($validatedClass, $modx) {
                 $ms3 = $modx->getService('MiniShop3', \MiniShop3\MiniShop3::class);
                 return new $validatedClass($modx, $ms3);
             });
-        } elseif (in_array($serviceKey, $servicesWithDependencies)) {
+        } elseif (in_array($serviceKey, self::SERVICES_WITH_DEPENDENCIES, true)) {
             // Services with dependencies - resolve them from DI
             $this->registerServiceWithDependencies($serviceKey, $validatedClass);
         } else {
@@ -505,6 +555,17 @@ protected function registerServiceWithDependencies(string $serviceKey, string $v
         $modx = $this->modx;
 
         switch ($serviceKey) {
+            case 'ms3_option_service':
+                // OptionService(xPDO, OptionLoaderService, OptionSyncService, OptionCategoryService)
+                $this->modx->services->add($serviceKey, function () use ($validatedClass, $modx) {
+                    $loader = $modx->services->get('ms3_option_loader');
+                    $sync = $modx->services->get('ms3_option_sync');
+                    $category = $modx->services->get('ms3_category_option_service');
+
+                    return new $validatedClass($modx, $loader, $sync, $category);
+                });
+                break;
+
             case 'ms3_order_field_manager':
                 // OrderFieldManager(modX, MiniShop3, OrderDraftManager)
                 $this->modx->services->add($serviceKey, function () use ($validatedClass, $modx) {

core/components/minishop3/src/Services/Option/OptionService.php

@@ -34,13 +34,20 @@ class OptionService
 
     /**
      * @param xPDO $xpdo
+     * @param OptionLoaderService $loader
+     * @param OptionSyncService $sync
+     * @param OptionCategoryService $category
      */
-    public function __construct(xPDO $xpdo)
-    {
+    public function __construct(
+        xPDO $xpdo,
+        OptionLoaderService $loader,
+        OptionSyncService $sync,
+        OptionCategoryService $category
+    ) {
         $this->xpdo = $xpdo;
-        $this->loader = new OptionLoaderService($xpdo);
-        $this->sync = new OptionSyncService($xpdo);
-        $this->category = new OptionCategoryService($xpdo);
+        $this->loader = $loader;
+        $this->sync = $sync;
+        $this->category = $category;
     }
 
     // ========== LOADING OPERATIONS ==========

core/components/minishop3/tests/ServiceRegistryDiTest.php

@@ -0,0 +1,154 @@
+<?php
+
+/**
+ * DI registry smoke for #363 — OptionSync/Loader/Category + ManagerOrderCostRecalculator.
+ *
+ * Run: php tests/ServiceRegistryDiTest.php
+ */
+
+declare(strict_types=1);
+
+require __DIR__ . '/stubs/ModxStub.php';
+require __DIR__ . '/../vendor/autoload.php';
+
+use MiniShop3\ServiceRegistry;
+use MODX\Revolution\modX;
+
+$fail = static function (string $message): never {
+    fwrite(STDERR, "FAIL: {$message}\n");
+    exit(1);
+};
+
+$registrySrc = file_get_contents(__DIR__ . '/../src/ServiceRegistry.php');
+if ($registrySrc === false || $registrySrc === '') {
+    $fail('unable to read ServiceRegistry.php');
+}
+
+foreach ([
+    'ms3_option_loader',
+    'ms3_option_sync',
+    'ms3_category_option_service',
+    'ms3_manager_order_cost_recalculator',
+] as $key) {
+    if (!str_contains($registrySrc, "'{$key}'")) {
+        $fail("ServiceRegistry missing {$key}");
+    }
+}
+
+if (!str_contains($registrySrc, "'ms3_option_service'")) {
+    $fail('ms3_option_service must stay registered');
+}
+
+if (!str_contains($registrySrc, 'SERVICES_WITH_DEPENDENCIES')) {
+    $fail('ServiceRegistry must declare SERVICES_WITH_DEPENDENCIES factory map');
+}
+
+if (!str_contains($registrySrc, "case 'ms3_option_service':")) {
+    $fail('registerServiceWithDependencies must wire ms3_option_service');
+}
+
+if (!str_contains($registrySrc, "services->get('ms3_category_option_service')")) {
+    $fail('registerServiceWithDependencies must resolve ms3_category_option_service for OptionService');
+}
+
+if (!str_contains($registrySrc, 'SERVICES_WITH_MODX_AND_MS3')) {
+    $fail('ServiceRegistry must declare SERVICES_WITH_MODX_AND_MS3 factory map');
+}
+
+$optionServiceSrc = file_get_contents(__DIR__ . '/../src/Services/Option/OptionService.php');
+if ($optionServiceSrc === false) {
+    $fail('unable to read OptionService.php');
+}
+if (preg_match('/new\s+OptionLoaderService\s*\(/', $optionServiceSrc)) {
+    $fail('OptionService must not instantiate OptionLoaderService directly');
+}
+if (preg_match('/new\s+OptionSyncService\s*\(/', $optionServiceSrc)) {
+    $fail('OptionService must not instantiate OptionSyncService directly');
+}
+if (preg_match('/new\s+OptionCategoryService\s*\(/', $optionServiceSrc)) {
+    $fail('OptionService must not instantiate OptionCategoryService directly');
+}
+if (!preg_match('/OptionLoaderService\s+\$loader,\s*OptionSyncService\s+\$sync,\s*OptionCategoryService\s+\$category/s', $optionServiceSrc)) {
+    $fail('OptionService constructor must accept loader/sync/category from DI');
+}
+
+$ordersCtrlSrc = file_get_contents(__DIR__ . '/../src/Controllers/Api/Manager/OrdersController.php');
+if ($ordersCtrlSrc === false) {
+    $fail('unable to read OrdersController.php');
+}
+if (preg_match('/new\s+ManagerOrderCostRecalculator\s*\(/', $ordersCtrlSrc)) {
+    $fail('OrdersController must not instantiate ManagerOrderCostRecalculator directly');
+}
+if (!str_contains($ordersCtrlSrc, "services->get('ms3_manager_order_cost_recalculator')")) {
+    $fail('OrdersController must resolve manager cost recalculator from DI');
+}
+
+$example = file_get_contents(__DIR__ . '/../config/ms3.services.example.php');
+if ($example === false) {
+    $fail('unable to read ms3.services.example.php');
+}
+foreach ([
+    'ms3_option_loader',
+    'ms3_option_sync',
+    'ms3_category_option_service',
+    'ms3_manager_order_cost_recalculator',
+] as $key) {
+    if (!str_contains($example, "'{$key}'")) {
+        $fail("ms3.services.example.php must document {$key} override");
+    }
+}
+
+// Behavior test: every factory-map key and every declared dependency must be
+// a registered service key, so the DI wiring can actually resolve at runtime
+// (#363 review: verify factory map keys exist).
+$modx = new modX();
+$registry = new class ($modx) extends ServiceRegistry {
+    protected function loadCustomServices(): void
+    {
+        // skip filesystem config loading in unit test
+    }
+};
+$registered = $registry->getRegisteredServices();
+$registeredSet = array_flip($registered);
+
+$assertRegistered = static function (string $key, string $map) use ($registeredSet, $fail): void {
+    if (!isset($registeredSet[$key])) {
+        $fail("{$map} references unregistered service key: {$key}");
+    }
+};
+
+foreach (ServiceRegistry::CONTROLLERS_WITH_MS3_ONLY as $key) {
+    $assertRegistered($key, 'CONTROLLERS_WITH_MS3_ONLY');
+}
+foreach (ServiceRegistry::SERVICES_WITH_MODX_AND_MS3 as $key) {
+    $assertRegistered($key, 'SERVICES_WITH_MODX_AND_MS3');
+    if ($key !== 'ms3_manager_order_cost_recalculator') {
+        continue;
+    }
+}
+if (!in_array('ms3_manager_order_cost_recalculator', ServiceRegistry::SERVICES_WITH_MODX_AND_MS3, true)) {
+    $fail('ms3_manager_order_cost_recalculator must use modX+MiniShop3 factory');
+}
+foreach (ServiceRegistry::SERVICES_WITH_DEPENDENCIES as $key) {
+    $assertRegistered($key, 'SERVICES_WITH_DEPENDENCIES');
+}
+foreach (ServiceRegistry::SERVICE_DEPENDENCIES as $service => $deps) {
+    $assertRegistered($service, 'SERVICE_DEPENDENCIES');
+    foreach ($deps as $dep) {
+        $assertRegistered($dep, "SERVICE_DEPENDENCIES[{$service}]");
+    }
+}
+
+$optionDeps = ServiceRegistry::SERVICE_DEPENDENCIES['ms3_option_service'] ?? [];
+if (!in_array('ms3_option_loader', $optionDeps, true)) {
+    $fail('ms3_option_service must depend on ms3_option_loader');
+}
+if (!in_array('ms3_option_sync', $optionDeps, true)) {
+    $fail('ms3_option_service must depend on ms3_option_sync');
+}
+if (!in_array('ms3_category_option_service', $optionDeps, true)) {
+    $fail('ms3_option_service must depend on ms3_category_option_service');
+}
+
+fwrite(STDOUT, "OK ServiceRegistryDiTest\n");
+exit(0);