Make the library more developer friendly

[xwero]

This Friday, 2020-07-10, I started to use the library and after two hours I just stopped and started to use the old library again. The problem I had was I couldn't figure out to get a batch of contacts by email.

The next day I decided to create a more user friendly library. And I discovered poor design choices in the generator code.
So what i did was to create a CMS class instead of a Factory that returns an instance of the Discovery class, https://github.com/xwero/hubspot-api-php/blob/master/lib/CMS.php. Adding an api key or an access token is two lines of code why would you want to hide that in a Factory class?
The same thing with the http client, you use a plain Guzzle client instance, and if middleware is needed the developer can add it. Why won't you expose that from the start?
Why should we have to type $hubspot->cms()->auditLogs()->defaultApi()->getPage(); when you can remove the discovery and type $cms->getAuditLogs($options) . The discovery system exposes too much of the file structure, what it you want to change that?

Next thing I noticed that different endpoint groups have a DefaultApi class. And in those classes there is the getPage method, which allows the developer to get a range of data. Naming is an important developer task because it's the way you communicate with other developers. In my CMS class the getPage methods are named getAuditLogs, getDomains, getPerformances, getRedirects.
I was thinking about making the getPage method wrappers nullable, ´function getAuditLogs(?AuditLogOptions)´ but it is going to be very rare that you want the default data from an endpoint. So $hubspot->cms()->auditLogs()->defaultApi()->getPage(); is not going to be seen in many codebases except for examples.

In the getPage method the generator calls the getPageWithHttpInfo method, in the getPageWithHttpInfo method the generator calls the getPageRequest method. Why are they all public? How many times are developers going to use the getPageWithHttpInfo and getPageRequest methods?

The getPage methods all have too many parameters. I understand that the endpoint has that many options but use a DTO to pass on all those parameters, it is more developer friendly because you don't have to count parameters to get the correct one (https://github.com/xwero/hubspot-api-php/blob/master/lib/PerformanceOptions.php). It also allows you to add convenience methods for static input values. I used class variable type hinting because I was lazy, so the DTO can only be used from php 7.4 on. But if you want to support older php versions you can use setters to type hint the values.

I read the documentation in the classes and in some classes you have to use a timestamp, and in other classes you can add a DateTime instance for dates. Wouldn't it be better that a date is always added as a DateTime instance, or even better a DateTimeImmutable instance? Let the library code pass on the correct date format to the endpoint. Consistency makes a library more developer friendly.

It took me a few hours and a bit of thinking to make the CMS endpoints easier to use. I understand the decision to go with the code generator, you can build libraries with minimal effort for many languages. But if there are so many quirks people will start writing wrappers of just ignore the library all together for a better option.

[yfaktor]

@xwero - thank you very much for your thoughts and the effort you put into this. You are making many valid points. In our design of the SDKs we were driven by the desire to make SDK methods to be close to the descriptions of the end points of the API itself hoping that it will make it more intuitive to the developers. However, you are right, in many cases it creates longer paths and instead makes it rather more confusing. Here is what we would like to do

• you touched on a number of issues here, so we will break it up to a number of separate issues to address
• we want to make sure we strike a balance between the length of the method path and ease to follow the API doc and also , importantly , maintain the automaticity of code generation to ensure that all of the changes/additions to the API flow into the SDK. We will make an experimental branch to try a different approach getting closer to what you did in the fork for CMS.php and put it out there for the community to comment -we hope to hear from you and other developers on the direction that we are going with it
  Hope it works - and thank you very much again for helping us with this issue.

[xwero]

@yfaktor Thank you for your feedback. I know the task you take on to improve the developer experience is not simple or easy. I will follow the progress and help you in any way I can.

I love the progress you made on the API v3 and it would be great to see a matching library.

As a sidenote: how do you get a batch of contacts by email? There is the /crm/v3/objects/contacts/batch/read endpoint that allows you to set the idProperty. So in the library you have $hs->crm()->contacts()->batchApi()->read(). It's the idProperty I can't figure out.

[lucabartoli]

Following, as I totally agree with xwero.
You should also consider that you will require a migration of old integrations at some point and the cost of this migrations will be reflected on your customers.
The smoother you make this process, the less economic effort is needed by our (and your) clients.

[tominal]

Oh good lord. I just realized that the "more developer friendly" docs for these little mystery functions are IN the HubSpot docs webpages.

For anyone googling, the HubSpot docs have a little PHP link which will show you the exact implementation for the request. I missed this somewhere along the line.

[jumpCircuit]

I think the code in the red rectangle, is also a syntax error.

I randomly learned the setInputs() method in another issue thread. It was a lot of reading, head-scratching, and navel gazing before I could make the code work.

These objects, PublicObjectId, BatchInputPublicObjectId, are difficult to understand, difficult to find, difficult to reason about. I don't believe the mythos of the genius programmer. Whenever possible, things should be intuitive, simple, easy to comprehend. The time that we spend decoding the code, must be minimized.

I might be opinionated, but the name of a function should imply what a thing does. In natural conversation, few of the uninitiated would say, "Oh, just PublicObjectID set the ID, and then you can create your BatchInputPublicObjectId."

Shouldn't the name of a function, and its usage, attempt to approach a spoken language? A function should do only one thing, but I can't think of a name for the procedure, because its child functions are approaching gibberish.

Wasn't the API machine-generated?

I can share something similar that I've used, for another API endpoint.

    public function dealAssociations($apiClient, $contactID)
    {
        $id[0] = new PublicObjectId;
        $id[0]->setId($contactID);
        $associationRequest = new BatchInputPublicObjectId();
        $associationRequest->setInputs($id);

        try {
            return $apiClient->crm()->associations()->batchApi()->read("contacts", "deals", $associationRequest);
        } catch (ApiException $e) {
            echo "Exception when calling batch_api->read: ", $e->getMessage();
            return false;
        }
    }

[atanasiuk-hubspot]

Hey team, this is indeed the true, the API clients are currently automatically generated with OpenApi Generator, so we're very limited with customzing names, objects, etc.

[jumpCircuit]

Can you confirm that there is a syntax error on this page?

https://developers.hubspot.com/docs/api/crm/associations
Read a batch of associations
PHP Line 7
$BatchInputPublicObjectId = new BatchInputPublicObjectId(['inputs' => [{"id":"37295"}]]);

I might begin with the cURL documentation instead, and try either the Guzzle PHP library, or the Symfony HTTPClient

[LosD]

@atanasiuk-hubspot If OpenApi Generator isn't able to produce something that looks and behaves like a sane SDK, why were you using it in the first place?

[obj63mc]

My main question for this sdk is when will 'all' the v3/api endpoints be actually supported. Right now things like adding a contact to a email subscription is not possible with this sdk. Your own examples on hubspot show to just use curl instead. If I was going to just use curl commands then why would we even use this SDK?

https://developers.hubspot.com/docs/api/marketing-api/subscriptions-preferences

Basically looking for a roadmap as right now I essentially have to include both this sdk as well as the old php sdk just to use the api.

[Synchro]

I've found the HubSpot API docs to be absolutely terrible – they might be complete, but they are unusable and cryptic. The code examples they generate make little sense, and mostly don't work. Similarly, I was really disappointed by the HubSpot PHP client libs for being similarly incomprehensible. I then found https://github.com/stechstudio/laravel-hubspot which is exactly what I want an API client lib to look like, I hope others might find that useful.

[moazam1]

@Synchro totally agreed. I copied demo code to create a table in hubdb but it won't work, however I was able to retrieve all tables.

[jakeandreoli]

Trying to do batch updates was an absolute nightmare today. Reading the code provided no insight, there are no examples on the internet and the docs were extremely cryptic.

It wasn't until I read this thread and saw someone mentioned HubSpot's API docs have examples tucked away that I made any progress.

I would like to see the docs for this improved upon somehow.

[MickeCast]

OpenApi Generator

Installed the api client with composer, all seem to be fine, but any of the samples in the documentations works, it always trows errors related to not finding a file inside the api client. Leaving this comment to receive updates.

[compwright]

Installed the api client with composer, all seem to be fine, but any of the samples in the documentations works, it always trows errors related to not finding a file inside the api client.

I'm getting this too:

Error: Class 'Hubspot\Client\Crm\Companies\Model\PublicObjectSearchRequest' not found in file src/Client/Hubspot/HubspotSearchRequestFactory.php on line 29