dannykopping / spore

ReSTful extensions for Slim
29 stars 2 forks source link

Middleware fix #17

Closed zenkimoto closed 1 year ago

zenkimoto commented 11 years ago

Let me know what you think!

zenkimoto commented 11 years ago

I just realized that the code for the deserializer should be this:

    $res = $this->app->response();

    $res->status(\Spore\ReST\Model\Status::BAD_REQUEST);
    $res->body(Serializer::getSerializedData($this->app, 
        array("error" => array(
                "message" => "An error occurred while attempting to deserialize data"
             ))
        ));
dannykopping commented 11 years ago

Thanks Alex - busy looking into this now

zenkimoto commented 11 years ago

Sorry Danny, I had visitors in town. I made a few changes to support partial sent XML and JSON with the POST and PUT verbs. I also looked at serialization/deserialization when dependencies are missing. They return a HTTP 500 just like before. I wrote some test cases with PHPUnit to test out the changes. (I did not intend for them to be included in Spore. If you want me to, I could comment them further following the PHP standards)

Take a look.

SporeTestCases.php

<?php

require_once "vendor/autoload.php";

class SporeTestCases extends PHPUnit_Framework_TestCase {

    private $baseUrl = "http://localhost:8080/";

    private function sendGetRequest($url, $contentType, $acceptContent) {
        $ch = curl_init();

        curl_setopt($ch, CURLOPT_URL, $url);
        curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
        curl_setopt($ch, CURLOPT_HTTPHEADER, array('Content-Type: '.$contentType, 'Accept: '.$acceptContent));

        $body = curl_exec($ch);

        $headers = curl_getinfo($ch);

        curl_close($ch);

        return array("body" => $body, "headers" => $headers);
    }

    private function sendPutRequest($url, $contentType, $acceptContent, $data) {
        $ch = curl_init();

        curl_setopt($ch, CURLOPT_CUSTOMREQUEST, "PUT");
        curl_setopt($ch, CURLOPT_URL, $url);
        curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
        curl_setopt($ch, CURLOPT_POSTFIELDS, $data);
        curl_setopt($ch, CURLOPT_HTTPHEADER, array('Content-Type: '.$contentType, 'Accept: '.$acceptContent));

        $body = curl_exec($ch);

        $headers = curl_getinfo($ch);

        curl_close($ch);

        return array("body" => $body, "headers" => $headers);
    }

    private function sendPostRequest($url, $contentType, $acceptContent, $data) {
        $ch = curl_init();

        curl_setopt($ch, CURLOPT_POST, 1);
        curl_setopt($ch, CURLOPT_URL, $url);
        curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
        curl_setopt($ch, CURLOPT_POSTFIELDS, $data);
        curl_setopt($ch, CURLOPT_HTTPHEADER, array('Content-Type: '.$contentType, 'Accept: '.$acceptContent));

        $body = curl_exec($ch);

        $headers = curl_getinfo($ch);

        curl_close($ch);

        return array("body" => $body, "headers" => $headers);
    }

    /***** EXCEPTION TESTING - JSON *****/

    public function testShould_Return_Bad_Request_When_POST_Invalid_JSON_Deserialization() {
        $url = $this->baseUrl."sporeTestCases/index.php/test";
        $json = '{"message":"Hello World fr';
        $errorMsg = '{"error":{"message":"An error occurred while attempting to deserialize data"}}';
        $response = $this->sendPostRequest($url, "application/json", "application/json", $json);

        $this->assertEquals(400, $response['headers']['http_code']);
        $this->assertEquals($errorMsg, $response['body']);
    }

    public function testShould_Return_Bad_Request_When_PUT_With_Invalid_JSON_Deserialization() {
        $url = $this->baseUrl."sporeTestCases/index.php/test";
        $json = '{"message":"Hello World fr';
        $errorMsg = '{"error":{"message":"An error occurred while attempting to deserialize data"}}';
        $response = $this->sendPutRequest($url, "application/json", "application/json", $json);

        $this->assertEquals(400, $response['headers']['http_code']);
        $this->assertEquals($errorMsg, $response['body']);  
    }

    /***** EXCEPTION TESTING - XML *****/

    public function testShould_Return_Bad_Request_When_POST_With_Invalid_XML_Deserialization() {
        $url = $this->baseUrl."sporeTestCases/index.php/test";
        $xml = '<?xml version="1.0" encoding="UTF-8" ?><data><message>Hello World from Spore (PO';
        $errorMsg = '<?xml version="1.0" encoding="UTF-8" ?><data><error><message>Unable to parse input data as XML.&lt;br/&gt;String could not be parsed as XML</message></error></data>';
        $response = $this->sendPostRequest($url, "text/xml", "text/xml", $xml);

        $this->assertEquals(400, $response['headers']['http_code']);
        $this->assertEquals($errorMsg, $response['body']);
    }

    public function testShould_Return_Bad_Request_When_PUT_With_Invalid_XML_Deserialization() {
        $url = $this->baseUrl."sporeTestCases/index.php/test";
        $xml = '<?xml version="1.0" encoding="UTF-8" ?><data><message>Hello World from Spore (PO';
        $errorMsg = '<?xml version="1.0" encoding="UTF-8" ?><data><error><message>Unable to parse input data as XML.&lt;br/&gt;String could not be parsed as XML</message></error></data>';
        $response = $this->sendPutRequest($url, "text/xml", "text/xml", $xml);

        $this->assertEquals(400, $response['headers']['http_code']);
        $this->assertEquals($errorMsg, $response['body']);
    }

    /***** EXCEPTION TESTING - NO SERIALIZATION DEPENDECY *****/

    public function testShould_Return_Server_Error_When_XML_Deserialization_AND_Serialization_Option_Does_Not_Exist() {
        $url = $this->baseUrl."sporeTestCases/index.php/test";
        $xml = '<?xml version="1.0" encoding="UTF-8" ?><data><message>Hello World from Spore (PO';
        $response = $this->sendPostRequest($url, "text/xml", "text/csv", $xml);

        $this->assertEquals(500, $response['headers']['http_code']);
    }

    public function testShould_Return_Server_Error_When_JSON_Deserialization_AND_Serialization_Option_Does_Not_Exist() {
        $url = $this->baseUrl."sporeTestCases/index.php/test";
        $json = '{"message":"Hello World fr';
        $response = $this->sendPostRequest($url, "application/json", "text/csv", $json);

        $this->assertEquals(500, $response['headers']['http_code']);
    }

    public function testShould_Return_Server_Error_When_JSON_Deserialization_AND_Invalid_Serialization_Option() {
        $url = $this->baseUrl."sporeTestCases/index.php/test";
        $json = '{"message":"Hello World fr';
        $response = $this->sendPostRequest($url, "application/json", "asdfasdf", $json);

        $this->assertEquals(500, $response['headers']['http_code']);
    }

}

index.php

<?php
    require_once "vendor/autoload.php";

    use Spore\Spore;

    $app = new Spore();

    $app->get("/test", function() 
    {
        return array("message" => "Hello World from Spore (GET)");
    });

    $app->put("/test", function ()
    {
        return array("message" => "Hello World from Spore (PUT)");
    });

    $app->post("/test", function ()
    {
        return array("message" => "Hello World from Spore (POST)");
    });

    $app->run();
zenkimoto commented 11 years ago

So ultimately what these changes are intended to support are cases where a request was made but only a partial request went through. More specifically, like cases when calling a web service from a smartphone where the connection can be flaky.

zenkimoto commented 11 years ago

Let me know what you think, we can discuss further. :-)

dannykopping commented 11 years ago

Hey Alex

Thanks for the effort! It's seriously appreciated I think what you've put together is great. I think it might be worth making the HTTP error types configurable so that users will not have to do a find/replace on the Spore codebase.

For example, if i - for whatever reason - wanted to throw a 500 error instead of a 400 error when a parser error occurs, i'd like to be able to do it like so:

<?php
$app = new Spore(array(
    "errors.parser-error" => \Spore\ReST\Model\Status::INTERNAL_SERVER_ERROR,
    "errors.invalid-accept-type" => \Spore\ReST\Model\Status::NOT_ACCEPTABLE,
));

What're your thoughts?

Also - many thanks for adding the unit tests. I think the whole library needs a bit of TDD love; maybe this is the metaphorical butt-kick for me to get cracking on that. I'd like to modify the tests a little bit - but we can discuss that first.

zenkimoto commented 11 years ago

Hey Danny,

Thanks! I'm glad you like it.

I also think that making HTTP error types configurable is a great idea. I agree, it would be beneficial to make the HTTP status configurable rather than having it hard coded. I'll implement that this coming week.

Also, would you like me to create a branch to add my unit tests? I have a few more unit tests that I didn't include. You're welcome to change the unit tests as you like. :-) I threw them together to test the changes I was making so it wasn't anything special. If you'd like, I can branch that in and you can change as you like. I'll just create a test folder and setup a phpunit script? Let me know what you think, I want to make sure you're fine with the unit testing framework.

zenkimoto commented 11 years ago

Hey Danny,

I made your suggested changes. Take a look and let me know what you think.

I will create another branch for tests after I have cleaned them up a bit? The above code was tested with my unit test cases. You'll see them when I add the unit tests.