Buy

DELETE is for Saying Goodbye

So you have to part ways with your programmer, and we all know goodbyes are hard. So let's delete them instead. We're going to create an rm -rf endpoint to send a programmer to /dev/null.

Start with the test! public function testDELETEProgrammer, because we'll send a DELETE request to terminate that programmer resource:

105 lines src/AppBundle/Tests/Controller/Api/ProgrammerControllerTest.php
... lines 1 - 5
class ProgrammerControllerTest extends ApiTestCase
{
... lines 8 - 93
public function testDELETEProgrammer()
{
... lines 96 - 102
}
}

Copy the guts of the GET test - fetching and deleting a programmer are almost the same, except for the HTTP method, change it to delete():

105 lines src/AppBundle/Tests/Controller/Api/ProgrammerControllerTest.php
... lines 1 - 93
public function testDELETEProgrammer()
{
$this->createProgrammer(array(
'nickname' => 'UnitTester',
'avatarNumber' => 3,
));
$response = $this->client->delete('/api/programmers/UnitTester');
... line 102
}
... lines 104 - 105

Now, what's the status code? What should we return? We can't return the JSON programmer, because we just finished truncating it's proverbial tables. I mean, it's deleted - so it doesn't make sense to return the resource. Instead, we'll return nothing and use a status code - 204 - that means "everything went super great, but I have no content to send back." Remove the asserts on the bottom... since there's nothing to look at:

105 lines src/AppBundle/Tests/Controller/Api/ProgrammerControllerTest.php
... lines 1 - 93
public function testDELETEProgrammer()
{
$this->createProgrammer(array(
'nickname' => 'UnitTester',
'avatarNumber' => 3,
));
$response = $this->client->delete('/api/programmers/UnitTester');
$this->assertEquals(204, $response->getStatusCode());
}
... lines 104 - 105

The Controller

Let's get straight to the controller: public function deleteAction(). Copy the route stuff from updateAction(). It's all the same again, except the method is different. Take out the route name - we don't need this unless we link here. And change the method to DELETE:

154 lines src/AppBundle/Controller/Api/ProgrammerController.php
... lines 1 - 117
/**
* @Route("/api/programmers/{nickname}")
* @Method("DELETE")
*/
public function deleteAction($nickname)
{
... lines 124 - 135
}
... lines 137 - 154

Grab the query code from updateAction() too, and make sure you have your $nickname argument:

154 lines src/AppBundle/Controller/Api/ProgrammerController.php
... lines 1 - 121
public function deleteAction($nickname)
{
$programmer = $this->getDoctrine()
->getRepository('AppBundle:Programmer')
->findOneByNickname($nickname);
if (!$programmer) {
throw $this->createNotFoundException(sprintf(
'No programmer found with nickname "%s"',
$nickname
));
}
// todo ...
}
... lines 137 - 154

So this will 404 if we don't find the programmer. Surprise! In the REST world, this is controversial! Since the job of this endpoint is to make sure the programmer resource is deleted, some people say that if the resource is already gone, then that's success! In other words, you should return the same 204 even if the programmer wasn't found. When you learn more about idempotency, this argument makes some sense. So let's do it! But really, either way is fine.

Change the if statement to be if ($programmer), then we'll delete it. Grab the EntityManager and call the normal remove() on it, then flush():

156 lines src/AppBundle/Controller/Api/ProgrammerController.php
... lines 1 - 121
public function deleteAction($nickname)
{
... lines 124 - 127
if ($programmer) {
// debated point: should we 404 on an unknown nickname?
// or should we just return a nice 204 in all cases?
// we're doing the latter
$em = $this->getDoctrine()->getManager();
$em->remove($programmer);
$em->flush();
}
... lines 136 - 137
}
... lines 139 - 156

And whether the Programmer was found or not, we'll always return the same new Response(null, 204):

156 lines src/AppBundle/Controller/Api/ProgrammerController.php
... lines 1 - 121
public function deleteAction($nickname)
{
$programmer = $this->getDoctrine()
->getRepository('AppBundle:Programmer')
->findOneByNickname($nickname);
if ($programmer) {
... lines 129 - 134
}
return new Response(null, 204);
}
... lines 139 - 156

Try the test and send a programmer to the trash! Filter for testDELETE.

phpunit -c app --filter testPUTProgrammer

Another endpoint bites the dust!

Leave a comment!

  • 2015-07-15 weaverryan

    Hey!

    I'm mixed on this. Well, obviously, doing what you're saying is probably best - and I like the idea that you're actually using the API to verify the delete. So why *wouldn't* I test this? Because I don't want to over-test. The only likely way that this could return a 204 but NOT delete is if we actually just forgot the save logic (any error would cause a 500). And as long as we manually play around with our API once or twice, we'll notice that it doesn't actually save. Unlike other bugs (the ones that cause errors), there's basically no risk that we'd add a regression where we accidentally deleted the save code later.

    I'll admit, this over-testing in functional tests is more possible when you're testing a web interface (e.g. don't submit a form 50 times to test all 50 different validation combinations), but that was the idea here.

    Anyways - if you want to test it, I like your way :)

    Cheers!

  • 2015-07-15 Victor Bocharsky

    I think we also should to send GET request and try to get this programmer by `$nickname` in tests and ensure that it doesn't exist anymore. What if we do something wrong in `deleteAction` (for example wrong DB query in `findOneByNickname` method) and programmer don't really delete? It almost always returns us a `204`, but programmer could not be deleted, so we need to check it too, right?