nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
67.82k stars 7.64k forks source link

Multer options asynchronously provided per request #14152

Open Azbesciak opened 1 day ago

Azbesciak commented 1 day ago

Is there an existing issue that is already proposing this?

Is your feature request related to a problem? Please describe it

Let us imagine the situation we need to upload a bunch of files, but constrains differs per endpoint, user privilage, server mode, region - whatever, it changes by time and context.

Currently I see we may have set static endpoint config when setting @UseInterceptors above the controller, or globally when setting up the MulterModule, which even has a method registerAsync that gives some hope of sharing the possibility requested above (even when asking Github copilot it says the same :) ), but unfortunatelly the method is called only at the beggining, when the app is setting up.

Describe the solution you'd like

Ideally would be to have another registerAsyncPerRequest (or just registerPerRequest because it is then async in most cases probably, or even if not it is simpler to assume that) which provides the same as the current MulterModule.registerAsync but takes the current request (and ExecutionContext) as an argument and provides options.

Teachability, documentation, adoption, migration strategy

https://docs.nestjs.com/techniques/file-upload#async-configuration - would need to be updated

What is the motivation / use case for changing the behavior?

As in the description, we fetch file upload constraints per user/company/app type from the database, dynamically. It is also configurable by the app admin, even on the file type constraint to the scope I wonder if it is not easier to make that whole validation on our side in the controller instead of using the multer middleware.

KostyaTretyak commented 1 day ago

@Azbesciak, try @ts-stack/multer.

Azbesciak commented 1 day ago

@KostyaTretyak Thank you, to clarify since you posted the official repository, you mean to implement the middleware by my own then?

KostyaTretyak commented 1 day ago

As in the description, we fetch file upload constraints per user/company/app type from the database, dynamically. It is also configurable by the app admin, even on the file type constraint to the scope I wonder if it is not easier to make that whole validation on our side in the controller instead of using the multer middleware.

I assumed from this quote of yours that you don't need to build middleware. @ts-stack/multer is intended for dynamic parsing, exactly as you described in this issue. Did I not understand you correctly?

Azbesciak commented 1 day ago

@KostyaTretyak Maybe it is rather my invalid understaning of the middleware's purpose. My goal is to reuse as much existing framework as possible. I just want to validate per each request, whether files meet conditions or not. The only problem is these conditions are not fixed and depend on the user's context, for instance. I have the whole information regarding user data already in the request, since it is provided by preceeding middlewares. But constraints regarding max files etc are fixed in MulterOptions, and the only place I can attach is fileFilter, but for the scope I would like to use it, it seems to me as a workaround - for instance, it processes one file only, whereas I need all files.

KostyaTretyak commented 1 day ago

Something like this is what you want to achieve, right?

import { createWriteStream } from 'node:fs';
import { Multer } from '@ts-stack/multer';
import { Controller, Scope, Post, Request, Req } from '@nestjs/common';
import { MyFileConfig } from './my-file-config';

@Controller({ scope: Scope.REQUEST })
export class AppController {
  constructor(private config: MyFileConfig) {}

  @Post()
  async parseFiles(@Req() request: Request) {
    // Here `photos` - is the name of the field in the HTML form.
    const multer = new Multer(this.config);
    const parsePhotos = multer.array('photos', 12);
    const parsedForm = await parsePhotos(request, request.headers);

    const promises: Promise<void>[] = [];
    parsedForm.files.forEach((file) => {
      const promise = new Promise<void>((resolve, reject) => {
        const path = `uploaded-files/${file.originalName}`;
        const writableStream = createWriteStream(path);
        writableStream.on('error', reject);
        writableStream.on('finish', resolve);
        file.stream.pipe(writableStream);
      });
      promises.push(promise);
    });

    await Promise.all(promises);

    return 'ok';
  }
}
Azbesciak commented 1 day ago

Yes, but I would rather keep the controller instance as a singleton, and provide the Multer's config per request. But at the end it all goes to the same thing - I cannot use the current Multer configuration options as these are static and global, and I need to find some workaround - or make things in my own way, without the framework help :)

Azbesciak commented 1 day ago

Actually the possible solution is to use MulterOptions.registerAsync with useClass as the one declared with @Injectable({ scope: Scope.REQUEST }). I was a bit sceptic if that may work, it did partially - not as I really wanted though, it is created before my global executors are created, and the context is not set. Thank you for the hint anyway! Maybe you have another advise how to change that not changing the whole app? Global interceptors are registered wit useGlobalInterceptors in the place I do not want to change anything. Still would be good to have it expressed explit.