From e8117a026673e6c20d06f49446451144bb8498c2 Mon Sep 17 00:00:00 2001 From: Yoan-Alexander Grigorov Date: Tue, 4 Mar 2025 12:10:25 +0100 Subject: [PATCH 1/2] :fire: Deprecate internal class ServiceMatcher, merge only used function in MyParcelComApi --- src/MyParcelComApi.php | 13 +- src/Shipments/ServiceMatcher.php | 74 -------- tests/Unit/Shipments/ServiceMatcherTest.php | 186 -------------------- 3 files changed, 10 insertions(+), 263 deletions(-) delete mode 100644 src/Shipments/ServiceMatcher.php delete mode 100644 tests/Unit/Shipments/ServiceMatcherTest.php diff --git a/src/MyParcelComApi.php b/src/MyParcelComApi.php index ce2f17dc..09887122 100644 --- a/src/MyParcelComApi.php +++ b/src/MyParcelComApi.php @@ -36,7 +36,6 @@ use MyParcelCom\ApiSdk\Resources\Service; use MyParcelCom\ApiSdk\Resources\ServiceRate; use MyParcelCom\ApiSdk\Resources\Shipment; -use MyParcelCom\ApiSdk\Shipments\ServiceMatcher; use MyParcelCom\ApiSdk\Utils\UrlBuilder; use MyParcelCom\ApiSdk\Validators\CollectionValidator; use MyParcelCom\ApiSdk\Validators\ManifestValidator; @@ -266,17 +265,25 @@ public function getServices( $services = $this->getResourcesArray($url->getUrl(), $ttl); - $matcher = new ServiceMatcher(); $services = array_values( array_filter( $services, - fn (ServiceInterface $service) => $matcher->matchesDeliveryMethod($shipment, $service), + fn (ServiceInterface $service) => $this->matchesDeliveryMethod($shipment, $service), ), ); return new ArrayCollection($services); } + protected function matchesDeliveryMethod(ShipmentInterface $shipment, ServiceInterface $service): bool + { + $deliveryMethod = $shipment->getPickupLocationCode() + ? ServiceInterface::DELIVERY_METHOD_PICKUP + : ServiceInterface::DELIVERY_METHOD_DELIVERY; + + return $service->getDeliveryMethod() === $deliveryMethod; + } + public function getServicesForCarrier( CarrierInterface $carrier, int $ttl = self::TTL_10MIN, diff --git a/src/Shipments/ServiceMatcher.php b/src/Shipments/ServiceMatcher.php deleted file mode 100644 index 8f470a82..00000000 --- a/src/Shipments/ServiceMatcher.php +++ /dev/null @@ -1,74 +0,0 @@ -getPhysicalProperties() === null - || $shipment->getPhysicalProperties()->getWeight() === null - || $shipment->getPhysicalProperties()->getWeight() < 0 - ) { - throw new InvalidResourceException( - 'Cannot match shipment and service without a valid shipment weight.' - ); - } - - // TODO: Add check for matching regions. We need to implement ancestor regions in the SDK in order to do this. - return $this->matchesDeliveryMethod($shipment, $service) - && ($serviceRates = $service->getServiceRates([ - 'has_active_contract' => 'true', - 'weight' => $shipment->getPhysicalProperties()->getWeight(), - 'volume' => $shipment->calculateVolume(DimensionUnitEnum::DM3), - ])) - && $this->getMatchedOptions($shipment, $serviceRates); - } - - /** - * Returns true if the service has a delivery method that matches the shipment. - */ - public function matchesDeliveryMethod(ShipmentInterface $shipment, ServiceInterface $service): bool - { - $deliveryMethod = $shipment->getPickupLocationCode() - ? ServiceInterface::DELIVERY_METHOD_PICKUP - : ServiceInterface::DELIVERY_METHOD_DELIVERY; - - return $service->getDeliveryMethod() === $deliveryMethod; - } - - /** - * Returns a subset of the given service rates that have all the options that the shipment requires. - */ - public function getMatchedOptions(ShipmentInterface $shipment, array $serviceRates): array - { - $optionIds = array_map(function (ServiceOptionInterface $option) { - return $option->getId(); - }, $shipment->getServiceOptions()); - - $matches = []; - foreach ($serviceRates as $serviceRate) { - $contractOptionIds = array_map(function (ServiceOptionInterface $option) use ($optionIds) { - return $option->getId(); - }, $serviceRate->getServiceOptions()); - - if (!array_diff($optionIds, $contractOptionIds)) { - $matches[] = $serviceRate; - } - } - - return $matches; - } -} diff --git a/tests/Unit/Shipments/ServiceMatcherTest.php b/tests/Unit/Shipments/ServiceMatcherTest.php deleted file mode 100644 index a510a92d..00000000 --- a/tests/Unit/Shipments/ServiceMatcherTest.php +++ /dev/null @@ -1,186 +0,0 @@ -matcher = new ServiceMatcher(); - - $this->proofOfDeliveryOption = $this->getMockedServiceOption('proof-of-delivery-option'); - $this->collectionOption = $this->getMockedServiceOption('collection-option'); - $this->weekendDeliveryOption = $this->getMockedServiceOption('weekend-delivery-option'); - - parent::setUp(); - } - - /** @test */ - public function testItMatchesDeliveryMethodsOnAServiceAndShipment() - { - $shipment = $this->getMockedShipment(); - $shipment->method('getPickupLocationCode')->willReturn(null); - - $matcher = new ServiceMatcher(); - - $pickupService = $this->getMockedService([], 'pick-up'); - $this->assertFalse($matcher->matchesDeliveryMethod($shipment, $pickupService)); - - $deliveryService = $this->getMockedService([], 'delivery'); - $this->assertTrue($matcher->matchesDeliveryMethod($shipment, $deliveryService)); - } - - /** @test */ - public function testItMatchesServiceOptionsOnShipmentAndService() - { - // Shipment only has collectionOption. - // Only serviceRates with collectionOption should be returned. - $shipment = $this->getMockedShipment(5000, $this->getMockedService(), [$this->collectionOption]); - - $serviceRates = [ - // Contains collectionOption and should thus be returned. - $serviceRateMockA = $this->getMockedServiceRate([ - $this->proofOfDeliveryOption, - $this->collectionOption, - ]), - - // Contains collectionOption and should thus be returned. - $serviceRateMockB = $this->getMockedServiceRate([ - $this->collectionOption, - ]), - - // Does NOT contain collectionOption and should thus NOT be returned. - $serviceRateMockC = $this->getMockedServiceRate([ - $this->proofOfDeliveryOption, - ]), - - // Contains collectionOption and should thus be returned. - $serviceRateMockD = $this->getMockedServiceRate([ - $this->proofOfDeliveryOption, - $this->collectionOption, - $this->weekendDeliveryOption, - ]), - - // Does NOT contain collectionOption and should thus NOT be returned. - $serviceRateMockE = $this->getMockedServiceRate([ - $this->proofOfDeliveryOption, - $this->weekendDeliveryOption, - ]), - ]; - - $matcher = new ServiceMatcher(); - - $this->assertEquals([ - $serviceRateMockA, - $serviceRateMockB, - $serviceRateMockD, - ], $matcher->getMatchedOptions($shipment, $serviceRates)); - } - - /** @test */ - public function testItThrowsAnExceptionIfShipmentWeightIsNegative() - { - $serviceMock = $this->getMockedService(); - $shipment = $this->getMockedShipment(-2345); - - $matcher = new ServiceMatcher(); - - $this->expectException(InvalidResourceException::class); - $this->expectExceptionMessage('Cannot match shipment and service without a valid shipment weight.'); - $matcher->matches($shipment, $serviceMock); - } - - /** @test */ - public function testItThrowsAnExceptionIfShipmentDoesNotHaveWeightSet() - { - $serviceMock = $this->getMockedService(); - $shipment = $this->getMockedShipment(null, $serviceMock); - - $matcher = new ServiceMatcher(); - - $this->expectException(InvalidResourceException::class); - $this->expectExceptionMessage('Cannot match shipment and service without a valid shipment weight.'); - $matcher->matches($shipment, $serviceMock); - } - - /** @test */ - public function testItMatchesAServiceAndShipment() - { - // A matching service should have the POD and collection options as well as have delivery method 'pick-up'. - $shipment = $this->getMockedShipment(2500, null, [$this->proofOfDeliveryOption, $this->collectionOption]); - $shipment->method('getPickupLocationCode')->willReturn('pickup-code'); - - $matchingServiceRates = [ - $this->getMockedServiceRate([ - $this->proofOfDeliveryOption, - $this->collectionOption, - $this->weekendDeliveryOption, - ]), - $this->getMockedServiceRate([ - $this->proofOfDeliveryOption, - $this->collectionOption, - ]), - ]; - - $nonMatchingServiceRates = [ - $this->getMockedServiceRate([ - $this->weekendDeliveryOption, - ]), - $this->getMockedServiceRate([ - $this->weekendDeliveryOption, - $this->proofOfDeliveryOption, - ]), - ]; - - // Note that in the service matcher, - // service rates are retrieved from the API using the shipment's weight as filter. - // An empty array here represents a mocked empty API response for the shipment's weight. - $nonMatchingServiceA = $this->getMockedService([], 'delivery'); - $this->assertFalse($this->matcher->matches($shipment, $nonMatchingServiceA)); - - // This service does have matching service rates, but the delivery method is wrong. - $nonMatchingServiceB = $this->getMockedService($matchingServiceRates, 'delivery'); - $this->assertFalse($this->matcher->matches($shipment, $nonMatchingServiceB)); - - // This service does match the shipment's delivery method, but the service rates don't match the - // shipment weight. - $nonMatchingServiceC = $this->getMockedService([], 'pick-up'); - $this->assertFalse($this->matcher->matches($shipment, $nonMatchingServiceC)); - - // This service does match the shipment's delivery method and the service rates match the - // shipment weight, but it doesn't have the right options. - $nonMatchingServiceC = $this->getMockedService($nonMatchingServiceRates, 'pick-up'); - $this->assertFalse($this->matcher->matches($shipment, $nonMatchingServiceC)); - - // This service matches the shipment's delivery method and all service rates match. - $matchingServiceA = $this->getMockedService($matchingServiceRates, 'pick-up'); - $this->assertTrue($this->matcher->matches($shipment, $matchingServiceA)); - - // This service matches the shipment's delivery method and has matching service rates. - // Note that even though it also has non-matching service rates, the service itself should still match. - $matchingServiceB = $this->getMockedService( - array_merge($matchingServiceRates, $nonMatchingServiceRates), - 'pick-up' - ); - $this->assertTrue($this->matcher->matches($shipment, $matchingServiceB)); - } -} From f6a3a9fab005267b3205a29e23cb6cb19f92693a Mon Sep 17 00:00:00 2001 From: Yoan-Alexander Grigorov Date: Tue, 4 Mar 2025 13:40:11 +0100 Subject: [PATCH 2/2] :zombie: Brought back SericeMatcher, but marked it as deprecated --- src/Shipments/ServiceMatcher.php | 77 ++++++++ tests/Unit/Shipments/ServiceMatcherTest.php | 186 ++++++++++++++++++++ 2 files changed, 263 insertions(+) create mode 100644 src/Shipments/ServiceMatcher.php create mode 100644 tests/Unit/Shipments/ServiceMatcherTest.php diff --git a/src/Shipments/ServiceMatcher.php b/src/Shipments/ServiceMatcher.php new file mode 100644 index 00000000..7a1fe9a7 --- /dev/null +++ b/src/Shipments/ServiceMatcher.php @@ -0,0 +1,77 @@ +getPhysicalProperties() === null + || $shipment->getPhysicalProperties()->getWeight() === null + || $shipment->getPhysicalProperties()->getWeight() < 0 + ) { + throw new InvalidResourceException( + 'Cannot match shipment and service without a valid shipment weight.' + ); + } + + // TODO: Add check for matching regions. We need to implement ancestor regions in the SDK in order to do this. + return $this->matchesDeliveryMethod($shipment, $service) + && ($serviceRates = $service->getServiceRates([ + 'has_active_contract' => 'true', + 'weight' => $shipment->getPhysicalProperties()->getWeight(), + 'volume' => $shipment->calculateVolume(DimensionUnitEnum::DM3), + ])) + && $this->getMatchedOptions($shipment, $serviceRates); + } + + /** + * Returns true if the service has a delivery method that matches the shipment. + */ + public function matchesDeliveryMethod(ShipmentInterface $shipment, ServiceInterface $service): bool + { + $deliveryMethod = $shipment->getPickupLocationCode() + ? ServiceInterface::DELIVERY_METHOD_PICKUP + : ServiceInterface::DELIVERY_METHOD_DELIVERY; + + return $service->getDeliveryMethod() === $deliveryMethod; + } + + /** + * Returns a subset of the given service rates that have all the options that the shipment requires. + */ + public function getMatchedOptions(ShipmentInterface $shipment, array $serviceRates): array + { + $optionIds = array_map(function (ServiceOptionInterface $option) { + return $option->getId(); + }, $shipment->getServiceOptions()); + + $matches = []; + foreach ($serviceRates as $serviceRate) { + $contractOptionIds = array_map(function (ServiceOptionInterface $option) use ($optionIds) { + return $option->getId(); + }, $serviceRate->getServiceOptions()); + + if (!array_diff($optionIds, $contractOptionIds)) { + $matches[] = $serviceRate; + } + } + + return $matches; + } +} diff --git a/tests/Unit/Shipments/ServiceMatcherTest.php b/tests/Unit/Shipments/ServiceMatcherTest.php new file mode 100644 index 00000000..a510a92d --- /dev/null +++ b/tests/Unit/Shipments/ServiceMatcherTest.php @@ -0,0 +1,186 @@ +matcher = new ServiceMatcher(); + + $this->proofOfDeliveryOption = $this->getMockedServiceOption('proof-of-delivery-option'); + $this->collectionOption = $this->getMockedServiceOption('collection-option'); + $this->weekendDeliveryOption = $this->getMockedServiceOption('weekend-delivery-option'); + + parent::setUp(); + } + + /** @test */ + public function testItMatchesDeliveryMethodsOnAServiceAndShipment() + { + $shipment = $this->getMockedShipment(); + $shipment->method('getPickupLocationCode')->willReturn(null); + + $matcher = new ServiceMatcher(); + + $pickupService = $this->getMockedService([], 'pick-up'); + $this->assertFalse($matcher->matchesDeliveryMethod($shipment, $pickupService)); + + $deliveryService = $this->getMockedService([], 'delivery'); + $this->assertTrue($matcher->matchesDeliveryMethod($shipment, $deliveryService)); + } + + /** @test */ + public function testItMatchesServiceOptionsOnShipmentAndService() + { + // Shipment only has collectionOption. + // Only serviceRates with collectionOption should be returned. + $shipment = $this->getMockedShipment(5000, $this->getMockedService(), [$this->collectionOption]); + + $serviceRates = [ + // Contains collectionOption and should thus be returned. + $serviceRateMockA = $this->getMockedServiceRate([ + $this->proofOfDeliveryOption, + $this->collectionOption, + ]), + + // Contains collectionOption and should thus be returned. + $serviceRateMockB = $this->getMockedServiceRate([ + $this->collectionOption, + ]), + + // Does NOT contain collectionOption and should thus NOT be returned. + $serviceRateMockC = $this->getMockedServiceRate([ + $this->proofOfDeliveryOption, + ]), + + // Contains collectionOption and should thus be returned. + $serviceRateMockD = $this->getMockedServiceRate([ + $this->proofOfDeliveryOption, + $this->collectionOption, + $this->weekendDeliveryOption, + ]), + + // Does NOT contain collectionOption and should thus NOT be returned. + $serviceRateMockE = $this->getMockedServiceRate([ + $this->proofOfDeliveryOption, + $this->weekendDeliveryOption, + ]), + ]; + + $matcher = new ServiceMatcher(); + + $this->assertEquals([ + $serviceRateMockA, + $serviceRateMockB, + $serviceRateMockD, + ], $matcher->getMatchedOptions($shipment, $serviceRates)); + } + + /** @test */ + public function testItThrowsAnExceptionIfShipmentWeightIsNegative() + { + $serviceMock = $this->getMockedService(); + $shipment = $this->getMockedShipment(-2345); + + $matcher = new ServiceMatcher(); + + $this->expectException(InvalidResourceException::class); + $this->expectExceptionMessage('Cannot match shipment and service without a valid shipment weight.'); + $matcher->matches($shipment, $serviceMock); + } + + /** @test */ + public function testItThrowsAnExceptionIfShipmentDoesNotHaveWeightSet() + { + $serviceMock = $this->getMockedService(); + $shipment = $this->getMockedShipment(null, $serviceMock); + + $matcher = new ServiceMatcher(); + + $this->expectException(InvalidResourceException::class); + $this->expectExceptionMessage('Cannot match shipment and service without a valid shipment weight.'); + $matcher->matches($shipment, $serviceMock); + } + + /** @test */ + public function testItMatchesAServiceAndShipment() + { + // A matching service should have the POD and collection options as well as have delivery method 'pick-up'. + $shipment = $this->getMockedShipment(2500, null, [$this->proofOfDeliveryOption, $this->collectionOption]); + $shipment->method('getPickupLocationCode')->willReturn('pickup-code'); + + $matchingServiceRates = [ + $this->getMockedServiceRate([ + $this->proofOfDeliveryOption, + $this->collectionOption, + $this->weekendDeliveryOption, + ]), + $this->getMockedServiceRate([ + $this->proofOfDeliveryOption, + $this->collectionOption, + ]), + ]; + + $nonMatchingServiceRates = [ + $this->getMockedServiceRate([ + $this->weekendDeliveryOption, + ]), + $this->getMockedServiceRate([ + $this->weekendDeliveryOption, + $this->proofOfDeliveryOption, + ]), + ]; + + // Note that in the service matcher, + // service rates are retrieved from the API using the shipment's weight as filter. + // An empty array here represents a mocked empty API response for the shipment's weight. + $nonMatchingServiceA = $this->getMockedService([], 'delivery'); + $this->assertFalse($this->matcher->matches($shipment, $nonMatchingServiceA)); + + // This service does have matching service rates, but the delivery method is wrong. + $nonMatchingServiceB = $this->getMockedService($matchingServiceRates, 'delivery'); + $this->assertFalse($this->matcher->matches($shipment, $nonMatchingServiceB)); + + // This service does match the shipment's delivery method, but the service rates don't match the + // shipment weight. + $nonMatchingServiceC = $this->getMockedService([], 'pick-up'); + $this->assertFalse($this->matcher->matches($shipment, $nonMatchingServiceC)); + + // This service does match the shipment's delivery method and the service rates match the + // shipment weight, but it doesn't have the right options. + $nonMatchingServiceC = $this->getMockedService($nonMatchingServiceRates, 'pick-up'); + $this->assertFalse($this->matcher->matches($shipment, $nonMatchingServiceC)); + + // This service matches the shipment's delivery method and all service rates match. + $matchingServiceA = $this->getMockedService($matchingServiceRates, 'pick-up'); + $this->assertTrue($this->matcher->matches($shipment, $matchingServiceA)); + + // This service matches the shipment's delivery method and has matching service rates. + // Note that even though it also has non-matching service rates, the service itself should still match. + $matchingServiceB = $this->getMockedService( + array_merge($matchingServiceRates, $nonMatchingServiceRates), + 'pick-up' + ); + $this->assertTrue($this->matcher->matches($shipment, $matchingServiceB)); + } +}