Compare commits

...

8 Commits

Author SHA1 Message Date
izzy
9698326702 chore: log restart event 2025-12-19 15:43:13 +00:00
izzy
6da85dd12b fix: send correct state in one shot 2025-12-19 15:43:07 +00:00
izzy
23bd27eb30 refactor: move one shot into app.repository 2025-12-19 15:23:48 +00:00
izzy
a2d4439e48 fix: ack may not exist depending on caller 2025-12-19 14:16:30 +00:00
izzy
00bafb899d chore: mock the app restart callback 2025-12-19 14:08:31 +00:00
izzy
ec63098020 chore(maintenance): validate app restart responses 2025-12-19 13:56:23 +00:00
izzy
8ca692bfb0 fix(maintenance): prevent CLI hanging on occassion
fix(maintenance): always ack messages
fix(maintenance): ensure Redis is connected first
2025-12-19 13:52:57 +00:00
Alex
125de91c71 fix: merged video in On This Device played with incorrect dimension (#24656)
* fix: merged video in On This Device played with incorrect dimension

* chore: pr feedback
2025-12-18 20:59:58 -06:00
8 changed files with 121 additions and 88 deletions

View File

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

View File

@@ -87,6 +87,25 @@ void main() {
verify(() => mockLocalAssetRepository.get('local-1')).called(1); 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 { test('returns 1.0 when dimensions are still unavailable after fetching', () async {
final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: null, height: null); final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: null, height: null);
@@ -112,7 +131,9 @@ void main() {
expect(result, 1.0); 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( final localAsset = TestUtils.createLocalAsset(
id: 'local-1', id: 'local-1',
remoteId: 'remote-1', remoteId: 'remote-1',
@@ -121,9 +142,24 @@ void main() {
orientation: 0, 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); final result = await sut.getAspectRatio(localAsset);

View File

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

View File

@@ -1,5 +1,10 @@
import { Injectable } from '@nestjs/common'; import { Injectable } from '@nestjs/common';
import { createAdapter } from '@socket.io/redis-adapter';
import Redis from 'ioredis';
import { Server as SocketIO } from 'socket.io';
import { ExitCode } from 'src/enum'; import { ExitCode } from 'src/enum';
import { ConfigRepository } from 'src/repositories/config.repository';
import { AppRestartEvent } from 'src/repositories/event.repository';
@Injectable() @Injectable()
export class AppRepository { export class AppRepository {
@@ -17,4 +22,26 @@ export class AppRepository {
setCloseFn(fn: () => Promise<void>) { setCloseFn(fn: () => Promise<void>) {
this.closeFn = fn; this.closeFn = fn;
} }
async sendOneShotAppRestart(state: AppRestartEvent): Promise<void> {
const server = new SocketIO();
const { redis } = new ConfigRepository().getEnv();
const pubClient = new Redis({ ...redis, lazyConnect: true });
const subClient = pubClient.duplicate();
await Promise.all([pubClient.connect(), subClient.connect()]);
server.adapter(createAdapter(pubClient, subClient));
// => corresponds to notification.service.ts#onAppRestart
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();
});
}
} }

View File

@@ -89,6 +89,7 @@ describe(CliService.name, () => {
alreadyDisabled: true, alreadyDisabled: true,
}); });
expect(mocks.app.sendOneShotAppRestart).toHaveBeenCalledTimes(0);
expect(mocks.systemMetadata.set).toHaveBeenCalledTimes(0); expect(mocks.systemMetadata.set).toHaveBeenCalledTimes(0);
expect(mocks.event.emit).toHaveBeenCalledTimes(0); expect(mocks.event.emit).toHaveBeenCalledTimes(0);
}); });
@@ -99,6 +100,7 @@ describe(CliService.name, () => {
alreadyDisabled: false, alreadyDisabled: false,
}); });
expect(mocks.app.sendOneShotAppRestart).toHaveBeenCalled();
expect(mocks.systemMetadata.set).toHaveBeenCalledWith(SystemMetadataKey.MaintenanceMode, { expect(mocks.systemMetadata.set).toHaveBeenCalledWith(SystemMetadataKey.MaintenanceMode, {
isMaintenanceMode: false, isMaintenanceMode: false,
}); });
@@ -114,6 +116,7 @@ describe(CliService.name, () => {
}), }),
); );
expect(mocks.app.sendOneShotAppRestart).toHaveBeenCalledTimes(0);
expect(mocks.systemMetadata.set).toHaveBeenCalledTimes(0); expect(mocks.systemMetadata.set).toHaveBeenCalledTimes(0);
expect(mocks.event.emit).toHaveBeenCalledTimes(0); expect(mocks.event.emit).toHaveBeenCalledTimes(0);
}); });
@@ -126,6 +129,7 @@ describe(CliService.name, () => {
}), }),
); );
expect(mocks.app.sendOneShotAppRestart).toHaveBeenCalled();
expect(mocks.systemMetadata.set).toHaveBeenCalledWith(SystemMetadataKey.MaintenanceMode, { expect(mocks.systemMetadata.set).toHaveBeenCalledWith(SystemMetadataKey.MaintenanceMode, {
isMaintenanceMode: true, isMaintenanceMode: true,
secret: expect.stringMatching(/^\w{128}$/), secret: expect.stringMatching(/^\w{128}$/),

View File

@@ -5,7 +5,7 @@ import { MaintenanceAuthDto } from 'src/dtos/maintenance.dto';
import { UserAdminResponseDto, mapUserAdmin } from 'src/dtos/user.dto'; import { UserAdminResponseDto, mapUserAdmin } from 'src/dtos/user.dto';
import { SystemMetadataKey } from 'src/enum'; import { SystemMetadataKey } from 'src/enum';
import { BaseService } from 'src/services/base.service'; import { BaseService } from 'src/services/base.service';
import { createMaintenanceLoginUrl, generateMaintenanceSecret, sendOneShotAppRestart } from 'src/utils/maintenance'; import { createMaintenanceLoginUrl, generateMaintenanceSecret } from 'src/utils/maintenance';
import { getExternalDomain } from 'src/utils/misc'; import { getExternalDomain } from 'src/utils/misc';
@Injectable() @Injectable()
@@ -55,8 +55,7 @@ export class CliService extends BaseService {
const state = { isMaintenanceMode: false as const }; const state = { isMaintenanceMode: false as const };
await this.systemMetadataRepository.set(SystemMetadataKey.MaintenanceMode, state); await this.systemMetadataRepository.set(SystemMetadataKey.MaintenanceMode, state);
await this.appRepository.sendOneShotAppRestart(state);
sendOneShotAppRestart(state);
return { return {
alreadyDisabled: false, alreadyDisabled: false,
@@ -89,7 +88,7 @@ export class CliService extends BaseService {
secret, secret,
}); });
sendOneShotAppRestart({ await this.appRepository.sendOneShotAppRestart({
isMaintenanceMode: true, isMaintenanceMode: true,
}); });

View File

@@ -2,6 +2,7 @@ import { Injectable } from '@nestjs/common';
import { OnEvent } from 'src/decorators'; import { OnEvent } from 'src/decorators';
import { MaintenanceAuthDto } from 'src/dtos/maintenance.dto'; import { MaintenanceAuthDto } from 'src/dtos/maintenance.dto';
import { SystemMetadataKey } from 'src/enum'; import { SystemMetadataKey } from 'src/enum';
import { ArgOf } from 'src/repositories/event.repository';
import { BaseService } from 'src/services/base.service'; import { BaseService } from 'src/services/base.service';
import { MaintenanceModeState } from 'src/types'; import { MaintenanceModeState } from 'src/types';
import { createMaintenanceLoginUrl, generateMaintenanceSecret, signMaintenanceJwt } from 'src/utils/maintenance'; import { createMaintenanceLoginUrl, generateMaintenanceSecret, signMaintenanceJwt } from 'src/utils/maintenance';
@@ -31,7 +32,10 @@ export class MaintenanceService extends BaseService {
} }
@OnEvent({ name: 'AppRestart', server: true }) @OnEvent({ name: 'AppRestart', server: true })
onRestart(): void { onRestart(event: ArgOf<'AppRestart'>, ack?: (ok: 'ok') => void): void {
this.logger.log(`Restarting due to event... ${JSON.stringify(event)}`);
ack?.('ok');
this.appRepository.exitApp(); this.appRepository.exitApp();
} }

View File

@@ -1,55 +1,6 @@
import { createAdapter } from '@socket.io/redis-adapter';
import Redis from 'ioredis';
import { SignJWT } from 'jose'; import { SignJWT } from 'jose';
import { randomBytes } from 'node:crypto'; import { randomBytes } from 'node:crypto';
import { Server as SocketIO } from 'socket.io';
import { MaintenanceAuthDto } from 'src/dtos/maintenance.dto'; 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 {
const server = new SocketIO();
const { redis } = new ConfigRepository().getEnv();
const pubClient = new Redis(redis);
const subClient = pubClient.duplicate();
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();
});
});
}
export async function createMaintenanceLoginUrl( export async function createMaintenanceLoginUrl(
baseUrl: string, baseUrl: string,