Exclude null query vars by default and add new @method annotation inc…

…ludeNullQueryVars (#372)

chopper/lib/src/annotations.dart

@@ -169,12 +169,39 @@ class Method {
   /// hxxp://path/to/script?user[name]=john&user[surname]=doe&user[age]=21
   final bool useBrackets;
 
+  /// Set to [true] to include query variables with null values. This includes nested maps.
+  /// The default is to exclude them.
+  ///
+  /// NOTE: Empty strings are always included.
+  ///
+  /// ```dart
+  /// @Get(
+  ///   path: '/script',
+  ///   includeNullQueryVars: true,
+  /// )
+  /// Future<Response<String>> getData({
+  ///   @Query('foo') String? foo,
+  ///   @Query('bar') String? bar,
+  ///   @Query('baz') String? baz,
+  /// });
+  ///
+  /// final response = await service.getData(
+  ///   foo: 'foo_val',
+  ///   bar: null, // omitting it would have the same effect
+  ///   baz: 'baz_val',
+  /// );
+  /// ```
+  ///
+  /// The above code produces hxxp://path/to/script&foo=foo_var&bar=&baz=baz_var
+  final bool includeNullQueryVars;
+
   const Method(
     this.method, {
     this.optionalBody = false,
     this.path = '',
     this.headers = const {},
     this.useBrackets = false,
+    this.includeNullQueryVars = false,
   });
 }
 
@@ -186,6 +213,7 @@ class Get extends Method {
     super.path,
     super.headers,
     super.useBrackets,
+    super.includeNullQueryVars,
   }) : super(HttpMethod.Get);
 }
 
@@ -199,6 +227,7 @@ class Post extends Method {
     super.path,
     super.headers,
     super.useBrackets,
+    super.includeNullQueryVars,
   }) : super(HttpMethod.Post);
 }
 
@@ -210,6 +239,7 @@ class Delete extends Method {
     super.path,
     super.headers,
     super.useBrackets,
+    super.includeNullQueryVars,
   }) : super(HttpMethod.Delete);
 }
 
@@ -223,6 +253,7 @@ class Put extends Method {
     super.path,
     super.headers,
     super.useBrackets,
+    super.includeNullQueryVars,
   }) : super(HttpMethod.Put);
 }
 
@@ -235,6 +266,7 @@ class Patch extends Method {
     super.path,
     super.headers,
     super.useBrackets,
+    super.includeNullQueryVars,
   }) : super(HttpMethod.Patch);
 }
 
@@ -246,6 +278,7 @@ class Head extends Method {
     super.path,
     super.headers,
     super.useBrackets,
+    super.includeNullQueryVars,
   }) : super(HttpMethod.Head);
 }
 
@@ -256,6 +289,7 @@ class Options extends Method {
     super.path,
     super.headers,
     super.useBrackets,
+    super.includeNullQueryVars,
   }) : super(HttpMethod.Options);
 }
 

chopper/lib/src/request.dart

@@ -14,6 +14,7 @@ class Request extends http.BaseRequest {
   final bool multipart;
   final List<PartValue> parts;
   final bool useBrackets;
+  final bool includeNullQueryVars;
 
   Request(
     String method,
@@ -25,9 +26,16 @@ class Request extends http.BaseRequest {
     this.multipart = false,
     this.parts = const [],
     this.useBrackets = false,
+    this.includeNullQueryVars = false,
   }) : super(
           method,
-          buildUri(origin, path, parameters, useBrackets: useBrackets),
+          buildUri(
+            origin,
+            path,
+            parameters,
+            useBrackets: useBrackets,
+            includeNullQueryVars: includeNullQueryVars,
+          ),
         ) {
     this.headers.addAll(headers);
   }
@@ -44,6 +52,7 @@ class Request extends http.BaseRequest {
     this.multipart = false,
     this.parts = const [],
     this.useBrackets = false,
+    this.includeNullQueryVars = false,
   })  : origin = url.origin,
         path = url.path,
         parameters = {...url.queryParametersAll, ...?parameters},
@@ -54,6 +63,7 @@ class Request extends http.BaseRequest {
             url.path,
             {...url.queryParametersAll, ...?parameters},
             useBrackets: useBrackets,
+            includeNullQueryVars: includeNullQueryVars,
           ),
         ) {
     this.headers.addAll(headers);
@@ -70,6 +80,7 @@ class Request extends http.BaseRequest {
     bool? multipart,
     List<PartValue>? parts,
     bool? useBrackets,
+    bool? includeNullQueryVars,
   }) =>
       Request(
         method ?? this.method,
@@ -81,6 +92,7 @@ class Request extends http.BaseRequest {
         multipart: multipart ?? this.multipart,
         parts: parts ?? this.parts,
         useBrackets: useBrackets ?? this.useBrackets,
+        includeNullQueryVars: includeNullQueryVars ?? this.includeNullQueryVars,
       );
 
   /// Builds a valid URI from [baseUrl], [url] and [parameters].
@@ -92,14 +104,19 @@ class Request extends http.BaseRequest {
     String url,
     Map<String, dynamic> parameters, {
     bool useBrackets = false,
+    bool includeNullQueryVars = false,
   }) {
     // If the request's url is already a fully qualified URL, we can use it
     // as-is and ignore the baseUrl.
     final Uri uri = url.startsWith('http://') || url.startsWith('https://')
         ? Uri.parse(url)
         : Uri.parse('${baseUrl.strip('/')}/${url.leftStrip('/')}');
 
-    final String query = mapToQuery(parameters, useBrackets: useBrackets);
+    final String query = mapToQuery(
+      parameters,
+      useBrackets: useBrackets,
+      includeNullQueryVars: includeNullQueryVars,
+    );
 
     return query.isNotEmpty
         ? uri.replace(query: uri.hasQuery ? '${uri.query}&$query' : query)

chopper/lib/src/utils.dart

@@ -56,13 +56,22 @@ final chopperLogger = Logger('Chopper');
 /// Creates a valid URI query string from [map].
 ///
 /// E.g., `{'foo': 'bar', 'ints': [ 1337, 42 ] }` will become 'foo=bar&ints=1337&ints=42'.
-String mapToQuery(Map<String, dynamic> map, {bool useBrackets = false}) =>
-    _mapToQuery(map, useBrackets: useBrackets).join('&');
+String mapToQuery(
+  Map<String, dynamic> map, {
+  bool useBrackets = false,
+  bool includeNullQueryVars = false,
+}) =>
+    _mapToQuery(
+      map,
+      useBrackets: useBrackets,
+      includeNullQueryVars: includeNullQueryVars,
+    ).join('&');
 
 Iterable<_Pair<String, String>> _mapToQuery(
   Map<String, dynamic> map, {
   String? prefix,
   bool useBrackets = false,
+  bool includeNullQueryVars = false,
 }) {
   final Set<_Pair<String, String>> pairs = {};
 
@@ -80,15 +89,22 @@ Iterable<_Pair<String, String>> _mapToQuery(
         pairs.addAll(_iterableToQuery(name, value, useBrackets: useBrackets));
       } else if (value is Map<String, dynamic>) {
         pairs.addAll(
-          _mapToQuery(value, prefix: name, useBrackets: useBrackets),
+          _mapToQuery(
+            value,
+            prefix: name,
+            useBrackets: useBrackets,
+            includeNullQueryVars: includeNullQueryVars,
+          ),
         );
       } else {
         pairs.add(
           _Pair<String, String>(name, _normalizeValue(value)),
         );
       }
     } else {
-      pairs.add(_Pair<String, String>(name, ''));
+      if (includeNullQueryVars) {
+        pairs.add(_Pair<String, String>(name, ''));
+      }
     }
   });
 

chopper/test/base_test.dart

@@ -110,7 +110,7 @@ void main() {
       final httpClient = MockClient((request) async {
         expect(
           request.url.toString(),
-          equals('$baseUrl/test/query?name=&int=&default_value='),
+          equals('$baseUrl/test/query?name='),
         );
         expect(request.method, equals('GET'));
 
@@ -132,7 +132,7 @@ void main() {
       final httpClient = MockClient((request) async {
         expect(
           request.url.toString(),
-          equals('$baseUrl/test/query?name=&int=&default_value=42'),
+          equals('$baseUrl/test/query?name=&default_value=42'),
         );
         expect(request.method, equals('GET'));
 
@@ -923,6 +923,34 @@ void main() {
     );
   });
 
+  test('Include null query vars', () async {
+    final httpClient = MockClient((request) async {
+      expect(
+        request.url.toString(),
+        equals('$baseUrl/test/query_param_include_null_query_vars'
+            '?foo=foo_val'
+            '&bar='
+            '&baz=baz_val'),
+      );
+      expect(request.method, equals('GET'));
+
+      return http.Response('get response', 200);
+    });
+
+    final chopper = buildClient(httpClient);
+    final service = chopper.getService<HttpTestService>();
+
+    final response = await service.getUsingQueryParamIncludeNullQueryVars(
+      foo: 'foo_val',
+      baz: 'baz_val',
+    );
+
+    expect(response.body, equals('get response'));
+    expect(response.statusCode, equals(200));
+
+    httpClient.close();
+  });
+
   test('List query param', () async {
     final httpClient = MockClient((request) async {
       expect(
@@ -1067,4 +1095,88 @@ void main() {
 
     httpClient.close();
   });
+
+  test('Map query param without including null query vars', () async {
+    final httpClient = MockClient((request) async {
+      expect(
+        request.url.toString(),
+        equals('$baseUrl/test/map_query_param'
+            '?value.bar=baz'
+            '&value.etc.abc=def'
+            '&value.etc.mno.opq=rst'
+            '&value.etc.mno.list=a'
+            '&value.etc.mno.list=123'
+            '&value.etc.mno.list=false'),
+      );
+      expect(request.method, equals('GET'));
+
+      return http.Response('get response', 200);
+    });
+
+    final chopper = buildClient(httpClient);
+    final service = chopper.getService<HttpTestService>();
+
+    final response = await service.getUsingMapQueryParam(<String, dynamic>{
+      'bar': 'baz',
+      'zap': null,
+      'etc': <String, dynamic>{
+        'abc': 'def',
+        'ghi': null,
+        'mno': <String, dynamic>{
+          'opq': 'rst',
+          'uvw': null,
+          'list': ['a', 123, false],
+        },
+      },
+    });
+
+    expect(response.body, equals('get response'));
+    expect(response.statusCode, equals(200));
+
+    httpClient.close();
+  });
+
+  test('Map query param including null query vars', () async {
+    final httpClient = MockClient((request) async {
+      expect(
+        request.url.toString(),
+        equals('$baseUrl/test/map_query_param_include_null_query_vars'
+            '?value.bar=baz'
+            '&value.zap='
+            '&value.etc.abc=def'
+            '&value.etc.ghi='
+            '&value.etc.mno.opq=rst'
+            '&value.etc.mno.uvw='
+            '&value.etc.mno.list=a'
+            '&value.etc.mno.list=123'
+            '&value.etc.mno.list=false'),
+      );
+      expect(request.method, equals('GET'));
+
+      return http.Response('get response', 200);
+    });
+
+    final chopper = buildClient(httpClient);
+    final service = chopper.getService<HttpTestService>();
+
+    final response = await service
+        .getUsingMapQueryParamIncludeNullQueryVars(<String, dynamic>{
+      'bar': 'baz',
+      'zap': null,
+      'etc': <String, dynamic>{
+        'abc': 'def',
+        'ghi': null,
+        'mno': <String, dynamic>{
+          'opq': 'rst',
+          'uvw': null,
+          'list': ['a', 123, false],
+        },
+      },
+    });
+
+    expect(response.body, equals('get response'));
+    expect(response.statusCode, equals(200));
+
+    httpClient.close();
+  });
 }