Completed
Pull Request — 4.0 (#4223)
by Kentaro
04:57
created

DeliveryFeePreprocessor::process()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 5

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 1
CRAP Score 1

Importance

Changes 0
Metric Value
cc 1
nc 1
nop 2
dl 0
loc 5
ccs 1
cts 1
cp 1
crap 1
rs 10
c 0
b 0
f 0
1
<?php
2
3
/*
4
 * This file is part of EC-CUBE
5
 *
6
 * Copyright(c) EC-CUBE CO.,LTD. All Rights Reserved.
7
 *
8
 * http://www.ec-cube.co.jp/
9
 *
10
 * For the full copyright and license information, please view the LICENSE
11
 * file that was distributed with this source code.
12
 */
13
14
namespace Eccube\Service\PurchaseFlow\Processor;
15
16
use Doctrine\ORM\EntityManagerInterface;
17
use Eccube\Entity\BaseInfo;
18
use Eccube\Entity\DeliveryFee;
19
use Eccube\Entity\ItemHolderInterface;
20
use Eccube\Entity\Master\OrderItemType;
21
use Eccube\Entity\Master\TaxDisplayType;
22
use Eccube\Entity\Master\TaxType;
23
use Eccube\Entity\Order;
24
use Eccube\Entity\OrderItem;
25
use Eccube\Entity\Shipping;
26
use Eccube\Repository\BaseInfoRepository;
27
use Eccube\Repository\DeliveryFeeRepository;
28
use Eccube\Repository\TaxRuleRepository;
29
use Eccube\Service\PurchaseFlow\ItemHolderPreprocessor;
30
use Eccube\Service\PurchaseFlow\PurchaseContext;
31
32
/**
33
 * 送料明細追加.
34
 */
35
class DeliveryFeePreprocessor implements ItemHolderPreprocessor
36
{
37
    /** @var BaseInfo */
38
    protected $BaseInfo;
39
40
    /**
41
     * @var EntityManagerInterface
42
     */
43
    protected $entityManager;
44
45
    /**
46
     * @var TaxRuleRepository
47
     */
48
    protected $taxRuleRepository;
49
50
    /**
51
     * @var DeliveryFeeRepository
52
     */
53
    protected $deliveryFeeRepository;
54
55
    /**
56 61
     * DeliveryFeePreprocessor constructor.
57
     *
58
     * @param BaseInfoRepository $baseInfoRepository
59
     * @param EntityManagerInterface $entityManager
60
     * @param TaxRuleRepository $taxRuleRepository
61 61
     * @param DeliveryFeeRepository $deliveryFeeRepository
62 61
     */
63 61
    public function __construct(
64
        BaseInfoRepository $baseInfoRepository,
65
        EntityManagerInterface $entityManager,
66
        TaxRuleRepository $taxRuleRepository,
67
        DeliveryFeeRepository $deliveryFeeRepository
68
    ) {
69
        $this->BaseInfo = $baseInfoRepository->get();
70
        $this->entityManager = $entityManager;
71
        $this->taxRuleRepository = $taxRuleRepository;
72 53
        $this->deliveryFeeRepository = $deliveryFeeRepository;
73
    }
74 53
75 52
    /**
76
     * @param ItemHolderInterface $itemHolder
77
     * @param PurchaseContext $context
78
     *
79
     * @throws \Doctrine\ORM\NoResultException
80
     */
81
    public function process(ItemHolderInterface $itemHolder, PurchaseContext $context)
82
    {
83
        $this->removeDeliveryFeeItem($itemHolder);
84 53
        $this->saveDeliveryFeeItem($itemHolder);
85
    }
86 53
87 53
    private function removeDeliveryFeeItem(ItemHolderInterface $itemHolder)
88 53
    {
89
        foreach ($itemHolder->getShippings() as $Shipping) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Eccube\Entity\ItemHolderInterface as the method getShippings() does only exist in the following implementations of said interface: Eccube\Entity\Order.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
90
            foreach ($Shipping->getOrderItems() as $item) {
91
                if ($item->getProcessorName() == DeliveryFeePreprocessor::class) {
92 52
                    $Shipping->removeOrderItem($item);
93
                    $itemHolder->removeOrderItem($item);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Eccube\Entity\ItemHolderInterface as the method removeOrderItem() does only exist in the following implementations of said interface: Eccube\Entity\Order.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
94
                    $this->entityManager->remove($item);
95
                }
96
            }
97
        }
98
    }
99
100 52
    /**
101
     * @param ItemHolderInterface $itemHolder
102 52
     *
103 52
     * @throws \Doctrine\ORM\NoResultException
104 52
     */
105 52
    private function saveDeliveryFeeItem(ItemHolderInterface $itemHolder)
106 52
    {
107 52
        $DeliveryFeeType = $this->entityManager
108
            ->find(OrderItemType::class, OrderItemType::DELIVERY_FEE);
109
        $TaxInclude = $this->entityManager
110 52
            ->find(TaxDisplayType::class, TaxDisplayType::INCLUDED);
111
        $Taxation = $this->entityManager
112 52
            ->find(TaxType::class, TaxType::TAXATION);
113
114 52
        /** @var Order $Order */
115 52
        $Order = $itemHolder;
116 52
        /* @var Shipping $Shipping */
117
        foreach ($Order->getShippings() as $Shipping) {
118 52
            // 送料の計算
119 52
            $deliveryFeeProduct = 0;
120
            if ($this->BaseInfo->isOptionProductDeliveryFee()) {
121 52
                /** @var OrderItem $item */
122 52
                foreach ($Shipping->getOrderItems() as $item) {
123 52
                    if (!$item->isProduct()) {
124 52
                        continue;
125 52
                    }
126 52
                    $deliveryFeeProduct += $item->getProductClass()->getDeliveryFee() * $item->getQuantity();
127 52
                }
128 52
            }
129 52
130
            /** @var DeliveryFee $DeliveryFee */
131 52
            $DeliveryFee = $this->deliveryFeeRepository->findOneBy([
132 52
                'Delivery' => $Shipping->getDelivery(),
133
                'Pref' => $Shipping->getPref(),
134
            ]);
135
136
            $OrderItem = new OrderItem();
137
            $OrderItem->setProductName($DeliveryFeeType->getName())
138
                ->setPrice($DeliveryFee->getFee() + $deliveryFeeProduct)
139
                ->setQuantity(1)
140
                ->setOrderItemType($DeliveryFeeType)
0 ignored issues
show
Bug introduced by
It seems like $DeliveryFeeType defined by $this->entityManager->fi...ItemType::DELIVERY_FEE) on line 107 can also be of type object; however, Eccube\Entity\OrderItem::setOrderItemType() does only seem to accept null|object<Eccube\Entity\Master\OrderItemType>, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
141
                ->setShipping($Shipping)
142
                ->setOrder($itemHolder)
0 ignored issues
show
Documentation introduced by
$itemHolder is of type object<Eccube\Entity\ItemHolderInterface>, but the function expects a null|object<Eccube\Entity\Order>.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
143
                ->setTaxDisplayType($TaxInclude)
0 ignored issues
show
Bug introduced by
It seems like $TaxInclude defined by $this->entityManager->fi...xDisplayType::INCLUDED) on line 109 can also be of type object; however, Eccube\Entity\OrderItem::setTaxDisplayType() does only seem to accept null|object<Eccube\Entity\Master\TaxDisplayType>, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
144
                ->setTaxType($Taxation)
0 ignored issues
show
Bug introduced by
It seems like $Taxation defined by $this->entityManager->fi...ster\TaxType::TAXATION) on line 111 can also be of type object; however, Eccube\Entity\OrderItem::setTaxType() does only seem to accept null|object<Eccube\Entity\Master\TaxType>, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
145
                ->setProcessorName(DeliveryFeePreprocessor::class);
146
147
            $itemHolder->addItem($OrderItem);
148
            $Shipping->addOrderItem($OrderItem);
149
        }
150
    }
151
152
    public function __toString()
153
    {
154
        return get_class($this);
155
    }
156
}
157