From e8fb529026c535b86e672f56afdee8f3b7436d23 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Mar 2024 17:16:32 -0600 Subject: [PATCH] fix(server): getAllAssets doesn't return all assets (#7752) * fix(server): getAllAssets doesn't return all assets * try reverting * fix: archive and remove unused method * update sql * remove unused code * linting --- .../domain/repositories/asset.repository.ts | 11 +---- .../immich/api-v1/asset/asset-repository.ts | 41 ++++++++++++++-- .../immich/api-v1/asset/asset.service.spec.ts | 1 + .../src/immich/api-v1/asset/asset.service.ts | 15 +----- .../infra/repositories/asset.repository.ts | 24 --------- server/src/infra/sql/asset.repository.sql | 49 ------------------- .../repositories/asset.repository.mock.ts | 1 - 7 files changed, 42 insertions(+), 100 deletions(-) diff --git a/server/src/domain/repositories/asset.repository.ts b/server/src/domain/repositories/asset.repository.ts index dd5e76577c..847c97aae3 100644 --- a/server/src/domain/repositories/asset.repository.ts +++ b/server/src/domain/repositories/asset.repository.ts @@ -1,9 +1,4 @@ -import { - AssetSearchOneToOneRelationOptions, - AssetSearchOptions, - ReverseGeocodeResult, - SearchExploreItem, -} from '@app/domain'; +import { AssetSearchOptions, ReverseGeocodeResult, SearchExploreItem } from '@app/domain'; import { AssetEntity, AssetJobStatusEntity, AssetType, ExifEntity } from '@app/infra/entities'; import { FindOptionsRelations, FindOptionsSelect } from 'typeorm'; import { Paginated, PaginationOptions } from '../domain.util'; @@ -140,10 +135,6 @@ export interface IAssetRepository { updateOfflineLibraryAssets(libraryId: string, originalPaths: string[]): Promise; deleteAll(ownerId: string): Promise; getAll(pagination: PaginationOptions, options?: AssetSearchOptions): Paginated; - getAllByFileCreationDate( - pagination: PaginationOptions, - options?: AssetSearchOneToOneRelationOptions, - ): Paginated; getAllByDeviceId(userId: string, deviceId: string): Promise; updateAll(ids: string[], options: Partial): Promise; save(asset: Pick & Partial): Promise; diff --git a/server/src/immich/api-v1/asset/asset-repository.ts b/server/src/immich/api-v1/asset/asset-repository.ts index 2f54db27d0..18feb65dce 100644 --- a/server/src/immich/api-v1/asset/asset-repository.ts +++ b/server/src/immich/api-v1/asset/asset-repository.ts @@ -1,13 +1,14 @@ -import { AssetEntity } from '@app/infra/entities'; +import { AssetEntity, ExifEntity } from '@app/infra/entities'; +import { OptionalBetween } from '@app/infra/infra.utils'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { In } from 'typeorm/find-options/operator/In.js'; import { Repository } from 'typeorm/repository/Repository.js'; +import { AssetSearchDto } from './dto/asset-search.dto'; import { CheckExistingAssetsDto } from './dto/check-existing-assets.dto'; import { SearchPropertiesDto } from './dto/search-properties.dto'; import { CuratedLocationsResponseDto } from './response-dto/curated-locations-response.dto'; import { CuratedObjectsResponseDto } from './response-dto/curated-objects-response.dto'; - export interface AssetCheck { id: string; checksum: Buffer; @@ -21,6 +22,7 @@ export interface IAssetRepositoryV1 { get(id: string): Promise; getLocationsByUserId(userId: string): Promise; getDetectedObjectsByUserId(userId: string): Promise; + getAllByUserId(userId: string, dto: AssetSearchDto): Promise; getSearchPropertiesByUserId(userId: string): Promise; getAssetsByChecksums(userId: string, checksums: Buffer[]): Promise; getExistingAssets(userId: string, checkDuplicateAssetDto: CheckExistingAssetsDto): Promise; @@ -31,7 +33,40 @@ export const IAssetRepositoryV1 = 'IAssetRepositoryV1'; @Injectable() export class AssetRepositoryV1 implements IAssetRepositoryV1 { - constructor(@InjectRepository(AssetEntity) private assetRepository: Repository) {} + constructor( + @InjectRepository(AssetEntity) private assetRepository: Repository, + @InjectRepository(ExifEntity) private exifRepository: Repository, + ) {} + + /** + * Retrieves all assets by user ID. + * + * @param ownerId - The ID of the owner. + * @param dto - The AssetSearchDto object containing search criteria. + * @returns A Promise that resolves to an array of AssetEntity objects. + */ + getAllByUserId(ownerId: string, dto: AssetSearchDto): Promise { + return this.assetRepository.find({ + where: { + ownerId, + isVisible: true, + isFavorite: dto.isFavorite, + isArchived: dto.isArchived, + updatedAt: OptionalBetween(dto.updatedAfter, dto.updatedBefore), + }, + relations: { + exifInfo: true, + tags: true, + stack: { assets: true }, + }, + skip: dto.skip || 0, + take: dto.take, + order: { + fileCreatedAt: 'DESC', + }, + withDeleted: true, + }); + } getSearchPropertiesByUserId(userId: string): Promise { return this.assetRepository diff --git a/server/src/immich/api-v1/asset/asset.service.spec.ts b/server/src/immich/api-v1/asset/asset.service.spec.ts index 48354d440e..9f0aa371e8 100644 --- a/server/src/immich/api-v1/asset/asset.service.spec.ts +++ b/server/src/immich/api-v1/asset/asset.service.spec.ts @@ -77,6 +77,7 @@ describe('AssetService', () => { beforeEach(() => { assetRepositoryMockV1 = { get: jest.fn(), + getAllByUserId: jest.fn(), getDetectedObjectsByUserId: jest.fn(), getLocationsByUserId: jest.fn(), getSearchPropertiesByUserId: jest.fn(), diff --git a/server/src/immich/api-v1/asset/asset.service.ts b/server/src/immich/api-v1/asset/asset.service.ts index 923cb4ebe8..821a7de82a 100644 --- a/server/src/immich/api-v1/asset/asset.service.ts +++ b/server/src/immich/api-v1/asset/asset.service.ts @@ -113,19 +113,8 @@ export class AssetService { public async getAllAssets(auth: AuthDto, dto: AssetSearchDto): Promise { const userId = dto.userId || auth.user.id; await this.access.requirePermission(auth, Permission.TIMELINE_READ, userId); - const assets = await this.assetRepository.getAllByFileCreationDate( - { take: dto.take ?? 1000, skip: dto.skip }, - { - ...dto, - userIds: [userId], - withDeleted: true, - orderDirection: 'DESC', - withExif: true, - isVisible: true, - withStacked: true, - }, - ); - return assets.items.map((asset) => mapAsset(asset, { withStack: true })); + const assets = await this.assetRepositoryV1.getAllByUserId(userId, dto); + return assets.map((asset) => mapAsset(asset, { withStack: true, auth })); } async serveThumbnail(auth: AuthDto, assetId: string, dto: GetAssetThumbnailDto): Promise { diff --git a/server/src/infra/repositories/asset.repository.ts b/server/src/infra/repositories/asset.repository.ts index 4813056659..15aa11523a 100644 --- a/server/src/infra/repositories/asset.repository.ts +++ b/server/src/infra/repositories/asset.repository.ts @@ -2,7 +2,6 @@ import { AssetBuilderOptions, AssetCreate, AssetExploreFieldOptions, - AssetSearchOneToOneRelationOptions, AssetSearchOptions, AssetStats, AssetStatsOptions, @@ -233,29 +232,6 @@ export class AssetRepository implements IAssetRepository { }); } - @GenerateSql({ - params: [ - { skip: 20_000, take: 10_000 }, - { - takenBefore: DummyValue.DATE, - userIds: [DummyValue.UUID], - }, - ], - }) - getAllByFileCreationDate( - pagination: PaginationOptions, - options: AssetSearchOneToOneRelationOptions = {}, - ): Paginated { - let builder = this.repository.createQueryBuilder('asset'); - builder = searchAssetBuilder(builder, options); - builder.orderBy('asset.fileCreatedAt', options.orderDirection ?? 'DESC'); - return paginatedBuilder(builder, { - mode: PaginationMode.LIMIT_OFFSET, - skip: pagination.skip, - take: pagination.take, - }); - } - /** * Get assets by device's Id on the database * @param ownerId diff --git a/server/src/infra/sql/asset.repository.sql b/server/src/infra/sql/asset.repository.sql index 54992e5f87..4e5dd2536b 100644 --- a/server/src/infra/sql/asset.repository.sql +++ b/server/src/infra/sql/asset.repository.sql @@ -428,55 +428,6 @@ WHERE AND "isOffline" = $4 ) --- AssetRepository.getAllByFileCreationDate -SELECT - "asset"."id" AS "asset_id", - "asset"."deviceAssetId" AS "asset_deviceAssetId", - "asset"."ownerId" AS "asset_ownerId", - "asset"."libraryId" AS "asset_libraryId", - "asset"."deviceId" AS "asset_deviceId", - "asset"."type" AS "asset_type", - "asset"."originalPath" AS "asset_originalPath", - "asset"."resizePath" AS "asset_resizePath", - "asset"."webpPath" AS "asset_webpPath", - "asset"."thumbhash" AS "asset_thumbhash", - "asset"."encodedVideoPath" AS "asset_encodedVideoPath", - "asset"."createdAt" AS "asset_createdAt", - "asset"."updatedAt" AS "asset_updatedAt", - "asset"."deletedAt" AS "asset_deletedAt", - "asset"."fileCreatedAt" AS "asset_fileCreatedAt", - "asset"."localDateTime" AS "asset_localDateTime", - "asset"."fileModifiedAt" AS "asset_fileModifiedAt", - "asset"."isFavorite" AS "asset_isFavorite", - "asset"."isArchived" AS "asset_isArchived", - "asset"."isExternal" AS "asset_isExternal", - "asset"."isReadOnly" AS "asset_isReadOnly", - "asset"."isOffline" AS "asset_isOffline", - "asset"."checksum" AS "asset_checksum", - "asset"."duration" AS "asset_duration", - "asset"."isVisible" AS "asset_isVisible", - "asset"."livePhotoVideoId" AS "asset_livePhotoVideoId", - "asset"."originalFileName" AS "asset_originalFileName", - "asset"."sidecarPath" AS "asset_sidecarPath", - "asset"."stackId" AS "asset_stackId" -FROM - "assets" "asset" -WHERE - ( - "asset"."fileCreatedAt" <= $1 - AND 1 = 1 - AND "asset"."ownerId" IN ($2) - AND 1 = 1 - AND "asset"."isArchived" = $3 - ) - AND ("asset"."deletedAt" IS NULL) -ORDER BY - "asset"."fileCreatedAt" DESC -LIMIT - 10001 -OFFSET - 20000 - -- AssetRepository.getAllByDeviceId SELECT "AssetEntity"."deviceAssetId" AS "AssetEntity_deviceAssetId", diff --git a/server/test/repositories/asset.repository.mock.ts b/server/test/repositories/asset.repository.mock.ts index 63f1229a23..6143d357c1 100644 --- a/server/test/repositories/asset.repository.mock.ts +++ b/server/test/repositories/asset.repository.mock.ts @@ -18,7 +18,6 @@ export const newAssetRepositoryMock = (): jest.Mocked => { getFirstAssetForAlbumId: jest.fn(), getLastUpdatedAssetForAlbumId: jest.fn(), getAll: jest.fn().mockResolvedValue({ items: [], hasNextPage: false }), - getAllByFileCreationDate: jest.fn(), getAllByDeviceId: jest.fn(), updateAll: jest.fn(), getByLibraryId: jest.fn(),