[WIP] v3.0.0 Overview Design (for brainstorming only, all inputs are welcomed)

[guzzilar]

!DISCLAIMER: This pull request is only here just for brainstorming purpose. It should not be merged directly to the dev-3-0-0 branch.

1. Objective

As previously I submitted pull requests for Omise-PHP v3.0.0 in different separate branches, it's hard to keep tracking for a reviewer to see where we are heading toward for a new Omise-PHP design and structure.

So I decided to have this pull request as a place to discuss, request feature, and a place where we can see the final design of Omise-PHP v3.0.0 (or at least, a road-map to a final design).

All inputs are more than welcomes.

Known Feature Requests

• Be able to get all properties from OmiseObject, as in some case, people want to log an output that they have received from API. Something like:

  $charge = Omise\Charge::retrieve('chrg_id');
  $charge->toArray();
  

  Related issue tickets: #53

• Unit test friendly. Decoupling Client, API Requestor, Response Handler, and so on. Related issue tickets: #40, #110

• Proposal ofr 3-0-0 design (thanks for @iwat, @liverbool) Related issue tickets: #59

Breaking Changes

All of the items in the list below are just ideas that may or may not be implemented, however, I would like to list it out just to see how far we agree on the changes.

1. No longer support for retrieve($id) with empty id for most of resources. Reason: retrieve() method has always been used as a method to retrieve a single object and a list object in one place. Sometimes it return object: [single-object] but sometimes object: list.

To add up to this topic, OmiseCharge::retrieve() gives us OmiseCharge object with an attribute object: list but when we call $charges->refunds() we instead, got OmiseRefundList object (hint: why don't we get OmiseChargeList object when we call for OmiseCharge::retrieve()?)

Improvement: This pull request is introducing a new method all(), which will always return Omise\Collection object for those object: list resources.

// This will raise an error $id parameter is required.
$charges = Omsie\Charge::retrieve(); 

// Use this one instead.
$charges = Omise\Charge::all(); 

Also, the another benefit of having Omise\Collection class is to improve code performance as well, as we can move around its array instead of trying to make a new request to Omise API every time we want to pull some data out.

Let's say we want to capture first 5 charge items. Previously, we have to retrieve a charge object 5 times.

$charges = OmiseCharge::retrieve('?limit=5&order=reverse_chronological');
foreach($charges['data'] as $charge) {
    $charge = OmiseCharge::retrieve($charge['id']); // Connect to Omise API.
    $charge->capture();
}

Now, we can simply do

$charges = OmiseCharge::all(['limit' => 5, 'order' => 'reverse_chronological']);
foreach($charges as $charge) {
    $charge->capture();
}

.. . 2. Removing ENDPOINT constant from resource object. Instead, using OBJECT_NAME. This one is a bit hard to explain its concept. But, this pull request is introducing a new way to think of resource classes.

Its origin was started from a simple question, why reload() method should pass url back to OmiseApiResource? Why doesn't it just reload itself from its location attribute?

I think because previously we were looking at these resource classes as just a class. Not really an object. By class, I mean, its responsibility was just passing variables around

public function reload()
{
    parent::g_reload(self::getUrl(...));
}

But if we think it as an object. This object should already known its id, endpoint location, etc since we call OmiseObject::retrieve('id');.

The reason of this one is not quite really related to the topic. But because of the starter-idea, so I took an advantage of OOP into these resource classes. Let's take a look at \Omise\Link class.

namespace Omise;

class Link extends \Omise\ApiResource
{
    const OBJECT_NAME = 'link';

    public static function all($query = array())
    {
        $resource = \Omise\Resource::newObject(static::OBJECT_NAME);
        $result   = $resource->request()->get($resource->url(), $resource->credential(), $query);

        return new \Omise\Collection($result);
    }

    public static function search($query = '')
    {
        return \Omise\Search::scope('link')->query($query);
    }

    public static function retrieve($id)
    {
        return parent::resourceRetrieve($id);
    }

    public function reload()
    {
        parent::resourceReload();
    }

    public static function create($params)
    {
        return parent::resourceCreate($params);
    }
}

[guzzilar]

Note: One of the breaking-changes would be Omise\Collection class.

By executing:

$transactions = Omise\Transaction::all(['limit' => 3]);
print_r($transactions);

Should give you the following output:

Omise\Collection Object
(
    [attributes:protected] => Array
        (
            [from] => 1970-01-01T00:00:00Z
            [to] => 2019-06-22T07:54:34Z
            [offset] => 0
            [limit] => 3
            [total] => 553
            [order] => chronological
            [location] => /transactions
        )

    [items:protected] => Array
        (
            [0] => Omise\Transaction Object
                (
                    [id:protected] => trxn_test_***1
                    [attributes:protected] => Array
                        (
                            [object] => transaction
                            ...
                            [amount] => 77836
                            [currency] => THB
                            [transferable_at] => 2017-11-06T10:02:59Z
                        )
                )

            [1] => Omise\Transaction Object
                (
                    [id:protected] => trxn_test_***2
                    [attributes:protected] => Array
                        (
                            [object] => transaction
                            ...
                            [amount] => 38919
                            [currency] => THB
                            [transferable_at] => 2017-11-06T10:06:25Z
                        )
                )

            [2] => Omise\Transaction Object
                (
                    [id:protected] => trxn_test_***3
                    [attributes:protected] => Array
                        (
                            [object] => transaction
                            ...
                            [amount] => 194592
                            [currency] => THB
                            [transferable_at] => 2017-11-07T07:47:43Z
                        )
                )
        )
)

Now, imagine if you want to capture a bunch of authorized charges? This would be possible to do, by executing the following code:

$charges = OmiseCharge::search()->filter(array('captured' => false));

foreach ($charges as $charge) {
    $charge->capture();
}

[guzzilar]

Also note for an overview of the current class structure:

Omise\Charge < Omise\ApiResource < Omise\OmiseObject
               |- Omise\ApiRequestor
                  |- Omise\Client\ClientInterface
	                  |- Omise\Client\CurlClient
	                  |- Omise\Client\UnitTestClient
	                  
               |- Omise\ApiResponse


Omise\Collection
	|- Omise\Charge (id: 1)
	|- Omise\Charge (id: 2)
	|- Omise\Charge (id: 3)
	|- Omise\Charge (id: 4)
	|- Omise\Charge (id: 5)


Omise\Omise::publicKey()
Omise\Omise::secretKey()
Omise\Omise::apiversion()
Omise\Omise::userAgent()
Omise\Omise::client() < You can swap a client class in case of unit-test.

[guzzilar]

As for unit test. Now it becomes more clean and neat. We will now be able to swap between CurlClient for the production, and UnitTestClient for the PHPUnit.

Also, the fixture files will now be separated by API version. You may check tests/fixtures/2019-05-29/*. We will also now, be able to choose what API version we would like to run the test, by simply set \Omise\Omise::setApiVersion('2019-05-29');

File: tests/omise/TestConfig.php

<?php
require_once dirname(__FILE__) . '/../../lib/Omise.php';

// Omise keys.
\Omise\Omise::setPublicKey('pkey');
\Omise\Omise::setSecretKey('skey');
\Omise\Omise::setApiVersion('2019-05-29');
\Omise\Omise::setClient('\Omise\Client\UnitTestClient');

abstract class TestConfig extends PHPUnit_Framework_TestCase
{
	// ...
}

[guzzilar]

Note, adding support to be able to convert an output to an array or a JSON string.

$account = \Omise\Account::retrieve();
print_r($account->toArray());

// Output
Array
(
    [object] => account
    [id] => account_test_fixture
    [livemode] => 
    [location] => /account
    [created_at] => 2017-11-01T17:10:42Z
    [team] => acct_fixture
    [email] => [email protected]
    [currency] => THB
    [supported_currencies] => Array
        (
            [0] => THB
            [1] => JPY
            [2] => USD
            [3] => EUR
            [4] => GBP
            [5] => SGD
        )
)


print_r($account->toJson());

// Output
{"object":"account","id":"account_test_fixture","livemode":false,"location":"\/account","created_at":"2017-11-01T17:10:42Z","team":"acct_fixture","email":"[email protected]","currency":"THB","supported_currencies":["THB","JPY","USD","EUR","GBP","SGD"]}

[guzzilar]

Note, as Omise-PHP v3.0.0 is introducing all method for all resources that can be listed. It's likely that we should break a support from retrieve without passing any id.

For example: This is how Omise-PHP v2.x work.

$charges = OmiseCharge::retrieve();
echo $charges['object']; // output: "list"

$charge = $charges['data'][0];
echo $charge['object']; // charge
echo $charge['id']; // chrg_***

We may want to make it like this at v3.0.0.

$charges = \Omise\Charge::all();
echo get_class($charges); // output: "Omise\Collection"

$charge = $charges->first();
echo $charge['object']; // charge
echo $charge['id']; // chrg_***

However, if we still keep the support of OmiseCharge::retrieve(), then this may confuse developers on which method exactly it should be called.

Also, in my opinion, we should not make it backward compatible as well. Not something like

$charges = OmiseCharge::retrieve();
echo get_class($charges); // output: "Omise\Collection"

As there might be a chance that someone would integrate Omise-PHP with

$charges = OmiseCharge::retrieve();
if ($charges instanceof OmiseCharge) {
    // ...
}

And it would eventually break the code anyway (as if we do backward compatible, then this $charges will be an instance of Omise\Collection).

So, why not just raise an error so developers will know and update their code.

[eNzyOfficial]

Do you guys have a release date for this? I've been waiting ~15 months for you guys to add namespaces. Or can you at least tag a version of this branch for composer so I can use it please?