feat: support sending gzip-encoded buffers #54
Conversation
WalkthroughAdds optional gzip compression for HTTP request bodies: new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HttpSender
participant BufferStreamContent
participant GZipStream
participant HttpRequest
participant IlpEndpoint
Client->>HttpSender: GenerateRequest(buffer)
HttpSender->>HttpSender: Check Options.gzip
alt gzip enabled
HttpSender->>BufferStreamContent: new(buffer, gzip=true)
HttpSender->>HttpRequest: Add header Content-Encoding: gzip
HttpSender->>HttpRequest: Omit Content-Length
else gzip disabled
HttpSender->>BufferStreamContent: new(buffer, gzip=false)
HttpSender->>HttpRequest: Set Content-Length
end
HttpRequest->>BufferStreamContent: SerializeToStreamAsync()
alt gzip=true
BufferStreamContent->>GZipStream: wrap output stream and write compressed data
GZipStream->>HttpRequest: Compressed body
else gzip=false
BufferStreamContent->>HttpRequest: Uncompressed body
end
HttpRequest->>IlpEndpoint: POST with body
IlpEndpoint->>IlpEndpoint: Inspect Content-Encoding
alt Content-Encoding: gzip
IlpEndpoint->>GZipStream: Decompress request stream
GZipStream->>IlpEndpoint: Decompressed data
else none
IlpEndpoint->>IlpEndpoint: Use original stream
end
IlpEndpoint->>IlpEndpoint: Process data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/dummy-http-server/IlpEndpoint.cs(2 hunks)src/net-questdb-client-tests/HttpTests.cs(51 hunks)src/net-questdb-client-tests/SenderOptionsTests.cs(2 hunks)src/net-questdb-client/Buffers/BufferStreamContent.cs(3 hunks)src/net-questdb-client/Senders/HttpSender.cs(1 hunks)src/net-questdb-client/Utils/SenderOptions.cs(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-17T16:38:52.474Z
Learnt from: RaphDal
Repo: questdb/net-questdb-client PR: 52
File: src/net-questdb-client/Senders/ISender.cs:410-418
Timestamp: 2025-10-17T16:38:52.474Z
Learning: In src/net-questdb-client/Senders/ISender.cs, the decimal type intentionally does not have a NullableColumn overload. This is a deliberate design decision because NullableColumn is deprecated in favor of using the Column method directly with nullable types.
Applied to files:
src/net-questdb-client-tests/HttpTests.cs
📚 Learning: 2025-10-17T16:37:03.583Z
Learnt from: RaphDal
Repo: questdb/net-questdb-client PR: 52
File: src/dummy-http-server/DummyHttpServer.cs:228-299
Timestamp: 2025-10-17T16:37:03.583Z
Learning: In src/dummy-http-server/DummyHttpServer.cs, the PrintBuffer() method is a test helper that produces human-readable output from received bytes. Decimal tests use binary representation directly via DecimalTestHelpers and do not call PrintBuffer, so DECIMAL type decoding in PrintBuffer is not required.
Applied to files:
src/net-questdb-client-tests/HttpTests.cs
🧬 Code graph analysis (4)
src/net-questdb-client/Buffers/BufferStreamContent.cs (2)
src/net-questdb-client/Buffers/IBuffer.cs (16)
IBuffer(66-66)IBuffer(74-74)IBuffer(84-84)IBuffer(97-97)IBuffer(105-105)IBuffer(113-113)IBuffer(121-121)IBuffer(129-129)IBuffer(137-137)IBuffer(145-145)IBuffer(153-153)IBuffer(228-228)IBuffer(235-235)IBuffer(248-248)IBuffer(260-260)WriteToStream(221-221)src/net-questdb-client/Buffers/BufferV1.cs (1)
WriteToStream(224-245)
src/net-questdb-client/Senders/HttpSender.cs (1)
src/net-questdb-client/Buffers/BufferStreamContent.cs (2)
BufferStreamContent(34-115)BufferStreamContent(41-45)
src/net-questdb-client-tests/HttpTests.cs (3)
src/dummy-http-server/DummyHttpServer.cs (8)
DummyHttpServer(36-306)DummyHttpServer(54-112)Exception(185-188)Task(141-152)Task(157-160)Task(162-165)Task(190-194)PrintBuffer(226-305)src/net-questdb-client/Buffers/BufferStreamContent.cs (3)
Task(51-62)Task(65-76)Task(93-99)src/net-questdb-client-tests/SenderOptionsTests.cs (12)
Test(36-42)Test(44-50)Test(52-59)Test(61-69)Test(71-77)Test(79-87)Test(89-97)Test(99-109)Test(111-116)Test(118-123)Test(125-130)Test(132-137)
src/net-questdb-client-tests/SenderOptionsTests.cs (1)
src/net-questdb-client/Utils/SenderOptions.cs (4)
SenderOptions(91-93)SenderOptions(99-125)SenderOptions(684-690)ToString(610-655)
🔇 Additional comments (14)
src/net-questdb-client/Utils/SenderOptions.cs (3)
56-56: LGTM!The gzip option is correctly added to the allowed keys set and the private field is appropriately initialized to false, ensuring backward compatibility.
Also applies to: 66-66
109-109: LGTM!The parsing logic correctly uses
ParseBoolWithDefaultwith a false default, maintaining consistency with other boolean configuration options.
220-232: LGTM!The property is well-documented with clear remarks about HTTP/HTTPS applicability. The implementation follows standard patterns.
src/net-questdb-client/Senders/HttpSender.cs (2)
238-242: LGTM!The BufferStreamContent is correctly instantiated with the gzip option, and the documentation accurately reflects the optional compression behavior.
245-256: LGTM!The header handling correctly omits Content-Length when gzip is enabled (since compressed size cannot be known ahead of time) and properly adds the Content-Encoding: gzip header. This follows HTTP standards for compressed content.
src/net-questdb-client-tests/SenderOptionsTests.cs (2)
76-76: LGTM!Existing tests correctly updated to include
gzip=Falsein the expected ToString() output, maintaining test validity.Also applies to: 108-108
111-137: LGTM!Comprehensive test coverage for the new gzip option, verifying default value, parsing of both true/false values, and serialization. Tests follow existing patterns in the file.
src/net-questdb-client-tests/HttpTests.cs (1)
1744-1814: Good test coverage, but depends on buggy server code.The three new tests provide comprehensive coverage of gzip behavior:
- Enabled: verifies compressed data is sent and received
- Disabled: verifies uncompressed data is sent
- Default: verifies backward compatibility (default is uncompressed)
However, these tests will fail due to the critical bug in IlpEndpoint.cs (lines 65-73) where the decompressed stream is disposed before use. Once that bug is fixed, these tests should work correctly.
src/net-questdb-client/Buffers/BufferStreamContent.cs (6)
40-48: LGTM!The constructor cleanly accepts an optional gzip parameter with a default of false, maintaining backward compatibility. Documentation is clear.
53-62: LGTM!Correct implementation using GZipStream with
leaveOpen: trueto prevent premature disposal of the underlying stream. The conditional logic properly handles both compressed and uncompressed paths.
67-76: LGTM!Correctly implements the cancellation token overload with the same gzip logic pattern, properly propagating the cancellation token through the call chain.
81-90: LGTM!Correctly returns false when gzip is enabled since compressed size cannot be determined ahead of time. The comment clearly explains the rationale.
97-97: LGTM!The Seek to the beginning ensures the content stream is positioned correctly for reading after serialization.
105-114: LGTM!The synchronous serialization method correctly mirrors the async implementation with proper cancellation token handling and gzip compression logic.
Summary by CodeRabbit
New Features
Tests