feat: Check that all AndroidManifest.xml files have the INTERNET permission (#219)

Co-authored-by: Felix Angelov <felix@shorebird.dev>

packages/shorebird_cli/lib/src/commands/doctor_command.dart

@@ -1,8 +1,7 @@
-import 'dart:io';
-
 import 'package:mason_logger/mason_logger.dart';
-import 'package:path/path.dart' as p;
 import 'package:shorebird_cli/src/command.dart';
+import 'package:shorebird_cli/src/doctor/doctor_validator.dart';
+import 'package:shorebird_cli/src/doctor/validators/validators.dart';
 import 'package:shorebird_cli/src/shorebird_version_mixin.dart';
 import 'package:shorebird_cli/src/version.dart';
 
@@ -14,7 +13,21 @@ import 'package:shorebird_cli/src/version.dart';
 /// {@endtemplate}
 class DoctorCommand extends ShorebirdCommand with ShorebirdVersionMixin {
   /// {@macro doctor_command}
-  DoctorCommand({required super.logger, super.runProcess});
+  DoctorCommand({
+    required super.logger,
+    List<DoctorValidator>? validators,
+    super.runProcess,
+  }) {
+    this.validators = validators ??
+        <DoctorValidator>[
+          ShorebirdVersionValidator(
+            isShorebirdVersionCurrent: isShorebirdVersionCurrent,
+          ),
+          AndroidInternetPermissionValidator(),
+        ];
+  }
+
+  late final List<DoctorValidator> validators;
 
   @override
   String get name => 'doctor';
@@ -24,26 +37,29 @@ class DoctorCommand extends ShorebirdCommand with ShorebirdVersionMixin {
 
   @override
   Future<int> run() async {
-    var numIssues = 0;
-    final workingDirectory = p.dirname(Platform.script.toFilePath());
     logger.info('''
-Doctor summary
 
 Shorebird v$packageVersion
 ''');
 
-    final isShorebirdUpToDate = await isShorebirdVersionCurrent(
-      workingDirectory: workingDirectory,
-    );
+    var numIssues = 0;
+    for (final validator in validators) {
+      final progress = logger.progress(validator.description);
+      final issues = await validator.validate();
+      numIssues += issues.length;
+      if (issues.isEmpty) {
+        progress.complete();
+      } else {
+        progress.fail();
 
-    if (!isShorebirdUpToDate) {
-      numIssues += 1;
-      logger.info('''
-A new version of shorebird is available!
-Run `shorebird upgrade` to upgrade.
-''');
+        for (final issue in issues) {
+          logger.info('  ${issue.displayMessage}');
+        }
+      }
     }
 
+    logger.info('');
+
     if (numIssues == 0) {
       logger.info('No issues detected!');
     } else {

packages/shorebird_cli/lib/src/doctor/doctor_validator.dart

@@ -0,0 +1,71 @@
+import 'package:mason_logger/mason_logger.dart';
+import 'package:meta/meta.dart';
+
+/// Severity level of a [ValidationIssue].
+///
+/// [error]s should be fixed before continuing development.
+/// [warning]s should be fixed before releasing your app, but are not as urgent.
+enum ValidationIssueSeverity {
+  error,
+  warning,
+}
+
+/// Display helpers for printing [ValidationIssue]s.
+extension Display on ValidationIssueSeverity {
+  String get leading {
+    switch (this) {
+      case ValidationIssueSeverity.error:
+        return red.wrap('[✗]')!;
+      case ValidationIssueSeverity.warning:
+        return yellow.wrap('[!]')!;
+    }
+  }
+}
+
+/// A (potential) problem with the current Shorebird installation or project.
+@immutable
+class ValidationIssue {
+  const ValidationIssue({required this.severity, required this.message});
+
+  /// How important it is to fix this issue.
+  final ValidationIssueSeverity severity;
+
+  /// A description of the issue.
+  final String message;
+
+  /// A console-friendly description of this issue.
+  String? get displayMessage {
+    return '${severity.leading} $message';
+  }
+
+  // coverage:ignore-start
+  @override
+  String toString() => '$severity $message';
+  // coverage:ignore-end
+
+  @override
+  bool operator ==(Object other) {
+    if (identical(this, other)) return true;
+
+    return other is ValidationIssue &&
+        other.severity == severity &&
+        other.message == message;
+  }
+
+  // coverage:ignore-start
+  @override
+  int get hashCode => Object.hashAll([severity, message]);
+  // coverage:ignore-end
+}
+
+/// Checks for a specific issue with either the Shorebird installation or the
+/// current Shorebird project.
+abstract class DoctorValidator {
+  /// A one-sentence explanation of what this validator is checking.
+  String get description;
+
+  /// Checks for [ValidationIssue]s.
+  ///
+  /// Returns an empty list if no issues are found.
+  Future<List<ValidationIssue>> validate();
+}

packages/shorebird_cli/lib/src/doctor/validators/android_internet_permission_validator.dart

@@ -0,0 +1,69 @@
+import 'dart:io';
+
+import 'package:path/path.dart' as p;
+import 'package:shorebird_cli/src/doctor/doctor_validator.dart';
+import 'package:xml/xml.dart';
+
+/// Checks that all AndroidManifest.xml files in android/app/src/{flavor}/
+/// contain the INTERNET permission, which is required for Shorebird to work.
+///
+/// See https://github.com/shorebirdtech/shorebird/issues/160.
+class AndroidInternetPermissionValidator extends DoctorValidator {
+  // coverage:ignore-start
+  @override
+  String get description =>
+      'AndroidManifest.xml files contain INTERNET permission';
+  // coverage:ignore-end
+
+  @override
+  Future<List<ValidationIssue>> validate() async {
+    const manifestFileName = 'AndroidManifest.xml';
+    final androidSrcDir = Directory(
+      p.join(
+        Directory.current.path,
+        'android',
+        'app',
+        'src',
+      ),
+    );
+    final manifestsWithoutInternetPermission = androidSrcDir
+        .listSync()
+        .whereType<Directory>()
+        .where((dir) {
+          return dir.listSync().whereType<File>().any(
+                (file) => p.basename(file.path) == 'AndroidManifest.xml',
+              );
+        })
+        .map((e) => p.join(e.path, manifestFileName))
+        .where((manifest) => !_androidManifestHasInternetPermission(manifest));
+
+    if (manifestsWithoutInternetPermission.isNotEmpty) {
+      return manifestsWithoutInternetPermission
+          .map(
+            (String manifestPath) => ValidationIssue(
+              severity: ValidationIssueSeverity.error,
+              message: '$manifestPath is missing the INTERNET permission.',
+            ),
+          )
+          .toList();
+    }
+
+    return [];
+  }
+
+  bool _androidManifestHasInternetPermission(String path) {
+    final xmlDocument = XmlDocument.parse(File(path).readAsStringSync());
+    return xmlDocument.rootElement.childElements
+        .any(_isInternetPermissionElement);
+  }
+
+  bool _isInternetPermissionElement(XmlElement element) {
+    if (element.localName != 'uses-permission') {
+      return false;
+    }
+
+    final attribute = element.attributes.first;
+    return attribute.qualifiedName == 'android:name' &&
+        attribute.value == 'android.permission.INTERNET';
+  }
+}

packages/shorebird_cli/lib/src/doctor/validators/shorebird_version_validator.dart

@@ -0,0 +1,39 @@
+import 'dart:io';
+
+import 'package:path/path.dart' as p;
+import 'package:shorebird_cli/src/doctor/doctor_validator.dart';
+
+/// Verifies that the currently installed version of Shorebird is the latest.
+class ShorebirdVersionValidator extends DoctorValidator {
+  ShorebirdVersionValidator({required this.isShorebirdVersionCurrent});
+
+  final Future<bool> Function({required String workingDirectory})
+      isShorebirdVersionCurrent;
+
+  // coverage:ignore-start
+  @override
+  String get description => 'Shorebird is up-to-date';
+  // coverage:ignore-end
+
+  @override
+  Future<List<ValidationIssue>> validate() async {
+    final workingDirectory = p.dirname(Platform.script.toFilePath());
+    final isShorebirdUpToDate = await isShorebirdVersionCurrent(
+      workingDirectory: workingDirectory,
+    );
+
+    if (!isShorebirdUpToDate) {
+      return [
+        const ValidationIssue(
+          severity: ValidationIssueSeverity.warning,
+          message: '''
+A new version of shorebird is available!
+Run `shorebird upgrade` to upgrade.
+''',
+        )
+      ];
+    }
+
+    return [];
+  }
+}

packages/shorebird_cli/lib/src/doctor/validators/validators.dart

@@ -0,0 +1,2 @@
+export 'android_internet_permission_validator.dart';
+export 'shorebird_version_validator.dart';

packages/shorebird_cli/pubspec.yaml

@@ -25,6 +25,7 @@ dependencies:
   pubspec_parse: ^1.2.2
   shorebird_code_push_client:
     path: ../shorebird_code_push_client
+  xml: ^6.2.2
   yaml: ^3.1.1
   yaml_edit: ^2.1.0
 

packages/shorebird_cli/test/src/commands/doctor/validators/android_internet_permission_validator_test.dart

@@ -0,0 +1,131 @@
+import 'dart:io';
+
+import 'package:collection/collection.dart';
+import 'package:path/path.dart' as p;
+import 'package:shorebird_cli/src/doctor/doctor_validator.dart';
+import 'package:shorebird_cli/src/doctor/validators/validators.dart';
+import 'package:test/test.dart';
+
+void main() {
+  const manifestWithInternetPermission = '''
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="dev.shorebird.u_shorebird_clock">
+    <uses-permission android:name="android.permission.INTERNET"/>
+</manifest>
+''';
+
+  const manifestWithCommentedOutInternetPermission = '''
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="dev.shorebird.u_shorebird_clock">
+    <!-- <uses-permission android:name="android.permission.INTERNET"/> -->
+</manifest>
+''';
+
+  const manifestWithNonInternetPermissions = '''
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="dev.shorebird.u_shorebird_clock">
+    <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION"/>
+</manifest>
+''';
+
+  const manifestWithNoPermissions = '''
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="dev.shorebird.u_shorebird_clock">
+</manifest>
+''';
+
+  group('AndroidInternetPermissionValidator', () {
+    Directory createTempDir() => Directory.systemTemp.createTempSync();
+
+    void writeManifestToPath(String manifestContents, String path) {
+      Directory(path).createSync(recursive: true);
+      File(p.join(path, 'AndroidManifest.xml'))
+          .writeAsStringSync(manifestContents);
+    }
+
+    test(
+      'returns successful result if all AndroidManifest.xml files have the '
+      'INTERNET permission',
+      () async {
+        final tempDirectory = createTempDir();
+        writeManifestToPath(
+          manifestWithInternetPermission,
+          p.join(tempDirectory.path, 'android/app/src/debug'),
+        );
+        writeManifestToPath(
+          manifestWithInternetPermission,
+          p.join(tempDirectory.path, 'android/app/src/main'),
+        );
+
+        final results = await IOOverrides.runZoned(
+          () => AndroidInternetPermissionValidator().validate(),
+          getCurrentDirectory: () => tempDirectory,
+        );
+
+        expect(results.map((res) => res.severity), isEmpty);
+      },
+    );
+
+    test(
+      'returns separate errors for all AndroidManifest.xml files without the '
+      'INTERNET permission',
+      () async {
+        final tempDirectory = createTempDir();
+        final manifestPaths = [
+          'internet_permission',
+          'debug',
+          'main',
+          'profile',
+        ]
+            .map(
+              (dir) => p.join(
+                tempDirectory.path,
+                'android',
+                'app',
+                'src',
+                dir,
+              ),
+            )
+            .toList();
+        final badManifestPaths = manifestPaths.slice(1);
+
+        writeManifestToPath(
+          manifestWithInternetPermission,
+          manifestPaths[0],
+        );
+        writeManifestToPath(
+          manifestWithCommentedOutInternetPermission,
+          manifestPaths[1],
+        );
+        writeManifestToPath(
+          manifestWithNonInternetPermissions,
+          manifestPaths[2],
+        );
+        writeManifestToPath(
+          manifestWithNoPermissions,
+          manifestPaths[3],
+        );
+
+        final results = await IOOverrides.runZoned(
+          () => AndroidInternetPermissionValidator().validate(),
+          getCurrentDirectory: () => tempDirectory,
+        );
+
+        expect(results, hasLength(3));
+
+        expect(
+          results,
+          containsAll(
+            badManifestPaths.map(
+              (path) => ValidationIssue(
+                severity: ValidationIssueSeverity.error,
+                message:
+                    '$path/AndroidManifest.xml is missing the INTERNET permission.',
+              ),
+            ),
+          ),
+        );
+      },
+    );
+  });
+}

packages/shorebird_cli/test/src/commands/doctor/validators/shorebird_version_validator_test.dart

@@ -0,0 +1,90 @@
+import 'dart:io';
+
+import 'package:mason_logger/mason_logger.dart';
+import 'package:mocktail/mocktail.dart';
+import 'package:shorebird_cli/src/commands/doctor_command.dart';
+import 'package:shorebird_cli/src/doctor/doctor_validator.dart';
+import 'package:shorebird_cli/src/doctor/validators/shorebird_version_validator.dart';
+import 'package:test/test.dart';
+
+class _MockLogger extends Mock implements Logger {}
+
+class _MockProcessResult extends Mock implements ProcessResult {}
+
+void main() {
+  const currentShorebirdRevision = 'revision-1';
+  const newerShorebirdRevision = 'revision-2';
+
+  group('ShorebirdVersionValidator', () {
+    late ShorebirdVersionValidator validator;
+    late Logger logger;
+    late DoctorCommand command;
+    late ProcessResult fetchCurrentVersionResult;
+    late ProcessResult fetchLatestVersionResult;
+
+    setUp(() {
+      logger = _MockLogger();
+      fetchCurrentVersionResult = _MockProcessResult();
+      fetchLatestVersionResult = _MockProcessResult();
+
+      command = DoctorCommand(
+        logger: logger,
+        runProcess: (
+          executable,
+          arguments, {
+          bool runInShell = false,
+          workingDirectory,
+        }) async {
+          if (executable == 'git') {
+            const revParseHead = ['rev-parse', '--verify', 'HEAD'];
+            if (arguments.every((arg) => revParseHead.contains(arg))) {
+              return fetchCurrentVersionResult;
+            }
+
+            const revParseUpstream = ['rev-parse', '--verify', '@{upstream}'];
+            if (arguments.every((arg) => revParseUpstream.contains(arg))) {
+              return fetchLatestVersionResult;
+            }
+          }
+          return _MockProcessResult();
+        },
+      );
+
+      validator = ShorebirdVersionValidator(
+        isShorebirdVersionCurrent: command.isShorebirdVersionCurrent,
+      );
+
+      when(
+        () => fetchCurrentVersionResult.exitCode,
+      ).thenReturn(ExitCode.success.code);
+      when(
+        () => fetchCurrentVersionResult.stdout,
+      ).thenReturn(currentShorebirdRevision);
+      when(
+        () => fetchLatestVersionResult.exitCode,
+      ).thenReturn(ExitCode.success.code);
+      when(
+        () => fetchLatestVersionResult.stdout,
+      ).thenReturn(currentShorebirdRevision);
+    });
+
+    test('returns no issues when shorebird is up-to-date', () async {
+      final results = await validator.validate();
+      expect(results, isEmpty);
+    });
+
+    test('returns a warning when a newer shorebird is available', () async {
+      when(
+        () => fetchLatestVersionResult.stdout,
+      ).thenReturn(newerShorebirdRevision);
+
+      final results = await validator.validate();
+      expect(results, hasLength(1));
+      expect(results.first.severity, ValidationIssueSeverity.warning);
+      expect(
+        results.first.message,
+        contains('A new version of shorebird is available!'),
+      );
+    });
+  });
+}

packages/shorebird_cli/test/src/commands/doctor_command_test.dart

@@ -1,83 +1,111 @@
-import 'dart:io';
-
 import 'package:mason_logger/mason_logger.dart';
 import 'package:mocktail/mocktail.dart';
 import 'package:shorebird_cli/src/commands/commands.dart';
+import 'package:shorebird_cli/src/doctor/doctor_validator.dart';
+import 'package:shorebird_cli/src/doctor/validators/validators.dart';
 import 'package:test/test.dart';
 
+class _MockShorebirdVersionValidator extends Mock
+    implements ShorebirdVersionValidator {}
+
+class _MockAndroidInternetPermissionValidator extends Mock
+    implements AndroidInternetPermissionValidator {}
+
 class _MockLogger extends Mock implements Logger {}
 
-class _MockProcessResult extends Mock implements ProcessResult {}
+class _MockProgress extends Mock implements Progress {}
 
 void main() {
-  const currentShorebirdRevision = 'revision-1';
-  const newerShorebirdRevision = 'revision-2';
-
   group('doctor', () {
     late Logger logger;
+    late Progress progress;
     late DoctorCommand command;
-    late ProcessResult fetchCurrentVersionResult;
-    late ProcessResult fetchLatestVersionResult;
+    late AndroidInternetPermissionValidator androidInternetPermissionValidator;
+    late ShorebirdVersionValidator shorebirdVersionValidator;
 
     setUp(() {
       logger = _MockLogger();
-      fetchCurrentVersionResult = _MockProcessResult();
-      fetchLatestVersionResult = _MockProcessResult();
+      progress = _MockProgress();
+
+      androidInternetPermissionValidator =
+          _MockAndroidInternetPermissionValidator();
+      shorebirdVersionValidator = _MockShorebirdVersionValidator();
 
       command = DoctorCommand(
         logger: logger,
-        runProcess: (
-          executable,
-          arguments, {
-          bool runInShell = false,
-          workingDirectory,
-        }) async {
-          if (executable == 'git') {
-            const revParseHead = ['rev-parse', '--verify', 'HEAD'];
-            if (arguments.every((arg) => revParseHead.contains(arg))) {
-              return fetchCurrentVersionResult;
-            }
-
-            const revParseUpstream = ['rev-parse', '--verify', '@{upstream}'];
-            if (arguments.every((arg) => revParseUpstream.contains(arg))) {
-              return fetchLatestVersionResult;
-            }
-          }
-          return _MockProcessResult();
-        },
+        validators: [
+          androidInternetPermissionValidator,
+          shorebirdVersionValidator,
+        ],
       );
 
-      when(
-        () => fetchCurrentVersionResult.exitCode,
-      ).thenReturn(ExitCode.success.code);
-      when(
-        () => fetchCurrentVersionResult.stdout,
-      ).thenReturn(currentShorebirdRevision);
-      when(
-        () => fetchLatestVersionResult.exitCode,
-      ).thenReturn(ExitCode.success.code);
-      when(
-        () => fetchLatestVersionResult.stdout,
-      ).thenReturn(currentShorebirdRevision);
+      when(() => logger.progress(any())).thenReturn(progress);
+
+      when(() => logger.info(any())).thenReturn(null);
+
+      when(() => androidInternetPermissionValidator.description)
+          .thenReturn('Android');
+      when(() => androidInternetPermissionValidator.validate())
+          .thenAnswer((_) async => []);
+
+      when(() => shorebirdVersionValidator.description)
+          .thenReturn('Shorebird Version');
+      when(() => shorebirdVersionValidator.validate())
+          .thenAnswer((_) async => []);
     });
 
     test('prints "no issues" when everything is OK', () async {
       await command.run();
+      for (final validator in command.validators) {
+        verify(validator.validate).called(1);
+      }
       verify(
         () => logger.info(captureAny(that: contains('No issues detected'))),
       ).called(1);
     });
 
-    test('prints that an upgrade is available', () async {
+    test('prints messages when warnings or errors found', () async {
       when(
-        () => fetchLatestVersionResult.stdout,
-      ).thenReturn(newerShorebirdRevision);
+        () => androidInternetPermissionValidator.validate(),
+      ).thenAnswer(
+        (_) async => [
+          const ValidationIssue(
+            severity: ValidationIssueSeverity.warning,
+            message: 'oh no!',
+          ),
+          const ValidationIssue(
+            severity: ValidationIssueSeverity.error,
+            message: 'OH NO!',
+          ),
+        ],
+      );
 
       await command.run();
+
+      for (final validator in command.validators) {
+        verify(validator.validate).called(1);
+      }
+
       verify(
         () => logger.info(
           captureAny(
-            that: contains('A new version of shorebird is available!'),
+            that: contains('[!] oh no!'),
+          ),
+        ),
+      ).called(1);
+
+      verify(
+        () => logger.info(
+          captureAny(
+            that: contains('[✗] OH NO!'),
+          ),
+        ),
+      ).called(1);
+
+      verify(
+        () => logger.info(
+          captureAny(
+            that: contains('2 issues detected.'),
           ),
         ),
       ).called(1);