Skip to content

Commit b664cb7

Browse files
CarsonFCarsonF
authored
Merge pull request #2878 from SeedCompany/refactor/ffprobe
2 parents a30e2a8 + dcff0b3 commit b664cb7

File tree

6 files changed

+71
-72
lines changed

6 files changed

+71
-72
lines changed

‎src/components/file/file.repository.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export class FileRepository extends CommonRepository {
154154
return result!;
155155
}
156156

157-
private hydrate() {
157+
hydrate() {
158158
return (query: Query) =>
159159
query
160160
.subQuery((sub) =>
@@ -463,13 +463,13 @@ export class FileRepository extends CommonRepository {
463463
}),
464464
)
465465
.apply(this.defaultPublicFromParent(input.public))
466-
.return<{ id: ID }>('node.id as id');
466+
.apply(this.hydrate());
467467

468468
const result = await createFile.first();
469469
if (!result) {
470470
throw new ServerException('Failed to create file version');
471471
}
472-
return result;
472+
return result.dto as FileVersion;
473473
}
474474

475475
async rename(fileNode: FileNode, newName: string): Promise<void> {

‎src/components/file/file.service.ts‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
DurationIn,
1515
generateId,
1616
ID,
17-
IdOf,
1817
InputException,
1918
NotFoundException,
2019
ServerException,
@@ -154,7 +153,9 @@ export class FileService {
154153
await this.bucket.headObject(id);
155154
return await this.bucket.getSignedUrl(GetObject, {
156155
Key: id,
157-
ResponseContentDisposition: `attachment; filename="${node.name}"`,
156+
ResponseContentDisposition: `attachment; filename="${encodeURIComponent(
157+
node.name,
158+
)}"`,
158159
ResponseContentType: node.mimeType,
159160
ResponseCacheControl: this.determineCacheHeader(node),
160161
signing: {
@@ -316,7 +317,7 @@ export class FileService {
316317
const mimeType =
317318
mimeTypeOverride ?? upload?.ContentType ?? 'application/octet-stream';
318319

319-
await this.repo.createFileVersion(
320+
const fv = await this.repo.createFileVersion(
320321
fileId,
321322
{
322323
id: uploadId,
@@ -355,16 +356,7 @@ export class FileService {
355356
}
356357
}
357358

358-
await this.mediaService.detectAndSave(
359-
this.asDownloadable(
360-
{
361-
file: uploadId as IdOf<FileVersion>,
362-
mimeType,
363-
...media,
364-
},
365-
uploadId,
366-
),
367-
);
359+
await this.mediaService.detectAndSave(fv, media);
368360

369361
// Change the file's name to match the latest version name
370362
await this.rename({ id: fileId, name }, session);

‎src/components/file/media/detect-existing-media.migration.ts‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
import { ModuleRef } from '@nestjs/core';
22
import { node, relation } from 'cypher-query-builder';
3-
import { IdOf } from '~/common';
43
import { BaseMigration, Migration } from '~/core/database';
54
import { ACTIVE } from '~/core/database/query';
65
import { FileVersion } from '../dto';
7-
import { FileService } from '../file.service';
6+
import { FileRepository } from '../file.repository';
87
import { MediaService } from './media.service';
98

10-
@Migration('2023-08-24T15:00:00')
9+
@Migration('2023-09-01T18:00:00')
1110
export class DetectExistingMediaMigration extends BaseMigration {
1211
constructor(
1312
private readonly mediaService: MediaService,
@@ -17,12 +16,10 @@ export class DetectExistingMediaMigration extends BaseMigration {
1716
}
1817

1918
async up() {
20-
const fileService = this.moduleRef.get(FileService, { strict: false });
21-
const detect = async (f: Row) => {
19+
const detect = async (f: FileVersion) => {
2220
this.logger.info('Detecting', f);
2321
try {
24-
const d = fileService.asDownloadable(f, f.file);
25-
const result = await this.mediaService.detectAndSave(d);
22+
const result = await this.mediaService.detectAndSave(f);
2623
this.logger.info('Detected and saved media', {
2724
...f,
2825
...(result ?? {}),
@@ -39,26 +36,40 @@ export class DetectExistingMediaMigration extends BaseMigration {
3936
}
4037

4138
private async *grabFileVersionsToDetect(type: string) {
39+
const fileRepo = this.moduleRef.get(FileRepository, { strict: false });
40+
4241
let page = 0;
4342
const size = 100;
4443
do {
4544
this.logger.info(`Grabbing page of files to detect ${page}`);
4645

4746
const currentPage = await this.db
4847
.query()
49-
.match([
50-
node('fv', 'FileVersion'),
51-
relation('out', '', 'mimeType', ACTIVE),
52-
node('mt', 'Property'),
53-
])
54-
.raw(
55-
`where not (fv)-[:media]->(:Media)
56-
and mt.value starts with '${type}/'`,
48+
// eslint-disable-next-line no-loop-func
49+
.subQuery((sub) =>
50+
sub
51+
.match([
52+
node('fv', 'FileVersion'),
53+
relation('out', '', 'mimeType', ACTIVE),
54+
node('mt', 'Property'),
55+
])
56+
.optionalMatch([
57+
node('fv'),
58+
relation('out', '', 'media'),
59+
node('media', 'Media'),
60+
])
61+
.with('fv, mt, media')
62+
.raw(
63+
`where mt.value starts with '${type}/' and (media is null or media.duration = 0)`,
64+
)
65+
.return('fv')
66+
.orderBy('fv.createdAt')
67+
.skip(page * size)
68+
.limit(size),
5769
)
58-
.return<Row>('fv.id as file, mt.value as mimeType')
59-
.orderBy('fv.createdAt')
60-
.skip(page * size)
61-
.limit(size)
70+
.with('fv as node')
71+
.apply(fileRepo.hydrate())
72+
.map((row) => row.dto as FileVersion)
6273
.run();
6374

6475
if (currentPage.length === 0) {
@@ -70,8 +81,3 @@ export class DetectExistingMediaMigration extends BaseMigration {
7081
} while (true);
7182
}
7283
}
73-
74-
interface Row {
75-
file: IdOf<FileVersion>;
76-
mimeType: string;
77-
}

‎src/components/file/media/media-detector.service.ts‎

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,29 @@
1-
import ffprobeBinary from '@ffprobe-installer/ffprobe';
2-
import { Injectable } from '@nestjs/common';
1+
import { path as ffprobeBinary } from '@ffprobe-installer/ffprobe';
2+
import { forwardRef, Inject, Injectable } from '@nestjs/common';
33
import { execa } from 'execa';
44
import { FFProbeResult } from 'ffprobe';
55
import { imageSize } from 'image-size';
66
import { ISize as ImageSize } from 'image-size/dist/types/interface';
7-
import { Readable } from 'stream';
87
import { Except } from 'type-fest';
98
import { retry } from '~/common/retry';
109
import { ILogger, Logger } from '~/core';
11-
import { Downloadable } from '../dto';
10+
import { FileVersion } from '../dto';
11+
import { FileService } from '../file.service';
1212
import { AnyMedia, Media } from './media.dto';
1313

1414
@Injectable()
1515
export class MediaDetector {
16-
constructor(@Logger('media:detector') private readonly logger: ILogger) {}
16+
constructor(
17+
@Inject(forwardRef(() => FileService))
18+
private readonly files: FileService & {},
19+
@Logger('media:detector') private readonly logger: ILogger,
20+
) {}
1721

1822
async detect(
19-
file: Downloadable<{ mimeType: string }>,
23+
file: FileVersion,
2024
): Promise<Except<AnyMedia, Exclude<keyof Media, '__typename'>> | null> {
2125
if (file.mimeType.startsWith('image/')) {
22-
const buffer = await file.download();
26+
const buffer = await this.files.asDownloadable(file).download();
2327

2428
let size: ImageSize = { width: 0, height: 0 };
2529
try {
@@ -43,9 +47,9 @@ export class MediaDetector {
4347
return null;
4448
}
4549

46-
const stream = await file.stream();
50+
const url = await this.files.getDownloadUrl(file);
4751

48-
const result = await this.ffprobe(stream);
52+
const result = await this.ffprobe(url);
4953
const { width, height, duration: rawDuration } = result.streams?.[0] ?? {};
5054

5155
const d = rawDuration as string | number | undefined; // I've seen as string
@@ -64,35 +68,25 @@ export class MediaDetector {
6468
};
6569
}
6670

67-
private async ffprobe(stream: Readable): Promise<Partial<FFProbeResult>> {
71+
private async ffprobe(url: string): Promise<Partial<FFProbeResult>> {
6872
try {
6973
return await retry(
7074
async () => {
7175
const probe = await execa(
72-
ffprobeBinary.path,
76+
ffprobeBinary,
7377
[
78+
'-v',
79+
'error',
7480
'-print_format',
7581
'json',
7682
'-show_format',
7783
'-show_streams',
78-
'-i',
79-
'pipe:0',
84+
url,
8085
],
8186
{
82-
reject: false, // just return error instead of throwing
83-
input: stream,
8487
timeout: 10_000,
8588
},
8689
);
87-
this.logger.info('ffprobe result', probe);
88-
// Ffprobe stops reading stdin & exits as soon as it has enough info.
89-
// This causes the input stream to SIGPIPE error.
90-
// In shells, this is normal and does not result in an error.
91-
// However, NodeJS converts/interrupts this as an EPIPE error.
92-
// https://github.com/sindresorhus/execa/issues/474#issuecomment-1640423498
93-
if (probe instanceof Error && (probe as any).code !== 'EPIPE') {
94-
throw probe;
95-
}
9690
if (probe.stdout.trim() === '') {
9791
return {};
9892
}

‎src/components/file/media/media.module.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Module } from '@nestjs/common';
1+
import { forwardRef, Module } from '@nestjs/common';
2+
import { FileModule } from '../file.module';
23
import { DetectExistingMediaMigration } from './detect-existing-media.migration';
34
import { DimensionsResolver } from './dimensions.resolver';
45
import { MediaByFileVersionLoader } from './media-by-file-version.loader';
@@ -9,6 +10,7 @@ import { MediaResolver } from './media.resolver';
910
import { MediaService } from './media.service';
1011

1112
@Module({
13+
imports: [forwardRef(() => FileModule)],
1214
providers: [
1315
DimensionsResolver,
1416
MediaByFileVersionLoader,

‎src/components/file/media/media.service.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { Injectable } from '@nestjs/common';
2-
import { Except, RequireAtLeastOne } from 'type-fest';
3-
import { NotFoundException, ServerException } from '~/common';
4-
import { Downloadable } from '../dto';
2+
import { RequireAtLeastOne } from 'type-fest';
3+
import { IdOf, NotFoundException, ServerException } from '~/common';
4+
import { FileVersion } from '../dto';
55
import { MediaDetector } from './media-detector.service';
6-
import { AnyMedia, Media, MediaUserMetadata } from './media.dto';
6+
import { AnyMedia, MediaUserMetadata } from './media.dto';
77
import { MediaRepository } from './media.repository';
88

99
@Injectable()
@@ -13,12 +13,17 @@ export class MediaService {
1313
private readonly repo: MediaRepository,
1414
) {}
1515

16-
async detectAndSave(input: Downloadable<Except<Media, 'id' | '__typename'>>) {
17-
const media = await this.detector.detect(input);
16+
async detectAndSave(file: FileVersion, metadata?: MediaUserMetadata) {
17+
const media = await this.detector.detect(file);
1818
if (!media) {
1919
return null;
2020
}
21-
return await this.repo.create({ ...input, ...media });
21+
return await this.repo.create({
22+
file: file.id as IdOf<FileVersion>,
23+
mimeType: file.mimeType,
24+
...media,
25+
...metadata,
26+
});
2227
}
2328

2429
async updateUserMetadata(

0 commit comments

Comments
 (0)