fix: ack may not exist depending on caller

fix(maintenance): always ack messages
fix(maintenance): ensure Redis is connected first

mobile/lib/domain/services/asset.service.dart

@@ -6,6 +6,8 @@ import 'package:immich_mobile/infrastructure/repositories/local_asset.repository
 import 'package:immich_mobile/infrastructure/repositories/remote_asset.repository.dart';
 import 'package:immich_mobile/infrastructure/utils/exif.converter.dart';
 
+typedef _AssetVideoDimension = ({double? width, double? height, bool isFlipped});
+
 class AssetService {
   final RemoteAssetRepository _remoteAssetRepository;
   final DriftLocalAssetRepository _localAssetRepository;
@@ -58,44 +60,48 @@ class AssetService {
   }
 
   Future<double> getAspectRatio(BaseAsset asset) async {
-    bool isFlipped;
-    double? width;
-    double? height;
+    final dimension = asset is LocalAsset
+        ? await _getLocalAssetDimensions(asset)
+        : await _getRemoteAssetDimensions(asset as RemoteAsset);
 
-    if (asset.hasRemote) {
-      final exif = await getExif(asset);
-      isFlipped = ExifDtoConverter.isOrientationFlipped(exif?.orientation);
-      width = asset.width?.toDouble();
-      height = asset.height?.toDouble();
-    } else if (asset is LocalAsset) {
-      isFlipped = CurrentPlatform.isAndroid && (asset.orientation == 90 || asset.orientation == 270);
-      width = asset.width?.toDouble();
-      height = asset.height?.toDouble();
-    } else {
-      isFlipped = false;
+    if (dimension.width == null || dimension.height == null || dimension.height == 0) {
+      return 1.0;
     }
 
+    return dimension.isFlipped ? dimension.height! / dimension.width! : dimension.width! / dimension.height!;
+  }
+
+  Future<_AssetVideoDimension> _getLocalAssetDimensions(LocalAsset asset) async {
+    double? width = asset.width?.toDouble();
+    double? height = asset.height?.toDouble();
+    int orientation = asset.orientation;
+
     if (width == null || height == null) {
-      if (asset.hasRemote) {
-        final id = asset is LocalAsset ? asset.remoteId! : (asset as RemoteAsset).id;
-        final remoteAsset = await _remoteAssetRepository.get(id);
-        width = remoteAsset?.width?.toDouble();
-        height = remoteAsset?.height?.toDouble();
-      } else {
-        final id = asset is LocalAsset ? asset.id : (asset as RemoteAsset).localId!;
-        final localAsset = await _localAssetRepository.get(id);
-        width = localAsset?.width?.toDouble();
-        height = localAsset?.height?.toDouble();
-      }
+      final fetched = await _localAssetRepository.get(asset.id);
+      width = fetched?.width?.toDouble();
+      height = fetched?.height?.toDouble();
+      orientation = fetched?.orientation ?? 0;
     }
 
-    final orientedWidth = isFlipped ? height : width;
-    final orientedHeight = isFlipped ? width : height;
-    if (orientedWidth != null && orientedHeight != null && orientedHeight > 0) {
-      return orientedWidth / orientedHeight;
+    // On Android, local assets need orientation correction for 90°/270° rotations
+    // On iOS, the Photos framework pre-corrects dimensions
+    final isFlipped = CurrentPlatform.isAndroid && (orientation == 90 || orientation == 270);
+    return (width: width, height: height, isFlipped: isFlipped);
+  }
+
+  Future<_AssetVideoDimension> _getRemoteAssetDimensions(RemoteAsset asset) async {
+    double? width = asset.width?.toDouble();
+    double? height = asset.height?.toDouble();
+
+    if (width == null || height == null) {
+      final fetched = await _remoteAssetRepository.get(asset.id);
+      width = fetched?.width?.toDouble();
+      height = fetched?.height?.toDouble();
     }
 
-    return 1.0;
+    final exif = await getExif(asset);
+    final isFlipped = ExifDtoConverter.isOrientationFlipped(exif?.orientation);
+    return (width: width, height: height, isFlipped: isFlipped);
   }
 
   Future<List<(String, String)>> getPlaces(String userId) {

mobile/test/domain/services/asset.service_test.dart

@@ -87,6 +87,25 @@ void main() {
       verify(() => mockLocalAssetRepository.get('local-1')).called(1);
     });
 
+    test('uses fetched asset orientation when dimensions are missing on Android', () async {
+      debugDefaultTargetPlatformOverride = TargetPlatform.android;
+      addTearDown(() => debugDefaultTargetPlatformOverride = null);
+
+      // Original asset has default orientation 0, but dimensions are missing
+      final localAsset = TestUtils.createLocalAsset(id: 'local-1', width: null, height: null, orientation: 0);
+
+      // Fetched asset has 90° orientation and proper dimensions
+      final fetchedAsset = TestUtils.createLocalAsset(id: 'local-1', width: 1920, height: 1080, orientation: 90);
+
+      when(() => mockLocalAssetRepository.get('local-1')).thenAnswer((_) async => fetchedAsset);
+
+      final result = await sut.getAspectRatio(localAsset);
+
+      // Should flip dimensions since fetched asset has 90° orientation
+      expect(result, 1080 / 1920);
+      verify(() => mockLocalAssetRepository.get('local-1')).called(1);
+    });
+
     test('returns 1.0 when dimensions are still unavailable after fetching', () async {
       final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: null, height: null);
 
@@ -112,7 +131,9 @@ void main() {
       expect(result, 1.0);
     });
 
-    test('handles local asset with remoteId and uses exif from remote', () async {
+    test('handles local asset with remoteId using local orientation not remote exif', () async {
+      // When a LocalAsset has a remoteId (merged), we should use local orientation
+      // because the width/height come from the local asset (pre-corrected on iOS)
       final localAsset = TestUtils.createLocalAsset(
         id: 'local-1',
         remoteId: 'remote-1',
@@ -121,9 +142,24 @@ void main() {
         orientation: 0,
       );
 
-      final exif = const ExifInfo(orientation: '6');
+      final result = await sut.getAspectRatio(localAsset);
 
-      when(() => mockRemoteAssetRepository.getExif('remote-1')).thenAnswer((_) async => exif);
+      expect(result, 1920 / 1080);
+      // Should not call remote exif for LocalAsset
+      verifyNever(() => mockRemoteAssetRepository.getExif(any()));
+    });
+
+    test('handles local asset with remoteId and 90 degree rotation on Android', () async {
+      debugDefaultTargetPlatformOverride = TargetPlatform.android;
+      addTearDown(() => debugDefaultTargetPlatformOverride = null);
+
+      final localAsset = TestUtils.createLocalAsset(
+        id: 'local-1',
+        remoteId: 'remote-1',
+        width: 1920,
+        height: 1080,
+        orientation: 90,
+      );
 
       final result = await sut.getAspectRatio(localAsset);
 

server/src/maintenance/maintenance-websocket.repository.ts

@@ -37,7 +37,10 @@ export class MaintenanceWebsocketRepository implements OnGatewayConnection, OnGa
 
   afterInit(websocketServer: Server) {
     this.logger.log('Initialized websocket server');
-    websocketServer.on('AppRestart', () => this.appRepository.exitApp());
+    websocketServer.on('AppRestart', (_, ack?: (ok: 'ok') => void) => {
+      ack?.('ok');
+      this.appRepository.exitApp();
+    });
   }
 
   clientBroadcast<T extends keyof ClientEventMap>(event: T, ...data: ClientEventMap[T]) {

server/src/services/cli.service.spec.ts

@@ -5,6 +5,8 @@ import { factory } from 'test/small.factory';
 import { newTestService, ServiceMocks } from 'test/utils';
 import { describe, it } from 'vitest';
 
+const mockSendRestart = vi.fn();
+
 describe(CliService.name, () => {
   let sut: CliService;
   let mocks: ServiceMocks;
@@ -85,7 +87,7 @@ describe(CliService.name, () => {
   describe('disableMaintenanceMode', () => {
     it('should not do anything if not in maintenance mode', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: false });
-      await expect(sut.disableMaintenanceMode()).resolves.toEqual({
+      await expect(sut.disableMaintenanceMode(mockSendRestart)).resolves.toEqual({
         alreadyDisabled: true,
       });
 
@@ -95,7 +97,7 @@ describe(CliService.name, () => {
 
     it('should disable maintenance mode', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: true, secret: 'secret' });
-      await expect(sut.disableMaintenanceMode()).resolves.toEqual({
+      await expect(sut.disableMaintenanceMode(mockSendRestart)).resolves.toEqual({
         alreadyDisabled: false,
       });
 
@@ -108,7 +110,7 @@ describe(CliService.name, () => {
   describe('enableMaintenanceMode', () => {
     it('should not do anything if in maintenance mode', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: true, secret: 'secret' });
-      await expect(sut.enableMaintenanceMode()).resolves.toEqual(
+      await expect(sut.enableMaintenanceMode(mockSendRestart)).resolves.toEqual(
         expect.objectContaining({
           alreadyEnabled: true,
         }),
@@ -120,7 +122,7 @@ describe(CliService.name, () => {
 
     it('should enable maintenance mode', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: false });
-      await expect(sut.enableMaintenanceMode()).resolves.toEqual(
+      await expect(sut.enableMaintenanceMode(mockSendRestart)).resolves.toEqual(
         expect.objectContaining({
           alreadyEnabled: false,
         }),
@@ -137,7 +139,7 @@ describe(CliService.name, () => {
     it('should return a valid login URL', async () => {
       mocks.systemMetadata.get.mockResolvedValue({ isMaintenanceMode: true, secret: 'secret' });
 
-      const result = await sut.enableMaintenanceMode();
+      const result = await sut.enableMaintenanceMode(mockSendRestart);
 
       expect(result).toEqual(
         expect.objectContaining({

server/src/services/cli.service.ts

@@ -42,7 +42,7 @@ export class CliService extends BaseService {
     await this.updateConfig(config);
   }
 
-  async disableMaintenanceMode(): Promise<{ alreadyDisabled: boolean }> {
+  async disableMaintenanceMode(sendAppRestartCallback = sendOneShotAppRestart): Promise<{ alreadyDisabled: boolean }> {
     const currentState = await this.systemMetadataRepository
       .get(SystemMetadataKey.MaintenanceMode)
       .then((state) => state ?? { isMaintenanceMode: false as const });
@@ -56,14 +56,16 @@ export class CliService extends BaseService {
     const state = { isMaintenanceMode: false as const };
     await this.systemMetadataRepository.set(SystemMetadataKey.MaintenanceMode, state);
 
-    sendOneShotAppRestart(state);
+    await sendAppRestartCallback(state);
 
     return {
       alreadyDisabled: false,
     };
   }
 
-  async enableMaintenanceMode(): Promise<{ authUrl: string; alreadyEnabled: boolean }> {
+  async enableMaintenanceMode(
+    sendAppRestartCallback = sendOneShotAppRestart,
+  ): Promise<{ authUrl: string; alreadyEnabled: boolean }> {
     const { server } = await this.getConfig({ withCache: true });
     const baseUrl = getExternalDomain(server);
 
@@ -89,7 +91,7 @@ export class CliService extends BaseService {
       secret,
     });
 
-    sendOneShotAppRestart({
+    await sendAppRestartCallback({
       isMaintenanceMode: true,
     });
 

server/src/services/maintenance.service.ts

@@ -31,7 +31,8 @@ export class MaintenanceService extends BaseService {
   }
 
   @OnEvent({ name: 'AppRestart', server: true })
-  onRestart(): void {
+  onRestart(_: undefined, ack?: (ok: 'ok') => void): void {
+    ack?.('ok');
     this.appRepository.exitApp();
   }
 

server/src/utils/maintenance.ts

@@ -7,47 +7,25 @@ import { MaintenanceAuthDto } from 'src/dtos/maintenance.dto';
 import { ConfigRepository } from 'src/repositories/config.repository';
 import { AppRestartEvent } from 'src/repositories/event.repository';
 
-export function sendOneShotAppRestart(state: AppRestartEvent): void {
+export async function sendOneShotAppRestart(state: AppRestartEvent): Promise<void> {
   const server = new SocketIO();
   const { redis } = new ConfigRepository().getEnv();
-  const pubClient = new Redis(redis);
+  const pubClient = new Redis({ ...redis, lazyConnect: true });
   const subClient = pubClient.duplicate();
+
+  await Promise.all([pubClient.connect(), subClient.connect()]);
+
   server.adapter(createAdapter(pubClient, subClient));
 
-  /**
-   * Keep trying until we manage to stop Immich
-   *
-   * Sometimes there appear to be communication
-   * issues between to the other servers.
-   *
-   * This issue only occurs with this method.
-   */
-  async function tryTerminate() {
-    while (true) {
-      try {
-        const responses = await server.serverSideEmitWithAck('AppRestart', state);
-        if (responses.length > 0) {
-          return;
-        }
-      } catch (error) {
-        console.error(error);
-        console.error('Encountered an error while telling Immich to stop.');
-      }
-
-      console.info(
-        "\nIt doesn't appear that Immich stopped, trying again in a moment.\nIf Immich is already not running, you can ignore this error.",
-      );
-
-      await new Promise((r) => setTimeout(r, 1e3));
-    }
-  }
-
   // => corresponds to notification.service.ts#onAppRestart
-  server.emit('AppRestartV1', state, () => {
-    void tryTerminate().finally(() => {
-      pubClient.disconnect();
-      subClient.disconnect();
-    });
+  server.emit('AppRestartV1', state, async () => {
+    const responses = await server.serverSideEmitWithAck('AppRestart', state);
+    if (responses.some((response) => response !== 'ok')) {
+      throw new Error("One or more node(s) returned a non-'ok' response to our restart request!");
+    }
+
+    pubClient.disconnect();
+    subClient.disconnect();
   });
 }