Add setting to compress HTTP request bodies#106
Add setting to compress HTTP request bodies#106bgrainger merged 7 commits intoFacilityApi:masterfrom
Conversation
This opt-in setting will cause the HttpContent to be compressed with "Content-Encoding: gzip".
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an opt-in setting to compress HTTP request bodies with gzip encoding. Key changes include:
- Adding a new setting/property to control request compression across HttpClientService.
- Implementing and integrating a helper method to compress HTTP content when enabled.
- Updating related defaults, settings, and tests to support the new behavior.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Facility.Core/Http/HttpClientService.cs | Implements request compression logic and integrates it into the HTTP request pipeline. |
| src/Facility.Core/Http/HttpClientServiceDefaults.cs | Adds a default property for compressing requests. |
| src/Facility.Core/Http/HttpClientServiceSettings.cs | Introduces the ShouldCompressRequest property and propagates it to clones. |
| tests/Facility.Core.UnitTests/Http/HttpClientServiceSettingsTests.cs | Updates tests to assign a compression callback for validation. |
|
Still needs a mechanism to set |
| /// </summary> | ||
| /// <remarks>Request bodies will be compressed with <c>Content-Encoding: gzip</c>. Even when this callback | ||
| /// returns <c>true</c>, the request may be sent uncompressed if compressing would make it larger.</remarks> | ||
| public Func<ServiceDto, bool>? ShouldCompressRequest { get; set; } |
There was a problem hiding this comment.
I'm open to just going with public bool? CompressRequests { get; set; } if the delegate is YAGNI.
There was a problem hiding this comment.
I can't think of an actual reason to enable compression on a per-request basis, so happy to switch to the simpler API.
There was a problem hiding this comment.
Great! We can always add this if we need it.
There was a problem hiding this comment.
I can't think of an actual reason to enable compression on a per-request basis
I thought of one: the API might know that POST /v1/smallData always has a very small payload (and shouldn't be compressed), but that POST /v1/bigData is large and always should be. This could still be YAGNI, though.
There was a problem hiding this comment.
Or that POST /v1/bigImage is already compressed and so shouldn't be compressed again.
A |
|
Multiple commits for code review; squash them when merging the PR. |
2 participants
This opt-in setting will cause the HttpContent to be compressed with
Content-Encoding: gzip.