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/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);
}
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) {

View File

@@ -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);

View File

@@ -37,7 +37,13 @@ export class MaintenanceWebsocketRepository implements OnGatewayConnection, OnGa
afterInit(websocketServer: 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]) {

View File

@@ -1,5 +1,10 @@
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 { ConfigRepository } from 'src/repositories/config.repository';
import { AppRestartEvent } from 'src/repositories/event.repository';
@Injectable()
export class AppRepository {
@@ -17,4 +22,26 @@ export class AppRepository {
setCloseFn(fn: () => Promise<void>) {
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,
});
expect(mocks.app.sendOneShotAppRestart).toHaveBeenCalledTimes(0);
expect(mocks.systemMetadata.set).toHaveBeenCalledTimes(0);
expect(mocks.event.emit).toHaveBeenCalledTimes(0);
});
@@ -99,6 +100,7 @@ describe(CliService.name, () => {
alreadyDisabled: false,
});
expect(mocks.app.sendOneShotAppRestart).toHaveBeenCalled();
expect(mocks.systemMetadata.set).toHaveBeenCalledWith(SystemMetadataKey.MaintenanceMode, {
isMaintenanceMode: false,
});
@@ -114,6 +116,7 @@ describe(CliService.name, () => {
}),
);
expect(mocks.app.sendOneShotAppRestart).toHaveBeenCalledTimes(0);
expect(mocks.systemMetadata.set).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, {
isMaintenanceMode: true,
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 { SystemMetadataKey } from 'src/enum';
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';
@Injectable()
@@ -55,8 +55,7 @@ export class CliService extends BaseService {
const state = { isMaintenanceMode: false as const };
await this.systemMetadataRepository.set(SystemMetadataKey.MaintenanceMode, state);
sendOneShotAppRestart(state);
await this.appRepository.sendOneShotAppRestart(state);
return {
alreadyDisabled: false,
@@ -89,7 +88,7 @@ export class CliService extends BaseService {
secret,
});
sendOneShotAppRestart({
await this.appRepository.sendOneShotAppRestart({
isMaintenanceMode: true,
});

View File

@@ -2,6 +2,7 @@ import { Injectable } from '@nestjs/common';
import { OnEvent } from 'src/decorators';
import { MaintenanceAuthDto } from 'src/dtos/maintenance.dto';
import { SystemMetadataKey } from 'src/enum';
import { ArgOf } from 'src/repositories/event.repository';
import { BaseService } from 'src/services/base.service';
import { MaintenanceModeState } from 'src/types';
import { createMaintenanceLoginUrl, generateMaintenanceSecret, signMaintenanceJwt } from 'src/utils/maintenance';
@@ -31,7 +32,10 @@ export class MaintenanceService extends BaseService {
}
@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();
}

View File

@@ -1,55 +1,6 @@
import { createAdapter } from '@socket.io/redis-adapter';
import Redis from 'ioredis';
import { SignJWT } from 'jose';
import { randomBytes } from 'node:crypto';
import { Server as SocketIO } from 'socket.io';
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(
baseUrl: string,