Refactored now endpoint results

README.md

@@ -18,8 +18,7 @@ View the OpenAPI documentation of the hosted demo at https://elliot.up.railway.a
 
 There are some things you should know before making requests to Elliot.
 First of all there are two different ways to generate PDF's. Synchronous (`now`) and asynchronous (`soon`).
-In the background they are basically doing the same thing, but the print `now` endpoints wait for the print job to finish and then redirects to the collect endpoint.
-Because of the redirect to the collect endpoint you should make sure that the api client that you're using is configured to allow redirects.
+In the background they are basically doing the same thing, but the print `now` endpoints wait for the print job to finish and then returns the result.
 The print `soon` endpoint returns a job id with whicht you can check the job status and collect the result later.
 
 ### Input types
@@ -45,7 +44,7 @@ Set the header `X-JSON-ERROR-RESPONSE` to recieve errors as a JSON response.
 | SECRET_KEY                  | Highly recomended if not run in a private network.                                                   |          |         |
 | BROWSERLESS_ENDPOINT        | Not strictly required but it will not work without it with the docker image.                         |          |         |
 | MAX_TIMEOUT                 | Maximum amount of miliseconds that the generation should last. Will cancel the request when reached. |          | 10000   |
-| PERSIST_PERIOD              | How long the job result should be persisted in miliseconds                                           |          | 3600000 |
+| PERSIST_PERIOD              | How long the job result should be persisted in miliseconds if not cleaned up on collect.             |          | 3600000 |
 | ADDITIONAL_SCRIPTS          | Additional js code that will be run in every print. Very useful for handlers.                        |          | []      |
 | HTTP_HEADERS                | HTTP headers that will be set on the get request for the page to be printed.                         |          | []      |
 | CLEANUP_JOB_AFTER_COLLECTED | Clean up the job data after the result has been collected.                                           |          | false   |

src/print/collect.controller.ts

@@ -1,4 +1,5 @@
 import {
+  BadRequestException,
   Controller,
   Get,
   Logger,
@@ -8,15 +9,17 @@ import {
   Res,
   StreamableFile,
   UseFilters,
+  UseGuards,
   ValidationPipe,
 } from '@nestjs/common';
-import { ApiTags } from '@nestjs/swagger';
+import { ApiOkResponse, ApiTags } from '@nestjs/swagger';
 import { Response } from 'express';
 
 import { ConfigService } from '@nestjs/config';
+import { ApiKeyAuthGuard } from '../auth/api-key-auth.guard';
 import { ConditionalHtmlExceptionsFilter } from '../common/conditional-html.filter';
 import { PrinterQueueService } from '../queue/printer-queue.service';
-import { PrintServiceFactory } from '../whatever/print.service.factory';
+import { CollectService } from './collect.service';
 import { PrintUrlOptionalDto } from './dto/print-url-optional.dto';
 
 @Controller('collect/:jobId')
@@ -26,13 +29,16 @@ export class CollectController {
 
   constructor(
     private readonly printerQueueService: PrinterQueueService,
-    private readonly printServiceFactory: PrintServiceFactory,
     private readonly configService: ConfigService,
+    private readonly collectService: CollectService,
   ) {}
 
   @Get()
-  // @UseGuards(ApiKeyAuthGuard) // hmmmmm
+  @UseGuards(ApiKeyAuthGuard)
   @UseFilters(new ConditionalHtmlExceptionsFilter())
+  @ApiOkResponse({
+    description: 'PDF file or HTML string depending on the job output type.',
+  })
   async getJobResult(
     @Res({ passthrough: true }) response: Response,
     @Param('jobId') jobId: string,
@@ -45,27 +51,18 @@ export class CollectController {
 
     if (!job) throw new NotFoundException('Job not found');
 
-    const jobReturnValue = await this.printerQueueService.getPrintJobResult(
-      job,
-    );
-
-    const printService = this.printServiceFactory.create(
-      jobReturnValue.outputType,
-    );
+    if (!(await job.isCompleted())) {
+      throw new BadRequestException('Job not finished yet');
+    }
 
-    const collectResponse = printService.createResponse(
-      jobReturnValue.data,
+    return this.collectService.buildCollectResponse(
+      job.returnvalue,
       download,
       fileName,
+      this.configService.get<boolean>('cleanupJobAfterCollected') || cleanupJob
+        ? job
+        : null,
       response,
     );
-
-    if (
-      this.configService.get<boolean>('cleanupJobAfterCollected') ||
-      cleanupJob
-    )
-      await this.printerQueueService.removePrintJob(job);
-
-    return collectResponse;
   }
 }

src/print/collect.service.ts

@@ -0,0 +1,40 @@
+import { Injectable, Logger, StreamableFile } from '@nestjs/common';
+import { Response } from 'express';
+
+import { Job } from 'bull';
+import { JobReturnValue } from '../queue/job-return-value.interface';
+import { PrinterQueueService } from '../queue/printer-queue.service';
+import { PrintServiceFactory } from '../whatever/print.service.factory';
+
+@Injectable()
+export class CollectService {
+  private readonly logger = new Logger(CollectService.name);
+
+  constructor(
+    private readonly printerQueueService: PrinterQueueService,
+    private readonly printServiceFactory: PrintServiceFactory,
+  ) {}
+
+  async buildCollectResponse(
+    jobReturnValue: JobReturnValue,
+    download: boolean,
+    fileName: string,
+    cleanupJob: Job | null,
+    response: Response,
+  ): Promise<StreamableFile | string> {
+    const printService = this.printServiceFactory.create(
+      jobReturnValue.outputType,
+    );
+
+    const collectResponse = printService.createResponse(
+      jobReturnValue.data,
+      download,
+      fileName,
+      response,
+    );
+
+    if (cleanupJob) await this.printerQueueService.removePrintJob(cleanupJob);
+
+    return collectResponse;
+  }
+}

src/print/dto/print-url-optional.dto.ts

@@ -13,7 +13,7 @@ export class PrintUrlOptionalDto extends PrintDto {
   @IsOptional()
   @ApiPropertyOptional({
     description: 'Cleanup job data.',
-    default: false,
+    default: true,
   })
-  cleanupJob: boolean = false;
+  cleanupJob: boolean = true;
 }

src/print/print-now.controller.ts

@@ -8,12 +8,19 @@ import {
   Post,
   Query,
   Res,
+  StreamableFile,
   UseFilters,
   UseGuards,
   ValidationPipe,
 } from '@nestjs/common';
 import { JwtService } from '@nestjs/jwt';
-import { ApiBody, ApiConsumes, ApiSecurity, ApiTags } from '@nestjs/swagger';
+import {
+  ApiBody,
+  ApiConsumes,
+  ApiOkResponse,
+  ApiSecurity,
+  ApiTags,
+} from '@nestjs/swagger';
 import { Response } from 'express';
 
 import { ApiKeyAuthGuard } from '../auth/api-key-auth.guard';
@@ -22,6 +29,7 @@ import { ConditionalHtmlExceptionsFilter } from '../common/conditional-html.filt
 import { PrinterQueueService } from '../queue/printer-queue.service';
 import { PrintOptions } from '../whatever/print-options.interface';
 import { PrintOutputType } from '../whatever/print-output-type.enum'; // TODO: dont konw if it should be here, mabye common
+import { CollectService } from './collect.service';
 import { PrintUrlOptionalDto } from './dto/print-url-optional.dto';
 import { PrintUrlRequiredDto } from './dto/print-url-required.dto';
 
@@ -33,12 +41,16 @@ export class PrintNowController {
   constructor(
     private readonly printerQueueService: PrinterQueueService,
     private readonly jwtService: JwtService,
+    private readonly collectService: CollectService,
   ) {}
 
   @Get()
-  @ApiSecurity('Api key')
   @UseGuards(ApiKeyAuthGuard)
   @UseFilters(ConditionalHtmlExceptionsFilter)
+  @ApiSecurity('Api key')
+  @ApiOkResponse({
+    description: 'PDF file or HTML string depending on the output type.',
+  })
   async printNowWithParams(
     @Res({ passthrough: true }) response: Response,
     @Param('outputType') outputType: PrintOutputType,
@@ -51,9 +63,8 @@ export class PrintNowController {
       fileName,
       injectPolyfill,
       httpHeaders,
-      cleanupJob,
     }: PrintUrlRequiredDto,
-  ) {
+  ): Promise<StreamableFile | string> {
     return this.doStuff(
       {
         input: { url },
@@ -67,16 +78,18 @@ export class PrintNowController {
       response,
       download,
       fileName,
-      cleanupJob,
     );
   }
 
   @Post()
-  @ApiSecurity('Api key')
   @UseGuards(ApiKeyAuthGuard)
   @UseFilters(ConditionalHtmlExceptionsFilter)
+  @ApiSecurity('Api key')
   @ApiConsumes('text/html')
   @ApiBody({ required: false })
+  @ApiOkResponse({
+    description: 'PDF file or HTML string depending on the output type.',
+  })
   async printNowWithParamsPost(
     @Res({ passthrough: true }) response: Response,
     @Param('outputType') outputType: PrintOutputType,
@@ -89,10 +102,9 @@ export class PrintNowController {
       fileName,
       injectPolyfill,
       httpHeaders,
-      cleanupJob,
     }: PrintUrlOptionalDto,
     @Body() html: string,
-  ) {
+  ): Promise<StreamableFile | string> {
     if (url === undefined && (typeof html !== 'string' || html === '')) {
       throw new BadRequestException(
         'You have to set either url or html parameter.',
@@ -120,18 +132,20 @@ export class PrintNowController {
       response,
       download,
       fileName,
-      cleanupJob,
     );
   }
 
   @Get(':jwt')
   @UseGuards(JwtParamAuthGuard)
   @UseFilters(ConditionalHtmlExceptionsFilter)
+  @ApiOkResponse({
+    description: 'PDF file or HTML string depending on the output type.',
+  })
   async printNowWithJwt(
     @Res({ passthrough: true }) response: Response,
     @Param('outputType') outputType: PrintOutputType,
     @Param('jwt') jwt: string,
-  ) {
+  ): Promise<StreamableFile | string> {
     const {
       url,
       download,
@@ -140,7 +154,6 @@ export class PrintNowController {
       fileName,
       injectPolyfill,
       httpHeaders,
-      cleanupJob,
     } = Object.assign(new PrintUrlRequiredDto(), this.jwtService.decode(jwt));
 
     return this.doStuff(
@@ -156,7 +169,6 @@ export class PrintNowController {
       response,
       download,
       fileName,
-      cleanupJob,
     );
   }
 
@@ -167,19 +179,23 @@ export class PrintNowController {
     response: Response,
     download: boolean,
     fileName: string,
-    cleanupJob: boolean,
-  ) {
+  ): Promise<StreamableFile | string> {
     const job = await this.printerQueueService.addPrintJob(options, priority);
 
+    let jobReturnValue = null;
+
     try {
-      await job.finished();
+      jobReturnValue = await job.finished();
     } catch (e) {
       throw new InternalServerErrorException(e.message);
     }
 
-    return response.redirect(
-      `/collect/${job.id}?download=${download}&cleanupJob=${cleanupJob}` +
-        (fileName !== undefined ? `&fileName=${fileName}` : ''), // TODO: make better
+    return this.collectService.buildCollectResponse(
+      jobReturnValue,
+      download,
+      fileName,
+      job,
+      response,
     );
   }
 }

src/print/print.module.ts

@@ -2,10 +2,11 @@ import { Module } from '@nestjs/common';
 import { JwtModule } from '@nestjs/jwt';
 
 import { ConfigModule } from '@nestjs/config';
-import { WhateverModule } from 'src/whatever/whatever.module';
 import { QueueModule } from '../queue/queue.module';
+import { WhateverModule } from '../whatever/whatever.module';
 import configuration from './collect.configuration';
 import { CollectController } from './collect.controller';
+import { CollectService } from './collect.service';
 import { PrintNowController } from './print-now.controller';
 import { PrintSoonController } from './print-soon.controller';
 
@@ -18,6 +19,7 @@ import { PrintSoonController } from './print-soon.controller';
       load: [configuration],
     }),
   ],
+  providers: [CollectService],
   controllers: [PrintNowController, PrintSoonController, CollectController],
 })
 export class PrintModule {}

src/queue/printer-queue.service.ts

@@ -1,12 +1,11 @@
 import { InjectQueue } from '@nestjs/bull';
-import { BadRequestException, Injectable, Logger } from '@nestjs/common';
+import { Injectable, Logger } from '@nestjs/common';
 import { ConfigService } from '@nestjs/config';
 import { Cron } from '@nestjs/schedule';
 import { Job, Queue } from 'bull';
 import { v4 as uuid } from 'uuid';
 
 import { PrintOptions } from '../whatever/print-options.interface'; // TODO: mabye put somewhere else
-import { JobReturnValue } from './job-return-value.interface';
 import { PrintStatus } from './print-status.interface';
 
 @Injectable()
@@ -37,14 +36,6 @@ export class PrinterQueueService {
     return job;
   }
 
-  public async getPrintJobResult(job: Job): Promise<JobReturnValue> {
-    if (!(await job.isCompleted())) {
-      throw new BadRequestException('Job not finished yet');
-    }
-
-    return job.returnvalue;
-  }
-
   public async getPrintJobStatus(job: Job): Promise<PrintStatus> {
     return {
       state: (await job.isActive())