Browse Source

Merge pull request #9946 from weseek/imprv/165843-165846-add-orgid-appid-to-bulk-export-path

imprv: Add orgId and appId to bulk export path
mergify[bot] 1 year ago
parent
commit
44525c1a05

+ 12 - 6
apps/app/src/features/page-bulk-export/server/service/page-bulk-export-job-cron/index.ts

@@ -62,7 +62,6 @@ class PageBulkExportJobCronService extends CronService implements IPageBulkExpor
   compressExtension = 'tar.gz';
 
   // temporal path of local fs to output page files before upload
-  // TODO: If necessary, change to a proper path in https://redmine.weseek.co.jp/issues/149512
   tmpOutputRootDir = '/tmp/page-bulk-export';
 
   // Keep track of the stream executed for PageBulkExportJob to destroy it on job failure.
@@ -104,10 +103,18 @@ class PageBulkExportJobCronService extends CronService implements IPageBulkExpor
    * @param isHtmlPath whether the tmp output path is for html files
    */
   getTmpOutputDir(pageBulkExportJob: PageBulkExportJobDocument, isHtmlPath = false): string {
-    if (isHtmlPath) {
-      return path.join(this.tmpOutputRootDir, 'html', pageBulkExportJob._id.toString());
+    const isGrowiCloud = configManager.getConfig('app:growiCloudUri') != null;
+    const appId = configManager.getConfig('app:growiAppIdForCloud')?.toString();
+    const jobId = pageBulkExportJob._id.toString();
+
+    if (isGrowiCloud) {
+      if (appId == null) {
+        throw new Error('appId is required for bulk export on GROWI.cloud');
+      }
     }
-    return path.join(this.tmpOutputRootDir, pageBulkExportJob._id.toString());
+
+    const basePath = isHtmlPath ? path.join(this.tmpOutputRootDir, 'html') : this.tmpOutputRootDir;
+    return path.join(basePath, appId ?? '', jobId);
   }
 
   /**
@@ -242,13 +249,12 @@ class PageBulkExportJobCronService extends CronService implements IPageBulkExpor
 
     const promises = [
       PageBulkExportPageSnapshot.deleteMany({ pageBulkExportJob }),
-      // delete /tmp/page-bulk-export/{jobId} dir
       fs.promises.rm(this.getTmpOutputDir(pageBulkExportJob), { recursive: true, force: true }),
     ];
 
+    // clean up html files exported for PDF conversion
     if (pageBulkExportJob.format === PageBulkExportFormat.pdf) {
       promises.push(
-        // delete /tmp/page-bulk-export/html/{jobId} dir
         fs.promises.rm(this.getTmpOutputDir(pageBulkExportJob, true), { recursive: true, force: true }),
       );
     }

+ 10 - 1
apps/app/src/features/page-bulk-export/server/service/page-bulk-export-job-cron/request-pdf-converter.ts

@@ -18,6 +18,12 @@ export async function requestPdfConverter(pageBulkExportJob: PageBulkExportJobDo
     throw new Error('createdAt is not set');
   }
 
+  const isGrowiCloud = configManager.getConfig('app:growiCloudUri') != null;
+  const appId = configManager.getConfig('app:growiAppIdForCloud');
+  if (isGrowiCloud && (appId == null)) {
+    throw new Error('appId is required for bulk export on GROWI.cloud');
+  }
+
   const exportJobExpirationSeconds = configManager.getConfig('app:bulkExportJobExpirationSeconds');
   const bulkExportJobExpirationDate = new Date(jobCreatedAt.getTime() + exportJobExpirationSeconds * 1000);
   let pdfConvertStatus: PdfCtrlSyncJobStatusBodyStatus = PdfCtrlSyncJobStatusBodyStatus.HTML_EXPORT_IN_PROGRESS;
@@ -41,7 +47,10 @@ export async function requestPdfConverter(pageBulkExportJob: PageBulkExportJobDo
     }
 
     const res = await pdfCtrlSyncJobStatus({
-      jobId: pageBulkExportJob._id.toString(), expirationDate: bulkExportJobExpirationDate.toISOString(), status: pdfConvertStatus,
+      appId: appId?.toString(),
+      jobId: pageBulkExportJob._id.toString(),
+      expirationDate: bulkExportJobExpirationDate.toISOString(),
+      status: pdfConvertStatus,
     }, { baseURL: configManager.getConfig('app:pageBulkExportPdfConverterUri') });
 
     if (res.data.status === PdfCtrlSyncJobStatus202Status.PDF_EXPORT_DONE) {

+ 10 - 7
apps/pdf-converter/src/controllers/pdf.ts

@@ -3,7 +3,7 @@ import { Controller } from '@tsed/di';
 import { InternalServerError } from '@tsed/exceptions';
 import { Logger } from '@tsed/logger';
 import {
-  Post, Returns, Enum, Description,
+  Post, Returns, Enum, Description, Required,
 } from '@tsed/schema';
 
 import PdfConvertService, { JobStatusSharedWithGrowi, JobStatus } from '../service/pdf-convert.js';
@@ -28,20 +28,23 @@ class PdfCtrl {
     Return resulting status of job to GROWI.
   `)
   async syncJobStatus(
-    @BodyParams('jobId') jobId: string,
-    @BodyParams('expirationDate') expirationDateStr: string,
-    @BodyParams('status') @Enum(Object.values(JobStatusSharedWithGrowi)) growiJobStatus: JobStatusSharedWithGrowi,
-  ): Promise<{ status: JobStatus }> {
+    @Required() @BodyParams('jobId') jobId: string,
+    @Required() @BodyParams('expirationDate') expirationDateStr: string,
+    @Required() @BodyParams('status') @Enum(Object.values(JobStatusSharedWithGrowi)) growiJobStatus: JobStatusSharedWithGrowi,
+    @BodyParams('appId') appId?: string,
+  ): Promise<{ status: JobStatus } | undefined> {
     const expirationDate = new Date(expirationDateStr);
     try {
-      await this.pdfConvertService.registerOrUpdateJob(jobId, expirationDate, growiJobStatus);
+      await this.pdfConvertService.registerOrUpdateJob(jobId, expirationDate, growiJobStatus, appId);
       const status = this.pdfConvertService.getJobStatus(jobId); // get status before cleanup
       this.pdfConvertService.cleanUpJobList();
       return { status };
     }
     catch (err) {
       this.logger.error('Failed to register or update job', err);
-      throw new InternalServerError(err);
+      if (err instanceof Error) {
+        throw new InternalServerError(err.message);
+      }
     }
   }
 

+ 1 - 1
apps/pdf-converter/src/server.ts

@@ -34,7 +34,7 @@ const PORT = Number(process.env.PORT || 3010);
 class Server {
 
   @Inject()
-    app: PlatformApplication;
+    app: PlatformApplication | undefined;
 
 }
 

+ 23 - 12
apps/pdf-converter/src/service/pdf-convert.ts

@@ -59,11 +59,17 @@ class PdfConvertService implements OnInit {
   /**
    * Register or update job inside jobList with given jobId, expirationDate, and status.
    * If job is new, start reading html files and convert them to pdf.
-   * @param jobId id of PageBulkExportJob
+   * @param jobId PageBulkExportJob ID
    * @param expirationDate expiration date of job
    * @param status status of job
+   * @param appId application ID for GROWI.cloud
    */
-  async registerOrUpdateJob(jobId: string, expirationDate: Date, status: JobStatusSharedWithGrowi): Promise<void> {
+  async registerOrUpdateJob(
+      jobId: string,
+      expirationDate: Date,
+      status: JobStatusSharedWithGrowi,
+      appId?: string,
+  ): Promise<void> {
     const isJobNew = !(jobId in this.jobList);
 
     if (isJobNew) {
@@ -83,7 +89,7 @@ class PdfConvertService implements OnInit {
     }
 
     if (isJobNew && status !== JobStatus.FAILED) {
-      this.readHtmlAndConvertToPdfUntilFinish(jobId);
+      this.readHtmlAndConvertToPdfUntilFinish(jobId, appId);
     }
   }
 
@@ -134,9 +140,10 @@ class PdfConvertService implements OnInit {
   /**
    * Read html files from shared fs path, convert them to pdf, and save them to shared fs path.
    * Repeat this until all html files are converted to pdf or job fails.
-   * @param jobId id of PageBulkExportJob
+   * @param jobId PageBulkExportJob ID
+   * @param appId application ID for GROWI.cloud
    */
-  private async readHtmlAndConvertToPdfUntilFinish(jobId: string): Promise<void> {
+  private async readHtmlAndConvertToPdfUntilFinish(jobId: string, appId?: string): Promise<void> {
     while (!this.isJobCompleted(jobId)) {
       // eslint-disable-next-line no-await-in-loop
       await new Promise(resolve => setTimeout(resolve, 10 * 1000));
@@ -146,7 +153,7 @@ class PdfConvertService implements OnInit {
           throw new Error('Job expired');
         }
 
-        const htmlReadable = this.getHtmlReadable(jobId);
+        const htmlReadable = this.getHtmlReadable(jobId, appId);
         const pdfWritable = this.getPdfWritable();
         this.jobList[jobId].currentStream = htmlReadable;
 
@@ -165,11 +172,13 @@ class PdfConvertService implements OnInit {
 
   /**
    * Get readable stream that reads html files from shared fs path
-   * @param jobId id of PageBulkExportJob
+   * @param jobId PageBulkExportJob ID
+   * @param appId application ID for GROWI.cloud
    * @returns readable stream
    */
-  private getHtmlReadable(jobId: string): Readable {
-    const htmlFileEntries = fs.readdirSync(path.join(this.tmpHtmlDir, jobId), { recursive: true, withFileTypes: true }).filter(entry => entry.isFile());
+  private getHtmlReadable(jobId: string, appId?: string): Readable {
+    const jobHtmlDir = path.join(this.tmpHtmlDir, appId ?? '', jobId);
+    const htmlFileEntries = fs.readdirSync(jobHtmlDir, { recursive: true, withFileTypes: true }).filter(entry => entry.isFile());
     let index = 0;
 
     const jobList = this.jobList;
@@ -215,7 +224,9 @@ class PdfConvertService implements OnInit {
           await fs.promises.rm(pageInfo.htmlFilePath, { force: true });
         }
         catch (err) {
-          callback(err);
+          if (err instanceof Error) {
+            callback(err);
+          }
           return;
         }
         callback();
@@ -229,9 +240,9 @@ class PdfConvertService implements OnInit {
    * @returns converted pdf
    */
   private async convertHtmlToPdf(htmlString: string): Promise<Buffer> {
-    const executeConvert = async(retries: number) => {
+    const executeConvert = async(retries: number): Promise<Buffer> => {
       try {
-        return this.puppeteerCluster.execute(htmlString);
+        return this.puppeteerCluster?.execute(htmlString);
       }
       catch (err) {
         if (retries > 0) {

+ 1 - 1
apps/pdf-converter/tsconfig.json

@@ -8,7 +8,7 @@
     "esModuleInterop": true,
     "experimentalDecorators": true,
     "emitDecoratorMetadata": true,
-    "strict": false
+    "strict": true
   },
   "include": ["./src/**/*", "./test/**/*"],
   "exclude": ["node_modules", "dist"]

+ 11 - 4
packages/pdf-converter-client/src/index.ts

@@ -24,6 +24,9 @@ export type PdfCtrlSyncJobStatus202 = {
   status: PdfCtrlSyncJobStatus202Status;
 };
 
+/**
+ * @minLength 1
+ */
 export type PdfCtrlSyncJobStatusBodyStatus = typeof PdfCtrlSyncJobStatusBodyStatus[keyof typeof PdfCtrlSyncJobStatusBodyStatus];
 
 
@@ -35,9 +38,13 @@ export const PdfCtrlSyncJobStatusBodyStatus = {
 } as const;
 
 export type PdfCtrlSyncJobStatusBody = {
-  expirationDate?: string;
-  jobId?: string;
-  status?: PdfCtrlSyncJobStatusBodyStatus;
+  appId?: string;
+  /** @minLength 1 */
+  expirationDate: string;
+  /** @minLength 1 */
+  jobId: string;
+  /** @minLength 1 */
+  status: PdfCtrlSyncJobStatusBodyStatus;
 };
 
 export interface GenericError {
@@ -85,7 +92,7 @@ export interface InternalServerError {
   
  */
 export const pdfCtrlSyncJobStatus = <TData = AxiosResponse<PdfCtrlSyncJobStatus202>>(
-    pdfCtrlSyncJobStatusBody?: PdfCtrlSyncJobStatusBody, options?: AxiosRequestConfig
+    pdfCtrlSyncJobStatusBody: PdfCtrlSyncJobStatusBody, options?: AxiosRequestConfig
  ): Promise<TData> => {
     return axios.post(
       `/pdf/sync-job`,