Feat/158222 158279 bulk export for file upload types other than s3/gcs #9467

arafubeatbox · 2024-12-05T23:56:28Z

目的

s3/gcs 以外のファイルアップロード設定でも bulk export を可能にする

実装内容

s3/gcs でしか許可していなかった bulk export を全てのファイルアップロード設定で有効化
PageBulkExportJobCronService で実行される compressAndUploadAsync を修正
- multipart upload ではなく uploadAttachment を使用するように変更
- compressAndUploadAsync 内での非同期処理 (非同期で pipeline を実行していた部分) はなくなったため、名前を compressAndUploadAsync -> compressAndUpload に変更
  - PageBulkExportJobCronService 内では await していないため、アップロード処理は変わらず非同期で実行される
PageBulkExportJob から multipart upload のリソース記憶用のフィールド (uploadId, uploadKey) を削除

task

https://redmine.weseek.co.jp/issues/158279

…ort-to-normal-fs

changeset-bot · 2024-12-05T23:56:31Z

⚠️ No Changeset found

Latest commit: a6a9607

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

arafubeatbox · 2024-12-06T00:45:20Z

apps/app/src/server/service/file-uploader/file-uploader.ts

@@ -164,7 +164,7 @@ export abstract class AbstractFileUploader implements FileUploader {
    throw new Error('Multipart upload not available for file upload type');
  }

-  abstract uploadAttachment(readStream: ReadStream, attachment: IAttachmentDocument): Promise<void>;
+  abstract uploadAttachment(readable: Readable, attachment: IAttachmentDocument): Promise<void>;


Archiver のインスタンスが渡せるように Readable に変更

ここデグレしないか慎重になる必要がある

attachment.js が TypeScript になっていないせいで createAttachment メソッドが型安全じゃない状態。
fs.createReadStream() の返り値は Readable にそのまま入れられる、でいいのだろうか。

デグレしちゃって戻した記録がある

#8981 の ba286f8

メモ: attachment.js で createReadStream を使用している部分

growi/apps/app/src/server/service/attachment.js

Line 31 in 383b5fe

const fileStream = fs.createReadStream(file.path, {

こちらですが、アップロード設定 "Local" と "GCS" でページにファイルを貼って動作確認してみましたが、問題なく動作しました。

ReadStream は Readable を継承しており (参考)、createReadStream は ReadStream のインスタンスを返す (参考) ので、uploadAttachment 内の操作が Readable で完結していればバグは生じなさそうです。

ただ、すみません、aws と gcs の uploadAttachment の引数の定義を Readable に書き換えるのを忘れており、gcs は ReadStream のメソッドも使っていたため、Readable で完結するように修正しました。
a6a9607

(詳細は apps/app/src/server/service/file-uploader/gcs/index.ts のコメントに記載

arafubeatbox · 2024-12-06T00:49:45Z

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

@@ -157,7 +157,7 @@ class PageBulkExportJobCronService extends CronService implements IPageBulkExpor
        exportPagesToFsAsync.bind(this)(pageBulkExportJob);
      }
      else if (pageBulkExportJob.status === PageBulkExportJobStatus.uploading) {
-        await compressAndUploadAsync.bind(this)(user, pageBulkExportJob);
+        compressAndUpload.bind(this)(user, pageBulkExportJob);


multipart upload の準備のエラーをキャッチするために await していたが、multipart upload しなくなったため await を除外。
(また、await してしまうと、compressAndUpload 内の uploadAttachment を待つようになってしまう

arafubeatbox · 2024-12-06T01:15:20Z

...page-bulk-export/server/service/page-bulk-export-job-cron/steps/compress-and-upload-async.ts

@@ -1,117 +0,0 @@
-import { Writable, pipeline } from 'stream';


ファイル名を compress-and-upload.ts に変更

arafubeatbox · 2024-12-06T01:15:43Z

...page-bulk-export/server/service/page-bulk-export-job-cron/steps/compress-and-upload-async.ts

-  return pageArchiver;
-}
-
-function getMultipartUploadWritable(


このメソッドは削除し、final で実行していた後処理だけ postProcess メソッドとして抽出。

yuki-takei · 2024-12-09T12:35:40Z

apps/app/src/server/service/file-uploader/file-uploader.ts

@@ -164,7 +164,7 @@ export abstract class AbstractFileUploader implements FileUploader {
    throw new Error('Multipart upload not available for file upload type');
  }

-  abstract uploadAttachment(readStream: ReadStream, attachment: IAttachmentDocument): Promise<void>;
+  abstract uploadAttachment(readable: Readable, attachment: IAttachmentDocument): Promise<void>;


ここデグレしないか慎重になる必要がある

attachment.js が TypeScript になっていないせいで createAttachment メソッドが型安全じゃない状態。
fs.createReadStream() の返り値は Readable にそのまま入れられる、でいいのだろうか。

yuki-takei · 2024-12-09T12:37:42Z

apps/app/src/server/service/file-uploader/file-uploader.ts

@@ -164,7 +164,7 @@ export abstract class AbstractFileUploader implements FileUploader {
    throw new Error('Multipart upload not available for file upload type');
  }

-  abstract uploadAttachment(readStream: ReadStream, attachment: IAttachmentDocument): Promise<void>;
+  abstract uploadAttachment(readable: Readable, attachment: IAttachmentDocument): Promise<void>;


デグレしちゃって戻した記録がある

#8981 の ba286f8

arafubeatbox · 2024-12-09T15:24:59Z

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

@@ -121,11 +122,12 @@ class GcsFileUploader extends AbstractFileUploader {
    const filePath = getFilePathOnStorage(attachment);
    const contentHeaders = new ContentHeaders(attachment);

-    await myBucket.upload(readStream.path.toString(), {


元々 ReadStream の path を読み取ってアップロードしていましたが、Readable に変更すると path がないため、これができなくなります。

ここでは ReadStream はパスを渡すためだけのインターフェース化しており、stream としての役割を果たしていません。

stream を使ってアップロードする方法が公式サンプルにあったため、そちらを参考にし、Readable stream を使用するコードに修正しました。
https://github.com/googleapis/nodejs-storage/blob/main/samples/streamFileUpload.js

arafubeatbox added 3 commits December 4, 2024 09:47

enable page bulk export for all file upload types

d2509d7

Merge branch 'feat/page-bulk-export' into feat/158222-158279-bulk-exp…

85ea3e6

…ort-to-normal-fs

Merge branch 'feat/page-bulk-export' into feat/158222-158279-bulk-exp…

b9c2f3d

…ort-to-normal-fs

arafubeatbox added 2 commits December 6, 2024 09:30

enable bulk export for file upload types other than s3/gcs

8b0b790

remove job stream when job completes

e38080f

arafubeatbox commented Dec 6, 2024

View reviewed changes

arafubeatbox changed the title ~~Feat/158222 158279 bulk export to normal fs~~ Feat/158222 158279 bulk export for file upload types other than s3/gcs Dec 6, 2024

arafubeatbox marked this pull request as ready for review December 6, 2024 01:12

arafubeatbox commented Dec 6, 2024

View reviewed changes

yuki-takei requested changes Dec 9, 2024

View reviewed changes

modify gcs uploadAttachment to use stream

a6a9607

arafubeatbox commented Dec 9, 2024

View reviewed changes

yuki-takei merged commit f8dfda8 into feat/page-bulk-export Dec 11, 2024
10 checks passed

yuki-takei deleted the feat/158222-158279-bulk-export-to-normal-fs branch December 11, 2024 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/158222 158279 bulk export for file upload types other than s3/gcs #9467

Feat/158222 158279 bulk export for file upload types other than s3/gcs #9467

arafubeatbox commented Dec 5, 2024 •

edited

Loading

changeset-bot bot commented Dec 5, 2024 •

edited

Loading

arafubeatbox Dec 6, 2024

yuki-takei Dec 9, 2024

yuki-takei Dec 9, 2024

arafubeatbox Dec 9, 2024

arafubeatbox Dec 9, 2024

arafubeatbox Dec 9, 2024

arafubeatbox Dec 6, 2024

arafubeatbox Dec 6, 2024

arafubeatbox Dec 6, 2024

yuki-takei Dec 9, 2024

yuki-takei Dec 9, 2024

arafubeatbox Dec 9, 2024

		@@ -1,117 +0,0 @@
		import { Writable, pipeline } from 'stream';

Feat/158222 158279 bulk export for file upload types other than s3/gcs #9467

Feat/158222 158279 bulk export for file upload types other than s3/gcs #9467

Conversation

arafubeatbox commented Dec 5, 2024 • edited Loading

目的

実装内容

task

changeset-bot bot commented Dec 5, 2024 • edited Loading

⚠️ No Changeset found

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arafubeatbox commented Dec 5, 2024 •

edited

Loading

changeset-bot bot commented Dec 5, 2024 •

edited

Loading