From 47c0b9892a9389b58afc99f865717cf6ceb6ef4d Mon Sep 17 00:00:00 2001 From: Nabil Azahaf Date: Wed, 29 Jul 2026 12:59:20 +0200 Subject: [PATCH 1/5] Guard carrier settings --- .../Service/DeliveryOptionsService.php | 9 ++- .../Model/DeliveryOptionsConfigTest.php | 5 ++ ...tionsServiceInvalidCarrierSettingsTest.php | 70 +++++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index f5d7cc3c5..a29769dc6 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -15,6 +15,7 @@ use MyParcelNL\Pdk\Base\Support\Collection; use MyParcelNL\Pdk\Base\Support\SettingKey; use MyParcelNL\Pdk\Base\Support\Utils; +use MyParcelNL\Pdk\Carrier\Collection\CarrierCollection; use MyParcelNL\Pdk\Carrier\Contract\CarrierRepositoryInterface; use MyParcelNL\Pdk\Carrier\Model\Carrier; use MyParcelNL\Pdk\Carrier\Service\CapabilitiesValidationService; @@ -229,8 +230,14 @@ private function getBaseSettings(CarrierSettings $carrierSettings, PdkCart $cart */ private function getValidCarrierOptions(PdkCart $cart): array { + $carrierSettings = Settings::get(CarrierSettings::ID); + $carrierSettings = is_array($carrierSettings) ? $carrierSettings : []; + + if (empty($carrierSettings)) { + return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()]; + } + $allCarriers = $this->carrierRepository->all(); - $carrierSettings = Settings::get(CarrierSettings::ID); $cc = $cart->shippingMethod->shippingAddress->cc ?? null; $candidatePackageTypes = $this->getCandidatePackageTypes($cart); diff --git a/tests/Unit/App/Context/Model/DeliveryOptionsConfigTest.php b/tests/Unit/App/Context/Model/DeliveryOptionsConfigTest.php index efa00e99b..1dec6909b 100644 --- a/tests/Unit/App/Context/Model/DeliveryOptionsConfigTest.php +++ b/tests/Unit/App/Context/Model/DeliveryOptionsConfigTest.php @@ -15,6 +15,7 @@ use MyParcelNL\Pdk\Facade\Pdk; use MyParcelNL\Pdk\Facade\Settings; use MyParcelNL\Pdk\Proposition\Proposition; +use MyParcelNL\Pdk\Settings\Model\CarrierSettings; use MyParcelNL\Pdk\Settings\Model\CheckoutSettings; use MyParcelNL\Pdk\Tests\Bootstrap\MockPdkProductRepository; use MyParcelNL\Pdk\Tests\Bootstrap\TestBootstrapper; @@ -134,6 +135,10 @@ ->withAllowPickupLocationsViewSelection(true) ->store(); + factory(CarrierSettings::class, RefCapabilitiesSharedCarrierV2::POSTNL) + ->withDeliveryOptionsEnabled(true) + ->store(); + /** @var \MyParcelNL\Pdk\Tests\Bootstrap\MockPdkProductRepository $productRepository */ $productRepository = Pdk::get(PdkProductRepositoryInterface::class); diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php new file mode 100644 index 000000000..1873b8e6c --- /dev/null +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php @@ -0,0 +1,70 @@ +group('checkout'); + +usesShared(new UsesMockPdkInstance(), new UsesAccountMock()); + +it('does not expose carriers when carrier settings are missing or invalid', function ($carrierSettings) { + $settingsManager = Mockery::mock(SettingsManagerInterface::class); + $settingsManager + ->shouldReceive('get') + ->andReturnUsing(static function ( + string $key, + ?string $namespace = null, + $default = null + ) use ($carrierSettings) { + return CarrierSettings::ID === $key && null === $namespace + ? $carrierSettings + : $default; + }); + $settingsManager + ->shouldReceive('all') + ->andReturn(new Settings()); + + Pdk::set(SettingsManagerInterface::class, $settingsManager); + + /** @var DeliveryOptionsServiceInterface $service */ + $service = Pdk::get(DeliveryOptionsServiceInterface::class); + + $result = $service->createAllCarrierSettings(new PdkCart([ + 'shippingMethod' => [ + 'shippingAddress' => ['cc' => 'NL'], + ], + 'lines' => [ + [ + 'quantity' => 1, + 'product' => [ + 'weight' => 1000, + 'isDeliverable' => true, + ], + ], + ], + ])); + + expect($result['packageType'])->toBe(DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME) + ->and($result['carrierSettings'])->toBe([]); +})->with([ + 'missing' => [null], + 'empty array' => [[]], + 'boolean value' => [false], + 'string value' => ['invalid'], +]); From 6cff6ae96c62704e3a0478b1f954ecda93e54ab1 Mon Sep 17 00:00:00 2001 From: Nabil Azahaf Date: Thu, 30 Jul 2026 09:01:49 +0200 Subject: [PATCH 2/5] test: clean carrier guard --- .../DeliveryOptionsServiceInvalidCarrierSettingsTest.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php index 1873b8e6c..95c69b454 100644 --- a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php @@ -12,7 +12,6 @@ use MyParcelNL\Pdk\Facade\Pdk; use MyParcelNL\Pdk\Settings\Contract\SettingsManagerInterface; use MyParcelNL\Pdk\Settings\Model\CarrierSettings; -use MyParcelNL\Pdk\Settings\Model\Settings; use MyParcelNL\Pdk\Shipment\Model\DeliveryOptions; use MyParcelNL\Pdk\Tests\Uses\UsesAccountMock; use MyParcelNL\Pdk\Tests\Uses\UsesMockPdkInstance; @@ -36,10 +35,6 @@ ? $carrierSettings : $default; }); - $settingsManager - ->shouldReceive('all') - ->andReturn(new Settings()); - Pdk::set(SettingsManagerInterface::class, $settingsManager); /** @var DeliveryOptionsServiceInterface $service */ From 13e4185715ae32cb41e1a5c6b65c41e0ba42cfed Mon Sep 17 00:00:00 2001 From: Nabil Azahaf Date: Thu, 30 Jul 2026 10:28:21 +0200 Subject: [PATCH 3/5] fix: validate carrier entries --- .../DeliveryOptions/Service/DeliveryOptionsService.php | 9 +++++++-- ...eliveryOptionsServiceInvalidCarrierSettingsTest.php | 10 ++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index a29769dc6..0201f2c7c 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -231,12 +231,17 @@ private function getBaseSettings(CarrierSettings $carrierSettings, PdkCart $cart private function getValidCarrierOptions(PdkCart $cart): array { $carrierSettings = Settings::get(CarrierSettings::ID); - $carrierSettings = is_array($carrierSettings) ? $carrierSettings : []; - if (empty($carrierSettings)) { + if (! is_array($carrierSettings) || empty($carrierSettings)) { return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()]; } + foreach ($carrierSettings as $settings) { + if (! is_array($settings)) { + return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()]; + } + } + $allCarriers = $this->carrierRepository->all(); $cc = $cart->shippingMethod->shippingAddress->cc ?? null; $candidatePackageTypes = $this->getCandidatePackageTypes($cart); diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php index 95c69b454..39719ac6a 100644 --- a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php @@ -58,8 +58,10 @@ expect($result['packageType'])->toBe(DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME) ->and($result['carrierSettings'])->toBe([]); })->with([ - 'missing' => [null], - 'empty array' => [[]], - 'boolean value' => [false], - 'string value' => ['invalid'], + 'missing' => [null], + 'empty array' => [[]], + 'malformed carrier entry' => [['carrier' => 'invalid']], + 'mixed carrier entries' => [['valid' => [], 'invalid' => 'invalid']], + 'boolean value' => [false], + 'string value' => ['invalid'], ]); From aee4821dfe677fed8763dad00057ab23d4251f78 Mon Sep 17 00:00:00 2001 From: Nabil Azahaf Date: Wed, 5 Aug 2026 13:53:40 +0200 Subject: [PATCH 4/5] fix: ignore malformed carrier settings --- .../Service/DeliveryOptionsService.php | 10 +++-- .../Service/CapabilitiesValidationService.php | 7 +++- .../AbstractPdkSettingsRepository.php | 14 +++++-- ...DeliveryOptionsServiceCapabilitiesTest.php | 38 ++++++++++++++++++- ...tionsServiceInvalidCarrierSettingsTest.php | 1 - .../AbstractSettingsRepositoryTest.php | 22 +++++++++++ 6 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index 0201f2c7c..c652bddba 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -236,10 +236,12 @@ private function getValidCarrierOptions(PdkCart $cart): array return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()]; } - foreach ($carrierSettings as $settings) { - if (! is_array($settings)) { - return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()]; - } + $carrierSettings = array_filter($carrierSettings, static function ($settings): bool { + return is_array($settings); + }); + + if (empty($carrierSettings)) { + return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()]; } $allCarriers = $this->carrierRepository->all(); diff --git a/src/Carrier/Service/CapabilitiesValidationService.php b/src/Carrier/Service/CapabilitiesValidationService.php index 707751ead..b69b2a787 100644 --- a/src/Carrier/Service/CapabilitiesValidationService.php +++ b/src/Carrier/Service/CapabilitiesValidationService.php @@ -123,11 +123,16 @@ private function getEnabledCarrierNames(): array { $carrierSettings = Settings::get(CarrierSettings::ID) ?? []; + if (! is_array($carrierSettings)) { + return []; + } + return array_keys( array_filter( $carrierSettings, static function ($settings): bool { - return ! empty($settings[CarrierSettings::DELIVERY_OPTIONS_ENABLED]); + return is_array($settings) + && ! empty($settings[CarrierSettings::DELIVERY_OPTIONS_ENABLED]); } ) ); diff --git a/src/Settings/Repository/AbstractPdkSettingsRepository.php b/src/Settings/Repository/AbstractPdkSettingsRepository.php index 121ba1da3..7e61f9db1 100644 --- a/src/Settings/Repository/AbstractPdkSettingsRepository.php +++ b/src/Settings/Repository/AbstractPdkSettingsRepository.php @@ -137,10 +137,18 @@ protected function updateSettingsFromCollection( ): Settings { $category = $this->get($this->createSettingsKey($settingsId)) ?? []; - foreach ($category as $key => $item) { - $values = ['id' => $key] + $this->get($this->createSettingsKey("$settingsId.$key")); + if (! is_array($category)) { + $category = []; + } + + foreach (array_keys($category) as $key) { + $values = $this->get($this->createSettingsKey("$settingsId.$key")); + + if (! is_array($values)) { + continue; + } - $collection->offsetSet($key, $values); + $collection->offsetSet($key, ['id' => $key] + $values); } $settings->setAttribute($settingsId, $collection); diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCapabilitiesTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCapabilitiesTest.php index 7516cd56c..0be175355 100644 --- a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCapabilitiesTest.php +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceCapabilitiesTest.php @@ -13,6 +13,7 @@ use MyParcelNL\Pdk\Carrier\Model\Carrier; use MyParcelNL\Pdk\Facade\FrontendData; use MyParcelNL\Pdk\Facade\Pdk; +use MyParcelNL\Pdk\Settings\Contract\PdkSettingsRepositoryInterface; use MyParcelNL\Pdk\Settings\Model\CarrierSettings; use MyParcelNL\Pdk\Settings\Model\Settings; use MyParcelNL\Pdk\Shipment\Model\DeliveryOptions; @@ -266,6 +267,42 @@ function enqueueCapabilitiesPerType(array $responsesPerType): void ->and($result['carrierSettings'])->not->toHaveKey($disabledId); }); +it('keeps valid carriers when another carrier setting is malformed', function () { + $carrierName = RefCapabilitiesSharedCarrierV2::getAllowableEnumValues()[0]; + + storeCarrierSettings([$carrierName => true]); + + /** @var \MyParcelNL\Pdk\Settings\Contract\PdkSettingsRepositoryInterface $settingsRepository */ + $settingsRepository = Pdk::get(PdkSettingsRepositoryInterface::class); + $settingsKey = Pdk::get('createSettingsKey')(CarrierSettings::ID); + $carrierSettings = $settingsRepository->get($settingsKey); + + $settingsRepository->store($settingsKey, array_merge($carrierSettings, ['invalid' => 'invalid'])); + + factory(Shop::class) + ->withCarriers( + factory(CarrierCollection::class) + ->push(factory(Carrier::class) + ->withCarrier($carrierName) + ->withCapabilityPackageTypes(['PACKAGE'])) + ) + ->store(); + + resetStorageCache(); + + enqueueCapabilitiesPerType([ + 'PACKAGE' => [capabilityResult($carrierName, 100, ['PACKAGE'])], + ]); + + /** @var DeliveryOptionsServiceInterface $service */ + $service = Pdk::get(DeliveryOptionsServiceInterface::class); + $result = $service->createAllCarrierSettings(makeCart('NL')); + + $carrierId = FrontendData::getLegacyCarrierIdentifier($carrierName); + + expect($result['carrierSettings'])->toHaveKey($carrierId); +}); + it('passes contract ID from capabilities to carrier settings output', function () { storeCarrierSettings([RefCapabilitiesSharedCarrierV2::POSTNL => true]); @@ -435,4 +472,3 @@ function enqueueCapabilitiesPerType(array $responsesPerType): void expect($result['packageType'])->toBe(DeliveryOptions::PACKAGE_TYPE_MAILBOX_NAME); }); - diff --git a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php index 39719ac6a..21df8e139 100644 --- a/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php +++ b/tests/Unit/App/DeliveryOptions/Service/DeliveryOptionsServiceInvalidCarrierSettingsTest.php @@ -61,7 +61,6 @@ 'missing' => [null], 'empty array' => [[]], 'malformed carrier entry' => [['carrier' => 'invalid']], - 'mixed carrier entries' => [['valid' => [], 'invalid' => 'invalid']], 'boolean value' => [false], 'string value' => ['invalid'], ]); diff --git a/tests/Unit/Settings/Repository/AbstractSettingsRepositoryTest.php b/tests/Unit/Settings/Repository/AbstractSettingsRepositoryTest.php index eb66121ad..efb1d5b13 100644 --- a/tests/Unit/Settings/Repository/AbstractSettingsRepositoryTest.php +++ b/tests/Unit/Settings/Repository/AbstractSettingsRepositoryTest.php @@ -49,6 +49,28 @@ assertMatchesJsonSnapshot(json_encode($settings->toArrayWithoutNull())); }); +it('skips malformed entries when retrieving collection settings', function () { + /** @var \MyParcelNL\Pdk\Settings\Contract\PdkSettingsRepositoryInterface $repository */ + $repository = Pdk::get(PdkSettingsRepositoryInterface::class); + $createSettingsKey = Pdk::get('createSettingsKey'); + + $currentCarrierSettings = $repository->get($createSettingsKey(CarrierSettings::ID)); + + try { + $repository->store($createSettingsKey(CarrierSettings::ID), array_merge($currentCarrierSettings, [ + 'valid' => [CarrierSettings::DELIVERY_OPTIONS_ENABLED => true], + 'invalid' => 'invalid', + ])); + + $carrierSettings = $repository->all()->carrier; + + expect($carrierSettings->has('valid'))->toBeTrue() + ->and($carrierSettings->has('invalid'))->toBeFalse(); + } finally { + $repository->store($createSettingsKey(CarrierSettings::ID), $currentCarrierSettings); + } +}); + it('retrieves a single setting from a category', function (string $key, $expected) { /** @var \MyParcelNL\Pdk\Settings\Contract\PdkSettingsRepositoryInterface $repository */ $repository = Pdk::get(PdkSettingsRepositoryInterface::class); From c63c28b87e8ac507db4952b2bae5a531314dfc78 Mon Sep 17 00:00:00 2001 From: Nabil Azahaf Date: Thu, 6 Aug 2026 15:54:18 +0200 Subject: [PATCH 5/5] refactor: simplify carrier settings guard --- .../Service/DeliveryOptionsService.php | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php index c652bddba..60219eaee 100644 --- a/src/App/DeliveryOptions/Service/DeliveryOptionsService.php +++ b/src/App/DeliveryOptions/Service/DeliveryOptionsService.php @@ -232,13 +232,10 @@ private function getValidCarrierOptions(PdkCart $cart): array { $carrierSettings = Settings::get(CarrierSettings::ID); - if (! is_array($carrierSettings) || empty($carrierSettings)) { - return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()]; - } - - $carrierSettings = array_filter($carrierSettings, static function ($settings): bool { - return is_array($settings); - }); + $carrierSettings = array_filter( + is_array($carrierSettings) ? $carrierSettings : [], + static fn($settings): bool => is_array($settings) + ); if (empty($carrierSettings)) { return [DeliveryOptions::DEFAULT_PACKAGE_TYPE_NAME, new CarrierCollection()];