Skip to content

Conversation

@StyleT
Copy link
Contributor

@StyleT StyleT commented Jul 12, 2015

getBody method always returns decoded string so decompressResponse method is useless.

@StyleT
Copy link
Contributor Author

StyleT commented Jul 12, 2015

It seems that unit test also should be updated, but I'm not sure how (because I saw that they follow some standards). However this code works correctly on production server.

@bakura10
Copy link
Member

Hi @StyleT,

Thank you for the fix. As you said, you'll need to update the test. You can see the failing test here: https://travis-ci.org/zf-fr/zfr-prerender/jobs/70649939#L189 (that basically needs to be removed if this PR is merged).

While you're at it, could you please add PHP 5.6 in the test matrix here: https://github.com/zf-fr/zfr-prerender/blob/master/.travis.yml#L5?

Thanks in advance

@thoop , can you just please have a look at this PR too, and confirm me that Prerender will actually return an uncompress response if ask for Gzip ? That seems a bit strange to me, what is the point of Gzip if Prerender actually already returns an uncompress answer? Or @StyleT , maybe ZF client uncompress it under the hood?

@StyleT
Copy link
Contributor Author

StyleT commented Jul 13, 2015

@bakura10 yes, you're right, ZF client will uncompress it under the hood.

public function getBody() { //Zend\Http\Response

    //...

    $contentEncoding = $this->getHeaders()->get('Content-Encoding');

    if (!empty($contentEncoding)) {
        $contentEncoding = $contentEncoding->getFieldValue();
        if ($contentEncoding =='gzip') {
            $body = $this->decodeGzip($body);
        } elseif ($contentEncoding == 'deflate') {
           $body = $this->decodeDeflate($body);
    }

    //...

}

@bakura10
Copy link
Member

Haaa yeah you're right ^^.

So you'll just need to update the tests ;).

@StyleT
Copy link
Contributor Author

StyleT commented Jul 13, 2015

@bakura10 If I understand you correctly, I should just remove ZfrPrerenderTest\Mvc\PrerenderListenerTest::testCanUncompressGzipResponses test? And add PHP 5.6

@StyleT
Copy link
Contributor Author

StyleT commented Jul 24, 2015

So what about pull request? Cay anyone merge it?

bakura10 added a commit that referenced this pull request Jul 24, 2015
Work with gzip requests fixed
@bakura10 bakura10 merged commit c620418 into zf-fr:master Jul 24, 2015
@bakura10
Copy link
Member

Merged, sorry for the delay! ;) I've just tagged as 3.1.1. Tahnks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants