[web_app] Minor clean up to API and HTTP related files

Author: parlough

pkg/web_app/lib/src/api_client/_rpc.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 import 'dart:html';
+
 import 'package:_pub_shared/pubapi.dart';
 
 import '../_dom_helper.dart';
@@ -17,7 +18,7 @@ Future<R?> rpc<R>({
 
   /// The async RPC call. If this throws, the error will be displayed as a modal
   /// popup, and then it will be re-thrown (or `onError` will be called).
-  Future<R?> Function()? fn,
+  required Future<R?> Function() fn,
 
   /// Message to show when the RPC returns without exceptions.
   required Element? successMessage,
@@ -37,12 +38,12 @@ Future<R?> rpc<R>({
     return null;
   }
 
-  // capture keys
+  // Capture key down events.
   final keyDownSubscription = window.onKeyDown.listen((event) {
     event.preventDefault();
     event.stopPropagation();
   });
-  // disable inputs and buttons that are not already disabled
+  // Disable inputs and buttons that are not already disabled.
   final inputs = document
       .querySelectorAll('input')
       .cast<InputElement>()
@@ -67,22 +68,21 @@ Future<R?> rpc<R>({
   }
 
   R? result;
-  Exception? error;
-  String? errorMessage;
+  ({Exception exception, String message})? error;
   try {
-    result = await fn!();
+    result = await fn();
   } on RequestException catch (e) {
     final asJson = e.bodyAsJson();
     if (e.status == 401 && asJson.containsKey('go')) {
-      final location = e.bodyAsJson()['go'] as String;
+      final location = asJson['go'] as String;
       final locationUri = Uri.tryParse(location);
       if (locationUri != null && locationUri.toString().isNotEmpty) {
         await cancelSpinner();
-        final errorObject = e.bodyAsJson()['error'] as Map?;
+        final errorObject = asJson['error'] as Map?;
         final message = errorObject?['message'];
         await modalMessage(
           'Further consent needed.',
-          Element.p()
+          ParagraphElement()
             ..text = [
               if (message != null) message,
               'You will be redirected, please authorize the action.',
@@ -101,21 +101,25 @@ Future<R?> rpc<R>({
         return null;
       }
     }
-    error = e;
-    errorMessage = _requestExceptionMessage(e) ?? 'Unexpected error: $e';
+    error = (
+      exception: e,
+      message: _requestExceptionMessage(asJson) ?? 'Unexpected error: $e'
+    );
   } catch (e) {
-    error = Exception('Unexpected error: $e');
-    errorMessage = 'Unexpected error: $e';
+    error = (
+      exception: Exception('Unexpected error: $e'),
+      message: 'Unexpected error: $e'
+    );
   } finally {
     await cancelSpinner();
   }
 
   if (error != null) {
-    await modalMessage('Error', await markdown(errorMessage!));
+    await modalMessage('Error', await markdown(error.message));
     if (onError != null) {
-      return await onError(error);
+      return await onError(error.exception);
     } else {
-      throw error;
+      throw error.exception;
     }
   }
 
@@ -128,37 +132,18 @@ Future<R?> rpc<R>({
   return result;
 }
 
-String? _requestExceptionMessage(RequestException e) {
-  try {
-    final map = e.bodyAsJson();
-    String? message;
-
-    if (map['error'] is Map) {
-      final errorMap = map['error'] as Map;
-      if (errorMap['message'] is String) {
-        message = errorMap['message'] as String;
-      }
-    }
-
-    // TODO: remove after the server is migrated to returns only `{'error': {'message': 'XX'}}`
-    if (message == null && map['message'] is String) {
-      message = map['message'] as String;
-    }
-
-    // TODO: check if we ever send responses like this and remove if not
-    if (message == null && map['error'] is String) {
-      message = map['error'] as String;
-    }
-
-    return message;
-  } on FormatException catch (_) {
-    // ignore bad body
-  }
-  return null;
-}
-
-Element _createSpinner() => Element.div()
+String? _requestExceptionMessage(Map<String, Object?> jsonBody) =>
+    switch (jsonBody) {
+      {'error': {'message': final String errorMessage}} => errorMessage,
+      // TODO: Remove after the server is migrated to return only `{'error': {'message': 'XX'}}`.
+      {'message': final String errorMessage} => errorMessage,
+      // TODO: Check if we ever send responses like this and remove if not.
+      {'error': final String errorMessage} => errorMessage,
+      _ => null,
+    };
+
+Element _createSpinner() => DivElement()
   ..className = 'spinner-frame'
   ..children = [
-    Element.div()..className = 'spinner',
+    DivElement()..className = 'spinner',
   ];

pkg/web_app/lib/src/api_client/api_client.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:html';
+
 import 'package:_pub_shared/pubapi.dart';
 import 'package:api_builder/_client_utils.dart';
 
@@ -24,18 +25,18 @@ PubApiClient get unauthenticatedClient =>
     PubApiClient(_baseUrl, client: http.Client());
 
 /// The pub API client to use with account credentials.
-PubApiClient get client {
-  return PubApiClient(_baseUrl, client: http.createClientWithCsrf());
-}
+PubApiClient get client =>
+    PubApiClient(_baseUrl, client: http.createClientWithCsrf());
 
-/// Sends a JSON request to the [path] endpoint using [verb] method with [body] content.
+/// Sends a JSON request to the [path] endpoint using
+/// [verb] method with [body] content.
 ///
 /// Sets the `Content-Type` header to `application/json; charset="utf-8` and
 /// expects a valid JSON response body.
-Future<Map<String, dynamic>> sendJson({
+Future<Map<String, Object?>> sendJson({
   required String verb,
   required String path,
-  required Map<String, dynamic>? body,
+  required Map<String, Object?>? body,
 }) async {
   final client = http.createClientWithCsrf();
   try {

pkg/web_app/lib/src/deferred/http.dart

@@ -4,60 +4,42 @@
 
 import 'dart:html';
 
+import 'package:collection/collection.dart' show IterableExtension;
 import 'package:http/browser_client.dart';
 import 'package:http/http.dart';
 
 export 'package:http/http.dart';
 
 /// Creates an authenticated [Client] that extends request with the CSRF header.
-Client createClientWithCsrf() {
-  return _AuthenticatedClient();
-}
+Client createClientWithCsrf() => _AuthenticatedClient();
 
-String? _getCsrfMetaContent() {
-  final values = document.head
-      ?.querySelectorAll('meta[name="csrf-token"]')
-      .map((e) => e.attributes['content'])
-      .nonNulls
-      .toList();
-  if (values == null || values.isEmpty) {
-    return null;
-  }
-  return values.first.trim();
-}
+String? get _csrfMetaContent => document.head
+    ?.querySelectorAll('meta[name="csrf-token"]')
+    .map((e) => e.getAttribute('content'))
+    .firstWhereOrNull((tokenContent) => tokenContent != null)
+    ?.trim();
 
 /// An HTTP [Client] which sends custom headers alongside each request:
 ///
 ///  - `x-pub-csrf-token` header when the HTML document's `<head>` contains the
 ///    `<meta name="csrf-token" content="<token>">` element.
 class _AuthenticatedClient extends _BrowserClient {
-  _AuthenticatedClient()
-      : super(
-          getHeadersFn: () async {
-            final csrfToken = _getCsrfMetaContent();
-            return {
-              if (csrfToken != null && csrfToken.isNotEmpty)
-                'x-pub-csrf-token': csrfToken,
-            };
-          },
-        );
+  @override
+  Future<Map<String, String>> get _sendHeaders async => {
+        if (_csrfMetaContent case final csrfToken? when csrfToken.isNotEmpty)
+          'x-pub-csrf-token': csrfToken,
+      };
 }
 
 /// An [Client] which updates the headers for each request.
-class _BrowserClient extends BrowserClient {
-  final Future<Map<String, String>> Function() getHeadersFn;
-  final _client = BrowserClient();
-  _BrowserClient({
-    required this.getHeadersFn,
-  });
+abstract class _BrowserClient extends BrowserClient {
+  final BrowserClient _client = BrowserClient();
 
   @override
   Future<StreamedResponse> send(BaseRequest request) async {
-    final headers = await getHeadersFn();
-
     final modifiedRequest = _RequestImpl.fromRequest(
       request,
-      updateHeaders: headers,
+      updateHeaders: await _sendHeaders,
     );
 
     return await _client.send(modifiedRequest);
@@ -67,6 +49,8 @@ class _BrowserClient extends BrowserClient {
   void close() {
     _client.close();
   }
+
+  Future<Map<String, String>> get _sendHeaders;
 }
 
 class _RequestImpl extends BaseRequest {

pkg/web_app/lib/src/deferred/markdown.dart

@@ -8,7 +8,7 @@ import 'package:markdown/markdown.dart' as md;
 
 /// Creates an [Element] with Markdown-formatted content.
 Future<Element> markdown(String text) async {
-  return Element.div()
+  return DivElement()
     ..setInnerHtml(
       md.markdownToHtml(text),
       validator: NodeValidator(uriPolicy: _UnsafeUriPolicy()),