Non-streaming ResponseWriters can't choose to not support gZip
This is probably low priority, but it doesn't seem like non-streaming ResponseWriters can choose to not support gZip encoding. This is because all responses are funneled through AbstractServiceResponseWriter (to enable streaming), which has this for its supportsGZip implementation:
@Override
public boolean supportsGZip(AbstractServiceResponse asr) {
if (isStreaming(asr)) {
return false;
}
return true;
}
In non-streaming cases, the supportsGZip method of the underlying ResponseWriter is never consulted (and the underlying ResponseWriter is only available during the write method).
Not sure how to solve this or even how important it is, but wanted to bring it up for discussion.
I want to work on this issue ,can you please assign this to me?
Hi @srstsavage , I've been looking into the ResponseWriters gZip support issue you pointed out (#107 on GitHub). I understand the problem: non-streaming ResponseWriters are forced to support gZip due to the AbstractServiceResponseWriter implementation, which bypasses the underlying ResponseWriter's supportsGZip method. Here's my initial thought process and proposed solution: Understanding the Issue:
-
Root Cause: The AbstractServiceResponseWriter's supportsGZip method directly returns true for non-streaming responses, regardless of the individual ResponseWriter's capabilities.
-
Impact: This prevents developers from explicitly disabling gZip compression for non-streaming responses when needed. Proposed Solution: I believe we can address this by modifying the AbstractServiceResponseWriter's supportsGZip method to:
-
Check for Streaming: Keep the existing check for isStreaming(asr).
-
Delegate to Underlying Writer: If it's not a streaming response, access the underlying ResponseWriter and call its supportsGZip method. Code Snippet (Conceptual): @Override public boolean supportsGZip(AbstractServiceResponseWriter asr) { if (isStreaming(asr)) { return false; }
// Assuming we can access the underlying ResponseWriter here ResponseWriter underlyingWriter = getUnderlyingResponseWriter(asr); // Need to implement this method if (underlyingWriter != null) { return underlyingWriter.supportsGZip(); }
return true; // Default to true if underlying writer is not available }
Next Steps:
- Implementation: I'll implement the getUnderlyingResponseWriter(asr) method (or find the appropriate way to access the underlying writer) and update the supportsGZip method accordingly.
- Testing: I'll write unit tests to ensure that the fix works correctly for both streaming and non-streaming scenarios, and that the underlying writer's supportsGZip method is respected.
- Pull Request: Once I've completed the implementation and testing, I'll create a pull request with the changes for review. I'm eager to get your feedback on this approach. Please let me know if you have any suggestions or if I'm missing anything. Thanks, shreyashkumar01