SlimPay / hapiclient-php

HAPI Client for PHP.
MIT License
14 stars 12 forks source link

Users of the lib cannot mock HapiClient because the class is final #9

Closed methylbro closed 6 years ago

methylbro commented 6 years ago

Suppose that you want to use the HapiClient in your code:

<?php

use HapiClient\Hal\CustomRel;
use HapiClient\Http\HapiClient;
use HapiClient\Http\Follow;
use HapiClient\Http\JsonBody;

class MandateValidityVerification
{
    private $hapiClient;
    private $creditorReference;

    public function __construct(HapiClient $hapiClient, $creditorReference)
    {
        $this->hapiClient = $hapiClient;
        $this->creditorReference = $creditorReference;
    }

    public function hasValidMandate($subscriberReference)
    {
        $follow = new Follow(
            new CustomRel('https://api.slimpay.net/alps#search-mandates'), 
            'GET',
            [
                'creditorReference' => $this->creditorReference,
                'subscriberReference' => $subscriberReference,
                'state' => 'active',
            ]
        );

        $result = $this->hapiClient->sendFollow($follow);
        $nbMandates = $result->getState()['page']['totalElements'];

        return $nbMandates > 0;
    }
}

You cannot test it because the class HapiClient (and more classes like Resource) is final.

<?php

use PHPUnit\Framework\TestCase;
use HapiClient\Hal\Resource;
use HapiClient\Http\HapiClient;

class MandateValidityVerificationTest extends TestCase
{
    public function testHasValidMandate()
    {
        $resource = $this
            ->getMockBuilder(Resource::class)
            ->disableOriginalConstructor()
            ->setMethods(['getState'])
            ->getMock()
        ;
        $hapiClient
            ->method('getState')
            ->willReturn(['page' => ['totalElements' => 1]])
        ;

        $resource = $this
            ->getMockBuilder(HapiClient::class)
            ->disableOriginalConstructor()
            ->setMethods(['sendFollow'])
            ->getMock()
        ;
        $hapiClient
            ->method('sendFollow')
            ->willReturn($resource)
        ;

        $mandateValidityVerification = new MandateValidityVerification($hapiClient, 'creditorReference');
        $hasValidMandate = $mandateValidityVerification->hasValidMandate('subscriberReference');

        $this->assertTrue($hasValidMandate);
    }
}
There was 1 warning:

1) MandateValidityVerificationTest::testHasValidMandate
Class "HapiClient\Hal\Resource" is declared "final" and cannot be mocked.

I think a lot of interfaces are missing or you shouldn't set all your classes as final.

Simply adding interfaces could do the work. For exemple:

<?php

interface HapiClientInterface
{
    /**
     * @param $request  Request The Request object containing all the parameters
     *                          necessary for the HTTP request.
     *
     * @return Resource The Resource object returned by the server.
     */
    public function sendRequest(Request $request);

    /**
     * @param $follow   array|Follow    The Follow object or an array of Follow objects containing
     *                                  the parameters necessary for the HTTP request(s).
     * @param $resource null|Resource   The resource containing the link you want to follow.
     *                                  If null, the entry point Resource will be used.
     *
     * @return Resource The Resource object contained in the last response.
     */
    public function sendFollow($follow, Resource $resource = null);

    /**
     * Attempts to refresh the Resource by sending a GET request
     * to the URL referenced by the "self" relation type.
     * If the resource doesn't have such relation type or the request fails,
     * the same resource is returned.
     * @param $resource Resource    The Resource to refresh.
     * @return  The refreshed Resource or the same Resource if failed to refresh it.
     */
    public function refresh($resource);
}

The HapiClient could implement this interface:

final class HapiClient implements HapiClientInterface
{
    //...
}

So lib users can mock this interface:

    $resource = $this
        ->getMockBuilder(HapiClientInterface::class)
        ->setMethods(['sendFollow'])
        ->getMock()
    ;

I'm open to ear your point of view on this matter. If needed I could do a pull-request.

YoussefBO commented 6 years ago

I agree with your approach. Please do send a pull-request and we will merge it.

methylbro commented 6 years ago

@YoussefBO See the pull-request #10. You could also watch another version to fix the problem (by removing final state on the classes) on a branch of my fork.