Thoughts and implementation on that daft FizzBuzz interview question

G'day:
I'm not quite sure where I'm gonna go with this article yet. Usually when I start an article like this it ends up being a bit rubbish, so... well sorry if it turns out this way this time too.

Right so there was a conversation on the CFML Slack channel last week stemming from someone deciding to use the old FizzBuzz question as part of a job interview. I'm not a great proponent of such things in interviews, but then back when I used to conduct such things I was shit at doing 'em anyhow, so what do I know?

But I don't think it's a sensible measure of anything to make someone code something in the actual interview: interviews can be high-stress environments, with restricted time-framing, and I don't think that reflects the general programming environment. Well not one I'd want to work in, anyhow. I've once been asked to design a database - in an interview for a developer role, I hasten to add - I just said "no, I'm not going to do that; I don't think that's a good use of anyone's time, and it casts you in a bad light for asking, and me in a bad light as I fumble around trying to do something that might normally take hours in only a few min. So: no" (that's a paraphrase, but it is the general gist of how the conversation went). I think perhaps had I actually wanted the job I might have humoured them. Maybe. It's dumb-arse anyhow.

Fortunately in this fizzbuzz situation the person conducting the interview was like-minded, and told the candidate to take it home with them and flick something back later. That's reasonable.

But what should one look for in a situation like this? And what should one deliver as a solution?

Firstly... in case you are like me and had no idea what people were on about when they started talking about FizzBuzz, this is the question:

Write a program that prints the numbers from 1 to 100. But for multiples of three print "Fizz" instead of the number and for the multiples of five print "Buzz". For numbers which are multiples of both three and five print "FizzBuzz".

(from http://wiki.c2.com/?FizzBuzzTest)

The Slack channel got to messing around, and as one can probably imagine, shortly people starting answering the question. A typical answer might be like this:

for ($i=1; $i <=100; $i++) {
    $s = "";
    if ($i % 3 == 0) $s .= "fizz";
    if ($i % 5 == 0) $s .= "buzz";
    
    echo $s ?: $i;
}

That's simple and to the point, but I would not be impressed with that answer. Why? It's not reusable at all. OK, cool, the question didn't ask for re-use. That being the case, why bother with the code? Just use the output:

echo "12fizz4buzzfizz78fizzbuzz[etc]buzz";

If yer not needing a reusable answer... don't piss about writing code to do it. Just answer it. If I was in the position to discuss this with the interviewer - rather than just submit the answer - I would indeed consider starting with that answer there. Not to be a smart arse, but to let them know I think about that sort of thing.

But I'd then go on to point out some flaws in the question and in the answer. I think it's reasonable to provide slightly reusable code, not least of all so one can test it. One could test a file that simply has that statement in it, but it's not so orthodox to do so.

I put the answer in a function:

function getFirst100FizzBuzzNumbers(){
    return "12fizz4buzzfizz78fizzbuzz[etc]buzz";
}

Then quickly realise that's still not very testable as testing frameworks usually expect code to be in classes. So I decide to do that: put it in a class.

However another thing I'd be wanting to do in this exercise is to demonstrate "test first" as a developmental approach. For this I just decided to use PHPSpec for a change today, as I heard someone mention it and thought "yeah, all right...". To get that strummed-up I need a composer.json file:

{
    "name" : "fizzBuzzChallenge",
    "require-dev" : {
        "phpspec/phpspec" : "^3.0"
    },
    "config": {
        "bin-dir": "bin"
    },
    "autoload": {
        "psr-4" : {
            "me\\adamcameron\\fizzBuzzChallenge\\" : "src/"
        }
    }
}

(some of that's from the PHPSpec docs, the rest is just "well I'll need to put the code somewhere, so might as well PSR-4 it and namespace it etc too).

Following the PHPSpec docs still, I start by describing my spec:

C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec desc me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
Specification for me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle created in C:\src\php\php.local\src\puzzle\fizzbuzz\spec\FizzBuzzPuzzleSpec.php.


C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  11  - it is initializable
      class me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle does not exist.

                                      100%                                       1
1 specs
1 example (1 broken)
25ms


  Do you want me to create `me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle`
  for you?
                                                                         [Y/n]

Class me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle created in C:\src\php\php.local\src\puzzle\fizzbuzz\src\FizzBuzzPuzzle.php.

                                      100%                                       1
1 specs
1 example (1 passed)
27ms


C:\src\php\php.local\src\puzzle\fizzbuzz>

So that's all quite cool. Notice that it created the spec class for me, and it also created the actual class for me too.

Now I have this:

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzPuzzleSpec extends ObjectBehavior
{
    function it_is_initializable()
    {
        $this->shouldHaveType(FizzBuzzPuzzle::class);
    }
}

namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzPuzzle
{
}

Now I need to spec-out my function (test first remember!):

class FizzBuzzPuzzleSpec extends ObjectBehavior {

    function its_getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue() {
        $this->getFirst100FizzBuzzNumbers()->shouldBe("12fizz4buzzfizz78fizzbuzz[etc]buzz");
    }
}

(It's not part of my requirement to spec-out the return type of the class, so I got rid of that one).

C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  10  - its getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue
      method me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers not found.

                                      100%                                       1
1 specs
1 example (1 broken)
227ms


  Do you want me to create
  `me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers
  ()` for you?
                                                                         [Y/n]

  Method me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers() has been created.

me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  10  - its getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue
      expected "12fizz4buzzfizz78fizzbuzz...", but got null.

                                      100%                                       1
1 specs
1 example (1 failed)
29ms


C:\src\php\php.local\src\puzzle\fizzbuzz>

Now I have the method skeleton as well:

public function getFirst100FizzBuzzNumbers()
{
    // TODO: write logic here
}

Which I replace with my implementation:

function getFirst100FizzBuzzNumbers(){
    return "12fizz4buzzfizz78fizzbuzz[etc]buzz";
}

Which works:

C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
                                      100%                                       1
1 specs
1 example (1 passed)
31ms


C:\src\php\php.local\src\puzzle\fizzbuzz>

However it's perhaps realistic to expect this thing to actually do the work dynamically instead of just returning a hard-coded string. So I shift my specification slightly to be "a function which takes an integer i and returns the first i FizzBuzz numbers.

The simplest spec for this is to just test that it returns a FizzBuzz number. The first FizzBuzz number is 1:

function its_getFizzBuzzNumbersMethodReturnsAFizzBuzzNumber() {
    $this->getFizzBuzzNumbers(1)->shouldBe(1);
}

Again I run the tests, and again it creates my method skeleton for me:

public function getFizzBuzzNumbers($argument1)
{
    // TODO: write logic here
}

Which I replace with enough code to pass.

public function getFizzBuzzNumbers($i) {
    return $i;
}

Now I'm intending to write enough of the algorithm to get me to 3, which is the first variation of the return value. I'm also gonna change the specification to return an array not a string. This is a nod to not assuming the use of the result, and an array is a better data type for a sequence of things. Coupled with this, I want to separate out the collection (the array) from the contents of the collection (FizzBuzz numbers). So I decide to spec a different function for now: getFizzBuzzNumber.

I create a spec:

function its_getFizzBuzzNumberMethodReturnsAFizzBuzzNumber() {
    $this->getFizzBuzzNumber(1)->shouldMatch('/\d+|fizz/');
}

And create a naive implementation of that:

public function getFizzBuzzNumber($i) {
    return $i;
}

And test the spec:

  16  - its getFizzBuzzNumberMethodReturnsAFizzBuzzNumber
      no match([array:1]) matcher found for [integer:1].

Erm... damn. What? This took a while to work out. PHPSpec seems to have forgotten that PHP is a dynamic, loosely-typed language, and seems to type-check the value before testing the pattern. If I returned a string, this spec worked:

public function getFizzBuzzNumber($i) {
    return (string) $i;
}

1 specs
1 example (1 passed)

Now I almost lived with that. I almost lived with returning the numeric FizzBuzz numbers as strings to make my pattern work. But that seemed arse-about-face. The spec was getting it "wrong", not my code. I chatted on the BDDLondon Slack Channel about how to test for "this OR that" (if not able to use a regex pattern"), and the answer was "cannae". This also had me thinking "I actually really hate methods that return two different types of thing, as it seems a bit messy. Then it occurred to me... it's not returning two different types of thing. It was returning one type of thing: it was returning a FizzBuzzNumber object. Aha! I was not after a collection of a mix of integers and strings "fizz", "buzz" or "fizzbuzz". They were all just FizzBuzzNumber objects. Cool.

So I spec-ed out a second class: FizzBuzzNumber. I'll spare you the stepped spec->code->repeat, and here's the end result. Spec first:

<?php

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzNumber;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzNumberSpec extends ObjectBehavior
{
    function it_isInitializable() {
        $this->beConstructedWith(1);
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithFizz() {
        $this->beConstructedWith("Fizz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithBuzz() {
        $this->beConstructedWith("Buzz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithFizzBuzz() {
        $this->beConstructedWith("FizzBuzz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithOtherInteger() {
        $this->beConstructedWith(2);
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isNotInitializableWithMultipleOfThree() {
        $this->beConstructedWith(6);
        $this->shouldThrow('\InvalidArgumentException')->duringInstantiation();
    }

    function it_isNotInitializableWithMultipleOfFive() {
        $this->beConstructedWith(10);
        $this->shouldThrow('\InvalidArgumentException')->duringInstantiation();
    }

    function it_returnsFizzWhenCreatingFromIntegerWithMultipleOfThree() {
        $this->beConstructedWith(1);
        $this::fromInteger(12)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(12)->__toString()->shouldBe("fizz");
    }

    function it_returnsBuzzWhenCreatingFromIntegerWithMultipleOfFive() {
        $this->beConstructedWith(1);
        $this::fromInteger(20)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(20)->__toString()->shouldBe("buzz");
    }

    function it_returnsFizzBuzzWhenCreatingFromIntegerWithMultipleOfFifteen() {
        $this->beConstructedWith(1);
        $this::fromInteger(30)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(30)->__toString()->shouldBe("fizzbuzz");
    }

    function it_returnsAStringFromToString() {
        $this->beConstructedWith(1);
        $this->__toString()->shouldBeString();
    }
}

(I'm writing this part of this article about a week later, and I still can't stand seeing those underscores in my method names!)

But I like what PHPSpec has made me do here: define my FizzBuzzNumber's behaviour:

• it can be constructed with a value of 1 (a fairly baseline "it works");
• or a value of "fizz"
• or "buzz";
• or "fizzbuzz";
• or some other integer not matching those rules;
• but not an integer multiple of three (because there's no such FizzBuzzNumber... that'd be "Fizz");
• Likewise with a multiple of five
• (multiple of 15 is implicit from those two).
• However its fromInteger method can create an object from multiples of 3
• or 5.
• And if required to do so, returns a string value of itself (like for output).

That's a reasonable description (ie: specification) of a FizzBuzzNumber, I reckon?

The implementation is pretty simple:

<?php

namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzNumber {

    private $value;

    function __construct($value) {
        if (!preg_match('/\d+|fizz|buzz|fizzbuzz/', $value)) {
            throw new \InvalidArgumentException("$value is not a valid FizzBuzzNumber");
        }
        if (is_integer($value) && !($value % 3 && $value % 5)){
            throw new \InvalidArgumentException("$value is not a valid FizzBuzzNumber");
        }

        $this->value = $value;
    }

    static function fromInteger($i) : FizzBuzzNumber {
        $result = "";

        if ($i % 3 == 0) {
            $result .= "fizz";
        }
        if ($i % 5 == 0) {
            $result .= "buzz";
        }
        if (empty($result)) {
            $result = $i;
        }

        return new self($result);
    }

    function __toString() {
        return (string) $this->value;
    }
}

Notes:

• The constructor does nothing clever. It takes a valid FizzBuzzNumber value and rejects values that aren't valid. EG: 15 is not a valid FizzBuzzNumber: there's no such thing. The integer 15 might exist, but the FizzBuzzNumber equivalent is "fizzbuzz".
• The fromInteger method now holds all the logic of converting an integer to a FizzBuzzNumber, which is pretty much what we've been asked for: take the integers 1-100 and return their FizzBuzzNumbers.
• We want to treat FizzBuzzNumbers just like we would an integer etc. So if we output it, we just spit out its string value.

Right, so that's our numbers sorted out. Now we need a collection of them. This is pretty devoid of requirement now given the "fizz buzz" logic is within FizzBuzzNumber, so I made an arbitrary decision to take a slightly more complex route than perhaps I could have. I've decided to use a generator rather than just return the series at once. Why? This is for a job interview right, so it's perhaps good to demonstrate one's understanding of the language extends further than the basics. Also - as a discussion point - it's perhaps worthwhile to observe that this "fizz buzz" exercise is nothing more than a very naive Sieve of Eratosthenes, and that kind of lends itself to be handled via an "on demand" collection, a generator makes sense. But it's mostly one of those "because I can" sort of things. Plus it's dead easy in PHP.

Again, spec first:

<?php

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzNumber;
use me\adamcameron\fizzBuzzChallenge\FizzBuzzSequence;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzSequenceSpec extends ObjectBehavior {

    function its_class_IsInitializable() {
        $this->shouldHaveType(FizzBuzzSequence::class);
    }

    function it_returnsaGeneratorfromGetFizzBuzzSequence() {
        $this->getFizzBuzzSequence()->shouldHaveType(\Generator::class);
    }

    function it_fulfilsTheRequirement(){
        $lengthToTest = 100;
        $this->beConstructedWith($lengthToTest);

        $expected = array_map(function($i){
            return FizzBuzzNumber::fromInteger($i);
        },range(1, $lengthToTest));

        $this->getFizzBuzzSequence()->shouldIterateAs($expected);
    }
}

Notes:

• This tests that is is a generator.
• Plus here is the test for the actual requirement! Yup. I don't output it on the screen and ask the examiner to eyeball it manually: I provide a test for them so they can see that the computer has verified my work.

And the actual code is thus:

<?php

namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzSequence {

    private $length;

    public function __construct($length=null) {
        $this->length = $length;
    }

    public function getFizzBuzzSequence() {
        $i = 0;
        while (is_null($this->length) || $i < $this->length){
            $i++;
            yield FizzBuzzNumber::fromInteger(($i));
        }
    }

    public function __invoke() {
        return $this->getFizzBuzzSequence();
    }
}

Simple, huh? OK, granted, not as simple as just a foreach loop, but it's not that much more complicated. The only twist is that I've just added an __invoke() method so that the object can be called as a method. Syntactical sugar.

OK, so if yer like me having that test there that proves the thing does the business ain't good enough. I also wrote a "manual test" to demonstrate it:

$seq = new FizzBuzzSequence(100);

foreach ($seq() as $fizzBuzzNumber) {
    echo $fizzBuzzNumber . PHP_EOL;
}

So we still get our simple foreach anyhow. But the implementation is hidden away in suitable classes.

And yeah it works:

C:\src\php\php.local\src\puzzle\fizzbuzz\public>php manualTest.php
1
2
fizz
4
buzz
fizz
7
8
fizz
buzz
11
fizz
13
14
fizzbuzz
16
17
fizz
19
buzz
fizz
22
23
fizz
buzz
26
fizz
28
29
fizzbuzz
31
32
fizz
34
buzz
fizz
37
38
fizz
buzz
41
fizz
43
44
fizzbuzz
46
47
fizz
49
buzz
fizz
52
53
fizz
buzz
56
fizz
58
59
fizzbuzz
61
62
fizz
64
buzz
fizz
67
68
fizz
buzz
71
fizz
73
74
fizzbuzz
76
77
fizz
79
buzz
fizz
82
83
fizz
buzz
86
fizz
88
89
fizzbuzz
91
92
fizz
94
buzz
fizz
97
98
fizz
buzz

C:\src\php\php.local\src\puzzle\fizzbuzz\public>

This is perhaps an overly-belaboured design of something simple, but I still think the design is sound, simple, and well-tested. I think it's an easy amount of work, and demonstrates a few sensible processes that are just as important to a developer job than actual code:

• be able to read the requirement, understand it, and deliver what's asked for (the hard-coded answer does this). This demonstrates one is paying attention and notices things.
• But question the requirement in a sensible way: is it just a one use thing? Is "100" cast in stone, or is it a parameter? This demonstrates one can then offer insight into where the client might have overlooked something, or "under-stated" their requirements due to using casual "human speak" when saying things like "I need the first 100 FizzBuzzNumbers". Questioning is always part of one's job.
• Any work one does should be spec'ed out (but only in a "just in time" fashion, not some horrendous waterfall). Know what yer doing before you set out to do it. It also helps collaboration with other devs, as it summarised things for them.
• and tested. All our work needs to be tested.
• Once one has teased-out the requirements from the initial statement, then one can crack on with the implementation, and piss around being "smart". Provided one can explain one's self.

That's the end of "my point" with this article.

---

As a digression though... obviously with these exercises "seasoned" devs might be wont to show-off how clever they are by making the solution as terse as they can. As if that's a good thing. It's not. But the conversation erred that way on the CFML Slack channel, and I rose to the bait.

Here's my foolishly terse solution to this, in a challenge I set for myself to implement the solution as a single expression in as fewer chars as possible:

[].set(1,100,"").reduce((s,_,i)=>s&(!i%15?"fizzbuzz":!i%3?"fizz":!i%5?"buzz":i),"")

(that only works on Lucee 5, btw. It will not work on ColdFusion)

That's absolutely ludicrous, and if anyone actually submitted that as an answer to the question they should be shot (yes, fine, a harsh penalty for failing a job interview).

But I was quite pleased that CFML can express the solution in one 84-char expression. I thought about trying to implement the same thing in PHP, but it simply can't do this sort of thing. Not being very OO and not doing a very good nod to FP doesn't help. CFML's abilities are def underrated here. In this example of "how to write shit code that'd get you shot".

Righto.

--
Adam

PHP: decorating async GuzzleHttp calls - more methods

G'day:
Here's yet another article about decorating an async GuzzleHttp adapter. So far in the series I've got this lot:

• PHP: decorating async GuzzleHttp calls
• PHP: decorating async GuzzleHttp calls - handling exceptions
• PHP: decorating async GuzzleHttp calls - handling exceptions a different way
• PHP: decorating async GuzzleHttp calls - caching
• PHP: decorating async GuzzleHttp calls - multiple decorators

To date my adapter has only had the one method to decorate: the get method. I also need to add more methods like post and put, which have their own considerations when doing decoration. Let's have a look.

Firstly I need some end points to test against. I decided I'd better shuffle-on from those CFML based ones I had, and use PHP instead. So I've rejigged them all as follows:

data.php

class Person{
    public $id;
    public $firstName;
    public $lastName;

    function __construct($id, $firstName, $lastName){
        $this->id = $id;
        $this->firstName = $firstName;
        $this->lastName = $lastName;
    }
}

$people = [
    new Person(1, "Donald", "Duck"),
    new Person(2, "Donald", "Cameron"),
    new Person(3, "Donald", "Trump"),
    new Person(4, "Donald", "Wearsyatroosas")
];

getById.php

$id = $_GET["id"];

require __DIR__ . "/data.php";

$filteredPeople = array_filter($people, function($person) use ($id) {
    return $person->id == $id;
});
$person = array_shift($filteredPeople);

$result = (array) $person;

sleep(5);
$result["retrieved"] = (new DateTime())->format("Y-m-d H:i:s");

header("type:application/json");
echo json_encode($result);

create.php

require __DIR__ . "/data.php";

$newPerson = new Person(
    uniqid(),
    $_POST['firstName'],
    $_POST['lastName']
);

$result = (array) $newPerson;

$result["retrieved"] = (new DateTime())->format("Y-m-d H:i:s");

header("type:application/json");
echo json_encode($result);

update.php

$id = $_GET["id"];
parse_str(file_get_contents("php://input"),$put_vars);

require __DIR__ . "/data.php";

$filteredPeople = array_filter($people, function($person) use ($id) {
    return $person->id == $id;
});

if (empty($filteredPeople)){
    http_response_code(400);
    throw new InvalidArgumentException("No Person with ID $id found");
}

$person = array_shift($filteredPeople);

$newPerson = new Person(
    $person->id,
    $put_vars['firstName'],
    $put_vars['lastName']
);

$result = [
    'before' => (array) $person,
    'after' => (array) $newPerson,
    'updated' => (new DateTime())->format("Y-m-d H:i:s")
];

header("type:application/json");
echo json_encode($result);

That lot is all pretty perfunctory. There was a coupla PHP things I didn't know about before hand, namely:

• how to extract the body for a put request. I was initially expecting them to be in the $_POST array for some reason, but that makes no sense.
• how to set the HTTP status. I'd never had to do that before with PHP (we use Silex / Symfony, and we just use API methods to do same).

(readers need to remember I'm fairly new to PHP ;-)

The next update is to the Adapter interface:

namespace me\adamcameron\testApp\adapter;

use GuzzleHttp\Promise\Promise;

interface Adapter {
    public function get($url, $parameters) : Promise;
    public function post($url, $body) : Promise;
    public function put($url, $body, $parameters) : Promise;
}

I've added the two new methods there. This now breaks all my Adapter implementations.

First I need to add the actual method implementations to the base adapter: the ones that actually makes the calls:

namespace me\adamcameron\testApp\adapter;

use GuzzleHttp\Client;
use GuzzleHttp\Promise\Promise;

class GuzzleAdapter implements Adapter {

    private $client;

    public function __construct() {
        $this->client = new Client();
    }

    public function get($url, $parameters) : Promise {
        $fullUrl = sprintf("%s?%s", $url, http_build_query($parameters));

        $response = $this->client->requestAsync("get", $fullUrl);

        return $response;
    }

    public function post($url, $body) : Promise {
        $options = ['form_params' => $body];

        $response = $this->client->requestAsync("post", $url, $options);

        return $response;
    }

    public function put($url, $body, $parameters) : Promise {
        $fullUrl = sprintf("%s?%s", $url, http_build_query($parameters));
        $options = ['form_params' => $body];

        $response = $this->client->requestAsync("put", $fullUrl, $options);

        return $response;
    }
}

Nothing surprising there. Note that each of the new methods have different argument requirements. This becomes relevant later.

Now I need to add equivalent methods to the decorators. I'll start with the caching one as that's easy: we have no caching requirements for POST or PUT requests, so the methods just pass through to the underlying adapter:

public function post($url, $body) : Promise {
    return $this->adapter->post($url, $body);
}

public function put($url, $body, $parameters) : Promise {
    return $this->adapter->put($url, $body, $parameters);
}

See: easy.

The logging adapter was more of a challenge. The first pass of the implementation looked like this sort of thing:

public function get($url, $parameters) : Promise {
    $encodedParameters = json_encode($parameters);
    $this->logger->logMessage(sprintf("%s: Requesting for %s", $this->thisFile, $encodedParameters));

    $response = $this->adapter->get($url, $parameters);

    $response->then(function($response) use ($encodedParameters) {
        $body = $response->getBody();
        $this->logger->logMessage(sprintf("%s: Response for %s: %s", $this->thisFile, $encodedParameters, $body));
        $body->rewind();
    });

    return $response;
}

public function post($url, $body) : Promise {
    $logDetails = json_encode($body);
    $this->logger->logMessage(sprintf("%s: Requesting for %s", $this->thisFile, $logDetails));

    $response = $this->adapter->post($url, $body);

    $response->then(function($response) use ($logDetails) {
        $body = $response->getBody();
        $this->logger->logMessage(sprintf("%s: Response for %s: %s", $this->thisFile, $logDetails, $body));
        $body->rewind();
    });

    return $response;
}

public function post($url, $body, $parameters) : Promise {
    $logDetails = json_encode([
        'parameters' => $parameters,
        'body' => $body
    ]);
    $this->logger->logMessage(sprintf("%s: Requesting for %s", $this->thisFile, $logDetails));

    $response = $this->adapter->put($url, $body, $parameters);

    $response->then(function($response) use ($logDetails) {
        $body = $response->getBody();
        $this->logger->logMessage(sprintf("%s: Response for %s: %s", $this->thisFile, $logDetails, $body));
        $body->rewind();
    });

    return $response;
}

That's fine, but look at how much similarity there is in there:

• We log some stuff
• We peform a request to a method
• We have a resolution handler

What we log is different for all three methods, but the handling after that is the same. So I figured there was a refactoring opportunity there. One slight challenge is that which I mentioned above: each method passes different arguments. That'll be tricky to refactor into one method. Well so I thought. Enter PHP's variadic functions. These allow me to specify a method signature that has a concrete argument list, then one last argument that captures any other arguments passed into the function when it's called.

This way I have three congruent parts:

• capture some stuff to log. The composition of what to log is method-specific, but that's fine
• call a method
• with some arguments

I was able to factor-out this commonality into a helper method:

private function performLoggedRequest($method, $logDetails, ...$requestArgs) : Promise {
    $this->logger->logMessage(sprintf("%s: Requesting for %s", $this->thisFile,  $logDetails));

    $response = call_user_func_array([$this->adapter, $method], $requestArgs);

    $response->then(function($response) use ($logDetails) {
        $body = $response->getBody();
        $this->logger->logMessage(sprintf("%s: Response for %s: %s", $this->thisFile, $logDetails, $body));
        $body->rewind();
    });

    return $response;
}

From there, it's just a matter of refactoring all three public methods to call this helper method:

public function get($url, $parameters) : Promise {
    $logDetails = json_encode($parameters);

    return $this->performLoggedRequest(__FUNCTION__, $logDetails, $url, $parameters);
}

public function post($url, $body) : Promise {
    $logDetails = json_encode($body);

    return $this->performLoggedRequest(__FUNCTION__, $logDetails, $url, $body);
}

public function put($url, $body, $parameters) : Promise {
    $logDetails = json_encode([
        'parameters' => $parameters,
        'body' => $body
    ]);

    return $this->performLoggedRequest(__FUNCTION__, $logDetails, $url, $body, $parameters);
}

For each call to performLoggedRequest, all the passed-in arguments after $logDetails end up in a single array, as $requestArgs All because of the ... in the method signature. Cool.

This enables use to call call_user_func_array. with both a dynamic method name, and a dynamic number of arguments in that array.

I then needed to do much the same with the StatusToExceptionAdapter:

public function get($url, $parameters) : Promise {
    return $this->request(__FUNCTION__, $url, $parameters);
}

public function post($url, $body) : Promise {
    return $this->request(__FUNCTION__, $url, $body);
}

public function put($url, $parameters, $body) : Promise {
    return $this->request(__FUNCTION__, $url, $parameters, $body);
}

private function request($method, ...$args) : Promise {
    return call_user_func_array([$this->adapter, $method], $args)
        ->then(function (Response $response) {
            return $this->handleThen($response);
        })
        ->otherwise(function ($exception) {
            $this->handleOtherwise($exception);
        });
}

Easy!

Finally a test. I used the same basic test rig as last time, so I'll just focus on the individual tests here. I also ran the equivalent test of getById as before: no change there so I'll not bother reproducing it.

The test for post was as follows:

function testCreate(PersonRepository $personRepository, LoggingService $loggingService) {
    $personDetails = [
        'firstName' => 'Donald',
        'lastName' => 'McLean'
    ];

    $loggingService->logMessage(sprintf("Test: calling create(%s)", json_encode($personDetails)));

    $response = $personRepository->create($personDetails);
    $body = (string) $response->wait()->getBody();

    $loggingService->logMessage(sprintf("Test: called create(): [%s]", $body));
}

That's all obvious. The log output for this was:

[2016-11-24 20:04:11] testPost.INFO: Test: calling create({"firstName":"Donald","lastName":"McLean"}) [] []
[2016-11-24 20:04:11] testPost.INFO: LoggingAdapter: Requesting for {"firstName":"Donald","lastName":"McLean"} [] []
[2016-11-24 20:04:11] testPost.INFO: LoggingAdapter: Response for {"firstName":"Donald","lastName":"McLean"}: {"id":"583747bb1a6d9","firstName":"Donald","lastName":"McLean","retrieved":"2016-11-24 20:04:11"} [] []
[2016-11-24 20:04:11] testPost.INFO: StatusToExceptionAdapter: non-exception status encountered: 200 [] []
[2016-11-24 20:04:11] testPost.INFO: Test: called create(): [{"id":"583747bb1a6d9","firstName":"Donald","lastName":"McLean","retrieved":"2016-11-24 20:04:11"}] [] []

We can see no caching was called (good), but the approrpiate logging and StatusToException checks were made. And we got a new record ID back after the insert. Excellent.

The test of the update had two things to do: doing an update of an existing record (happy path) then trying to update a non-existent record (which should yield an exception):

function testUpdate(PersonRepository $personRepository, LoggingService $loggingService) {
    testUpdateByPhase("Valid ID", 3, $personRepository, $loggingService);
    testUpdateByPhase("Invalid ID", -1, $personRepository, $loggingService);
}

function testUpdateByPhase($phase, $id, PersonRepository $personRepository, LoggingService $loggingService) {
    $personDetails = [
        'firstName' => 'Donald',
        'lastName' => 'Corleone'
    ];

    $logDetails = [
        'id' => $id,
        'details' => $personDetails
    ];

    $loggingService->logMessage(sprintf("Test (phase: %s): calling update(%s)", $phase, json_encode($logDetails)));

    $response = $personRepository->update($id, $personDetails);
    try {
        $body = (string) $response->wait()->getBody();
        $loggingService->logMessage(sprintf("Test (phase %s): called update(): [%s]", $phase, $body));
    } catch (Exception $e) {
        $previous = $e->getPrevious();
        $loggingService->logMessage(
            sprintf(
                "Test (phase %s): call to update failed: "
                . "Class: %s; "
                . "Code: %s; "
                . "Message: %s"
                . "Previous class: %s"
                . "Previous message: %s"
                , $phase, get_class($e), $e->getCode(), $e->getMessage()
                , get_class($previous), $previous->getMessage()
            )
        );
    } finally {
        $loggingService->logMessage(sprintf("Test (phase %s): complete", $phase));
    }
}

That's more code, but most of it is error handling, and there's nothing noteworthy. The log shows the outcome. First the valid update:

[2016-11-24 20:04:11] testPost.INFO: Test (phase: Valid ID): calling update({"id":3,"details":{"firstName":"Donald","lastName":"Corleone"}}) [] []
[2016-11-24 20:04:11] testPost.INFO: LoggingAdapter: Requesting for {"parameters":{"id":3},"body":{"firstName":"Donald","lastName":"Corleone"}} [] []
[2016-11-24 20:04:11] testPost.INFO: LoggingAdapter: Response for {"parameters":{"id":3},"body":{"firstName":"Donald","lastName":"Corleone"}}: {"before":{"id":3,"firstName":"Donald","lastName":"Trump"},"after":{"id":3,"firstName":"Donald","lastName":"Corleone"},"updated":"2016-11-24 20:04:11"} [] []
[2016-11-24 20:04:11] testPost.INFO: StatusToExceptionAdapter: non-exception status encountered: 200 [] []
[2016-11-24 20:04:11] testPost.INFO: Test (phase Valid ID): called update(): [{"before":{"id":3,"firstName":"Donald","lastName":"Trump"},"after":{"id":3,"firstName":"Donald","lastName":"Corleone"},"updated":"2016-11-24 20:04:11"}] [] []
[2016-11-24 20:04:11] testPost.INFO: Test (phase Valid ID): complete [] []

Success!

And the invalid ID:

[2016-11-24 20:04:11] testPost.INFO: LoggingAdapter: Requesting for {"parameters":{"id":-1},"body":{"firstName":"Donald","lastName":"Corleone"}} [] []
[2016-11-24 20:04:11] testPost.INFO: StatusToExceptionAdapter: remapped status encountered: 400 [] []
[2016-11-24 20:04:11] testPost.INFO: Test (phase Invalid ID): call to update failed: Class: me\adamcameron\testApp\exception\BadRequestException; Code: 400; Message: A response of 400 was received from the service Previous class: GuzzleHttp\Exception\ClientExceptionPrevious message: Client error: `PUT http://php.local:8070/experiment/guzzle/update.php?id=-1` resulted in a `400 Bad Request` response: <br /> <font size='1'><table class='xdebug-error xe-uncaught-exception' dir='ltr' border='1' cellspacing='0' cellpadding (truncated...)  [] []
[2016-11-24 20:04:11] testPost.INFO: Test (phase Invalid ID): complete [] []

That worked as expected too. Although I now note I should have returned a 404 there, not a 400. Oops. But it's doing what I told it to do, anyhow. And pleasingly, the StatusToExceptionAdapter did its job.

And that's about it. I learned a few new things whilst investigating this one, which is good.

That's the end of my foray into decorating GuzzleHttp adapters. I'll get onto something different next.

Righto.

--
Adam

PHP: decorating async GuzzleHttp calls - multiple decorators

G'day:
Here's yet another article about decorating an async GuzzleHttp adapter. So far in the series I've got this lot:

• PHP: decorating async GuzzleHttp calls
• PHP: decorating async GuzzleHttp calls - handling exceptions
• PHP: decorating async GuzzleHttp calls - handling exceptions a different way
• PHP: decorating async GuzzleHttp calls - caching

Over the course of those articles, I've created three decorators:

• logging - wraps the async call in "before" and "after" logging;
• exception handling - remapping GuzzleHttp's default exceptions to HTTP-response-status-specific exceptions;
• caching - caching successful responses so future calls for the same info won't need to hit the underlying  web service.

These all mirror real-world requirements I have.

I've tested them all separately, but the theory with decoration is that one should be able to further-decorate an already-decorated object. I've decided to give this a go.

Firstly I've revised my approach somewhat. Previously I was baking my endpoint into the adapter as that was convenient for how i was demonstrating the code. This isn't a good approach as the adapter should just focus on making the calls and handling them, not having a position on the destination of the calls. So I've removed the endpoint from the basic adapter:

use GuzzleHttp\Client;
use GuzzleHttp\Promise\Promise;

class GuzzleAdapter implements Adapter {

    private $client;

    public function __construct() {
        $this->client = new Client();
    }

    public function get($url, $parameters) : Promise {
        $fullUrl = sprintf("%s?%s", $url, http_build_query($parameters));

        $response = $this->client->requestAsync("get", $fullUrl);

        return $response;
    }
}

Now I pass the URL into the call to get, and append the passed-in parameters to make a complete URL. In real life I'd probably also be optionally passing in some headers, and perhaps some GuzzleHttp options, but that's just noise for the purposes of what I'm doing here.

Also note I've gone all PHP7 for a change, and added in some stricter typing. To that end, I'm also enforcing an interface on these classes now:

use GuzzleHttp\Promise\Promise;

interface Adapter {
    public function get($url, $parameters) : Promise;
}

In dynamic languages like PHP and CFML I generally try to shy away from being to forceful about type checking as it doesn't really add much, and sometimes gets in the way. But I think in this situation it makes some sense. And I also want to start getting into the swing of things of writing PHP 7 code too.

I've also changed my approach to calling the adapter somewhat. Now that the endpoint has been moved out of the adapter, I need it to go somewhere else. I don't want to be passing it in with every get call. We use the Repository Pattern at work to wrap up details of external service implementation, so I've run with that here.

use GuzzleHttp\Promise\Promise;
use me\adamcameron\testApp\adapter\Adapter;

class PersonRepository {

    private $adapter;

    private static $baseUrl = "http://cf2016.local:8516/cfml/misc/guzzleTestEndpoints/";
    private static $getByIdUrl = "getById.cfm";
    private static $getByNameUrl = "getByName.cfm";
    private static $returnStatusCodeUrl = "returnStatusCode.cfm";

    public function __construct(Adapter $adapter)
    {
        $this->adapter = $adapter;
    }

    public function getById($id) : Promise {
        $response = $this->adapter->get(
            self::$baseUrl . self::$getByIdUrl,
            ["id" => $id]
        );

        return $response;
    }

    public function getByName($firstName, $lastName) : Promise {
        $response = $this->adapter->get(
            self::$baseUrl . self::$getByNameUrl,
            [
                "firstName" => $firstName,
                "lastName" => $lastName
            ]
        );

        return $response;
    }

    public function returnStatusCode($statusCode) : Promise {
        $response = $this->adapter->get(
            self::$baseUrl . self::$returnStatusCodeUrl,
            ["statusCode" => $statusCode]
        );

        return $response;
    }
}

Notes on this one:

• The repo knows about the endpoints it calls. Usually we'd probably have the base URL as a constructor param so we can easily change version of the web service, but that's just noise here, so I've "hard-coded" it.
• I've added a new method - getByName - that I've not used before. I just wanted to have a second method which had a different method signature from the others I'd previously used. This was mostly for an experiment that I was gonna do but decided against for this article, but left the method in anyhow.

For the sake of completeness, here's the revised endpoint code:

getById.cfm

<cfscript>
cfcontent(type="application/json");

writeLog(file="testApp", text="[ID: #URL.id#] request received");

sleep(5000);
include "./data.cfm";

record = queryExecute(
    "SELECT id, firstName, lastName FROM people WHERE id = :id",
    {id=URL.id},
    {dbtype="query", query="people", maxrows=1}
);

result = structNew("ordered");
result["id"] = record.id;
result["firstName"] = record.firstName;
result["lastName"] = record.lastName;
result["retrieved"] = now().dateTimeFormat("HH:nn:ss.lll");

writeOutput(serializeJson(result));
writeLog(file="testApp", text="[ID: #URL.id#] response returned");
</cfscript>

getByName.cfm

<cfscript>
cfcontent(type="application/json");

writeLog(file="testApp", text="[firstName: #URL.firstName#][lastName: #URL.lastName#] request received");

sleep(2500);
include "./data.cfm";

record = queryExecute(
    "SELECT id, firstName, lastName FROM people WHERE firstName = :firstName AND lastName = :lastName",
    {firstName=URL.firstName, lastName=URL.lastName},
    {dbtype="query", query="people", maxrows=1}
);

result = structNew("ordered");
result["id"] = record.id;
result["firstName"] = record.firstName;
result["lastName"] = record.lastName;
result["retrieved"] = now().dateTimeFormat("HH:nn:ss.lll");

sleep(2500);

writeOutput(serializeJson(result));
writeLog(file="testApp", text="[firstName: #URL.firstName#][lastName: #URL.lastName#] response returned");
</cfscript>

data.cfm

<cfscript>
people = queryNew("id,firstName,lastName", "integer,varchar,varchar", [
    {id=1, firstName="Donald", lastName="Duck"},
    {id=2, firstName="Donald", lastName="Cameron"},
    {id=3, firstName="Donald", lastName="Trump"},
    {id=4, firstName="Donald", lastName="Wearsyatroosas"}
]);
</cfscript>

returnStatusCode.cfm

Remains unchanged:

<cfset sleep(2000)>
<cfheader statusCode="#URL.statusCode#">
<cfoutput>The request resulted in a #URL.statusCode# status code @ #now().dateTimeFormat("HH:nn:ss.lll")#</cfoutput>

The only thing to note really is that these endpoints are kinda data-driven now.

And I have a test of all this. It's long-winded, so I'll go through it section by section, before presenting it again in its entirety.

Firstly - and the only really important bit for the purposes of this article - is setting our test objects:

$loggingService = getLoggingService();
$personRepository = getPersonRepository($loggingService);

// ...


function getLoggingService() : LoggingService {
    $ts = (new DateTime())->format("His");
    $loggingService = new LoggingService("all_$ts");

    return $loggingService;
}

function getPersonRepository(LoggingService $loggingService) : PersonRepository {
    $guzzleAdapter = getGuzzleAdapter();
    $loggingAdapter = getLoggingAdapter($guzzleAdapter, $loggingService);
    $statusToExceptionAdapter = getStatusToExceptionAdapter($loggingAdapter, $loggingService);
    $cachingAdapter = getCachingAdapter($statusToExceptionAdapter, $loggingService);

    $personRepository = new PersonRepository($cachingAdapter);

    return $personRepository;
}

function getGuzzleAdapter() : GuzzleAdapter {
    $guzzleAdapter = new GuzzleAdapter();

    return $guzzleAdapter;
}

function getLoggingAdapter(Adapter $adapter, LoggingService $loggingService) : LoggingAdapter {
    $loggingAdapter = new LoggingAdapter($adapter, $loggingService);

    return $loggingAdapter;
}

function getStatusToExceptionAdapter(Adapter $adapter, LoggingService $loggingService) : StatusToExceptionAdapter {
    $exceptionMap = [
        400 => '\me\adamcameron\testApp\exception\BadRequestException',
        503 => '\me\adamcameron\testApp\exception\ServerException'
    ];

    $statusToExceptionAdapter = new StatusToExceptionAdapter($adapter, $exceptionMap, $loggingService);

    return $statusToExceptionAdapter;
}

function getCachingAdapter(Adapter $adapter, LoggingService $loggingService) : CachingAdapter {
    $cachingService = new CachingService($loggingService);
    $cachingAdapter = new CachingAdapter($adapter, $cachingService);

    return $cachingAdapter;
}

That looks like a chunk of code (well: it is), but it's mostly just compartmentalising some calls. Notes:

• we need a LoggingService and a PersonRepository for our tests.
• The PersonRepository needs an Adapter.
• For our purposes we are decorating the baseline GuzzleAdapter with:
  • a LoggingAdapter. We only want to log calls made to the service, so we put this closest to the baseline Adapter.
  • a StatusToExceptionAdapter. We only want to cache successful calls, and intrinsically previously successful calls will not need the logic in this adapter, so put this next.
  • a CachingAdapter. This goes closest to the calling code so that there's a minimum of complexity if a successful call has been previously made.

The order that the decorations are layers is important, as hopefully I've explained above. The ordering needs to be considered on a case-by-case basis, determined by the purpose of each decorator, and how one decorator might interrupt the execution of other ones; eg: if the StatusToExceptionAdapter throws an exception, the CachingAdapter never does its work. And depending on what those two do, the GuzzleAdapter itself might not be needed, so the LoggingAdapter is skipped too.

From there I test each of the three endpoints again, in much the same way I did before, but noting how the decorators combine where appropriate.

makeRequestsToGetById($personRepository, $loggingService);
logTestSeparator($loggingService);

makeRequestsToGetByName($personRepository, $loggingService);
logTestSeparator($loggingService);

makeRequestsToReturnStatusCode($personRepository, $loggingService);
logTestSeparator($loggingService);

Testing getById I just check the caching still works:

function makeRequestsToGetById(PersonRepository $personRepository, LoggingService $loggingService){
    $id = 4;

    makeRequestToGetById($id, "Uncached", $personRepository, $loggingService);
    pause(5, $loggingService);
    makeRequestToGetById($id, "Cached", $personRepository, $loggingService);
}

function makeRequestToGetById($id, $phase, PersonRepository $personRepository, LoggingService $loggingService){
    $startTime = time();

    $loggingService->logMessage("Test (phase: $phase): calling getById($id)");
    $response = $personRepository->getById($id);
    $loggingService->logMessage("Test (phase: $phase): call to getById complete");
    $body = (string) $response->wait()->getBody();
    $loggingService->logMessage("Test (phase: $phase): Body: $body");

    $duration = time() - $startTime;
    $loggingService->logMessage("Test (phase: $phase): Process duration: {$duration}sec");
}

Sample output:

[2016-11-22 20:39:21] all_203921.INFO: Test: Getting not-yet cached results [] []
[2016-11-22 20:39:21] all_203921.INFO: Test (phase: Uncached): calling getById(4) [] []
[2016-11-22 20:39:21] all_203921.INFO: LoggingAdapter: Requesting for {"id":4} [] []
[2016-11-22 20:39:21] all_203921.INFO: Test (phase: Uncached): call to getById complete [] []
[2016-11-22 20:39:26] all_203921.INFO: LoggingAdapter: Response for {"id":4}: {"id":4,"firstName":"Donald","lastName":"Wearsyatroosas","retrieved":"20:39:26.161"} [] []
[2016-11-22 20:39:26] all_203921.INFO: StatusToExceptionAdapter: non-exception status encountered: 200 [] []
[2016-11-22 20:39:26] all_203921.INFO: CachingService: put called with cd51039fde58b80cf261c3f1a99da3ef [] []
[2016-11-22 20:39:26] all_203921.INFO: Test (phase: Uncached): Body: {"id":4,"firstName":"Donald","lastName":"Wearsyatroosas","retrieved":"20:39:26.161"} [] []
[2016-11-22 20:39:26] all_203921.INFO: Test (phase: Uncached): Process duration: 5sec [] []
[2016-11-22 20:39:26] all_203921.INFO: Test: waiting 5sec [] []
[2016-11-22 20:39:31] all_203921.INFO: Test: waited 5sec [] []
[2016-11-22 20:39:31] all_203921.INFO: Test (phase: Cached): calling getById(4) [] []
no log entry here, because the LoggingAdapter was never hit: the CachingAdapter took care of it
[2016-11-22 20:39:31] all_203921.INFO: Test (phase: Cached): call to getById complete [] []
[2016-11-22 20:39:31] all_203921.INFO: CachingService: get called with cd51039fde58b80cf261c3f1a99da3ef [] []
[2016-11-22 20:39:31] all_203921.INFO: Test (phase: Cached): Body: {"id":4,"firstName":"Donald","lastName":"Wearsyatroosas","retrieved":"20:39:26.161"} [] []
[2016-11-22 20:39:31] all_203921.INFO: Test (phase: Cached): Process duration: 0sec [] []

Also note this demonstrates the LoggingAdapter at work too. Specifically note that there's only a log entry for the request when the web service is actually being called, not when it's coming from cache, further down.

Next I test getByName. It's exactly the same test as the getById one, so I won't bore you with it. It demonstrates the same thing.

Lastly I test returnStatusCode:

function makeRequestsToReturnStatusCode(PersonRepository $personRepository, LoggingService $loggingService) {
    makeRequestToReturnStatusCode200("First", $personRepository, $loggingService);
    pause(2, $loggingService);
    makeRequestToReturnStatusCode200("Second", $personRepository, $loggingService);
    logTestSeparator($loggingService);
    makeRequestToMappedStatusCode("First", $personRepository, $loggingService);
    $loggingService->logMessage("Test: Repeating an erroring call will not touch cache");
    makeRequestToMappedStatusCode("Second", $personRepository, $loggingService);
}

The test here is twofold. First I test that requests that are not intercepted and don't error are logged and cached correctly:

function makeRequestToReturnStatusCode200($phase, PersonRepository $personRepository, LoggingService $loggingService){
    $statusCode = 200;
    $loggingService->logMessage("Test (phase: $phase): calling returnStatusCode($statusCode)");
    $response = $personRepository->returnStatusCode($statusCode);
    $loggingService->logMessage("Test (phase: $phase): call to returnStatusCode complete");
    $body = (string) $response->wait()->getBody();
    $loggingService->logMessage("Test (phase: $phase): Body: $body");
}

Sample log output:

[2016-11-22 20:39:41] all_203921.INFO: Test (phase: First): calling returnStatusCode(200) [] []
[2016-11-22 20:39:41] all_203921.INFO: LoggingAdapter: Requesting for {"statusCode":200} [] []
[2016-11-22 20:39:41] all_203921.INFO: Test (phase: First): call to returnStatusCode complete [] []
[2016-11-22 20:39:43] all_203921.INFO: LoggingAdapter: Response for {"statusCode":200}: The request resulted in a 200 status code @ 20:39:43.319 [] []
[2016-11-22 20:39:43] all_203921.INFO: StatusToExceptionAdapter: non-exception status encountered: 200 [] []
[2016-11-22 20:39:43] all_203921.INFO: CachingService: put called with 8670b5e29a709e36ae186bce7715de68 [] []
[2016-11-22 20:39:43] all_203921.INFO: Test (phase: First): Body: The request resulted in a 200 status code @ 20:39:43.319 [] []
[2016-11-22 20:39:43] all_203921.INFO: Test: waiting 2sec [] []
[2016-11-22 20:39:45] all_203921.INFO: Test: waited 2sec [] []
[2016-11-22 20:39:45] all_203921.INFO: Test (phase: Second): calling returnStatusCode(200) [] []
[2016-11-22 20:39:45] all_203921.INFO: Test (phase: Second): call to returnStatusCode complete [] []
[2016-11-22 20:39:45] all_203921.INFO: CachingService: get called with 8670b5e29a709e36ae186bce7715de68 [] []
[2016-11-22 20:39:45] all_203921.INFO: Test (phase: Second): Body: The request resulted in a 200 status code @ 20:39:43.319 [] []

The second test of returnStatusCode test a remapped status to make sure it works to spec:

function makeRequestToMappedStatusCode($phase, PersonRepository $personRepository, LoggingService $loggingService) {
    $statusCode = 400;
    $loggingService->logMessage("Test (phase: $phase): calling returnStatusCode($statusCode)");
    $response = $personRepository->returnStatusCode($statusCode);
    $loggingService->logMessage("Test (phase: $phase): call to returnStatusCode complete");
    try {
        $response->wait();
        $loggingService->logMessage("Test (phase: $phase): failed to throw expected exception");
    } catch (Exception $e){
        logException($e, $loggingService);
    }
}

Sample logging:

[2016-11-22 20:39:45] all_203921.INFO: Test (phase: First): calling returnStatusCode(400) [] []
[2016-11-22 20:39:45] all_203921.INFO: LoggingAdapter: Requesting for {"statusCode":400} [] []
[2016-11-22 20:39:45] all_203921.INFO: Test (phase: First): call to returnStatusCode complete [] []
[2016-11-22 20:39:47] all_203921.INFO: StatusToExceptionAdapter: remapped status encountered: 400 [] []
[2016-11-22 20:39:47] all_203921.INFO: Test: Request errored with 400  [] []
[2016-11-22 20:39:47] all_203921.INFO: Test: Class: me\adamcameron\testApp\exception\BadRequestException  [] []
[2016-11-22 20:39:47] all_203921.INFO: Test: Message A response of 400 was received from the service  [] []
[2016-11-22 20:39:47] all_203921.INFO: Test: Previous class GuzzleHttp\Exception\ClientException  [] []
[2016-11-22 20:39:47] all_203921.INFO: Test: Previous code 400  [] []
[2016-11-22 20:39:47] all_203921.INFO: Test: Previous message Client error: `GET http://cf2016.local:8516/cfml/misc/guzzleTestEndpoints/returnStatusCode.cfm?statusCode=400` resulted in a `400 Bad Request` response: The request resulted in a 400 status code @ 20:39:47.359   [] []
[2016-11-22 20:39:47] all_203921.INFO: Test: Repeating an erroring call will not touch cache [] []
[2016-11-22 20:39:47] all_203921.INFO: Test (phase: Second): calling returnStatusCode(400) [] []
[2016-11-22 20:39:47] all_203921.INFO: LoggingAdapter: Requesting for {"statusCode":400} [] []
[2016-11-22 20:39:47] all_203921.INFO: Test (phase: Second): call to returnStatusCode complete [] []
[2016-11-22 20:39:49] all_203921.INFO: StatusToExceptionAdapter: remapped status encountered: 400 [] []
[2016-11-22 20:39:49] all_203921.INFO: Test: Request errored with 400  [] []
[2016-11-22 20:39:49] all_203921.INFO: Test: Class: me\adamcameron\testApp\exception\BadRequestException  [] []
[2016-11-22 20:39:49] all_203921.INFO: Test: Message A response of 400 was received from the service  [] []
[2016-11-22 20:39:49] all_203921.INFO: Test: Previous class GuzzleHttp\Exception\ClientException  [] []
[2016-11-22 20:39:49] all_203921.INFO: Test: Previous code 400  [] []
[2016-11-22 20:39:49] all_203921.INFO: Test: Previous message Client error: `GET http://cf2016.local:8516/cfml/misc/guzzleTestEndpoints/returnStatusCode.cfm?statusCode=400` resulted in a `400 Bad Request` response: The request resulted in a 400 status code @ 20:39:49.392   [] []

So:

• The status code is converted to the correct exception.
• The initial log entry is made for the request, but not the one for the response (see the log extract from the getById tests for that one.
• Nothing is cached or attempted to be fetched from cache at any point because the appropriate handlers are never invoked.

That'sit really. I'm happy with that.

Oh, I use a coupla other helper functions in that lot. For completeness here they are:

function logException(Exception $e, LoggingService $loggingService) {
    $previous = $e->getPrevious();
    $loggingService->logMessage(sprintf("Test: Request errored with %s%s", $e->getCode(), PHP_EOL));
    $loggingService->logMessage(sprintf("Test: Class: %s%s", get_class($e), PHP_EOL));
    $loggingService->logMessage(sprintf("Test: Message %s%s", $e->getMessage(), PHP_EOL));
    $loggingService->logMessage(sprintf("Test: Previous class %s%s", get_class($previous), PHP_EOL));
    $loggingService->logMessage(sprintf("Test: Previous code %s%s", $previous->getCode(), PHP_EOL));
    $loggingService->logMessage(sprintf("Test: Previous message %s%s", $previous->getMessage(), PHP_EOL));
}

function logTestSeparator(LoggingService $loggingService) {
    $loggingService->logMessage("==================================================================");
}

function pause($seconds, LoggingService $loggingService) {
    $loggingService->logMessage("Test: waiting {$seconds}sec");
    sleep($seconds);
    $loggingService->logMessage("Test: waited {$seconds}sec");
}

Indeed the code for everything here is up at GitHub, but I'm changing it with each article I write, so it gets out of date quickly. Here's the current commit, anyhow, and that's the code I've been using for this article.

I've got yet another article to write on this, but I haven't even worked out how I'm gonna write the code yet, so that might be a day or so still.

Righto.

--
Adam

PHP: decorating async GuzzleHttp calls - caching

G'day:
This continues my series of exercise in decorating GuzzleHttp calls. Recap:

• PHP: decorating async GuzzleHttp calls
• PHP: decorating async GuzzleHttp calls - handling exceptions
• PHP: decorating async GuzzleHttp calls - handling exceptions a different way

The last piece of the puzzle I need to deal with is caching. We need to decorate our web service adapter with a caching tier so that we don't continually pester it over the wire for info we have already asked for.

The treatment here is traditional:

• we get a request for data  based on an ID;
• we check the cache for data stored against that ID;
• if we have it, we returned the cached version;
• if not, we call the web service;
• and before returning the result, we cache it for "next time".

I've implemented a stub caching service which simply sticks stuff in an array. This'll do for the purposes of testing:

class CachingService {

    public $cache;

    public function __construct(){
        $this->cache = [];
    }

    public function contains($id){
        return array_key_exists($id, $this->cache);
    }

    public function get($id){
        echo "Getting $id from cache" . PHP_EOL . PHP_EOL;
        return $this->cache[$id];
    }

    public function put($id, $value){
        echo "Putting $id into cache" . PHP_EOL . PHP_EOL;
        return $this->cache[$id] = $value;
    }
}

In reality we'd use redis or something like that, but it's the interface that matters here, not the implementation. Note I'm also outputing a message when the cache methods are hit, just so we can tell what's going on when the code is running.

Next I have a CachingGuzzleAdapter which is the decorator for a GuzzleAdapter. This has a chunk of code in it, so I'll break it down:

class CachingGuzzleAdapter {

    private $adapter;
    private $cache;

    public function __construct($adapter, $cache) {
        $this->adapter = $adapter;
        $this->cache = $cache;
    }

    // ...

}

No surprises here. The decorator takes the adapter it's decorating, as well as an instance of the cache service it'll be talking to. I probably shoulda called that $cachingService, now that I look at it. We're taking the same approach here as we did with the other decorators (in the earlier articles): all the guzzling is offloaded to the adapter here; this class only deals with the caching side of things.

To that end, our get method is very simple:

public function get($id){
    if ($this->cache->contains($id)) {
        return $this->returnResponseFromCache($id);
    }
    return $this->returnResponseFromWebService($id);
}

If it's in the cache: use that; if not: go get it. The details of how to get the thing from cache or how to get things from the adapter are hidden in helper functions.

Here's the tricky bit: getting the response back from cache:

private function returnResponseFromCache($id) {
    $p = new Promise(function() use (&$p, $id){
        echo "GETTING FROM CACHE" . PHP_EOL;
        $cachedResult = $this->cache->get($id);

        $newResponse = new Response(
            $cachedResult['status'],
            $cachedResult['headers'],
            $cachedResult['body']
        );

        $p->resolve($newResponse);
    });

    return $p;
}

Here we:

• get some data from the cache. This method only gets called if the data is there, so we can dive straight in.
• We create a new Guzzle Response object, and put the data back into it.
• We resolve a promise with that as its value. We need to return a promise here because that's what the get method we're decorating returns.

I'm not entirely comfortable with the self-referential bit around $p, but that was from the docs as the recommended way of doing this. So be it.

If the stuff wasn't in cache, we need to make an actual web service call:

private function returnResponseFromWebService($id){
    $response = $this->adapter->get($id);

    $response->then(function(Response $response) use ($id) {
        echo "PUTTING IN CACHE" . PHP_EOL;

        $detailsToCache = [
            'status' => $response->getStatusCode(),
            'headers' => $response->getHeaders(),
            'body' => $response->getBody()->getContents()
        ];
        $this->cache->put($id, $detailsToCache);
    });

    return $response;
}

This is straight forward:

• make the call;
• in its resolution handler grab the data and bung it in the cache.
• Note that I'm only caching the significant bits of the response, not the response itself. My original version of this cached the entire response object: because the cache is just an in-memory array I can do that; in reality I'd need to serialise it somehow, so I wanted to make a point of only caching the data here.

That's it!

Of course I need to test this, so I wrote a test rig, similar to the ones I've already done:

$endPoint  = "http://cf2016.local:8516/cfml/misc/guzzleTestEndpoints/getById.cfm?id=";

$guzzleAdapter = new GuzzleAdapter($endPoint);
$cache = new CachingService();
$adapter = new CachingGuzzleAdapter($guzzleAdapter, $cache);

printf("Getting not-yet cached results @ %s%s", (new DateTime())->format("H:i:s"), PHP_EOL . PHP_EOL);
makeRequests($adapter);

echo "===================================================" .PHP_EOL . PHP_EOL;

sleep(10);

printf("Getting results again (from cache) @ %s%s", (new DateTime())->format("H:i:s"), PHP_EOL . PHP_EOL);
makeRequests($adapter);


function makeRequests($adapter){
    $startTime = time();
    $ids = ["001", "002"];
    $responses = [];
    foreach ($ids as $id) {
        echo "Making requests" . PHP_EOL . PHP_EOL;
        echo "Requesting: $id" . PHP_EOL;
        $responses[] = $adapter->get($id);
        echo "Requests made" . PHP_EOL . PHP_EOL;
    }
    echo "Fetching responses" . PHP_EOL . PHP_EOL;
    foreach ($responses as $response){
        $body = (string) $response->wait()->getBody();
        echo "Body: $body" . PHP_EOL . PHP_EOL;
    }
    echo "Responses fetched" . PHP_EOL . PHP_EOL;
    $duration = time() - $startTime;
    echo "Process duration: {$duration}sec" . PHP_EOL . PHP_EOL;
}

This just:

• calls the adapter for a coupla IDs,
• waits 10sec
• and gets the same data again.

The expectation is that the first call will take around 5sec as that's how long each web service call takes:

<cfscript>
cfcontent(type="application/json");

writeLog(file="testApp", text="[ID: #URL.id#] request received");

sleep(5000);

writeOutput(serializeJson({"id"=URL.id, "retrieved"=now().dateTimeFormat("HH:nn:ss.lll")}));
writeLog(file="testApp", text="[ID: #URL.id#] response returned");
</cfscript>

It'll only be 5sec in total not 10sec because the calls are being made asynchronously, remember?

The second round of calls should take no time at all because the adapter will be able to get the results from cache. In theory. Let's see:

>php testCachingAdapter.php
Getting not-yet cached results @ 09:24:25

Making requests

Requesting: 001
Requests made

Making requests

Requesting: 002
Requests made

Fetching responses

PUTTING IN CACHE
Putting 001 into cache

PUTTING IN CACHE
Putting 002 into cache

Body: {"retrieved":"09:24:30.453","id":"001"}

Body: {"retrieved":"09:24:30.457","id":"002"}

Responses fetched

Process duration: 5sec

===================================================

Getting results again (from cache) @ 09:24:40

Making requests

Requesting: 001
Requests made

Making requests

Requesting: 002
Requests made

Fetching responses

GETTING FROM CACHE
Getting 001 from cache

Body: {"retrieved":"09:24:30.453","id":"001"}

GETTING FROM CACHE
Getting 002 from cache

Body: {"retrieved":"09:24:30.457","id":"002"}

Responses fetched

Process duration: 0sec


>

Hurrah! As predicted the first call takes 5sec; the second takes 0sec. And the intermediary telemetry confirms that the cache is being used correctly. Also note the timestamps on the returned data reflect when the initial requests were made, not when the second round of calls were made.

That was pretty easy this time. It took me a while to find the code for returning a resolved promise with the Response value in it, but wiring everything up was simple.

As an exercise I'm now gonna wire up a CachedLoggedErrorMappedGuzzleAdapter. Or perhaps a LoggedErrorMappedCacheGuzzleAdapter. Not sure. But it's all a difference in wiring, and that's it really. I'll write that up next.

Righto.

--
Adam

PHP: decorating async GuzzleHttp calls - handling exceptions a different way

G'day:
The previous article ("PHP: decorating async GuzzleHttp calls - handling exceptions") detailed how I've used a simple decorator to intercept 4xx & 5xx HTTP responses from my web service and remap Guzzle's default exceptions and use my own ones.  It was basically just this:

public function get($status){
    return $this->adapter
        ->get($status)
        ->otherwise(function($exception){
            $code = $exception->getCode();

            if (!array_key_exists($code, $this->exceptionMap)) {
                throw $exception;
            }
            throw new $this->exceptionMap[$code](
                "A response of $code was received from the service",
                $code,
                $exception
            );
        });
}

Notes:

• get is a wrapper for a call to Guzzle, and it returns a GuzzleHttp\Promise
• if the promise rejects (which it will on a 4xx or 5xx response), then I have a handler to check if it's a status that I'm interested in, and if so return (well: throw) a different exception.
• If not, I just rethrow the original exception.

Simple enough and that works nicely. Read the article though, for more detail on this.

Along the way to that solution I ballsed something up in a way I still don't quite understand and I cannot replicate now, so abandoned that easy solution and approached things a different way. SO I've now got two ways of doing the same thing, and that seems liek a cheap win for me as far as blog content goes, so yer hearing about both of 'em ;-)

If I could not work out how to intercept Guzzle's own exception handling, I decided to replace it.

Reading through the docs I found one could switch off Guzzle's exception handling via the http_errors option. That was no use in itself (as it turns out), but looking at the matches for http_errors in the docs, I got down to the Middleware section, which detailed how the error handler (the one that pays attention to the http_errors setting) is one of the default middelware that Guzzle configures if no bespoke middleware is specified. It also details how one can set one's own middleware. Ergo: one can set one's own error handling middleware.

First things first, I took their own bespoke middleware example and modified it to handle my error remapping code (as per the previous article). As I was only really copying and pasting still into their example, I'll just present the final job:

private function handleHttpErrors($exceptionMap)
{
    return function (callable $handler) use ($exceptionMap) {
        return function ($request, array $options) use ($handler, $exceptionMap) {
            return $handler($request, $options)->then(
                function (ResponseInterface $response) use ($request, $handler, $exceptionMap) {
                    $code = $response->getStatusCode();

                    if ($code < 400) {
                        return $response;
                    }

                    if (array_key_exists($code, $exceptionMap)){
                        throw new $exceptionMap[$code](
                            (string)$response->getBody(),
                            $code
                        );
                    }
                    throw RequestException::create($request, $response);
                }
            );
        };
    };
}

Notes:

• most of this is copy and paste from the docs and I have no idea why it is the way it is, so... erm... well there you go.
• The rest just checks the response for an "error" situation;
• if it's not just return it... the next middleware will continue with it.
• If it is an exceptional situation: consult the exception map for how to handle it (which exception to throw);
• or if there's no remapping consideration, just throw the same exception one would normally throw (this was pinched from a different part of the Guzzle source code: Middleware.php).

That's all lovely, but I still need to get it into the middleware stack. Remember this is just a decorator, and it doesn't itself have its own Guzzle client... that's hidden away in the base Guzzle Adapter. But I need to have access to its middleware stack.

Here I cheat slightly and add a method to the base GuzzleAdapter:

namespace me\adamcameron\testApp;

use GuzzleHttp\Client;

class GuzzleAdapter {

    private $client;
    private $endPoint;

    public function __construct($endPoint){
        $this->endPoint = $endPoint;
        $this->client = new Client();
    }

    public function get($id){
        $response = $this->client->requestAsync(
            "get",
            $this->endPoint . $id,
            ["http_errors" => true]
        );

        return $response;
    }

    public function getHandlerStack()
    {
        return $this->client->getConfig('handler');
    }

}

This means the ExceptionHandler decorator can call that method, and get a reference to the handler stack. One caveat with this is that all decorators now need a proxy for this method too. This kinda suggests to me I need to have a GuzzleAdapter interface which specifies which methods decorators need to implement, but I've not got around to that yet.

Now from the ExceptionHandler decorator, I can horse around slightly, replacing the default exception handling with my own version:

public function __construct($adapter, $exceptionMap) {
    $this->adapter = $adapter;
    $this->exceptionMap = $exceptionMap;

    $this->replaceClientErrorHandler($exceptionMap);
}

// ...

private function replaceClientErrorHandler($exceptionMap)
{
    $stack = $this->adapter->getHandlerStack();

    $stack->remove('http_errors');

    $stack->unshift($this->handleHttpErrors($exceptionMap));
}

The handler stack doesn't have a "set" method that works on a named handler like $stack->set('http_errors', $this->handleHttpErrors($exceptionMap)), hence having to remove the default and then stick my own back in. But that's OK I guess.

And that all works.

I have the same test code as I had in the previous article, and it outputs much the same results.

My question is now... which of these approaches is better?

• The previous article's code is simpler, and easier to follow as one needs to know less about the inner workings of Guzzle.
• However letting Guzzle handle exceptions in one way, then intercepting that and going "actually now I'll do it meself thanks" seems like doubling up on work a bit.

I think from the perspective of code maintenance, the previous approach is probably "better" in the long run. But I'll just give both options to the chief, and see what he reckons. As my lunch break is about to be over, I think he'll be reckoning "get back to work, Cameron", so I better do that.

Righto.

--
Adam