Просмотр исходного кода

Revert changes to file handler constructors and not wanted changes

arvid-e 8 месяцев назад
Родитель
Сommit
f90ac852da

+ 12 - 45
apps/app/src/server/service/file-uploader/aws/index.ts

@@ -1,6 +1,5 @@
 import type { Readable } from 'stream';
 
-
 import type { GetObjectCommandInput, HeadObjectCommandInput } from '@aws-sdk/client-s3';
 import {
   S3Client,
@@ -30,7 +29,7 @@ import { configManager } from '../../config-manager';
 import {
   AbstractFileUploader, type TemporaryUrl, type SaveFileParam,
 } from '../file-uploader';
-import { ContentHeaders, applyHeaders } from '../utils';
+import { ContentHeaders } from '../utils';
 
 import { AwsMultipartUploader } from './multipart-uploader';
 
@@ -51,7 +50,7 @@ const isFileExists = async(s3: S3Client, params: HeadObjectCommandInput) => {
     await s3.send(new HeadObjectCommand(params));
   }
   catch (err) {
-    if (err != null && (err as any).code === 'NotFound') {
+    if (err != null && err.code === 'NotFound') {
       return false;
     }
     throw err;
@@ -133,13 +132,7 @@ class AwsFileUploader extends AbstractFileUploader {
    * @inheritdoc
    */
   override isValidUploadSettings(): boolean {
-    return this.configManager.getConfig('aws:s3AccessKeyId') != null
-      && this.configManager.getConfig('aws:s3SecretAccessKey') != null
-      && (
-        this.configManager.getConfig('aws:s3Region') != null
-        || this.configManager.getConfig('aws:s3CustomEndpoint') != null
-      )
-      && this.configManager.getConfig('aws:s3Bucket') != null;
+    throw new Error('Method not implemented.');
   }
 
   /**
@@ -167,7 +160,7 @@ class AwsFileUploader extends AbstractFileUploader {
    * @inheritdoc
    */
   override determineResponseMode() {
-    return this.configManager.getConfig('aws:referenceFileWithRelayMode')
+    return configManager.getConfig('aws:referenceFileWithRelayMode')
       ? ResponseMode.RELAY
       : ResponseMode.REDIRECT;
   }
@@ -201,25 +194,7 @@ class AwsFileUploader extends AbstractFileUploader {
 
 
   override async respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): Promise<void> {
-    const responseMode = this.determineResponseMode();
-
-    if (responseMode === ResponseMode.REDIRECT) {
-      // In REDIRECT mode, generate a temporary URL and redirect
-      const temporaryUrl = await this.generateTemporaryUrl(attachment, opts);
-      res.redirect(temporaryUrl.url);
-    }
-    else if (responseMode === ResponseMode.RELAY) {
-      // In RELAY mode, stream the file content
-      const readableStream = await this.findDeliveryFile(attachment);
-      const isDownload = opts?.download ?? false;
-      const contentHeaders = new ContentHeaders(attachment, { inline: !isDownload });
-      applyHeaders(res, contentHeaders.toExpressHttpHeaders());
-      readableStream.pipe(res);
-    }
-    else {
-      // Fallback for any unexpected ResponseMode
-      throw new Error(`AwsFileUploader encountered an unsupported or unexpected ResponseMode: ${responseMode}.`);
-    }
+    throw new Error('AwsFileUploader does not support ResponseMode.DELEGATE.');
   }
 
   /**
@@ -253,8 +228,8 @@ class AwsFileUploader extends AbstractFileUploader {
 
       // eslint-disable-next-line no-nested-ternary
       return 'stream' in body
-        ? (body as any).transformToWebStream() as unknown as NodeJS.ReadableStream // get stream from Blob and cast force
-        : body as unknown as NodeJS.ReadableStream; // cast force -- Updated for aws-sdk v3 'Body' type
+        ? body.stream() as unknown as NodeJS.ReadableStream // get stream from Blob and cast force
+        : body as unknown as NodeJS.ReadableStream; // cast force
     }
     catch (err) {
       logger.error(err);
@@ -272,12 +247,12 @@ class AwsFileUploader extends AbstractFileUploader {
 
     const s3 = S3Factory();
     const filePath = getFilePathOnStorage(attachment);
-    const lifetimeSecForTemporaryUrl = this.configManager.getConfig('aws:lifetimeSecForTemporaryUrl'); // <-- Changed
+    const lifetimeSecForTemporaryUrl = configManager.getConfig('aws:lifetimeSecForTemporaryUrl');
 
     // issue signed url (default: expires 120 seconds)
     // https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#getSignedUrl-property
     const isDownload = opts?.download ?? false;
-    const contentHeaders = new ContentHeaders(attachment, { inline: !isDownload }); // <-- Changed
+    const contentHeaders = new ContentHeaders(attachment, { inline: !isDownload });
     const params: GetObjectCommandInput = {
       Bucket: getS3Bucket(),
       Key: filePath,
@@ -309,11 +284,7 @@ class AwsFileUploader extends AbstractFileUploader {
       }));
     }
     catch (e) {
-      // allow duplicate abort requests to ensure abortion
-      // Note: In AWS SDK v3, 'e.response' might not exist directly.
-      // You may need to inspect the 'name' or 'Code' property for specific error types,
-      // e.g., if (e instanceof NotFound) or (e as any).name === 'NotFound'
-      if ((e as any)?.name !== 'NotFound') {
+      if (e.response?.status !== 404) {
         throw e;
       }
     }
@@ -321,19 +292,15 @@ class AwsFileUploader extends AbstractFileUploader {
 
 }
 
-// This is the factory function that creates an instance of AwsFileUploader
-// It receives 'crowi' and needs to pass the 'configManager' to the constructor
 module.exports = (crowi: Crowi) => {
-  const lib = new AwsFileUploader(crowi, configManager);
+  const lib = new AwsFileUploader(crowi);
 
-  // For ideal consistency, should be moved into the class as 'override' methods
-  // and use 'this.configManager'.
   lib.isValidUploadSettings = function() {
     return configManager.getConfig('aws:s3AccessKeyId') != null
       && configManager.getConfig('aws:s3SecretAccessKey') != null
       && (
         configManager.getConfig('aws:s3Region') != null
-        || configManager.getConfig('aws:s3CustomEndpoint') != null
+          || configManager.getConfig('aws:s3CustomEndpoint') != null
       )
       && configManager.getConfig('aws:s3Bucket') != null;
   };

+ 1 - 6
apps/app/src/server/service/file-uploader/azure.ts

@@ -85,11 +85,6 @@ function getFilePathOnStorage(attachment: IAttachmentDocument) {
 
 class AzureFileUploader extends AbstractFileUploader {
 
-  constructor(crowi: Crowi) {
-    super(crowi, configManager);
-    this.configManager = configManager;
-  }
-
   /**
    * @inheritdoc
    */
@@ -160,7 +155,7 @@ class AzureFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override respond(): Promise<void> {
+  override respond(): void {
     throw new Error('AzureFileUploader does not support ResponseMode.DELEGATE.');
   }
 

+ 7 - 10
apps/app/src/server/service/file-uploader/file-uploader.ts

@@ -9,7 +9,7 @@ import { type RespondOptions, ResponseMode } from '~/server/interfaces/attachmen
 import { Attachment, type IAttachmentDocument } from '~/server/models/attachment';
 import loggerFactory from '~/utils/logger';
 
-import type { ConfigManager } from '../config-manager';
+import { configManager } from '../config-manager';
 
 import type { MultipartUploader } from './multipart-uploader';
 
@@ -52,15 +52,12 @@ export abstract class AbstractFileUploader implements FileUploader {
 
   private crowi: Crowi;
 
-  protected configManager: ConfigManager;
-
-  constructor(crowi: Crowi, configManager: ConfigManager) {
+  constructor(crowi: Crowi) {
     this.crowi = crowi;
-    this.configManager = configManager;
   }
 
   getIsUploadable() {
-    return !this.configManager.getConfig('app:fileUploadDisabled') && this.isValidUploadSettings();
+    return !configManager.getConfig('app:fileUploadDisabled') && this.isValidUploadSettings();
   }
 
   /**
@@ -99,7 +96,7 @@ export abstract class AbstractFileUploader implements FileUploader {
       return false;
     }
 
-    return !!this.configManager.getConfig('app:fileUpload');
+    return !!configManager.getConfig('app:fileUpload');
   }
 
   abstract listFiles();
@@ -115,10 +112,10 @@ export abstract class AbstractFileUploader implements FileUploader {
    * @returns file upload total limit in bytes
    */
   getFileUploadTotalLimit() {
-    const fileUploadTotalLimit = this.configManager.getConfig('app:fileUploadType') === 'mongodb'
+    const fileUploadTotalLimit = configManager.getConfig('app:fileUploadType') === 'mongodb'
       // Use app:fileUploadTotalLimit if gridfs:totalLimit is null (default for gridfs:totalLimit is null)
-      ? this.configManager.getConfig('app:fileUploadTotalLimit')
-      : this.configManager.getConfig('app:fileUploadTotalLimit');
+      ? configManager.getConfig('app:fileUploadTotalLimit')
+      : configManager.getConfig('app:fileUploadTotalLimit');
     return fileUploadTotalLimit;
   }
 

+ 12 - 15
apps/app/src/server/service/file-uploader/gcs/index.ts

@@ -79,19 +79,11 @@ class GcsFileUploader extends AbstractFileUploader {
    */
   override isValidUploadSettings(): boolean {
     try {
-      const gcsBucket = toNonBlankStringOrUndefined(this.configManager.getConfig('gcs:bucket'));
-      // You can add a check for apiKeyJsonPath here if it's strictly required
-      // const apiKeyJsonPath = toNonBlankStringOrUndefined(this.configManager.getConfig('gcs:apiKeyJsonPath'));
-
-      if (gcsBucket == null) {
-        throw new Error('GCS bucket is not configured.');
-      }
-      // if (apiKeyJsonPath == null) { /* throw error */ } // Uncomment if API key is strictly required
-
+      getGcsBucket();
       return true;
     }
     catch (err) {
-      logger.error('GcsFileUploader isValidUploadSettings error:', err);
+      logger.error(err);
       return false;
     }
   }
@@ -122,7 +114,7 @@ class GcsFileUploader extends AbstractFileUploader {
    */
   override determineResponseMode() {
     // This is already correct in your provided code, using this.configManager
-    return this.configManager.getConfig('gcs:referenceFileWithRelayMode')
+    return configManager.getConfig('gcs:referenceFileWithRelayMode')
       ? ResponseMode.RELAY
       : ResponseMode.REDIRECT;
   }
@@ -180,7 +172,7 @@ class GcsFileUploader extends AbstractFileUploader {
     }
     catch (err) {
       logger.error(err);
-      throw new Error(`Couldn't get file from GCS for the Attachment (${attachment._id.toString()})`);
+      throw new Error(`Coudn't get file from AWS for the Attachment (${attachment._id.toString()})`);
     }
   }
 
@@ -196,7 +188,7 @@ class GcsFileUploader extends AbstractFileUploader {
     const myBucket = gcs.bucket(getGcsBucket());
     const filePath = getFilePathOnStorage(attachment);
     const file = myBucket.file(filePath);
-    const lifetimeSecForTemporaryUrl = this.configManager.getConfig('gcs:lifetimeSecForTemporaryUrl');
+    const lifetimeSecForTemporaryUrl = configManager.getConfig('gcs:lifetimeSecForTemporaryUrl');
 
     // issue signed url (default: expires 120 seconds)
     // https://cloud.google.com/storage/docs/access-control/signed-urls
@@ -230,7 +222,7 @@ class GcsFileUploader extends AbstractFileUploader {
       // allow 404: allow duplicate abort requests to ensure abortion
       // allow 499: it is the success response code for canceling upload
       // ref: https://cloud.google.com/storage/docs/performing-resumable-uploads#cancel-upload
-      if ((e as any).response?.status !== 404 && (e as any).response?.status !== 499) { // Added (e as any) for type safety
+      if (e.response?.status !== 404 && e.response?.status !== 499) {
         throw e;
       }
     }
@@ -240,7 +232,12 @@ class GcsFileUploader extends AbstractFileUploader {
 
 
 module.exports = function(crowi: Crowi) {
-  const lib = new GcsFileUploader(crowi, configManager);
+  const lib = new GcsFileUploader(crowi);
+
+  lib.isValidUploadSettings = function() {
+    return configManager.getConfig('gcs:apiKeyJsonPath') != null
+      && configManager.getConfig('gcs:bucket') != null;
+  };
 
   (lib as any).deleteFile = function(attachment) {
     const filePath = getFilePathOnStorage(attachment);

+ 2 - 2
apps/app/src/server/service/file-uploader/gridfs.ts

@@ -80,7 +80,7 @@ class GridfsFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override respond(): Promise<void> {
+  override respond(): void {
     throw new Error('GridfsFileUploader does not support ResponseMode.DELEGATE.');
   }
 
@@ -102,7 +102,7 @@ class GridfsFileUploader extends AbstractFileUploader {
 
 
 module.exports = function(crowi: Crowi) {
-  const lib = new GridfsFileUploader(crowi, configManager);
+  const lib = new GridfsFileUploader(crowi);
 
   // get Collection instance of chunk
   const chunkCollection = mongoose.connection.collection(CHUNK_COLLECTION_NAME);

+ 4 - 4
apps/app/src/server/service/file-uploader/local.ts

@@ -83,7 +83,7 @@ class LocalFileUploader extends AbstractFileUploader {
   /**
    * @inheritdoc
    */
-  override respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): Promise<void> {
+  override respond(res: Response, attachment: IAttachmentDocument, opts?: RespondOptions): void {
     throw new Error('Method not implemented.');
   }
 
@@ -104,7 +104,7 @@ class LocalFileUploader extends AbstractFileUploader {
 }
 
 module.exports = function(crowi: Crowi) {
-  const lib = new LocalFileUploader(crowi, configManager);
+  const lib = new LocalFileUploader(crowi);
 
   const basePath = path.posix.join(crowi.publicDir, 'uploads');
 
@@ -221,7 +221,7 @@ module.exports = function(crowi: Crowi) {
    * @param {Response} res
    * @param {Response} attachment
    */
-  lib.respond = async function(res, attachment, opts) {
+  lib.respond = function(res, attachment, opts) {
     // Responce using internal redirect of nginx or Apache.
     const storagePath = getFilePathOnStorage(attachment);
     const relativePath = path.relative(crowi.publicDir, storagePath);
@@ -236,7 +236,7 @@ module.exports = function(crowi: Crowi) {
       { field: 'X-Sendfile', value: storagePath },
     ]);
 
-    res.end();
+    return res.end();
   };
 
   /**

+ 1 - 1
apps/app/src/server/service/file-uploader/utils/headers.ts

@@ -89,7 +89,7 @@ export class ContentHeaders implements IContentHeaders {
         value: attachment.fileSize.toString(),
       };
     }
-  } // End of constructor
+  }
 
   /**
    * Convert to ExpressHttpHeader[]