From 3bfbfef513953cc0c944c9b63c0448d3a278f723 Mon Sep 17 00:00:00 2001 From: Luis Correa Date: Sat, 27 Jan 2024 23:32:21 -0300 Subject: [PATCH 1/4] feat(dart_frog): allow streamed response --- packages/dart_frog/lib/src/response.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/dart_frog/lib/src/response.dart b/packages/dart_frog/lib/src/response.dart index 52cc4da07..799ea4c56 100644 --- a/packages/dart_frog/lib/src/response.dart +++ b/packages/dart_frog/lib/src/response.dart @@ -24,11 +24,13 @@ class Response { int statusCode = 200, Stream>? body, Map? headers, + bool useBufferOutput = true, }) : this._( shelf.Response( statusCode, body: body, headers: headers, + context: {'shelf.io.buffer_output': useBufferOutput}, ), ); From dd35fc90223be8a5958d8c078dd3b1c3bc1e9865 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Mon, 29 Jan 2024 13:22:20 +0000 Subject: [PATCH 2/4] docs(dart_frog): included documentation about useBufferOutput --- packages/dart_frog/lib/src/response.dart | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/dart_frog/lib/src/response.dart b/packages/dart_frog/lib/src/response.dart index 799ea4c56..052c196c0 100644 --- a/packages/dart_frog/lib/src/response.dart +++ b/packages/dart_frog/lib/src/response.dart @@ -20,6 +20,16 @@ class Response { ); /// Create a [Response] with a stream of bytes. + /// + /// If [useBufferOutput] is `true` (the default), streamed responses will be + /// buffered to improve performance. If `false`, all chunks will be pushed + /// over the wire as they're received. Note that, disabling buffering of the + /// output can result in very poor performance, when writing many small + /// chunks. + /// + /// See also: + /// + /// * [HttpResponse.bufferOutput](https://api.flutter.dev/flutter/dart-io/HttpResponse/bufferOutput.html) Response.stream({ int statusCode = 200, Stream>? body, From 0fbb88b64e59d8043f34978ce2f58288fa030e32 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Mon, 29 Jan 2024 13:58:25 +0000 Subject: [PATCH 3/4] test: useBufferOutput --- packages/dart_frog/lib/src/_internal.dart | 1 + packages/dart_frog/lib/src/response.dart | 21 +++++++++- .../dart_frog/test/src/response_test.dart | 40 +++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/packages/dart_frog/lib/src/_internal.dart b/packages/dart_frog/lib/src/_internal.dart index bea82913f..d1f503e3e 100644 --- a/packages/dart_frog/lib/src/_internal.dart +++ b/packages/dart_frog/lib/src/_internal.dart @@ -6,6 +6,7 @@ import 'dart:io'; import 'package:dart_frog/dart_frog.dart'; import 'package:dart_frog/src/body_parsers/body_parsers.dart'; import 'package:http_methods/http_methods.dart' show isHttpMethod; +import 'package:meta/meta.dart'; import 'package:shelf/shelf.dart' as shelf; import 'package:shelf/shelf_io.dart' as shelf_io; diff --git a/packages/dart_frog/lib/src/response.dart b/packages/dart_frog/lib/src/response.dart index 052c196c0..4c151d381 100644 --- a/packages/dart_frog/lib/src/response.dart +++ b/packages/dart_frog/lib/src/response.dart @@ -18,6 +18,7 @@ class Response { encoding: encoding, ), ); + Response._(this._response); /// Create a [Response] with a stream of bytes. /// @@ -40,7 +41,9 @@ class Response { statusCode, body: body, headers: headers, - context: {'shelf.io.buffer_output': useBufferOutput}, + context: !useBufferOutput + ? {Response.shelfUseBufferOutputContextKey: useBufferOutput} + : null, ), ); @@ -92,7 +95,15 @@ class Response { encoding: encoding, ); - Response._(this._response); + /// A [shelf.Response.context] key used to determine if if the + /// [HttpResponse.bufferOutput] should be enabled or disabled. + /// + /// See also: + /// + /// * [shelf_io library](https://pub.dev/documentation/shelf/latest/shelf_io/shelf_io-library.html) + /// * [HttpResponse.bufferOutput](https://api.flutter.dev/flutter/dart-io/HttpResponse/bufferOutput.html) + @visibleForTesting + static const shelfUseBufferOutputContextKey = 'shelf.io.buffer_output'; shelf.Response _response; @@ -103,6 +114,12 @@ class Response { /// The returned map is unmodifiable. Map get headers => _response.headers; + /// Extra context that can be used by for middleware and handlers. + /// + /// The value is immutable. + @visibleForTesting + Map get context => _response.context; + /// Returns a [Stream] representing the body. Stream> bytes() => _response.read(); diff --git a/packages/dart_frog/test/src/response_test.dart b/packages/dart_frog/test/src/response_test.dart index 0e9e48b87..94bc9d2d1 100644 --- a/packages/dart_frog/test/src/response_test.dart +++ b/packages/dart_frog/test/src/response_test.dart @@ -107,6 +107,46 @@ void main() { final response = Response.stream(body: stream); expect(response.bytes(), emits(equals(bytes))); }); + + group('useBufferOutput', () { + test('is omitted by default', () { + final response = Response.stream( + body: const Stream.empty(), + // ignore: avoid_redundant_argument_values + useBufferOutput: true, + ); + + expect( + response.context, + isNot(contains(Response.shelfUseBufferOutputContextKey)), + reason: + '''The context should not have the '${Response.shelfUseBufferOutputContextKey}' key.''', + ); + }); + + test('can be disabled', () { + final response = Response.stream( + body: const Stream.empty(), + useBufferOutput: false, + ); + + expect( + response.context, + contains(Response.shelfUseBufferOutputContextKey), + reason: + '''The context should have the '${Response.shelfUseBufferOutputContextKey}' key.''', + ); + + final useBufferOutput = + response.context[Response.shelfUseBufferOutputContextKey]; + expect( + useBufferOutput, + isFalse, + reason: + '''The '${Response.shelfUseBufferOutputContextKey}' should be 'false' when disabled.''', + ); + }); + }); }); group('formData', () { From a805aa161cbdaf0aed6d0f6f7b02cc2b43781ce6 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Mon, 29 Jan 2024 14:03:05 +0000 Subject: [PATCH 4/4] refactor: renamed to bufferOutput --- packages/dart_frog/lib/src/response.dart | 10 ++++----- .../dart_frog/test/src/response_test.dart | 22 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/dart_frog/lib/src/response.dart b/packages/dart_frog/lib/src/response.dart index 4c151d381..ca5a5466c 100644 --- a/packages/dart_frog/lib/src/response.dart +++ b/packages/dart_frog/lib/src/response.dart @@ -22,7 +22,7 @@ class Response { /// Create a [Response] with a stream of bytes. /// - /// If [useBufferOutput] is `true` (the default), streamed responses will be + /// If [bufferOutput] is `true` (the default), streamed responses will be /// buffered to improve performance. If `false`, all chunks will be pushed /// over the wire as they're received. Note that, disabling buffering of the /// output can result in very poor performance, when writing many small @@ -35,14 +35,14 @@ class Response { int statusCode = 200, Stream>? body, Map? headers, - bool useBufferOutput = true, + bool bufferOutput = true, }) : this._( shelf.Response( statusCode, body: body, headers: headers, - context: !useBufferOutput - ? {Response.shelfUseBufferOutputContextKey: useBufferOutput} + context: !bufferOutput + ? {Response.shelfBufferOutputContextKey: bufferOutput} : null, ), ); @@ -103,7 +103,7 @@ class Response { /// * [shelf_io library](https://pub.dev/documentation/shelf/latest/shelf_io/shelf_io-library.html) /// * [HttpResponse.bufferOutput](https://api.flutter.dev/flutter/dart-io/HttpResponse/bufferOutput.html) @visibleForTesting - static const shelfUseBufferOutputContextKey = 'shelf.io.buffer_output'; + static const shelfBufferOutputContextKey = 'shelf.io.buffer_output'; shelf.Response _response; diff --git a/packages/dart_frog/test/src/response_test.dart b/packages/dart_frog/test/src/response_test.dart index 94bc9d2d1..c8c70a39f 100644 --- a/packages/dart_frog/test/src/response_test.dart +++ b/packages/dart_frog/test/src/response_test.dart @@ -108,42 +108,42 @@ void main() { expect(response.bytes(), emits(equals(bytes))); }); - group('useBufferOutput', () { + group('bufferOutput', () { test('is omitted by default', () { final response = Response.stream( body: const Stream.empty(), // ignore: avoid_redundant_argument_values - useBufferOutput: true, + bufferOutput: true, ); expect( response.context, - isNot(contains(Response.shelfUseBufferOutputContextKey)), + isNot(contains(Response.shelfBufferOutputContextKey)), reason: - '''The context should not have the '${Response.shelfUseBufferOutputContextKey}' key.''', + '''The context should not have the '${Response.shelfBufferOutputContextKey}' key.''', ); }); test('can be disabled', () { final response = Response.stream( body: const Stream.empty(), - useBufferOutput: false, + bufferOutput: false, ); expect( response.context, - contains(Response.shelfUseBufferOutputContextKey), + contains(Response.shelfBufferOutputContextKey), reason: - '''The context should have the '${Response.shelfUseBufferOutputContextKey}' key.''', + '''The context should have the '${Response.shelfBufferOutputContextKey}' key.''', ); - final useBufferOutput = - response.context[Response.shelfUseBufferOutputContextKey]; + final bufferOutput = + response.context[Response.shelfBufferOutputContextKey]; expect( - useBufferOutput, + bufferOutput, isFalse, reason: - '''The '${Response.shelfUseBufferOutputContextKey}' should be 'false' when disabled.''', + '''The '${Response.shelfBufferOutputContextKey}' should be 'false' when disabled.''', ); }); });