From 911dde39c939ab8fb165f7067ffb255bb583274a Mon Sep 17 00:00:00 2001 From: shenlong <139912620+shenlong-tanwen@users.noreply.github.com> Date: Wed, 3 Jun 2026 20:51:23 +0530 Subject: [PATCH] ci: verify mobile backward compatibility (#28786) Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> --- .github/workflows/check-openapi.yml | 33 +++++++ mobile/lib/utils/openapi_patching.dart | 115 ++++++++++------------ mobile/test/openapi_patches_coverage.dart | 111 +++++++++++++++++++++ 3 files changed, 196 insertions(+), 63 deletions(-) create mode 100644 mobile/test/openapi_patches_coverage.dart diff --git a/.github/workflows/check-openapi.yml b/.github/workflows/check-openapi.yml index f2b3e3c248..ddd33925fa 100644 --- a/.github/workflows/check-openapi.yml +++ b/.github/workflows/check-openapi.yml @@ -4,6 +4,7 @@ on: pull_request: paths: - 'open-api/**' + - 'mobile/lib/utils/openapi_patching.dart' - '.github/workflows/check-openapi.yml' concurrency: @@ -29,3 +30,35 @@ jobs: base: https://raw.githubusercontent.com/${{ github.repository }}/main/open-api/immich-openapi-specs.json revision: open-api/immich-openapi-specs.json fail-on: ERR + + check-mobile-patches: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + + - name: Setup Mise + uses: immich-app/devtools/actions/use-mise@7b8610a904d57da241e4ddba17fa62b62b15aed4 # use-mise-action-v2.0.2 + with: + github_token: ${{ github.token }} + + - name: Get packages + working-directory: ./mobile + run: flutter pub get + + - name: Fetch base spec from main + run: | + curl -fsSL \ + "https://raw.githubusercontent.com/${{ github.repository }}/main/open-api/immich-openapi-specs.json" \ + -o /tmp/base-spec.json + + - name: Check newly-required fields have a backward-compat patch + working-directory: ./mobile + env: + OPENAPI_BASE_SPEC: /tmp/base-spec.json + OPENAPI_REVISION_SPEC: ../open-api/immich-openapi-specs.json + run: flutter test test/openapi_patches_coverage.dart diff --git a/mobile/lib/utils/openapi_patching.dart b/mobile/lib/utils/openapi_patching.dart index cbd6c2bcce..eca190ce8b 100644 --- a/mobile/lib/utils/openapi_patching.dart +++ b/mobile/lib/utils/openapi_patching.dart @@ -1,69 +1,58 @@ +import 'package:flutter/foundation.dart'; import 'package:openapi/api.dart'; -dynamic upgradeDto(dynamic value, String targetType) { - switch (targetType) { - case 'UserPreferencesResponseDto': - if (value is Map) { - addDefault(value, 'download.includeEmbeddedVideos', false); - addDefault(value, 'folders', FoldersResponse(enabled: false, sidebarWeb: false).toJson()); - addDefault(value, 'memories', MemoriesResponse(enabled: true, duration: 5).toJson()); - addDefault(value, 'ratings', RatingsResponse(enabled: false).toJson()); - addDefault(value, 'people', PeopleResponse(enabled: true, sidebarWeb: false).toJson()); - addDefault(value, 'tags', TagsResponse(enabled: false, sidebarWeb: false).toJson()); - addDefault(value, 'sharedLinks', SharedLinksResponse(enabled: true, sidebarWeb: false).toJson()); - addDefault(value, 'cast', CastResponse(gCastEnabled: false).toJson()); - addDefault(value, 'albums', {'defaultAssetOrder': 'desc'}); - } - break; - case 'ServerConfigDto': - if (value is Map) { - addDefault(value, 'mapLightStyleUrl', 'https://tiles.immich.cloud/v1/style/light.json'); - addDefault(value, 'mapDarkStyleUrl', 'https://tiles.immich.cloud/v1/style/dark.json'); - addDefault(value, 'minFaces', 3); - } - case 'UserResponseDto': - if (value is Map) { - addDefault(value, 'profileChangedAt', DateTime.now().toIso8601String()); - } - break; - case 'AssetResponseDto': - if (value is Map) { - addDefault(value, 'visibility', 'timeline'); - addDefault(value, 'createdAt', DateTime.now().toIso8601String()); - addDefault(value, 'isEdited', false); - } - break; - case 'UserAdminResponseDto': - if (value is Map) { - addDefault(value, 'profileChangedAt', DateTime.now().toIso8601String()); - } - break; - case 'LoginResponseDto': - if (value is Map) { - addDefault(value, 'isOnboarded', false); - } - break; - case 'SyncUserV1': - if (value is Map) { - addDefault(value, 'profileChangedAt', DateTime.now().toIso8601String()); - addDefault(value, 'hasProfileImage', false); - } - case 'SyncAssetV1': - if (value is Map) { - addDefault(value, 'isEdited', false); - } - case 'ServerFeaturesDto': - if (value is Map) { - addDefault(value, 'ocr', false); - addDefault(value, 'realtimeTranscoding', false); - } - break; - case 'MemoriesResponse': - if (value is Map) { - addDefault(value, 'duration', 5); - } - break; +abstract interface class _Dynamic { + Object? resolve(); +} + +class _CurrentTimestamp implements _Dynamic { + const _CurrentTimestamp(); + + @override + Object? resolve() => DateTime.now().toIso8601String(); +} + +const _now = _CurrentTimestamp(); + +@visibleForTesting +final Map> openApiPatches = { + 'UserPreferencesResponseDto': { + 'download.includeEmbeddedVideos': false, + 'folders': FoldersResponse(enabled: false, sidebarWeb: false).toJson(), + 'memories': MemoriesResponse(enabled: true, duration: 5).toJson(), + 'ratings': RatingsResponse(enabled: false).toJson(), + 'people': PeopleResponse(enabled: true, sidebarWeb: false).toJson(), + 'tags': TagsResponse(enabled: false, sidebarWeb: false).toJson(), + 'sharedLinks': SharedLinksResponse(enabled: true, sidebarWeb: false).toJson(), + 'cast': CastResponse(gCastEnabled: false).toJson(), + 'albums': {'defaultAssetOrder': 'desc'}, + }, + 'ServerConfigDto': { + 'mapLightStyleUrl': 'https://tiles.immich.cloud/v1/style/light.json', + 'mapDarkStyleUrl': 'https://tiles.immich.cloud/v1/style/dark.json', + 'minFaces': 3, + }, + 'UserResponseDto': {'profileChangedAt': _now}, + 'AssetResponseDto': {'visibility': 'timeline', 'createdAt': _now, 'isEdited': false}, + 'UserAdminResponseDto': {'profileChangedAt': _now}, + 'LoginResponseDto': {'isOnboarded': false}, + 'SyncUserV1': {'profileChangedAt': _now, 'hasProfileImage': false}, + 'SyncAssetV1': {'isEdited': false}, + 'ServerFeaturesDto': {'ocr': false, 'realtimeTranscoding': false}, + 'MemoriesResponse': {'duration': 5}, +}; + +void upgradeDto(dynamic value, String targetType) { + if (value is! Map) { + return; } + final fields = openApiPatches[targetType]; + if (fields == null) { + return; + } + fields.forEach((key, defaultValue) { + addDefault(value, key, defaultValue is _Dynamic ? defaultValue.resolve() : defaultValue); + }); } addDefault(dynamic value, String keys, dynamic defaultValue) { diff --git a/mobile/test/openapi_patches_coverage.dart b/mobile/test/openapi_patches_coverage.dart new file mode 100644 index 0000000000..c4225d82c6 --- /dev/null +++ b/mobile/test/openapi_patches_coverage.dart @@ -0,0 +1,111 @@ +// Intentionally NOT named `*_test.dart`: that suffix makes `flutter test` +// auto-discover it, which would run it on every mobile PR. This check is only +// relevant when the OpenAPI spec changes, so the `Check OpenAPI` workflow runs +// it by explicit path with the spec locations in the environment. + +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/utils/openapi_patching.dart'; + +void main() { + test('every newly-required response field has a backward-compat patch', () { + final basePath = Platform.environment['OPENAPI_BASE_SPEC']; + final revisionPath = Platform.environment['OPENAPI_REVISION_SPEC']; + if (basePath == null || revisionPath == null) { + markTestSkipped('set OPENAPI_BASE_SPEC and OPENAPI_REVISION_SPEC to run'); + return; + } + + final baseRequired = _requiredBySchema(_loadSpec(basePath)); + final revisionSpec = _loadSpec(revisionPath); + final revisionRequired = _requiredBySchema(revisionSpec); + final deserialized = _deserializedSchemas(revisionSpec); + final patched = openApiPatches.map( + (type, fields) => MapEntry(type, fields.keys.toSet()), + ); + + final missing = []; + for (final entry in revisionRequired.entries) { + if (!deserialized.contains(entry.key)) { + continue; + } + final have = patched[entry.key] ?? const {}; + final newlyRequired = entry.value.difference( + baseRequired[entry.key] ?? const {}, + ); + for (final field in newlyRequired) { + if (!have.contains(field)) { + missing.add('${entry.key}.$field'); + } + } + } + missing.sort(); + + expect( + missing, + isEmpty, + reason: + 'Detected a breaking change: $missing\n' + 'Either add a default to openApiPatches in lib/utils/openapi_patching.dart, or make it optional', + ); + }); +} + +Map _loadSpec(String path) => + jsonDecode(File(path).readAsStringSync()) as Map; + +Map _schemas(Map spec) => + ((spec['components'] as Map?)?['schemas'] as Map?) + ?.cast() ?? + const {}; + +Map> _requiredBySchema(Map spec) { + final result = >{}; + _schemas(spec).forEach((name, schema) { + final required = (schema as Map)['required'] as List? ?? const []; + result[name] = required.cast().toSet(); + }); + return result; +} + +Iterable _refsIn(Object? node) sync* { + if (node is Map) { + if (node[r'$ref'] case final String ref) { + yield ref.split('/').last; + } + for (final value in node.values) { + yield* _refsIn(value); + } + } else if (node is List) { + for (final value in node) { + yield* _refsIn(value); + } + } +} + +Set _deserializedSchemas(Map spec) { + final schemas = _schemas(spec); + final reachable = {}; + + final queue = []; + for (final path in (spec['paths'] as Map?)?.values ?? const []) { + if (path is! Map) { + continue; + } + for (final operation in path.values) { + if (operation is Map) { + queue.addAll(_refsIn(operation['responses'])); + } + } + } + while (queue.isNotEmpty) { + final name = queue.removeLast(); + if (!schemas.containsKey(name) || !reachable.add(name)) { + continue; + } + queue.addAll(_refsIn(schemas[name])); + } + return reachable; +}