Nascom / TeamleaderApiClient

PHP Client to connect to the Teamleader API
MIT License
9 stars 9 forks source link

Figure out if we want to maintain the classes for the Entities ourselves or use a helper library #8

Open nickveenhof opened 6 years ago

nickveenhof commented 6 years ago

https://github.com/swaggest/php-code-builder could for example elp if we could embed the json schemas in here. That does raise the pressure on Teamleader to be accurate with their schemas, but could help in later versioning and overall making it easier to not create getters & setters. It would allow us to generate the object classes everytime Teamleader makes a certain change of bugfix to their schema's. The IDE's won't notice as the generated classes are actual classes.

Curious for ideas/suggestions here.

Classes are generated from the JSON Schemas, and allow something like the following. The generated class can be seen here: https://github.com/swaggest/php-code-builder/blob/master/tests/src/Tmp/Example/User.php

User class: // Importing raw data to entity class instance will do validation and mapping $user = User::import( json_decode('{"name":"John","info":{"lastName":"Doe","birthDate":"1980-01-01","options":{"rememberSession":true,"allowNotifications":false}}}') );

var_dump($user->info->options->allowNotifications); // bool(false)

nickveenhof commented 6 years ago

@BramG Could use some Nascom feedback here as well.

stivni commented 6 years ago

Hi Nick

Our intention at Teamleader is to never break backwards compatibility with our api v2.

We are looking at the Stripe versioning model to make sure that once an integration starts working with a certain (minor) version, that version stays working for several years. Once we start introducing backwards incompatible changes, we would make a new minor version, but only available for new integrations or integrators that intentionally upgrade.

This might affect the design of your SDK because the SDK can and should not make any assumptions about the format of the request or responses, as it might differ from client to client. I’m not sure if I’m making any sense here, without going too deep in the topic.

Although we don’t have a concrete timing yet, it is our intention to provide a minimal SDK for php and JavaScript too, but we will keep it as minimal as possible. We’ll help with authentication, and general stuff like paging (interators etc) and side-loading,

In some way, we hope our data structures are as straight forward possible, so there is minimal need to convert them to models or at least there is minimal effort for someone to do it themselves in their codebase instead of in an SDK.

I hope this gives you some more context and insights to be able to make better informed decisions for this great initiative of yours!

If you have any questions (no feature requests :wink:) please feel free to mention me in one of your issues. Although I don’t look at my github notifications regularly enough...

Kind regards Stijn Vannieuwenhuyse Head of Engineering at Teamleader

nickveenhof commented 6 years ago

@stivni regardless of the change, it would help in reducing tedious copy/paste work towards classes as we do now. API Blueprint currently does not code-wise (to my knowledge) allow the extraction of the JSON schemas.

Right now we need to verify all the properties and make something that is easy to use in terms of converting it from JSON to proper Objects and back. To do this, we use the concept of an ArrayObject as you can see in our current implementation: https://github.com/Nascom/TeamleaderApiClient/blob/v2/src/Entity/Contact.php, or something more deeply nested in there: https://github.com/Nascom/TeamleaderApiClient/blob/v2/src/Entity/Address.php

If we could have the JSON Schema's, we wouldn't be needing to convert these ourselves but there could be tooling to do this for us and it would speed up the process in getting more and more endpoints available more quickly to the SDK, and hence, faster integrations with the V2 to be built.

And with regards to the version. As you mention, the response could tell the client which version it is on, and it could use that information to return the correct objects for the specific endpoint for that version. I don't really see an issue there.

We could have different generated objects depending on the version, as even the most minor change has impact on certain properties of objects and their use in the SDK. If your intention is to build the minimal SDK, I would certainly suggest looking at this SDK in terms of Architecture as it is built according to the latests standards. If something is not "minimal" about this architecture yet, I would love to hear it. Also, it would be sad to duplicate efforts and "rethink" it all in my opinion without taking this work into consideration.

We could also just return the object as is to the integrator/IDE but that would not be the most ideal scenario for the developer experience in my opinion. Examples for these SDK's can be found at the AWS SDK's. Examples for the autogenerated classes, and improved DX can be found at https://github.com/google/google-api-php-client-services/tree/master/src/Google/Service/CloudIot.

Up to Teamleader which direction you'd like to go :)

jenssegers commented 6 years ago

@nickveenhof API Blueprint does provide some parsing tools, and it looks like there are some SDK generators for some languages on https://apiblueprint.org/tools.html#converters. Although not sure how much that is going to speed up to process.

nickveenhof commented 6 years ago

https://apimatic.io/transformer looks like a cool toy to play with. I'll report back with my findings!