-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Convert TestServer to pipes #11611
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
Convert TestServer to pipes #11611
Conversation
private readonly ResponseBodyPipeWriter _responseWriter; | ||
private readonly Func<bool> _allowSynchronousIO; | ||
|
||
public ResponseBodyWriterStream(ResponseBodyPipeWriter responseWriter, Func<bool> allowSynchronousIO) |
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.
Why not have this ctor take the return value of responseWriter.AsStream()
instead of copying all the AsStream()
logic? Turning the ResponseBodyWriterStream into a decorator will probably be less fragile since any fixes/improvements to the PipeWriterStream will get picked up automatically.
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.
Maybe if there were anything novel in PipeWriterStream, but there isn't. I still have to override just as many Stream APIs. https://github.com/dotnet/corefx/blob/master/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeWriterStream.cs
Justin was right, deriving from PipeWriterStream would have been the right thing if it were public.
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.
We can make it public, file an issue though.
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.
#11598 This makes response.BodyWriter a first class feature in TestServer rather than an adapter. There was already an internal pipe, but exposing it required a wrapper with added semantics such as FirstWrite for flushing headers.