[DX] Static resource access is bad for mockability

[kenloburn]

In the current implementation of the SDK static methods on classes are used to make requests.

For example:

// ...
$users = User::listing($apiContext)->getValue();

This is not good for developer experience as it makes it non-trivial to implement the SDK in a project with high unit test coverage.

Imagine for example that in my project I have a FinanceService that has a method to accumulate total spend on saturday + sunday. If I expressed the service like this:

<?php

namespace Bla;

use bunq\Context\ApiContext;
use bunq\Model\Generated\Payment;

class FinanceService
{
    private $apiContext;

    public function __construct(ApiContext $apiContext)
    {
        $this->apiContext = $apiContext;
    }

    public function getWeekendSpending($userId, $monetaryAccountId)
    {
        $amount = 0;
        $paymentList = Payment::listing($this->apiContext, $userId, $monetaryAccountId);

        foreach ($paymentList as $payment) {
            $date = new \DateTime($payment->getCreated());

            if (in_array($date->format('D'), ['Sat', 'Sun'])) {
                $amount += $payment->getAmount();
            }
        }

        return $amount;
    }
}

Writing the unit test would not allow me to mock the API HTTP request very easily as there is no simple way to ensure that Payment::listing does not make an actual HTTP request, or to influence what the return value would be.

There is a workaround I could do on my end, but it involves wrapping each individual resource that makes HTTP requests in a wrapper object like this:

class PaymentResource implements PaymentResourceInterface
{
    private $apiContext;

    public function __construct(ApiContext $apiContext)
    {
        $this->apiContext = $apiContext;
    }

    public function listing($userId, $monetaryAccountId)
    {
        return Payment::listing($this->apiContext, $userId, $monetaryAccountId);
    }
}

To be able to mock (the result of) API HTTP requests in a unit test designed to test only the logic of the service method, and not invoke an actual API HTTP request (or even obtain/instantiate an ApiContext/session with the API):

class FinanceServiceTest extends \PHPUnit\Framework\TestCase
{
    public function testGetWeekendSpending()
    {
        // Friday
        $payment0 = new Payment();
        $payment0->setAmount(new Amount('199', 'EUR'));
        $payment0->setCreated('2017-08-18 10:33:17.097996');

        // Sunday
        $payment1 = new Payment();
        $payment1->setAmount(new Amount('199', 'EUR'));
        $payment1->setCreated('2017-08-20 16:21:57.295726');

        $paymentResource = $this
            ->createMock(PaymentResource::class)
            ->method('listing')
            ->willReturn([
                $payment0,
                $payment1
            ]);

        $s = new FinanceService($paymentResource);
        $this->assertEquals(199, $s->getWeekendSpending(1, 2));
    }
}

This would mean that I would end up maintaining code that is domain specific to bunq but somehow physically located in the codebase of my own projects.

Ideally, the SDK is designed in a way that there is no need to invoke static methods to make API HTTP requests - this will make the DX much smoother in modern PHP projects.

[dnl-blkv]

@kenloburn great point!

[kenloburn]

@OGKevin @dnl-blkv Any idea if/when this will be looked into?

[dnl-blkv]

@kenloburn this is a major rework. So, not providing any dates yet.

[OGKevin]

@kenloburn Just a little update, we haven't forgot about this. It will be on the roadmap after callback models, decoder improvement for callback models and some minor patches 👍

[OGKevin]

After internal discussions, we concluded that ws will not move away from the static HTTP methods.

I would suggest you create a mock object and mock the static call via this mock object.