Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First steps to add encoding capability to outbound traffic #73

Merged
merged 37 commits into from
Feb 28, 2014

Conversation

nicolas-rempulski
Copy link
Contributor

Need Review
Merge Candidate

Here are first steps of adding encoding to Wisdom as stated in #10

Done :

  • Add content encoding constants
  • Add content encoders in the content manager service (GZIP, DEFLATE, IDENTITY)
  • Add content encoder retrieval in content engine (with encoding)
  • Add Accepting_Encoding parsing (partially done) + encoding sorting based on order and qValue (lots of TODO in code)
  • Add content encoding to result processing
  • Add a configuration key to enable / disable encoding globally
  • Add annotations to enable / disable encoding for a route / controller
  • Add size limit to @AllowEncoding and global configuration (code is still rough)
  • Review Accepting_Encoding parsing as I'm not sure of some behaviors + "*;qValue" is not handled
  • Don't work on /assets/xxx routes, they have a length of -1 => Not RenderableFile but RenderableStream. Added a configuration key to allow encoding of ALL stream. Still disable with the @DenyEncoding annotation.
  • Documentation for annotations, default behaviour
  • Filter on mime types
  • Stream are not encoded (no size no mime type). RenderableUrl are encoded if encoding.url is set and mime type not in exclude list
  • Filter on already encoded files (Content_Encoding set and different from identitity)
  • Unit tests (done for encoders)
  • Documentation
  • Encoding is now asynchrone

ToDo :

  • Check encoding behavior with cache (Compressed each time or the result is cached ?)

Future Work :

To discuss :

I also made a change to RenderableString, as the Charset was not used to convert the string to bytes. Need reviews. (I think it is linked to #42)

String.getBytes() encode a string using the platform default encoding.

In RenderableString, the string was encoded without using the Charset specified or didn't specify the default Charset if no Charset was set.

Now, when using RenderableString, it will always specify the charset.
A ContentEncoder encodes an InputStream to a given compression format
- GZIP
- DEFLATE
- IDENTITY
EncodingHelp allows to parse the Accept_Encoding header and sort the encoding methods according (mostly) to HTTP/1.1
Result parameter as null was not handled
These annotations allow to override the application configuration regarding content encoding.

They can be set both on class & methods
* maxSize is the upper boundary for encoding
* minSize is the lower boundary for encoding
* if renderable.length is not in encoding boundaries, abort encoding

* shouldEncode now take a renderable as parameter, preparing for filter on extensions
Changing package names to org.wisdom.content.codecs
* Adding AbstractDefInfCodec for Gzip and Deflate

* Adding in file documentation
Moving shouldEncode from WisdomHandler to EncodingHelper
* shouldEncode is now in EncodingHelper

* parseAcceptEncodingHeader now correctly handle wildcard "*"
* Add default value for encoding.global : true

* Add encoding.stream configuration key to allow stream encoding (no size check !)

* Add default value for encoding.stream : true
Should encode will not check stream size in encoding.stream is set to true in the configuration
* Adding get functions

* Update Configuration Unit Test
* Added a ContentEncodingHelper interface
* Provide a ContentEncodingHelperImpl in content-manager as a service
* Retrieval through content-engine
* Updated UT (was throwing null pointer)
* Add UT and FakeControllers
* Add mockito dependency
* Modifications to ContentEncodingHelperImpl to satisfy tests
* Configuration keys
* URL encoding
* MimeType filtering
* Cosmetic changes
@nicolas-rempulski
Copy link
Contributor Author

This PR is now ready for review & merge.

Interaction between cache and encoding should be examined.

* add fromAsync parameter to write / finalizeWrite to know is their call is from an async result and need cleanup at the end
* Remove cleanup after AsyncResult handling as there could be an async content encoding running
* Extract content encoding from process result to handle it in writeResponse
* Add a proceedAsyncEncoding function to encode content InputStream asynchronously
* FinalizeWrite now check if the call is from an Async function and if keepAlive is not set, cleanup this handler
Conflicts:
	wisdom-api/src/main/java/org/wisdom/api/bodies/RenderableString.java
	wisdom-api/src/main/java/org/wisdom/api/utils/KnownMimeTypes.java
	wisdom-engine/src/main/java/org/wisdom/engine/server/WisdomHandler.java
Conflicts:
	application-configuration/src/main/java/org/wisdom/configuration/ApplicationConfigurationImpl.java
	application-configuration/src/test/java/org/wisdom/configuration/ApplicationConfigurationTest.java
	wisdom-api/src/main/java/org/wisdom/api/configuration/ApplicationConfiguration.java
@nicolas-rempulski
Copy link
Contributor Author

Conflicts too high

cescoffier added a commit that referenced this pull request Feb 28, 2014
First steps to add encoding capability to outbound traffic
@cescoffier cescoffier merged commit d28e7c0 into master Feb 28, 2014
@cescoffier cescoffier added this to the 0.4.0 milestone Apr 4, 2014
@cescoffier cescoffier deleted the encoding-implem branch August 25, 2014 09:13
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