brikis98 / ping-play

BigPipe streaming for the Play Framework
MIT License
307 stars 60 forks source link

Extension with HTMLCompressorFilter and GzipFilter #15

Open mantsak opened 9 years ago

mantsak commented 9 years ago

I am trying to extend the loader/Filters.java with the HTMLCompressorFilter from

https://github.com/mohiva/play-html-compressor

and standard Play GzipFilter, but do not see any change in the returned HTML code.

It might be possible, that chunked encoding does not support a combination with gzip-encoding, but a text-compressed HTML code should be possible.

brikis98 commented 9 years ago

How are you looking at the HTML code? If it's through a browser, I would think the browser is decompressing it for you.

mantsak commented 9 years ago

I just created a new branch in my fork with the extension of both HTMLCompressorFilter and gzipFilter

https://github.com/mantsak/ping-play/tree/withcompressors

I was using curl with following line:

curl -v -H "Accept-Encoding: gzip,deflate" http://localhost:9000/withoutBigPipe

Interestingly, withoutBigPipe does work 100% correct, the HTML code gets somewhat compressed and then it's getting gzipped. The order of the filters is important of course. gzipFilter is the first one and done last.

withBigPipe does neither. I found in @mohiva 's code that chunked encoding is not compressed by default,

The same in @playframework code. GzipFilter.scale has a comment in there:

* It will only gzip non chunked responses. Chunked responses are often comet responses, gzipping will interfere in
* that case. If you want to gzip a chunked response, you can apply the gzip enumeratee manually to the enumerator.

This is no bug at all on your side. If, then only a feature request. But not important at all. I think the priority here is to be able to save waiting time for the user and not saving bandwith.

brikis98 commented 9 years ago

Ah, gotcha, thanks for following up. I didn't realize the GzipFilter skipped chunked responses. It seems like a reasonable feature request and I'd be happy to merge in a PR that does it :)