SOS icon indicating copy to clipboard operation
SOS copied to clipboard

Non-streaming ResponseWriters can't choose to not support gZip

Open srstsavage opened this issue 11 years ago • 2 comments

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.

srstsavage avatar Aug 16 '14 04:08 srstsavage

I want to work on this issue ,can you please assign this to me?

shreyashkumar01 avatar Mar 21 '25 20:03 shreyashkumar01

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

shreyashkumar01 avatar Mar 21 '25 20:03 shreyashkumar01