-
Notifications
You must be signed in to change notification settings - Fork 73
Add HTTP request compression support (Gzip, Deflate, Brotli) #695
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
base: main
Are you sure you want to change the base?
Conversation
This commit adds opt-in HTTP request compression to reduce bandwidth usage and improve performance when uploading large document payloads. Features: - Support for Gzip (netstandard2.0+), Deflate (net6.0+), and Brotli (netstandard2.1+) - Client-level configuration with sensible defaults (1400-byte threshold) - Automatic compression via DelegatingHandler pattern - Configurable minimum payload size threshold - Optional response decompression support - Zero breaking changes - fully backward compatible Implementation: - CompressionOptions class for configuration - CompressionHelper for compression logic with platform-aware conditional compilation - Enhanced MeilisearchMessageHandler to intercept and compress requests - Updated MeilisearchClient constructors to accept CompressionOptions Testing: - 13 comprehensive tests covering all algorithms - Tests for error handling on unsupported platforms - Integration tests with Meilisearch server - Payload size verification tests - Different threshold value tests - All tests passing (100% success rate) Documentation: - Updated README with usage examples and best practices - XML documentation on all public APIs - Clear error messages with actionable guidance
WalkthroughAdds HTTP request compression support: new public CompressionOptions and CompressionAlgorithm types, an internal CompressionHelper implementing Gzip/Deflate/Brotli compression with runtime guards, integration into MeilisearchMessageHandler and MeilisearchClient constructors/properties, README docs, and tests covering behavior and compatibility. Changes
Sequence DiagramsequenceDiagram
participant Client as MeilisearchClient
participant Handler as MeilisearchMessageHandler
participant Helper as CompressionHelper
participant HTTP as HTTP Pipeline
Client->>Handler: Send request
Handler->>Handler: Check CompressionOptions & content size
alt Compression enabled and threshold met
Handler->>Helper: CompressAsync(content, algorithm)
Helper->>Helper: Validate support & compress (Gzip/Deflate/Brotli)
Helper-->>Handler: Return compressed HttpContent (with Content-Encoding)
Handler->>Handler: Replace request content
end
alt EnableResponseDecompression
Handler->>Handler: Add Accept-Encoding header(s)
end
Handler->>HTTP: Forward HTTP request
HTTP-->>Handler: Return HTTP response
Handler-->>Client: Deliver response (existing processing)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/Meilisearch/CompressionOptions.cs (3)
1-1: Unusedusing System;directive.The
Systemnamespace import appears unused in this file.-using System; - namespace Meilisearch
58-75: MissingBrotli()factory method for consistency.The enum includes
Brotlibut there's no corresponding static factory method likeGzip()andDeflate(). This creates an inconsistent API surface.Add a
Brotli()factory method:public static CompressionOptions Deflate(int minimumSizeBytes = 1400) => new CompressionOptions { Algorithm = CompressionAlgorithm.Deflate, MinimumSizeBytes = minimumSizeBytes }; + + /// <summary> + /// Creates compression options with Brotli compression enabled. + /// </summary> + /// <param name="minimumSizeBytes">Minimum payload size to compress. Default is 1400 bytes.</param> + /// <returns>Compression options configured for Brotli.</returns> + public static CompressionOptions Brotli(int minimumSizeBytes = 1400) => + new CompressionOptions + { + Algorithm = CompressionAlgorithm.Brotli, + MinimumSizeBytes = minimumSizeBytes + }; }
39-39: Consider validatingMinimumSizeBytesfor negative values.The property accepts any
intvalue, including negative numbers, which would cause unexpected behavior (all payloads would be compressed sincedata.Length >= negativeValueis always true).Consider adding validation in a setter or using
uint:- public int MinimumSizeBytes { get; set; } = 1400; + private int _minimumSizeBytes = 1400; + + public int MinimumSizeBytes + { + get => _minimumSizeBytes; + set => _minimumSizeBytes = value < 0 ? 0 : value; + }src/Meilisearch/Compression/CompressionHelper.cs (1)
78-97: Potential duplicateContent-Encodingheader if original content has one.When copying headers from original content (line 85), if the original already has a
Content-Encodingheader, it gets copied first. ThenClear()is called (line 90) and the new encoding is added. This works correctly, but consider that theTryAddWithoutValidationmight add multiple values to Content-Encoding before clearing.For clarity, consider excluding
Content-EncodingandContent-Lengthfrom the copied headers since they're explicitly set afterward:// Copy headers from original content foreach (var header in originalContent.Headers) { + // Skip headers that will be explicitly set + if (header.Key.Equals("Content-Encoding", StringComparison.OrdinalIgnoreCase) || + header.Key.Equals("Content-Length", StringComparison.OrdinalIgnoreCase)) + { + continue; + } compressedContent.Headers.TryAddWithoutValidation(header.Key, header.Value); } // Set Content-Encoding header var contentEncoding = GetContentEncoding(algorithm); - compressedContent.Headers.ContentEncoding.Clear(); compressedContent.Headers.ContentEncoding.Add(contentEncoding); // Update Content-Length compressedContent.Headers.ContentLength = compressedBytes.Length;tests/Meilisearch.Tests/CompressionTests.cs (1)
156-166: Consider adding behavioral test for EnableResponseDecompression.While the default value is validated (line 165), there's no integration test exercising the EnableResponseDecompression option. If server-side compression support is available in your test environment, consider adding a test that verifies the Accept-Encoding header is set and compressed responses are handled correctly.
Example test structure:
[Fact] public async Task EnableResponseDecompression_ShouldRequestCompressedResponses() { // Arrange var options = new CompressionOptions { EnableResponseDecompression = true }; var client = new MeilisearchClient(_fixture.MeilisearchAddress(), "masterKey", options); // Act & Assert // Verify Accept-Encoding header is set and responses work correctly var version = await client.GetVersionAsync(); version.Should().NotBeNull(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(2 hunks)src/Meilisearch/Compression/CompressionHelper.cs(1 hunks)src/Meilisearch/CompressionOptions.cs(1 hunks)src/Meilisearch/MeilisearchClient.cs(2 hunks)src/Meilisearch/MeilisearchMessageHandler.cs(3 hunks)tests/Meilisearch.Tests/CompressionTests.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/Meilisearch/MeilisearchMessageHandler.cs (2)
src/Meilisearch/CompressionOptions.cs (3)
CompressionOptions(26-76)CompressionOptions(58-63)CompressionOptions(70-75)src/Meilisearch/Compression/CompressionHelper.cs (2)
CompressionHelper(12-211)IsAlgorithmSupported(186-210)
tests/Meilisearch.Tests/CompressionTests.cs (4)
tests/Meilisearch.Tests/ServerConfigs/BaseUriServer.cs (1)
BaseUriServer(7-104)src/Meilisearch/MeilisearchClient.cs (19)
Task(67-73)Task(95-108)Task(117-130)Task(139-149)Task(158-162)Task(171-174)Task(182-190)Task(198-211)Task(220-223)Task(231-237)Task(245-249)Task(257-260)Task(270-278)Task(285-289)Task(296-301)MeilisearchClient(19-495)MeilisearchClient(37-42)MeilisearchClient(51-59)Index(81-86)src/Meilisearch/CompressionOptions.cs (3)
CompressionOptions(26-76)CompressionOptions(58-63)CompressionOptions(70-75)tests/Meilisearch.Tests/Movie.cs (1)
Movie(6-13)
src/Meilisearch/MeilisearchClient.cs (3)
src/Meilisearch/CompressionOptions.cs (3)
CompressionOptions(26-76)CompressionOptions(58-63)CompressionOptions(70-75)src/Meilisearch/MeilisearchMessageHandler.cs (4)
MeilisearchMessageHandler(15-120)MeilisearchMessageHandler(23-26)MeilisearchMessageHandler(33-37)MeilisearchMessageHandler(45-49)src/Meilisearch/Extensions/HttpExtensions.cs (2)
AddApiKeyToHeader(72-78)AddDefaultUserAgent(80-85)
🔇 Additional comments (19)
README.md (1)
392-456: Documentation is comprehensive and well-structured.The new compression documentation section clearly explains supported algorithms, platform requirements, usage examples, and when to use compression. The performance notes and threshold rationale are helpful.
src/Meilisearch/MeilisearchMessageHandler.cs (2)
57-85: Compression handling is well-implemented.The implementation correctly validates algorithm support before attempting compression, provides helpful error messages, and properly awaits the async compression operation with
ConfigureAwait(false).
87-99: Accept-Encoding header setup looks correct.The conditional compilation for Brotli support and the clearing of existing headers before adding new ones is appropriate. However, consider that if the server sends a compressed response, the HttpClient may not automatically decompress it unless
HttpClientHandler.AutomaticDecompressionis configured.Verify that response decompression works as expected. The
EnableResponseDecompressionoption adds Accept-Encoding headers, but actual decompression requiresHttpClientHandler.AutomaticDecompressionto be set. Is this intentional behavior (delegating to the server's response handling) or should automatic decompression be enabled on the handler?src/Meilisearch/Compression/CompressionHelper.cs (4)
20-36: Implementation correctly handles compression flow.The logic properly checks
ShouldCompress, then validates size threshold before compressing. The use ofConfigureAwait(false)is appropriate for library code.
102-113: Gzip compression implementation is correct.Uses
GZipStreamwithCompressionLevel.Fastestfor optimal performance. TheleaveOpen: trueparameter and explicitFlush()before disposing ensures all data is written to the output stream.
120-139: Good platform-specific handling for Deflate.The documentation correctly notes that Meilisearch expects zlib format (RFC 1950) and that
ZLibStreamis only available in .NET 6.0+. The error message provides clear guidance for users.
145-163: Brotli compression with appropriate platform guards.The preprocessor directives correctly guard the
BrotliStreamusage, and the error message is informative.src/Meilisearch/MeilisearchClient.cs (2)
25-28: Property addition looks good.The
CompressionOptionsproperty is properly documented and provides read-only access to the configured compression settings.
37-41: Constructor correctly configures compression for default HttpClient.The compression options are properly passed to
MeilisearchMessageHandlerwhen creating the defaultHttpClient.tests/Meilisearch.Tests/CompressionTests.cs (10)
1-27: LGTM! Well-structured test class setup.The test class follows xUnit best practices with proper fixture injection, collection-based test organization, and async lifecycle management. The InitializeAsync cleanup ensures test isolation.
53-76: LGTM! Platform-specific exception handling is correctly tested.The test properly validates that Deflate compression throws NotSupportedException on netstandard2.0 targets, with clear explanatory comments.
78-103: LGTM! Update operation with compression is properly tested.The test validates that compression works correctly for document updates, ensuring the feature is compatible with all document operations.
105-121: LGTM! Backward compatibility is validated.This test confirms that the compression feature is truly opt-in and doesn't break existing client usage patterns.
123-154: LGTM! Comprehensive threshold testing.The theory test validates various threshold configurations including the edge case (0) and recommended default (1400), ensuring flexible configuration options work correctly.
156-166: LGTM! Default values are properly validated.This unit test ensures the CompressionOptions defaults are correctly set, establishing the API contract.
168-188: LGTM! Factory methods are properly tested.Both Gzip and Deflate factory methods are validated, with the Deflate test also verifying custom parameter handling. The absence of a Brotli factory method is intentional given platform limitations.
190-217: LGTM! Brotli platform limitation is properly tested.The test validates that Brotli compression correctly throws NotSupportedException with an appropriate error message on unsupported platforms.
219-251: LGTM! Large payload compression is thoroughly validated.This test confirms that compression works correctly with realistic, sizable payloads and that decompression on the server side retrieves the correct data without corruption.
36-36: No action needed. The hardcoded"masterKey"is the standard test configuration across the Meilisearch .NET SDK and matches the test infrastructure setup.The fixture itself defines
private const string ApiKey = "masterKey";, and all tests are designed to work with the Docker-based test environment configured with--master-key=masterKey. This approach is consistent with Meilisearch's standard testing practices and is not a portability or configuration concern.
| public MeilisearchClient(HttpClient client, string apiKey = default, CompressionOptions compressionOptions = default) | ||
| { | ||
| client.BaseAddress = client.BaseAddress.OriginalString.ToSafeUri(); | ||
| _http = client; | ||
| _http.AddApiKeyToHeader(apiKey); | ||
| _http.AddDefaultUserAgent(); | ||
| ApiKey = apiKey; | ||
| CompressionOptions = compressionOptions; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compression options are stored but not applied when using custom HttpClient.
When a user provides their own HttpClient, the compressionOptions parameter is stored in the property but not actually used for compression. The custom HttpClient may have a different handler pipeline that doesn't include MeilisearchMessageHandler with compression.
This could lead to unexpected behavior where users configure compression but it silently doesn't work.
Consider one of these approaches:
-
Document the limitation - Make it clear that compression only works with the URL-based constructor.
-
Validate and warn - If compression is requested with a custom HttpClient, throw or log a warning.
-
Wrap the custom HttpClient - Create a new handler chain that includes compression:
public MeilisearchClient(HttpClient client, string apiKey = default, CompressionOptions compressionOptions = default)
{
+ if (compressionOptions != null && compressionOptions.Algorithm != CompressionAlgorithm.None)
+ {
+ throw new ArgumentException(
+ "Compression options cannot be used with a custom HttpClient. " +
+ "Use the URL-based constructor or configure compression in your custom handler.",
+ nameof(compressionOptions));
+ }
client.BaseAddress = client.BaseAddress.OriginalString.ToSafeUri();
_http = client;Or update the XML documentation to clarify:
/// <param name="client">Injects the reusable HttpClient.</param>
/// <param name="apiKey">API Key to connect to the Meilisearch server. Best practice is to use HttpClient default header rather than this parameter.</param>
- /// <param name="compressionOptions">Compression configuration options.</param>
+ /// <param name="compressionOptions">Compression configuration options. Note: This parameter is stored for reference only when using a custom HttpClient. To enable compression with a custom HttpClient, configure it in your handler pipeline.</param>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public MeilisearchClient(HttpClient client, string apiKey = default, CompressionOptions compressionOptions = default) | |
| { | |
| client.BaseAddress = client.BaseAddress.OriginalString.ToSafeUri(); | |
| _http = client; | |
| _http.AddApiKeyToHeader(apiKey); | |
| _http.AddDefaultUserAgent(); | |
| ApiKey = apiKey; | |
| CompressionOptions = compressionOptions; | |
| public MeilisearchClient(HttpClient client, string apiKey = default, CompressionOptions compressionOptions = default) | |
| { | |
| if (compressionOptions != null && compressionOptions.Algorithm != CompressionAlgorithm.None) | |
| { | |
| throw new ArgumentException( | |
| "Compression options cannot be used with a custom HttpClient. " + | |
| "Use the URL-based constructor or configure compression in your custom handler.", | |
| nameof(compressionOptions)); | |
| } | |
| client.BaseAddress = client.BaseAddress.OriginalString.ToSafeUri(); | |
| _http = client; | |
| _http.AddApiKeyToHeader(apiKey); | |
| _http.AddDefaultUserAgent(); | |
| ApiKey = apiKey; | |
| CompressionOptions = compressionOptions; |
🤖 Prompt for AI Agents
In src/Meilisearch/MeilisearchClient.cs around lines 51 to 58, the constructor
stores CompressionOptions when a custom HttpClient is provided but never applies
compression because the custom client's handler pipeline may not include
MeilisearchMessageHandler; fix by detecting when compressionOptions != null and
either (a) validate and log/throw: if the provided HttpClient's handler chain
does not include MeilisearchMessageHandler, throw an ArgumentException or log a
clear warning that compression will be ignored, or (b) wrap the custom client:
create a new HttpMessageHandler pipeline that inserts MeilisearchMessageHandler
(configured with CompressionOptions) delegating to the original client's primary
handler, then construct a new HttpClient with that pipeline to replace _http;
also update the constructor XML docs to state the chosen behavior (limitation or
automatic wrapping).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vaceslav thanks for your PR
Warn about the limitation seems to be a good first approach, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
- Remove unused 'using System;' directive from CompressionOptions.cs - Add Brotli() factory method for API consistency with Gzip() and Deflate() - Add validation for MinimumSizeBytes property to prevent negative values - Optimize CreateCompressedContent to skip copying Content-Encoding and Content-Length headers - All compression tests pass (13/13) - Code style compliant with dotnet format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/Meilisearch/Compression/CompressionHelper.cs (1)
118-122: Documentation inconsistency: Deflate RFC reference differs across files.This comment correctly states "Meilisearch expects zlib format (RFC 1950)", but in
CompressionOptions.csline 14, theDeflateenum member is documented as "Deflate compression (RFC 1951)". RFC 1951 describes raw DEFLATE, while RFC 1950 describes zlib, which wraps DEFLATE with a header and checksum. The enum documentation should clarify that the Deflate option uses zlib format.Consider updating the enum documentation in
CompressionOptions.cs:-/// <summary>Deflate compression (RFC 1951).</summary> +/// <summary>Deflate compression with zlib wrapper (RFC 1950). Requires .NET 6.0+.</summary> Deflate = 2,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Meilisearch/Compression/CompressionHelper.cs(1 hunks)src/Meilisearch/CompressionOptions.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Meilisearch/Compression/CompressionHelper.cs (1)
src/Meilisearch/CompressionOptions.cs (4)
CompressionOptions(24-100)CompressionOptions(70-75)CompressionOptions(82-87)CompressionOptions(94-99)
🔇 Additional comments (7)
src/Meilisearch/Compression/CompressionHelper.cs (4)
41-52: LGTM!The predicate methods correctly implement applicability checks with appropriate null handling and threshold comparison.
57-100: LGTM!The compression dispatcher correctly delegates to algorithm-specific implementations, and the compressed content creation properly handles header propagation while updating Content-Encoding and Content-Length.
105-166: LGTM!The algorithm-specific compression methods follow a consistent pattern, use appropriate platform guards, and provide helpful error messages for unsupported runtimes. The use of
CompressionLevel.Fastestis appropriate for request compression where latency matters.
171-213: LGTM!The utility methods correctly map algorithms to HTTP header values and provide runtime capability checks that align with the conditional compilation guards in the implementation methods.
src/Meilisearch/CompressionOptions.cs (3)
6-19: LGTM!The enum is well-structured with explicit values and clear documentation. The platform requirement note for Brotli is helpful.
Note: The Deflate RFC reference inconsistency has been addressed in the
CompressionHelper.csreview.
24-32: LGTM!The class structure is appropriate for a configuration type, with sensible defaults aligning with the PR objectives (1400-byte threshold).
63-99: LGTM!The factory methods provide convenient configuration with consistent patterns. The
Noneproperty correctly returns a new instance each time to prevent shared mutable state.
Critical Fixes: - Fix HttpContent stream consumption bug where content was returned after being read - Reconstruct ByteArrayContent when below compression threshold to prevent stream errors - Copy headers to reconstructed content to maintain consistency Improvements: - Add validation in factory methods (Gzip, Deflate, Brotli) for better exception context - Exception now reports correct parameter name 'minimumSizeBytes' instead of 'value' - Update Deflate enum documentation to clarify RFC 1950 (zlib) vs RFC 1951 (raw deflate) - Add .NET 6.0+ requirement note to Deflate enum documentation All 13 compression tests passing Code style compliant with dotnet format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Meilisearch/CompressionOptions.cs (1)
40-51: Past review concern addressed in factory methods.The factory methods now include proper validation with
nameof(minimumSizeBytes), which addresses the previous review comment. The setter's use ofnameof(value)remains technically correct for direct property assignment scenarios.
🧹 Nitpick comments (1)
src/Meilisearch/CompressionOptions.cs (1)
60-63: Consider caching theNoneinstance if immutability is enforced.Currently, each access creates a new instance. This is acceptable given the class is mutable. If you later make properties init-only (C# 9+), you could cache this as
public static CompressionOptions None { get; } = new CompressionOptions();for minor efficiency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Meilisearch/Compression/CompressionHelper.cs(1 hunks)src/Meilisearch/CompressionOptions.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Meilisearch/Compression/CompressionHelper.cs
🔇 Additional comments (2)
src/Meilisearch/CompressionOptions.cs (2)
6-19: Well-structured enum with explicit values and platform documentation.Good practice assigning explicit values to prevent accidental breaking changes. The XML docs correctly document the .NET version requirements for each algorithm.
70-117: Clean factory method pattern with proper validation.The consistent structure across
Gzip,Deflate, andBrotliprovides a clear, discoverable API. The validation logic is correctly implemented with appropriate parameter names in exceptions.
Summary
This PR adds opt-in HTTP request compression support to reduce bandwidth usage and improve performance when uploading large document payloads.
Features
Implementation Details
New Files
CompressionOptions.cs- Configuration class with factory methodsCompression/CompressionHelper.cs- Platform-aware compression logicCompressionTests.cs- Comprehensive test suiteModified Files
MeilisearchMessageHandler.cs- Request interception and compressionMeilisearchClient.cs- Constructor overloads with CompressionOptionsREADME.md- Usage documentation and examplesTesting
Usage Example
Compliance
Platform Support
Related
This addresses the need for bandwidth optimization when uploading large document collections, especially beneficial for:
Summary by CodeRabbit
New Features
Documentation
Tests