Skip to content

Instantly share code, notes, and snippets.

@atroche
Created June 5, 2015 06:55
Show Gist options
  • Save atroche/8c29908a322bc619ffbf to your computer and use it in GitHub Desktop.
Save atroche/8c29908a322bc619ffbf to your computer and use it in GitHub Desktop.
PHP client blog post
# Giving our PHP client some love
At Zendesk we're principally a Ruby and JavaScript shop. Our data team in Melbourne uses Clojure, and some of the infrastructure team in Dublin use Go, but everyone else sticks to those languages. And no one wants to do PHP.
Which is a problem, because the world uses PHP. A lot. And they want to have their Magento stores and their WordPress plugins interact with Zendesk.
We have a PHP API client, but it was written by an external contractor years ago, and hasn't been actively maintained since then. Bugs and new features have been added on in a slapdash fashion since then, and it shows — we've had feedback from partners that it's hard to use, and hard to contribute to.
This sucks. Real programmers are polyglot programmers. Using a different language is just a design constraint, and as everyone knows, constraints lead to creativity.
I've never touched PHP in my life (just scoffed for afar), but this week I flew over to work with the engineering team in Manila (so far just two awesomely talented Filipinos, Jose and Mio) to turn the PHP client from something the company is embarrassed about to something we're proud of.
This was our process.
## Designing the interface
We practiced README-driven development. This made us face this question up front: When we tell someone how to use our client, how annoying does it sound?
So we looked at common use cases, and thought about the ideal way we'd solve those problems using a PHP library. For example, when you want to make a comment on a ticket, we wanted it to look like this:
`$zendesk->tickets()->find($yourTicketId)->comments->create(["body" => "this is a reply!"])`
We also took a look at the most popular API clients out there at the moment (e.g. Stripe and Mailchimp) to get a feel for what their interfaces are like, because it's important that we don't break existing patterns in the ecosystem.
## Deciding whether to rewrite
It's a situation no one likes to be in: an old, poorly tested, creaky codebase, where the original authors are long gone. Rewriting is always the most seductive choice — no one likes to read other people's code, let alone edit it. But it's also a big risk — we wanted to start shipping useful code to people THIS WEEK, not after we'd rewritten every feature of the client. So we decided to resist temptation and gradually, iteratively refactor the codebase.
It's worked well — a big plus has been that because we've been forced to deeply understand the current implementation, we learnt from the mistakes made by the original implementors. Those are mistakes we might have made again if we had done things from scratch.
## Making our test suite trustworthy
As everyone knows, you can't refactor code that isn't well-tested. Because how do you know whether or not your refactor has broken functionality? So we started by looking through the existing test suite.
It consisted of a few token unit tests, and then a massive collection of “live tests”, AKA ones that were making real live API calls to our production servers. These were incredibly slow and flaky, and a lot of them didn't even work.
So our first coding challenge on this project was converting the live tests to use request mocking. Our tests became cleaner, simpler and multiple orders of magnitude faster. Here's an example of what they look like:
```
$ticketIds = [$this->testTicket['id'], $this->testTicket2['id']];
$this->mockApiCall(
"PUT",
"tickets/update_many.json",
[
"queryParams" => ["ids" => implode(",", $ticketIds)],
"bodyParams" => ["ticket" => ["status" => "solved"]]
]
);
$this->client->tickets()->updateMany(
[
"ids" => $ticketIds,
"status" => "solved"
]
);
$this->httpMock->verify();
```
## Abstracting boilerplate
We used CodeClimate and PHP code sniffer to identify duplication. That duplication was making refactoring tedious, so it was an early win to get rid of it. Many of the “resources” on the API (e.g. tickets, comments, users) have a shared set of basic CRUD operations.
We were able to pull those out into a base class, which allowed us to kill of five methods on each of the tens of different resource classes. Big wins, early on! Not only did we get to kill redundant code (and improve our Code Climate GPA!), but now we could kill a lot of tests and just test the base operations on their own.
After that, we started pulling out other bits of shared functionality like side-loading and bulk operations into traits. It was around this time that I realised that the new versions of PHP really aren't that bad — especially their OO model. I'd even go so far as to call it sensible. There are still warts, but the main thing I'm missing from Ruby is the syntactic sugar, but PHPStorm is able to write a lot of the boilerplate for me anyway.
## Using a real HTTP library
The existing client wrapped Curl directly. The code was ugly, ugly, ugly, and it meant that all our exception handling was done manually (and badly — this was a common complaint by users). There's an awesome HTTP client out there called Guzzle, which is extremely well-designed, and actively maintained.
Because we now had a test suite we trusted and could run quickly, it didn't take long to swap out the parts of the code that were making requests to the API. They were mostly contained in a couple of files.
Using Guzzle meant that we got back sensible, rich exceptions that mapped well to the domain of HTTP, which is a lot better than what was there before — you used to get back a generic exception that said “uh, something went wrong with the request” and has no more information. Now you can handle it, and get back error codes and the messages from the server, etc.
One of the next wins for the client will be creating our own exceptions that map more closely to the kind of errors Zendesk might throw back at you — e.g. `TicketAlreadyClosedException`. Building this on top of Guzzle will be a whole lot nicer.
## Allowing chaining
So we had that fake README full of examples that we wanted our client to allow, examples like this one:
`$zendesk->tickets()->find($yourTicketId)->comments->findAll()
Even though this looks like multiple operations, behind the scenes it's make a single API call to a our [ticket comments endpoint](https://developer.zendesk.com/rest_api/docs/core/ticket_comments#list-comments), which looks a little something like this: `https://subdomain.zendesk.com/api/v2/tickets/{ticket_id}/comments.json`. After a couple of whiteboarding sessions, Mio and Jose came up with a solution that a) didn't require too much magic metaprogramming and b) is reusable for any resource that has nested resources under it (in fact, it's pulled out into a trait and unit tested on its own!).
## Planning for the future
Our work is up on a `v2` branch, and we'll be ready to merge once we cover more of the Zendesk API's edge cases (the ones not covered by our base classes and traits). We'd love it if you tried it out right now and gave us feedback — this is an open source project, and one of our chief goals with this work was to make it easy for everyone who uses this client to contribute.
Zendesk Engineering has a high standard of code quality, and there's still plenty of places where we can pull out reusable classes, traits and methods. We're committed to keeping our Code Climate score up, our code style consistent and our Travis builds passing.
Some of the examples in our README aren't done yet — they're mainly syntactic sugar and helper methods built on top of the lower level interface that will make certain common use cases extremely easy to write.
We're going to flesh out documentation and a contributors guide, add a Gitter channel for the repo, and make sure that we get big flashing notifications when someone leaves an issue or makes a pull request.
Now that we've built a steady foundation of strong tests and useful abstractions, work on this client is going to get even faster — I'm excited to see what Jose and Mio are able to produce in the next few weeks of focused work, and how they'll help create a community of engaged users and contributors.
## Key lessons
* Treating awful legacy code as a challenging puzzle to solve is a great way to stay motivated and do good work
* Taking a whole week to focus on paying down technical debt is necessary sometimes when the fundamentals of a project are badly broken
* Knowing that our short sprint of work is going to make lots of developers lives' nicer is incredibly motivating
* PHP isn't that bad anymore, and there are good patterns, tools and libraries in its ecosystem
* Manila is the most hospitable of all of Zendesk's many outposts around the world =)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment