slince / shopify-api-php

:rocket: Shopify API Client for PHP
MIT License
128 stars 48 forks source link

API versioning #52

Open maximzasorin opened 4 years ago

maximzasorin commented 4 years ago

Hi, @slince.

I see you've added versioning and cursor-based pagination support to the library, great job!

I have a little remark on versioning, I would like to discuss it.

In summary, I agree that the user should be able to query a different version of the API than the default client version, but I do not agree that the user can change the version of the API for which the library models and managers are implemented.

I will try to explain it in more detail.

After the introduction of API versioning, we have received several versions of the API working simultaneously. That is, theoretically, one application using the library can work with several versions of the API. Libraries to work with this API should provide this feature, but there is a challenge here in relation to this library.

In addition to the basic implementation of CRUD methods in the library there is an abstract provided by the models and managers. Now this abstraction implements a certain one version of the API, and cannot work with several versions of the API. I will explain on a fictitious example (I could give a real example, but unfortunately there is no implementation of all models in the library now).

For example, there is a model Model, in which the version 2019-07 has a property property, in the next version 2019-10 Shopify developers decide to remove this property. It turns out that for correct versioning support there should be separate models for each version. In one model there should be a property property, in another this property should not be. If we have only one implementation of the model, and we allow the user to set the API version, we get a mess.

For example, we have a model implementation:

class Model
{
    // ...

    private $property;

    public function getProperty()
    {
        return $this->property;
    }

    // ...
}

And the default version of the client is 2019-07:

class Client
{
    private $apiVersion = '2019-07';
}

The user makes a request through the manager and gets the correct result:

$client = new Client(...);
$model = $client->getModelManager()->get();
$model->getProperty(); // property value

If the user changes the API version, the result is not as expected:

$client = new Client(..., [
    'apiVersion' => '2019-07',
]);
$model = $client->getModelManager()->get();
$model->getProperty(); // no value

It turns out that the model does not correspond to the used version of the API, the same is true if any of the properties will be added in the new version of the APU, in this case, the model will not provide a definition of the property, which is in the new version of the API.

There are the same changes for Checkout and Payment models in release note for 2019-10: https://help.shopify.com/en/api/versioning/release-notes/2019-10#rest-admin-api-changes

Everything can also be applied to managers, for example, now managers have implemented the paginate method to use cursor-based pagination, but if the user changes to an earlier version and tries to call this method, it will not work.

I see two solutions: 1) We need to either maintain a separate abstraction for each version of the API, or: 2) Lock a version of the abstraction and not allow users to modify it and requests for versions of the API other than the default one are allowed only through CRUD. I think the second option is more preferable because it would be too difficult to support multiple versions of the abstraction.

That's how it may work.

  1. When working with managers, the default version should always be used, for which an abstraction is defined. It will be necessary to sync the library version and the API version, and after a new version of the API is released, the abstraction and the library version must be updated. For example, let the version of API 2020-01 match the version of the library 3.x, then, after the release of version 2020-04, the version of the library will change to 4.x and so on.

  2. When working with CRUD methods it should be possible to use a custom version, for example, as below or in another similar way:

    $client->get('2019-10', 'products');
    
    $client->post('2019-10', 'products', [
        'product' => [
            ...
        ]
    ]);
    
    $client->put('2019-10', 'products/12800', [
        'product' => [
            ...
        ]
    ]);
    
    $client->delete('2019-10', 'products/12800');
slince commented 4 years ago

Hello, yes, that's why I didn't mark it on readme.md after adding API version support. I haven't come up with a better way to solve this problem. Yes, I think your idea is right, I am now thinking about how to implement 3.x version.

maximzasorin commented 4 years ago

Okay. It would also be very useful if you could tell about your plans for the 3.x version, maybe I could help.

slince commented 4 years ago

Ideas about 3.x include (just an idea. It hasn't been decided yet):

  1. Remove jms/serializer and use symfony/serializer instead to implement hydrator; The former relies too much on third-party code.

  2. Reorganize the directory structure of the manager, use Entity to store all resource models, and use Managerto store all resource managers; Use the Contracts to store all resource manager interfaces.

  3. Some framework integrations, such as symfony bundle or laravel service provider. May be a separate repo

  4. Oauth support, webhook validation and so on ...