-
Notifications
You must be signed in to change notification settings - Fork 295
Add HTTP Response Compression Support #3003
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
Co-authored-by: JerryNixon <[email protected]>
Co-authored-by: JerryNixon <[email protected]>
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.
Pull request overview
This PR adds HTTP response compression support to Data API Builder, enabling Gzip and Brotli compression for REST, GraphQL, and MCP endpoints to reduce payload sizes and improve performance over slow networks.
Key Changes:
- Added configurable compression middleware with three levels:
optimal(default),fastest, andnone - Implemented JSON schema, CLI commands, and configuration model for compression settings
- Added unit tests for CLI configuration updates
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Service/Startup.cs | Adds ConfigureResponseCompression method to register Gzip and Brotli providers, and calls UseResponseCompression middleware when compression is enabled |
| src/Config/ObjectModel/CompressionOptions.cs | Defines the CompressionOptions record with Level property and UserProvidedLevel flag for JSON serialization control |
| src/Config/ObjectModel/CompressionLevel.cs | Defines the CompressionLevel enum with Optimal, Fastest, and None values |
| src/Config/Converters/CompressionOptionsConverterFactory.cs | Implements custom JSON converter for reading and writing compression configuration with validation |
| src/Config/RuntimeConfigLoader.cs | Registers the CompressionOptionsConverterFactory in the serialization options |
| src/Config/ObjectModel/RuntimeOptions.cs | Adds Compression property to runtime configuration options |
| src/Cli/ConfigGenerator.cs | Implements CLI configuration update logic for compression level settings |
| src/Cli/Commands/ConfigureOptions.cs | Adds RuntimeCompressionLevel command-line option for the dab configure command |
| src/Cli.Tests/ConfigureOptionsTests.cs | Adds test cases for all three compression levels (Optimal, Fastest, None) |
| schemas/dab.draft.schema.json | Adds JSON schema definition for the compression configuration section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
| } | ||
| } |
Copilot
AI
Dec 4, 2025
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.
The converter doesn't properly handle unknown properties with complex values (objects or arrays). After reading a property name on line 58, if the property is not "level", the code continues the loop without skipping the property value. This works for simple values but could cause parsing issues if an unknown property contains an object or array.
Add a default case to properly skip unknown properties:
if (string.Equals(propertyName, "level", StringComparison.OrdinalIgnoreCase))
{
string? levelStr = reader.GetString();
if (levelStr is not null)
{
if (Enum.TryParse<CompressionLevel>(levelStr, ignoreCase: true, out CompressionLevel parsedLevel))
{
level = parsedLevel;
userProvidedLevel = true;
}
else
{
throw new JsonException($"Invalid compression level: '{levelStr}'. Valid values are: optimal, fastest, none.");
}
}
}
else
{
// Skip unknown properties
reader.Skip();
}| } | |
| else | |
| { | |
| // Skip unknown properties and their values (including objects/arrays) | |
| reader.Skip(); | |
| } |
| /// <summary> | ||
| /// Tests that running "dab configure --runtime.compression.level {value}" on a config with various values results | ||
| /// in runtime config update. Takes in updated value for compression.level and | ||
| /// validates whether the runtime config reflects those updated values |
Copilot
AI
Dec 4, 2025
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.
Missing closing period in the documentation comment. The summary should end with a period for consistency with other test documentation in the file.
| /// validates whether the runtime config reflects those updated values | |
| /// validates whether the runtime config reflects those updated values. |
| private void ConfigureResponseCompression(IServiceCollection services, RuntimeConfig? runtimeConfig) | ||
| { | ||
| CompressionLevel compressionLevel = runtimeConfig?.Runtime?.Compression?.Level ?? CompressionOptions.DEFAULT_LEVEL; | ||
|
|
||
| // Only configure compression if level is not None | ||
| if (compressionLevel == CompressionLevel.None) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| System.IO.Compression.CompressionLevel systemCompressionLevel = compressionLevel switch | ||
| { | ||
| CompressionLevel.Fastest => System.IO.Compression.CompressionLevel.Fastest, | ||
| CompressionLevel.Optimal => System.IO.Compression.CompressionLevel.Optimal, | ||
| _ => System.IO.Compression.CompressionLevel.Optimal | ||
| }; | ||
|
|
||
| services.AddResponseCompression(options => | ||
| { | ||
| options.EnableForHttps = true; | ||
| options.Providers.Add<Microsoft.AspNetCore.ResponseCompression.GzipCompressionProvider>(); | ||
| options.Providers.Add<Microsoft.AspNetCore.ResponseCompression.BrotliCompressionProvider>(); | ||
| }); | ||
|
|
||
| services.Configure<Microsoft.AspNetCore.ResponseCompression.GzipCompressionProviderOptions>(options => | ||
| { | ||
| options.Level = systemCompressionLevel; | ||
| }); | ||
|
|
||
| services.Configure<Microsoft.AspNetCore.ResponseCompression.BrotliCompressionProviderOptions>(options => | ||
| { | ||
| options.Level = systemCompressionLevel; | ||
| }); | ||
|
|
||
| _logger.LogInformation("Response compression enabled with level '{compressionLevel}' for REST, GraphQL, and MCP endpoints.", compressionLevel); | ||
| } |
Copilot
AI
Dec 4, 2025
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.
The response compression middleware lacks integration test coverage. While unit tests verify the CLI configuration, there are no tests validating that:
- The compression middleware is actually registered when configured
- Responses are compressed when clients send
Accept-Encodingheaders - Compression is disabled when the level is set to
None - Different compression levels (Optimal vs Fastest) behave as expected
Consider adding integration tests similar to CorrelationIdTests.cs that verify the middleware behavior with actual HTTP requests and responses.
Why make this change?
Closes #2711
DAB does not currently support response compression (gzip/brotli), which can reduce payload sizes by ~79% for large JSON responses. This is especially beneficial for REST and GraphQL endpoints over slow or high-latency networks.
What is this change?
Adds response compression middleware with configurable compression levels.
Configuration Model:
CompressionLevelenum:optimal,fastest,noneCompressionOptionsrecord with JSON converter and validationoptimalRuntime Behavior:
none)Schema & CLI:
{ "runtime": { "compression": { "level": "optimal" } } }How was this tested?
Added 3 tests in
ConfigureOptionsTests.cscovering all compression levels.Sample Request(s)
CLI Configuration:
Sample Request/Response:
Startup Output (when enabled):
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.