From e2a2c86a31aaf6ba9f4d8bc5a797ded841254d87 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 13 Jun 2024 09:21:47 -0500 Subject: [PATCH] chore(server): optional originalMimeType in asset response payload (#10272) * chore(server): optional originalMimeType in asset response payload * lint * Update web/src/lib/utils/asset-utils.ts Co-authored-by: Jason Rasmussen * fix permission of shared link * test * test * test * test server --------- Co-authored-by: Jason Rasmussen --- .../openapi/lib/model/asset_response_dto.dart | 19 +++++++--- open-api/immich-openapi-specs.json | 1 - open-api/typescript-sdk/src/fetch-client.ts | 2 +- server/src/dtos/asset-response.dto.ts | 2 +- .../src/services/shared-link.service.spec.ts | 30 +++++++++++++++ server/src/services/shared-link.service.ts | 2 +- .../asset-viewer/asset-viewer.svelte | 3 +- .../asset-viewer/photo-viewer.spec.ts | 38 +++++++++++++++++++ .../asset-viewer/photo-viewer.svelte | 8 +++- .../create-shared-link-modal.svelte | 12 +++++- web/src/lib/utils/asset-utils.ts | 4 ++ .../factories/shared-link-factory.ts | 19 ++++++++++ 12 files changed, 126 insertions(+), 14 deletions(-) create mode 100644 web/src/test-data/factories/shared-link-factory.ts diff --git a/mobile/openapi/lib/model/asset_response_dto.dart b/mobile/openapi/lib/model/asset_response_dto.dart index ced0230f3e..61e33ef4e0 100644 --- a/mobile/openapi/lib/model/asset_response_dto.dart +++ b/mobile/openapi/lib/model/asset_response_dto.dart @@ -31,7 +31,7 @@ class AssetResponseDto { this.livePhotoVideoId, required this.localDateTime, required this.originalFileName, - required this.originalMimeType, + this.originalMimeType, required this.originalPath, this.owner, required this.ownerId, @@ -92,7 +92,13 @@ class AssetResponseDto { String originalFileName; - String originalMimeType; + /// + /// Please note: This property should have been non-nullable! Since the specification file + /// does not include a default value (using the "default:" property), however, the generated + /// source code must fall back to having a nullable type. + /// Consider adding a "default:" property in the specification file to hide this note. + /// + String? originalMimeType; String originalPath; @@ -191,7 +197,7 @@ class AssetResponseDto { (livePhotoVideoId == null ? 0 : livePhotoVideoId!.hashCode) + (localDateTime.hashCode) + (originalFileName.hashCode) + - (originalMimeType.hashCode) + + (originalMimeType == null ? 0 : originalMimeType!.hashCode) + (originalPath.hashCode) + (owner == null ? 0 : owner!.hashCode) + (ownerId.hashCode) + @@ -246,7 +252,11 @@ class AssetResponseDto { } json[r'localDateTime'] = this.localDateTime.toUtc().toIso8601String(); json[r'originalFileName'] = this.originalFileName; + if (this.originalMimeType != null) { json[r'originalMimeType'] = this.originalMimeType; + } else { + // json[r'originalMimeType'] = null; + } json[r'originalPath'] = this.originalPath; if (this.owner != null) { json[r'owner'] = this.owner; @@ -310,7 +320,7 @@ class AssetResponseDto { livePhotoVideoId: mapValueOfType(json, r'livePhotoVideoId'), localDateTime: mapDateTime(json, r'localDateTime', r'')!, originalFileName: mapValueOfType(json, r'originalFileName')!, - originalMimeType: mapValueOfType(json, r'originalMimeType')!, + originalMimeType: mapValueOfType(json, r'originalMimeType'), originalPath: mapValueOfType(json, r'originalPath')!, owner: UserResponseDto.fromJson(json[r'owner']), ownerId: mapValueOfType(json, r'ownerId')!, @@ -386,7 +396,6 @@ class AssetResponseDto { 'isTrashed', 'localDateTime', 'originalFileName', - 'originalMimeType', 'originalPath', 'ownerId', 'resized', diff --git a/open-api/immich-openapi-specs.json b/open-api/immich-openapi-specs.json index 84a1a7a1ce..ca2f1735c7 100644 --- a/open-api/immich-openapi-specs.json +++ b/open-api/immich-openapi-specs.json @@ -7785,7 +7785,6 @@ "isTrashed", "localDateTime", "originalFileName", - "originalMimeType", "originalPath", "ownerId", "resized", diff --git a/open-api/typescript-sdk/src/fetch-client.ts b/open-api/typescript-sdk/src/fetch-client.ts index d772cb6245..7cd939a8f3 100644 --- a/open-api/typescript-sdk/src/fetch-client.ts +++ b/open-api/typescript-sdk/src/fetch-client.ts @@ -182,7 +182,7 @@ export type AssetResponseDto = { livePhotoVideoId?: string | null; localDateTime: string; originalFileName: string; - originalMimeType: string; + originalMimeType?: string; originalPath: string; owner?: UserResponseDto; ownerId: string; diff --git a/server/src/dtos/asset-response.dto.ts b/server/src/dtos/asset-response.dto.ts index d75a0c6329..03fa2f8b3d 100644 --- a/server/src/dtos/asset-response.dto.ts +++ b/server/src/dtos/asset-response.dto.ts @@ -20,7 +20,7 @@ export class SanitizedAssetResponseDto { @ApiProperty({ enumName: 'AssetTypeEnum', enum: AssetType }) type!: AssetType; thumbhash!: string | null; - originalMimeType!: string; + originalMimeType?: string; resized!: boolean; localDateTime!: Date; duration!: string; diff --git a/server/src/services/shared-link.service.spec.ts b/server/src/services/shared-link.service.spec.ts index 65776167b5..a5a24cfd7b 100644 --- a/server/src/services/shared-link.service.spec.ts +++ b/server/src/services/shared-link.service.spec.ts @@ -164,6 +164,36 @@ describe(SharedLinkService.name, () => { key: Buffer.from('random-bytes', 'utf8'), }); }); + + it('should create a shared link with allowDownload set to false when showMetadata is false', async () => { + accessMock.asset.checkOwnerAccess.mockResolvedValue(new Set([assetStub.image.id])); + shareMock.create.mockResolvedValue(sharedLinkStub.individual); + + await sut.create(authStub.admin, { + type: SharedLinkType.INDIVIDUAL, + assetIds: [assetStub.image.id], + showMetadata: false, + allowDownload: true, + allowUpload: true, + }); + + expect(accessMock.asset.checkOwnerAccess).toHaveBeenCalledWith( + authStub.admin.user.id, + new Set([assetStub.image.id]), + ); + expect(shareMock.create).toHaveBeenCalledWith({ + type: SharedLinkType.INDIVIDUAL, + userId: authStub.admin.user.id, + albumId: null, + allowDownload: false, + allowUpload: true, + assets: [{ id: assetStub.image.id }], + description: null, + expiresAt: null, + showExif: false, + key: Buffer.from('random-bytes', 'utf8'), + }); + }); }); describe('update', () => { diff --git a/server/src/services/shared-link.service.ts b/server/src/services/shared-link.service.ts index 489b5e5f0a..50ddba65cf 100644 --- a/server/src/services/shared-link.service.ts +++ b/server/src/services/shared-link.service.ts @@ -84,7 +84,7 @@ export class SharedLinkService { password: dto.password, expiresAt: dto.expiresAt || null, allowUpload: dto.allowUpload ?? true, - allowDownload: dto.allowDownload ?? true, + allowDownload: dto.showMetadata === false ? false : dto.allowDownload ?? true, showExif: dto.showMetadata ?? true, }); diff --git a/web/src/lib/components/asset-viewer/asset-viewer.svelte b/web/src/lib/components/asset-viewer/asset-viewer.svelte index f8f1891616..0225a92df2 100644 --- a/web/src/lib/components/asset-viewer/asset-viewer.svelte +++ b/web/src/lib/components/asset-viewer/asset-viewer.svelte @@ -629,6 +629,7 @@ {preloadAssets} on:close={closeViewer} haveFadeTransition={false} + {sharedLink} /> {:else} {:else} - + {/if} {:else} { expect(getAssetThumbnailUrlSpy).not.toBeCalled(); expect(getAssetOriginalUrlSpy).toBeCalledWith({ id: asset.id, checksum: asset.checksum }); }); + + it('loads original for shared link when download permission is true and showMetadata permission is true', () => { + const asset = assetFactory.build({ originalPath: 'image.gif', originalMimeType: 'image/gif' }); + const sharedLink = sharedLinkFactory.build({ allowDownload: true, showMetadata: true, assets: [asset] }); + render(PhotoViewer, { asset, sharedLink }); + + expect(getAssetThumbnailUrlSpy).not.toBeCalled(); + expect(getAssetOriginalUrlSpy).toBeCalledWith({ id: asset.id, checksum: asset.checksum }); + }); + + it('not loads original image when shared link download permission is false', () => { + const asset = assetFactory.build({ originalPath: 'image.gif', originalMimeType: 'image/gif' }); + const sharedLink = sharedLinkFactory.build({ allowDownload: false, assets: [asset] }); + render(PhotoViewer, { asset, sharedLink }); + + expect(getAssetThumbnailUrlSpy).toBeCalledWith({ + id: asset.id, + size: AssetMediaSize.Preview, + checksum: asset.checksum, + }); + + expect(getAssetOriginalUrlSpy).not.toBeCalled(); + }); + + it('not loads original image when shared link showMetadata permission is false', () => { + const asset = assetFactory.build({ originalPath: 'image.gif', originalMimeType: 'image/gif' }); + const sharedLink = sharedLinkFactory.build({ showMetadata: false, assets: [asset] }); + render(PhotoViewer, { asset, sharedLink }); + + expect(getAssetThumbnailUrlSpy).toBeCalledWith({ + id: asset.id, + size: AssetMediaSize.Preview, + checksum: asset.checksum, + }); + + expect(getAssetOriginalUrlSpy).not.toBeCalled(); + }); }); diff --git a/web/src/lib/components/asset-viewer/photo-viewer.svelte b/web/src/lib/components/asset-viewer/photo-viewer.svelte index 47d92b0d53..cd4010b135 100644 --- a/web/src/lib/components/asset-viewer/photo-viewer.svelte +++ b/web/src/lib/components/asset-viewer/photo-viewer.svelte @@ -9,7 +9,7 @@ import { isWebCompatibleImage } from '$lib/utils/asset-utils'; import { getBoundingBox } from '$lib/utils/people-utils'; import { getAltText } from '$lib/utils/thumbnail-util'; - import { AssetTypeEnum, type AssetResponseDto, AssetMediaSize } from '@immich/sdk'; + import { AssetTypeEnum, type AssetResponseDto, AssetMediaSize, type SharedLinkResponseDto } from '@immich/sdk'; import { zoomImageAction, zoomed } from '$lib/actions/zoom-image'; import { canCopyImagesToClipboard, copyImageToClipboard } from 'copy-image-clipboard'; import { onDestroy } from 'svelte'; @@ -23,7 +23,7 @@ export let preloadAssets: AssetResponseDto[] | undefined = undefined; export let element: HTMLDivElement | undefined = undefined; export let haveFadeTransition = true; - + export let sharedLink: SharedLinkResponseDto | undefined = undefined; export let copyImage: (() => Promise) | null = null; export let zoomToggle: (() => void) | null = null; @@ -67,6 +67,10 @@ }; const getAssetUrl = (id: string, useOriginal: boolean, checksum: string) => { + if (sharedLink && (!sharedLink.allowDownload || !sharedLink.showMetadata)) { + return getAssetThumbnailUrl({ id, size: AssetMediaSize.Preview, checksum }); + } + return useOriginal ? getAssetOriginalUrl({ id, checksum }) : getAssetThumbnailUrl({ id, size: AssetMediaSize.Preview, checksum }); diff --git a/web/src/lib/components/shared-components/create-share-link-modal/create-shared-link-modal.svelte b/web/src/lib/components/shared-components/create-share-link-modal/create-shared-link-modal.svelte index a87774c9e9..3df849e187 100644 --- a/web/src/lib/components/shared-components/create-share-link-modal/create-shared-link-modal.svelte +++ b/web/src/lib/components/shared-components/create-share-link-modal/create-shared-link-modal.svelte @@ -51,7 +51,11 @@ }; $: shareType = albumId ? SharedLinkType.Album : SharedLinkType.Individual; - + $: { + if (!showMetadata) { + allowDownload = false; + } + } if (editingLink) { if (editingLink.description) { description = editingLink.description; @@ -227,7 +231,11 @@
- +
diff --git a/web/src/lib/utils/asset-utils.ts b/web/src/lib/utils/asset-utils.ts index 9fa851aa39..648b66e361 100644 --- a/web/src/lib/utils/asset-utils.ts +++ b/web/src/lib/utils/asset-utils.ts @@ -270,6 +270,10 @@ const supportedImageMimeTypes = new Set([ * Returns true if the asset is an image supported by web browsers, false otherwise */ export function isWebCompatibleImage(asset: AssetResponseDto): boolean { + if (!asset.originalMimeType) { + return false; + } + return supportedImageMimeTypes.has(asset.originalMimeType); } diff --git a/web/src/test-data/factories/shared-link-factory.ts b/web/src/test-data/factories/shared-link-factory.ts new file mode 100644 index 0000000000..a057bc936b --- /dev/null +++ b/web/src/test-data/factories/shared-link-factory.ts @@ -0,0 +1,19 @@ +import { faker } from '@faker-js/faker'; +import { SharedLinkType, type SharedLinkResponseDto } from '@immich/sdk'; +import { Sync } from 'factory.ts'; + +export const sharedLinkFactory = Sync.makeFactory({ + id: Sync.each(() => faker.string.uuid()), + description: Sync.each(() => faker.word.sample()), + password: Sync.each(() => faker.word.sample()), + token: Sync.each(() => faker.word.sample()), + userId: Sync.each(() => faker.string.uuid()), + key: Sync.each(() => faker.word.sample()), + type: Sync.each(() => faker.helpers.enumValue(SharedLinkType)), + createdAt: Sync.each(() => faker.date.past().toISOString()), + expiresAt: Sync.each(() => faker.date.past().toISOString()), + assets: [], + allowUpload: Sync.each(() => faker.datatype.boolean()), + allowDownload: Sync.each(() => faker.datatype.boolean()), + showMetadata: Sync.each(() => faker.datatype.boolean()), +});