#2039 Buffer request body and copy request body to downstreams during multiplexing

ThreeMammals / Ocelot

.NET API Gateway

https://www.nuget.org/packages/Ocelot

MIT License

8.35k stars 1.63k forks source link

#2039 Buffer request body and copy request body to downstreams during multiplexing #2044

Closed PaulARoy closed 5 months ago

PaulARoy commented 5 months ago

Closes #2039

2039

Buffers the body of the request in some cases and copy the buffered body to the downstreams.

Proposed Changes

Creates a MemoryStream buffer for the body when Content-Length > 0
Copy the buffer to the downstream requests through a MemoryStream
Fixes broken tests due to CreateThreadContext name and parameter changes + acceptance test on multiple body forwarding
Updates the body stream in-place

WIP

Decide wether we want to implement more complex buffer logic BufferingMiddleware.BufferBodyAsync:L87
Decide wether we want to use a MemoryStream for downstream body MultiplexingMiddleware.CopyBufferToTargetRequestAsync:L253

PaulARoy commented 5 months ago

Hello, Thanks, please add some acceptance tests.

Acceptance test added for multiplexer body copy (tested — wasn't passing before modifications and passes after).

I wonder if I could add as acceptance test for buffering, taking suggestions as it's more of an internal mechanism.

PaulARoy commented 5 months ago

All tests on Visual Studio (+ dotnet cake) pass on my machine and the failing test from CI (should_reload_config_on_change) doesn't look like something I touched. Maybe the new CI PR I saw? I have the dotnet cake log file if needed.

raman-m commented 5 months ago

@PaulARoy wrote

No need to worry about the should_reload_config_on_change test; it's an unstable one due to the file system's intensive operations within the Docker environment. The build should not fail following the next commit. If tests pass locally in VS but fail on CircleCI, consider triggering a new build asking me or making a minor commit—useful or not—as a DevOps CircleCI workaround.

The build is 🟢 now

raman-m commented 5 months ago

@PaulARoy It appears that you’ve used the PaulARoy:develop branch as a feature branch which is wrong approach. Here’s the recommended approach:

Ensure that your forked repository has an identical develop branch to the original ThreeMammals:develop branch of the head repository.
When working on multiple tickets or features, create a new branch from the develop branch. This will prevent any potential blocking issues.

While it may not be critical for single-ticket work, following this practice will help you avoid complications when dealing with multiple feature branches.

raman-m commented 5 months ago

Follow up PR is #2050