fix(shorebird_cli): doctor should ignore non-main AndroidManifest.xml…

… files (#1439)
shorebirdtech · Oct 30, 2023 · 3b6c28d · 3b6c28d
1 parent 9eb0801
commit 3b6c28d
Show file tree

Hide file tree

Showing 2 changed files with 114 additions and 118 deletions.
diff --git a/packages/shorebird_cli/lib/src/validators/android_internet_permission_validator.dart b/packages/shorebird_cli/lib/src/validators/android_internet_permission_validator.dart
@@ -4,8 +4,8 @@ import 'package:path/path.dart' as p;
 import 'package:shorebird_cli/src/validators/validators.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.
+/// Checks that android/app/src/main/AndroidManifest.xml contains the INTERNET
+/// permission, which is required for Shorebird to work.
 ///
 /// See https://github.com/shorebirdtech/shorebird/issues/160.
 class AndroidInternetPermissionValidator extends Validator {
@@ -34,46 +34,30 @@ The command you are running must be run at the root of a Flutter app project tha
 
   @override
   Future<List<ValidationIssue>> validate() async {
-    const manifestFileName = 'AndroidManifest.xml';
-
-    final manifestFiles = _androidSrcDirectory
-        .listSync()
-        .whereType<Directory>()
-        .where(
-          (dir) => dir
-              .listSync()
-              .whereType<File>()
-              .any((file) => p.basename(file.path) == manifestFileName),
-        )
-        .map((e) => p.join(e.path, manifestFileName));
-
-    if (manifestFiles.isEmpty) {
+    final manifestFilePath = p.join(
+      _androidSrcDirectory.path,
+      'main',
+      'AndroidManifest.xml',
+    );
+    final manifestFile = File(manifestFilePath);
+    if (!manifestFile.existsSync()) {
       return [
         ValidationIssue(
           severity: ValidationIssueSeverity.error,
-          message:
-              '''No AndroidManifest.xml files found in ${_androidSrcDirectory.path}''',
+          message: '''No AndroidManifest.xml file found at $manifestFilePath''',
         ),
       ];
     }
 
-    final manifestsWithoutInternetPermission = manifestFiles
-        .where((manifest) => !_androidManifestHasInternetPermission(manifest));
-
-    if (manifestsWithoutInternetPermission.isNotEmpty) {
-      return manifestsWithoutInternetPermission.map(
-        (String manifestPath) {
-          return ValidationIssue(
-            severity: manifestPath.contains(mainAndroidManifestPath)
-                ? ValidationIssueSeverity.error
-                : ValidationIssueSeverity.warning,
-            message:
-                '${p.relative(manifestPath, from: Directory.current.path)} '
-                'is missing the INTERNET permission.',
-            fix: () => _addInternetPermissionToFile(manifestPath),
-          );
-        },
-      ).toList();
+    if (!_androidManifestHasInternetPermission(manifestFilePath)) {
+      return [
+        ValidationIssue(
+          severity: ValidationIssueSeverity.error,
+          message:
+              '$mainAndroidManifestPath is missing the INTERNET permission.',
+          fix: () => _addInternetPermissionToFile(manifestFilePath),
+        ),
+      ];
     }
 
     return [];

diff --git a/packages/shorebird_cli/test/src/validators/android_internet_permission_validator_test.dart b/packages/shorebird_cli/test/src/validators/android_internet_permission_validator_test.dart
@@ -1,6 +1,5 @@
 import 'dart:io';
 
-import 'package:collection/collection.dart';
 import 'package:path/path.dart' as p;
 import 'package:shorebird_cli/src/validators/validators.dart';
 import 'package:test/test.dart';
@@ -75,14 +74,9 @@ void main() {
     });
 
     test(
-      'returns successful result if all AndroidManifest.xml files have the '
-      'INTERNET permission',
+      '''returns successful result if the main AndroidManifest.xml file has 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'),
@@ -97,10 +91,10 @@ void main() {
       },
     );
 
-    test('returns an error if no AndroidManifest.xml files are found',
+    test('returns an error if AndroidManifest.xml file does not exist',
         () async {
       final tempDirectory = createTempDir();
-      Directory(p.join(tempDirectory.path, 'android', 'app', 'src', 'debug'))
+      Directory(p.join(tempDirectory.path, 'android', 'app', 'src', 'main'))
           .createSync(recursive: true);
 
       final results = await IOOverrides.runZoned(
@@ -112,106 +106,124 @@ void main() {
       expect(results.first.severity, ValidationIssueSeverity.error);
       expect(
         results.first.message,
-        startsWith('No AndroidManifest.xml files found in'),
+        startsWith('No AndroidManifest.xml file found at'),
       );
       expect(results.first.fix, isNull);
     });
 
-    test(
-      'returns separate errors for all AndroidManifest.xml files without the '
-      'INTERNET permission',
-      () async {
+    group('when the INTERNET permission is commented out', () {
+      test('returns error', () async {
         final tempDirectory = createTempDir();
-        final relativeManifestPaths = [
-          'internet_permission',
-          'debug',
-          'main',
-          'profile',
-        ]
-            .map(
-              (dir) => p.join(
-                'android',
-                'app',
-                'src',
-                dir,
-              ),
-            )
-            .toList();
-        final absoluteManifestPaths = relativeManifestPaths
-            .map((path) => p.join(tempDirectory.path, path))
-            .toList();
-        final badManifestPaths = relativeManifestPaths.slice(1);
+        final manifestPath =
+            p.join(tempDirectory.path, 'android', 'app', 'src', 'main');
 
-        writeManifestToPath(
-          manifestWithInternetPermission,
-          absoluteManifestPaths[0],
-        );
         writeManifestToPath(
           manifestWithCommentedOutInternetPermission,
-          absoluteManifestPaths[1],
+          manifestPath,
         );
-        writeManifestToPath(
-          manifestWithNonInternetPermissions,
-          absoluteManifestPaths[2],
+
+        final results = await IOOverrides.runZoned(
+          AndroidInternetPermissionValidator().validate,
+          getCurrentDirectory: () => tempDirectory,
         );
-        writeManifestToPath(
-          manifestWithNoPermissions,
-          absoluteManifestPaths[3],
+
+        expect(results, hasLength(1));
+        expect(
+          results.first,
+          equals(
+            const ValidationIssue(
+              severity: ValidationIssueSeverity.error,
+              message:
+                  '''android/app/src/main/AndroidManifest.xml is missing the INTERNET permission.''',
+            ),
+          ),
         );
+      });
+    });
+
+    group('when the INTERNET permission is missing', () {
+      test('returns error', () async {
+        final tempDirectory = createTempDir();
+        final manifestPath =
+            p.join(tempDirectory.path, 'android', 'app', 'src', 'main');
+
+        writeManifestToPath(manifestWithNoPermissions, manifestPath);
 
         final results = await IOOverrides.runZoned(
           AndroidInternetPermissionValidator().validate,
           getCurrentDirectory: () => tempDirectory,
         );
 
-        expect(results, hasLength(3));
+        expect(results, hasLength(1));
+        expect(
+          results.first,
+          equals(
+            const ValidationIssue(
+              severity: ValidationIssueSeverity.error,
+              message:
+                  '''android/app/src/main/AndroidManifest.xml is missing the INTERNET permission.''',
+            ),
+          ),
+        );
+      });
+    });
+
+    group('when manifest has non-INTERNET permissions', () {
+      test('returns error', () async {
+        final tempDirectory = createTempDir();
+        final manifestPath =
+            p.join(tempDirectory.path, 'android', 'app', 'src', 'main');
+
+        writeManifestToPath(
+          manifestWithNonInternetPermissions,
+          manifestPath,
+        );
+
+        final results = await IOOverrides.runZoned(
+          AndroidInternetPermissionValidator().validate,
+          getCurrentDirectory: () => tempDirectory,
+        );
 
+        expect(results, hasLength(1));
         expect(
-          results,
-          containsAll(
-            badManifestPaths.map(
-              (path) => ValidationIssue(
-                // The AndroidManifest.xml used for release builds is located at
-                // android/app/src/main/AndroidManifest.xml. This is the only
-                // manifest file that fail validation with an error if the
-                // INTERNET permission is missing.
-                severity: path.contains('main')
-                    ? ValidationIssueSeverity.error
-                    : ValidationIssueSeverity.warning,
-                message:
-                    '${p.join(path, 'AndroidManifest.xml')} is missing the '
-                    'INTERNET permission.',
-              ),
+          results.first,
+          equals(
+            const ValidationIssue(
+              severity: ValidationIssueSeverity.error,
+              message:
+                  '''android/app/src/main/AndroidManifest.xml is missing the INTERNET permission.''',
             ),
           ),
         );
-      },
-    );
+      });
+    });
 
-    test('fix() adds permission to manifest file', () async {
-      final tempDirectory = createTempDir();
-      writeManifestToPath(
-        manifestWithNonInternetPermissions,
-        p.join(tempDirectory.path, 'android', 'app', 'src', 'debug'),
-      );
+    group('fix', () {
+      test('adds permission to manifest file', () async {
+        final tempDirectory = createTempDir();
+        writeManifestToPath(
+          manifestWithNonInternetPermissions,
+          p.join(tempDirectory.path, 'android', 'app', 'src', 'main'),
+        );
 
-      var results = await IOOverrides.runZoned(
-        AndroidInternetPermissionValidator().validate,
-        getCurrentDirectory: () => tempDirectory,
-      );
-      expect(results, hasLength(1));
-      expect(results.first.fix, isNotNull);
+        var results = await IOOverrides.runZoned(
+          AndroidInternetPermissionValidator().validate,
+          getCurrentDirectory: () => tempDirectory,
+        );
+        expect(results, hasLength(1));
+        expect(results.first.fix, isNotNull);
 
-      await IOOverrides.runZoned(
-        () => results.first.fix!(),
-        getCurrentDirectory: () => tempDirectory,
-      );
+        await IOOverrides.runZoned(
+          () => results.first.fix!(),
+          getCurrentDirectory: () => tempDirectory,
+        );
 
-      results = await IOOverrides.runZoned(
-        AndroidInternetPermissionValidator().validate,
-        getCurrentDirectory: () => tempDirectory,
-      );
-      expect(results, isEmpty);
+        results = await IOOverrides.runZoned(
+          AndroidInternetPermissionValidator().validate,
+          getCurrentDirectory: () => tempDirectory,
+        );
+        expect(results, isEmpty);
+      });
     });
   });
 }