Completed
Push — master ( 1483b5...e14ece )
by Jeroen De
11s
created

testGivenExistingTransactionIdForBookedDonation_handler()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 15
Code Lines 10

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 1
eloc 10
nc 1
nop 0
dl 0
loc 15
rs 9.4285
c 0
b 0
f 0
1
<?php
2
3
declare( strict_types = 1 );
4
5
namespace WMDE\Fundraising\Frontend\DonationContext\Tests\Integration\UseCases\HandlePayPalPaymentNotification;
6
7
use WMDE\Fundraising\Frontend\DonationContext\DataAccess\DoctrineDonationRepository;
8
use WMDE\Fundraising\Frontend\DonationContext\Domain\Model\Donation;
9
use WMDE\Fundraising\Frontend\DonationContext\Domain\Model\DonorName;
10
use WMDE\Fundraising\Frontend\DonationContext\Infrastructure\DonationConfirmationMailer;
11
use WMDE\Fundraising\Frontend\DonationContext\Infrastructure\DonationEventLogger;
12
use WMDE\Fundraising\Frontend\DonationContext\Tests\Data\ValidPayPalNotificationRequest;
13
use WMDE\Fundraising\Frontend\DonationContext\Tests\Fixtures\DonationEventLoggerSpy;
14
use WMDE\Fundraising\Frontend\DonationContext\Tests\Fixtures\DonationRepositorySpy;
15
use WMDE\Fundraising\Frontend\DonationContext\Tests\Fixtures\FailingDonationAuthorizer;
16
use WMDE\Fundraising\Frontend\DonationContext\Tests\Fixtures\FakeDonationRepository;
17
use WMDE\Fundraising\Frontend\DonationContext\Tests\Fixtures\SucceedingDonationAuthorizer;
18
use WMDE\Fundraising\Frontend\DonationContext\UseCases\HandlePayPalPaymentNotification\HandlePayPalPaymentNotificationUseCase;
19
use WMDE\Fundraising\Frontend\PaymentContext\Domain\Model\PayPalData;
20
use WMDE\Fundraising\Frontend\DonationContext\Tests\Data\ValidDonation;
21
use WMDE\Fundraising\Frontend\Tests\Fixtures\ThrowingEntityManager;
22
23
/**
24
 * @covers WMDE\Fundraising\Frontend\DonationContext\UseCases\HandlePayPalPaymentNotification\HandlePayPalPaymentNotificationUseCase
25
 *
26
 * @licence GNU GPL v2+
27
 * @author Kai Nissen < [email protected] >
28
 * @author Gabriel Birke < [email protected] >
29
 */
30
class HandlePayPalPaymentNotificationUseCaseTest extends \PHPUnit_Framework_TestCase {
31
32
	public function testWhenRepositoryThrowsException_errorResponseIsReturned() {
33
		$useCase = new HandlePayPalPaymentNotificationUseCase(
34
			new DoctrineDonationRepository( ThrowingEntityManager::newInstance( $this ) ),
35
			new FailingDonationAuthorizer(),
36
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
37
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
38
		);
39
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
40
		$reponse = $useCase->handleNotification( $request );
41
		$this->assertFalse( $reponse->notificationWasHandled() );
42
		$this->assertTrue( $reponse->hasErrors() );
43
	}
44
45
	public function testWhenAuthorizationFails_unhandledResponseIsReturned() {
46
		$fakeRepository = new FakeDonationRepository();
47
		$fakeRepository->storeDonation( ValidDonation::newIncompletePayPalDonation() );
48
49
		$useCase = new HandlePayPalPaymentNotificationUseCase(
50
			$fakeRepository,
51
			new FailingDonationAuthorizer(),
52
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
53
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
54
		);
55
56
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
57
		$this->assertFalse( $useCase->handleNotification( $request )->notificationWasHandled() );
58
	}
59
60
	public function testWhenAuthorizationSucceeds_successResponseIsReturned() {
61
		$fakeRepository = new FakeDonationRepository();
62
		$fakeRepository->storeDonation( ValidDonation::newIncompletePayPalDonation() );
63
64
		$useCase = new HandlePayPalPaymentNotificationUseCase(
65
			$fakeRepository,
66
			new SucceedingDonationAuthorizer(),
67
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
68
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
69
		);
70
71
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
72
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
73
	}
74
75
	public function testWhenPaymentTypeIsNonPayPal_unhandledResponseIsReturned() {
76
		$fakeRepository = new FakeDonationRepository();
77
		$fakeRepository->storeDonation( ValidDonation::newDirectDebitDonation() );
78
79
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
80
		$useCase = new HandlePayPalPaymentNotificationUseCase(
81
			$fakeRepository,
82
			new SucceedingDonationAuthorizer(),
83
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
84
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
85
		);
86
87
		$this->assertFalse( $useCase->handleNotification( $request )->notificationWasHandled() );
88
	}
89
90
	public function testWhenPaymentStatusIsPending_unhandledResponseIsReturned() {
91
		$request = ValidPayPalNotificationRequest::newPendingPayment();
92
93
		$useCase = new HandlePayPalPaymentNotificationUseCase(
94
			new FakeDonationRepository(),
95
			new SucceedingDonationAuthorizer(),
96
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
97
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
98
		);
99
100
		$this->assertFalse( $useCase->handleNotification( $request )->notificationWasHandled() );
101
	}
102
103
	public function testWhenPaymentStatusIsPending_responseContainsMoreInformation() {
104
		$request = $request = ValidPayPalNotificationRequest::newPendingPayment();
105
106
		$useCase = new HandlePayPalPaymentNotificationUseCase(
107
			new FakeDonationRepository(),
108
			new SucceedingDonationAuthorizer(),
109
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
110
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
111
		);
112
113
		$response = $useCase->handleNotification( $request );
114
		$this->assertSame( 'Unhandled PayPal instant payment notification', $response->getContext()['message'] );
115
		$this->assertSame( 'Pending', $response->getContext()['request']['paymentStatus'] );
116
	}
117
118
	public function testWhenTransactionTypeIsForSubscriptionChanges_unhandledResponseIsReturned() {
119
		$request = ValidPayPalNotificationRequest::newSubscriptionModification();
120
121
		$useCase = new HandlePayPalPaymentNotificationUseCase(
122
			new FakeDonationRepository(),
123
			new SucceedingDonationAuthorizer(),
124
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
125
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
126
		);
127
		$this->assertFalse( $useCase->handleNotification( $request )->notificationWasHandled() );
128
	}
129
130
	public function testWhenTransactionTypeIsForSubscriptionChanges_responseContainsMoreInformation() {
131
		$useCase = new HandlePayPalPaymentNotificationUseCase(
132
			new FakeDonationRepository(),
133
			new SucceedingDonationAuthorizer(),
134
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
135
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
136
		);
137
138
		$request = ValidPayPalNotificationRequest::newSubscriptionModification();
139
140
		$response = $useCase->handleNotification( $request );
141
		$this->assertSame( 'Unhandled PayPal subscription notification', $response->getContext()['message'] );
142
		$this->assertSame( 'subscr_modify', $response->getContext()['request']['transactionType'] );
143
	}
144
145
	public function testWhenAuthorizationSucceeds_confirmationMailIsSent() {
146
		$donation = ValidDonation::newIncompletePayPalDonation();
147
		$fakeRepository = new FakeDonationRepository();
148
		$fakeRepository->storeDonation( $donation );
149
150
		$mailer = $this->getMailer();
151
		$mailer->expects( $this->once() )
0 ignored issues
show
Bug introduced by
The method expects does only exist in PHPUnit_Framework_MockObject_MockObject, but not in WMDE\Fundraising\Fronten...ationConfirmationMailer.

It seems like the method you are trying to call exists only in some of the possible types.

Let’s take a look at an example:

class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

Available Fixes

  1. Add an additional type-check:

    /**
     * @param A|B $x
     */
    function someFunction($x)
    {
        $x->foo();
    
        if ($x instanceof B) {
            $x->bar();
        }
    }
    
  2. Only allow a single type to be passed if the variable comes from a parameter:

    function someFunction(B $x) { /** ... */ }
    
Loading history...
152
			->method( 'sendConfirmationMailFor' );
153
			// TODO: assert that the correct values are passed to the mailer
154
155
		$useCase = new HandlePayPalPaymentNotificationUseCase(
156
			$fakeRepository,
157
			new SucceedingDonationAuthorizer(),
158
			$mailer,
0 ignored issues
show
Bug introduced by
It seems like $mailer defined by $this->getMailer() on line 150 can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, 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...
159
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
160
		);
161
162
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
163
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
164
	}
165
166
	public function testWhenAuthorizationSucceedsForAnonymousDonation_confirmationMailIsNotSent() {
167
		$donation = ValidDonation::newIncompleteAnonymousPayPalDonation();
168
		$fakeRepository = new FakeDonationRepository();
169
		$fakeRepository->storeDonation( $donation );
170
171
		$mailer = $this->getMailer();
172
		$mailer->expects( $this->never() )
0 ignored issues
show
Bug introduced by
The method expects does only exist in PHPUnit_Framework_MockObject_MockObject, but not in WMDE\Fundraising\Fronten...ationConfirmationMailer.

It seems like the method you are trying to call exists only in some of the possible types.

Let’s take a look at an example:

class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

Available Fixes

  1. Add an additional type-check:

    /**
     * @param A|B $x
     */
    function someFunction($x)
    {
        $x->foo();
    
        if ($x instanceof B) {
            $x->bar();
        }
    }
    
  2. Only allow a single type to be passed if the variable comes from a parameter:

    function someFunction(B $x) { /** ... */ }
    
Loading history...
173
			->method( 'sendConfirmationMailFor' );
174
175
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
176
		$useCase = new HandlePayPalPaymentNotificationUseCase(
177
			$fakeRepository,
178
			new SucceedingDonationAuthorizer(),
179
			$mailer,
0 ignored issues
show
Bug introduced by
It seems like $mailer defined by $this->getMailer() on line 171 can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, 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...
180
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
181
		);
182
183
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
184
	}
185
186
	public function testWhenAuthorizationSucceeds_donationIsStored() {
187
		$donation = ValidDonation::newIncompletePayPalDonation();
188
		$repositorySpy = new DonationRepositorySpy( $donation );
189
190
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
191
		$useCase = new HandlePayPalPaymentNotificationUseCase(
192
			$repositorySpy,
193
			new SucceedingDonationAuthorizer(),
194
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
195
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
196
		);
197
198
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
199
		$this->assertCount( 1, $repositorySpy->getStoreDonationCalls() );
200
	}
201
202
	public function testWhenAuthorizationSucceeds_donationIsBooked() {
203
		$donation = ValidDonation::newIncompletePayPalDonation();
204
		$repository = new FakeDonationRepository( $donation );
205
206
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
207
		$useCase = new HandlePayPalPaymentNotificationUseCase(
208
			$repository,
209
			new SucceedingDonationAuthorizer(),
210
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
211
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
212
		);
213
214
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
215
		$this->assertTrue( $repository->getDonationById( $donation->getId() )->isBooked() );
216
	}
217
218
	public function testWhenAuthorizationSucceeds_bookingEventIsLogged() {
219
		$donation = ValidDonation::newIncompletePayPalDonation();
220
		$repositorySpy = new DonationRepositorySpy( $donation );
221
222
		$eventLogger = new DonationEventLoggerSpy();
223
224
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
225
		$useCase = new HandlePayPalPaymentNotificationUseCase(
226
			$repositorySpy,
227
			new SucceedingDonationAuthorizer(),
228
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
229
			$eventLogger
230
		);
231
232
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
233
234
		$this->assertEventLogContainsExpression( $eventLogger, $donation->getId(), '/booked/' );
235
	}
236
237
	public function testWhenSendingConfirmationMailFails_handlerReturnsTrue() {
238
		$fakeRepository = new FakeDonationRepository();
239
		$fakeRepository->storeDonation( ValidDonation::newIncompletePayPalDonation() );
240
241
		$mailer = $this->getMailer();
242
		$mailer->expects( $this->once() )
0 ignored issues
show
Bug introduced by
The method expects does only exist in PHPUnit_Framework_MockObject_MockObject, but not in WMDE\Fundraising\Fronten...ationConfirmationMailer.

It seems like the method you are trying to call exists only in some of the possible types.

Let’s take a look at an example:

class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

Available Fixes

  1. Add an additional type-check:

    /**
     * @param A|B $x
     */
    function someFunction($x)
    {
        $x->foo();
    
        if ($x instanceof B) {
            $x->bar();
        }
    }
    
  2. Only allow a single type to be passed if the variable comes from a parameter:

    function someFunction(B $x) { /** ... */ }
    
Loading history...
243
			->method( 'sendConfirmationMailFor' )
244
			->willThrowException( new \RuntimeException( 'Oh noes!' ) );
245
246
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
247
		$useCase = new HandlePayPalPaymentNotificationUseCase(
248
			$fakeRepository,
249
			new SucceedingDonationAuthorizer(),
250
			$mailer,
0 ignored issues
show
Bug introduced by
It seems like $mailer defined by $this->getMailer() on line 241 can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, 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...
251
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
252
		);
253
254
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
255
	}
256
257
	public function testGivenNewTransactionIdForBookedDonation_transactionIdShowsUpInChildPayments() {
258
		$donation = ValidDonation::newBookedPayPalDonation();
259
		$transactionId = '16R12136PU8783961';
260
261
		$fakeRepository = new FakeDonationRepository();
262
		$fakeRepository->storeDonation( $donation );
263
264
		$request = ValidPayPalNotificationRequest::newDuplicatePaymentForDonation( $donation->getId(), $transactionId );
265
266
		$useCase = new HandlePayPalPaymentNotificationUseCase(
267
			$fakeRepository,
268
			new SucceedingDonationAuthorizer(),
269
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
270
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
271
		);
272
273
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
274
275
		/** @var \WMDE\Fundraising\Frontend\PaymentContext\Domain\Model\PayPalPayment $payment */
276
		$payment = $fakeRepository->getDonationById( $donation->getId() )->getPaymentMethod();
277
278
		$this->assertTrue(
279
			$payment->getPayPalData()->hasChildPayment( $transactionId ),
280
			'Parent payment must have new transaction ID in its list'
281
		);
282
	}
283
284
	public function testGivenNewTransactionIdForBookedDonation_childTransactionWithSameDataIsCreated() {
285
		$donation = ValidDonation::newBookedPayPalDonation();
286
		$transactionId = '16R12136PU8783961';
287
288
		$fakeRepository = new FakeDonationRepository();
289
		$fakeRepository->storeDonation( $donation );
290
291
		$request = ValidPayPalNotificationRequest::newDuplicatePaymentForDonation( $donation->getId(), $transactionId );
292
293
		$useCase = new HandlePayPalPaymentNotificationUseCase(
294
			$fakeRepository,
295
			new SucceedingDonationAuthorizer(),
296
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
297
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
298
		);
299
300
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
301
302
		$donation = $fakeRepository->getDonationById( $donation->getId() );
303
		/** @var \WMDE\Fundraising\Frontend\PaymentContext\Domain\Model\PayPalPayment $payment */
304
		$payment = $donation->getPaymentMethod();
305
		$childDonation = $fakeRepository->getDonationById( $payment->getPayPalData()->getChildPaymentEntityId( $transactionId ) );
306
		$this->assertNotNull( $childDonation );
307
		/** @var \WMDE\Fundraising\Frontend\PaymentContext\Domain\Model\PayPalPayment $childDonationPaymentMethod */
308
		$childDonationPaymentMethod = $childDonation->getPaymentMethod();
309
		$this->assertEquals( $transactionId, $childDonationPaymentMethod->getPayPalData()->getPaymentId() );
310
		$this->assertEquals( $donation->getAmount(), $childDonation->getAmount() );
311
		$this->assertEquals( $donation->getDonor(), $childDonation->getDonor() );
312
		$this->assertEquals( $donation->getPaymentIntervalInMonths(), $childDonation->getPaymentIntervalInMonths() );
313
		$this->assertTrue( $childDonation->isBooked() );
314
	}
315
316
	public function testGivenNewTransactionIdForBookedDonation_childCreationeventIsLogged() {
317
		$donation = ValidDonation::newBookedPayPalDonation();
318
		$transactionId = '16R12136PU8783961';
319
320
		$fakeRepository = new FakeDonationRepository();
321
		$fakeRepository->storeDonation( $donation );
322
323
		$request = ValidPayPalNotificationRequest::newDuplicatePaymentForDonation( $donation->getId(), $transactionId );
324
325
		$eventLogger = new DonationEventLoggerSpy();
326
327
		$useCase = new HandlePayPalPaymentNotificationUseCase(
328
			$fakeRepository,
329
			new SucceedingDonationAuthorizer(),
330
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
331
			$eventLogger
332
		);
333
334
		$this->assertTrue( $useCase->handleNotification( $request )->notificationWasHandled() );
335
336
		$donation = $fakeRepository->getDonationById( $donation->getId() );
337
		/** @var \WMDE\Fundraising\Frontend\PaymentContext\Domain\Model\PayPalPayment $payment */
338
		$payment = $donation->getPaymentMethod();
339
		$childDonationId = $payment->getPayPalData()->getChildPaymentEntityId( $transactionId );
340
341
		$this->assertEventLogContainsExpression( $eventLogger, $donation->getId(), '/child donation.*' . $childDonationId .'/' );
342
		$this->assertEventLogContainsExpression( $eventLogger, $childDonationId, '/parent donation.*' . $donation->getId() .'/' );
343
	}
344
345
	public function testGivenExistingTransactionIdForBookedDonation_handlerReturnsFalse() {
346
		$fakeRepository = new FakeDonationRepository();
347
		$fakeRepository->storeDonation( ValidDonation::newBookedPayPalDonation() );
348
349
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 1 );
350
351
		$useCase = new HandlePayPalPaymentNotificationUseCase(
352
			$fakeRepository,
353
			new SucceedingDonationAuthorizer(),
354
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
355
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
356
		);
357
358
		$this->assertFalse( $useCase->handleNotification( $request )->notificationWasHandled() );
359
	}
360
361
	public function testGivenTransactionIdInBookedChildDonation_noNewDonationIsCreated() {
362
		$transactionId = '16R12136PU8783961';
363
		$fakeChildEntityId = 2;
364
		$donation = ValidDonation::newBookedPayPalDonation();
365
		$donation->getPaymentMethod()->getPaypalData()->addChildPayment( $transactionId, $fakeChildEntityId );
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface WMDE\Fundraising\Fronten...ain\Model\PaymentMethod as the method getPaypalData() does only exist in the following implementations of said interface: WMDE\Fundraising\Fronten...ain\Model\PayPalPayment.

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...
366
367
		$fakeRepository = new FakeDonationRepository();
368
		$fakeRepository->storeDonation( $donation );
369
370
		$request = ValidPayPalNotificationRequest::newDuplicatePaymentForDonation( $donation->getId(), $transactionId );
371
372
		$useCase = new HandlePayPalPaymentNotificationUseCase(
373
			$fakeRepository,
374
			new SucceedingDonationAuthorizer(),
375
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
376
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
377
		);
378
379
		$this->assertFalse( $useCase->handleNotification( $request )->notificationWasHandled() );
380
	}
381
382
	public function testWhenNotificationIsForNonExistingDonation_newDonationIsCreated() {
383
		$repositorySpy = new DonationRepositorySpy();
384
385
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 12345 );
386
		$useCase = new HandlePayPalPaymentNotificationUseCase(
387
			$repositorySpy,
388
			new SucceedingDonationAuthorizer(),
389
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
390
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
391
		);
392
393
		$useCase->handleNotification( $request );
394
395
		$storeDonationCalls = $repositorySpy->getStoreDonationCalls();
396
		$this->assertCount( 1, $storeDonationCalls, 'Donation is stored' );
397
		$this->assertNull( $storeDonationCalls[0]->getId(), 'ID is not taken from request' );
398
		$this->assertDonationIsCreatedWithNotficationRequestData( $storeDonationCalls[0] );
399
	}
400
401
	public function testGivenRecurringPaymentForIncompleteDonation_donationIsBooked() {
402
		$donation = ValidDonation::newIncompletePayPalDonation();
403
		$repositorySpy = new DonationRepositorySpy( $donation );
404
405
		$request = ValidPayPalNotificationRequest::newRecurringPayment( $donation->getId() );
406
407
		$useCase = new HandlePayPalPaymentNotificationUseCase(
408
			$repositorySpy,
409
			new SucceedingDonationAuthorizer(),
410
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
411
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
412
		);
413
414
		$useCase->handleNotification( $request );
415
		$donation = $repositorySpy->getDonationById( $donation->getId() );
416
417
		$this->assertCount( 1, $repositorySpy->getStoreDonationCalls() );
418
		$this->assertEquals( $donation, $repositorySpy->getStoreDonationCalls()[0] );
419
		$this->assertTrue( $donation->isBooked() );
420
	}
421
422
	public function testWhenNotificationIsForNonExistingDonation_confirmationMailIsSent() {
423
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 12345 );
424
		$mailer = $this->getMailer();
425
		$mailer->expects( $this->once() )
0 ignored issues
show
Bug introduced by
The method expects does only exist in PHPUnit_Framework_MockObject_MockObject, but not in WMDE\Fundraising\Fronten...ationConfirmationMailer.

It seems like the method you are trying to call exists only in some of the possible types.

Let’s take a look at an example:

class A
{
    public function foo() { }
}

class B extends A
{
    public function bar() { }
}

/**
 * @param A|B $x
 */
function someFunction($x)
{
    $x->foo(); // This call is fine as the method exists in A and B.
    $x->bar(); // This method only exists in B and might cause an error.
}

Available Fixes

  1. Add an additional type-check:

    /**
     * @param A|B $x
     */
    function someFunction($x)
    {
        $x->foo();
    
        if ($x instanceof B) {
            $x->bar();
        }
    }
    
  2. Only allow a single type to be passed if the variable comes from a parameter:

    function someFunction(B $x) { /** ... */ }
    
Loading history...
426
			->method( 'sendConfirmationMailFor' )
427
			->with( $this->anything() );
428
		$useCase = new HandlePayPalPaymentNotificationUseCase(
429
			new FakeDonationRepository(),
430
			new SucceedingDonationAuthorizer(),
431
			$mailer,
0 ignored issues
show
Bug introduced by
It seems like $mailer defined by $this->getMailer() on line 424 can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, 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...
432
			$this->getEventLogger()
0 ignored issues
show
Bug introduced by
It seems like $this->getEventLogger() targeting WMDE\Fundraising\Fronten...eTest::getEventLogger() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...re\DonationEventLogger>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
433
		);
434
435
		$useCase->handleNotification( $request );
436
	}
437
438
	public function testWhenNotificationIsForNonExistingDonation_bookingEventIsLogged() {
439
		$request = ValidPayPalNotificationRequest::newInstantPaymentForDonation( 12345 );
440
		$eventLogger = new DonationEventLoggerSpy();
441
442
		$useCase = new HandlePayPalPaymentNotificationUseCase(
443
			new FakeDonationRepository(),
444
			new SucceedingDonationAuthorizer(),
445
			$this->getMailer(),
0 ignored issues
show
Bug introduced by
It seems like $this->getMailer() targeting WMDE\Fundraising\Fronten...seCaseTest::getMailer() can also be of type object<PHPUnit_Framework_MockObject_MockObject>; however, WMDE\Fundraising\Fronten...nUseCase::__construct() does only seem to accept object<WMDE\Fundraising\...tionConfirmationMailer>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
446
			$eventLogger
447
		);
448
449
		$useCase->handleNotification( $request );
450
451
		$this->assertEventLogContainsExpression( $eventLogger, 1, '/booked/' ); // 1 is the generated donation id
452
	}
453
454
	private function assertDonationIsCreatedWithNotficationRequestData( Donation $donation ) {
455
		$this->assertSame( 0, $donation->getPaymentIntervalInMonths(), 'Payment interval is always empty' );
456
		$this->assertTrue( $donation->isBooked() );
457
458
		$donorName = $donation->getDonor()->getName();
459
		$this->assertSame( DonorName::PERSON_PRIVATE, $donorName->getPersonType(), 'Person is always private' );
460
		$this->assertSame( ValidPayPalNotificationRequest::PAYER_ADDRESS_NAME, $donorName->getFullName() );
461
462
		$this->assertSame( ValidPayPalNotificationRequest::PAYER_EMAIL, $donation->getDonor()->getEmailAddress() );
463
464
		$address = $donation->getDonor()->getPhysicalAddress();
465
		$this->assertSame( ValidPayPalNotificationRequest::PAYER_ADDRESS_STREET, $address->getStreetAddress() );
466
		$this->assertSame( ValidPayPalNotificationRequest::PAYER_ADDRESS_CITY, $address->getCity() );
467
		$this->assertSame( ValidPayPalNotificationRequest::PAYER_ADDRESS_POSTAL_CODE, $address->getPostalCode() );
468
		$this->assertSame( ValidPayPalNotificationRequest::PAYER_ADDRESS_COUNTRY_CODE, $address->getCountryCode() );
469
470
		$payment = $donation->getPayment();
471
		$this->assertSame( ValidPayPalNotificationRequest::AMOUNT_GROSS_CENTS, $payment->getAmount()->getEuroCents() );
472
473
		/** @var PayPalData $paypalData */
474
		$paypalData = $payment->getPaymentMethod()->getPaypalData();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface WMDE\Fundraising\Fronten...ain\Model\PaymentMethod as the method getPaypalData() does only exist in the following implementations of said interface: WMDE\Fundraising\Fronten...ain\Model\PayPalPayment.

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...
475
		$this->assertSame( ValidPayPalNotificationRequest::PAYER_ADDRESS_NAME, $paypalData->getAddressName() );
476
	}
477
478
	private function assertEventLogContainsExpression( DonationEventLoggerSpy $eventLoggerSpy, int $donationId, string $expr ) {
479
		$foundCalls = array_filter( $eventLoggerSpy->getLogCalls(), function( $call ) use ( $donationId, $expr ) {
480
			return $call[0] == $donationId && preg_match( $expr, $call[1] );
481
		} );
482
		$assertMsg = 'Failed to assert that donation event log log contained "' . $expr . '" for donation id '.$donationId;
483
		$this->assertCount( 1, $foundCalls, $assertMsg );
484
	}
485
486
	/**
487
	 * @return DonationConfirmationMailer|\PHPUnit_Framework_MockObject_MockObject
488
	 */
489
	private function getMailer(): DonationConfirmationMailer {
490
		return $this->getMockBuilder( DonationConfirmationMailer::class )->disableOriginalConstructor()->getMock();
491
	}
492
493
	/**
494
	 * @return DonationEventLogger|\PHPUnit_Framework_MockObject_MockObject
495
	 */
496
	private function getEventLogger(): DonationEventLogger {
497
		return $this->createMock( DonationEventLogger::class );
498
	}
499
500
}
501