-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat: add support for s3 and minio as storage providers #873
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,99 @@ | ||||||
import { S3Client, PutObjectCommand, DeleteObjectCommand } from '@aws-sdk/client-s3'; | ||||||
import 'multer'; | ||||||
import { makeId } from '@gitroom/nestjs-libraries/services/make.is'; | ||||||
import mime from 'mime-types'; | ||||||
// @ts-ignore | ||||||
import { getExtension } from 'mime'; | ||||||
Comment on lines
+5
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using @ts-ignore suppresses TypeScript errors without addressing the underlying issue. Consider using proper type definitions or a more specific type assertion.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
import { IUploadProvider } from './upload.interface'; | ||||||
|
||||||
class MinIOStorage implements IUploadProvider { | ||||||
private _client: S3Client; | ||||||
|
||||||
constructor( | ||||||
private _accessKeyId: string, | ||||||
private _secretAccessKey: string, | ||||||
private _region: string, | ||||||
private _bucketName: string, | ||||||
private _endpoint: string | ||||||
) { | ||||||
this._client = new S3Client({ | ||||||
endpoint: _endpoint, | ||||||
region: _region, | ||||||
credentials: { | ||||||
accessKeyId: _accessKeyId, | ||||||
secretAccessKey: _secretAccessKey, | ||||||
}, | ||||||
forcePathStyle: true, // Required for MinIO | ||||||
}); | ||||||
} | ||||||
|
||||||
private getUploadUrl(fileName: string): string { | ||||||
// For MinIO with path-style, the URL format is: endpoint/bucket/file | ||||||
return `${this._endpoint}/${this._bucketName}/${fileName}`; | ||||||
} | ||||||
|
||||||
async uploadSimple(path: string) { | ||||||
const loadImage = await fetch(path); | ||||||
const contentType = | ||||||
loadImage?.headers?.get('content-type') || | ||||||
loadImage?.headers?.get('Content-Type'); | ||||||
const extension = getExtension(contentType)!; | ||||||
const id = makeId(10); | ||||||
|
||||||
const params = { | ||||||
Bucket: this._bucketName, | ||||||
Key: `${id}.${extension}`, | ||||||
Body: Buffer.from(await loadImage.arrayBuffer()), | ||||||
ContentType: contentType, | ||||||
ACL: 'public-read', | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting ACL to 'public-read' makes uploaded files publicly accessible. Consider if this is intended behavior and whether access should be more restricted based on the file type or user permissions.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
}; | ||||||
|
||||||
const command = new PutObjectCommand({ ...params }); | ||||||
await this._client.send(command); | ||||||
|
||||||
return this.getUploadUrl(`${id}.${extension}`); | ||||||
} | ||||||
|
||||||
async uploadFile(file: Express.Multer.File): Promise<any> { | ||||||
const id = makeId(10); | ||||||
const extension = mime.extension(file.mimetype) || ''; | ||||||
|
||||||
const command = new PutObjectCommand({ | ||||||
Bucket: this._bucketName, | ||||||
Key: `${id}.${extension}`, | ||||||
Body: file.buffer, | ||||||
ContentType: file.mimetype, | ||||||
ACL: 'public-read', | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting ACL to 'public-read' makes uploaded files publicly accessible. Consider if this is intended behavior and whether access should be more restricted based on the file type or user permissions.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
}); | ||||||
|
||||||
await this._client.send(command); | ||||||
|
||||||
const uploadUrl = this.getUploadUrl(`${id}.${extension}`); | ||||||
return { | ||||||
filename: `${id}.${extension}`, | ||||||
mimetype: file.mimetype, | ||||||
size: file.size, | ||||||
buffer: file.buffer, | ||||||
originalname: `${id}.${extension}`, | ||||||
fieldname: 'file', | ||||||
path: uploadUrl, | ||||||
destination: uploadUrl, | ||||||
encoding: '7bit', | ||||||
stream: file.buffer as any, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using 'as any' bypasses type safety. Consider defining a proper type or using a more specific type assertion if the buffer needs to be treated as a stream.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
}; | ||||||
} | ||||||
|
||||||
async removeFile(filePath: string): Promise<void> { | ||||||
const fileName = filePath.split('/').pop(); | ||||||
if (fileName) { | ||||||
const command = new DeleteObjectCommand({ | ||||||
Bucket: this._bucketName, | ||||||
Key: fileName, | ||||||
}); | ||||||
await this._client.send(command); | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
export { MinIOStorage }; | ||||||
export default MinIOStorage; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,103 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { S3Client, PutObjectCommand, DeleteObjectCommand } from '@aws-sdk/client-s3'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import 'multer'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { makeId } from '@gitroom/nestjs-libraries/services/make.is'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import mime from 'mime-types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// @ts-ignore | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { getExtension } from 'mime'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+3
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAddress import issues and improve type safety There are several concerns with the imports:
Let me verify the correct import path for the makeId function: For the -// @ts-ignore
-import { getExtension } from 'mime';
+import { extension } from 'mime-types'; Then update the usage accordingly in the methods. 🏁 Script executed: #!/bin/bash
# Check if the makeId import path is correct
fd -t f "make.is" . | head -10
fd -t f -e ts -e js | xargs grep -l "makeId" | head -10 Length of output: 764 Update MIME extension handling and remove The • Remove these lines: - // @ts-ignore
- import { getExtension } from 'mime'; • Add this line (you can coalesce with the existing + import { extension } from 'mime-types'; • Then update any - const ext = getExtension(file.mimetype)!;
+ const ext = extension(file.mimetype)!; This removes the 🤖 Prompt for AI Agents
Comment on lines
+5
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using @ts-ignore suppresses TypeScript errors without addressing the underlying issue. Consider using proper type definitions or a more specific type assertion.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { IUploadProvider } from './upload.interface'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class S3Storage implements IUploadProvider { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private _client: S3Client; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
constructor( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private _accessKeyId: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private _secretAccessKey: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private _region: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private _bucketName: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private _endpoint?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this._client = new S3Client({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
endpoint: _endpoint, // Optional custom endpoint for S3-compatible services like MinIO | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
region: _region, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
credentials: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
accessKeyId: _accessKeyId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
secretAccessKey: _secretAccessKey, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
private getUploadUrl(fileName: string): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (this._endpoint) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// For custom S3-compatible endpoints (like MinIO), use endpoint/bucket/file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return `${this._endpoint}/${this._bucketName}/${fileName}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// For standard AWS S3, use bucket.s3.region.amazonaws.com/file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return `https://${this._bucketName}.s3.${this._region}.amazonaws.com/${fileName}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async uploadSimple(path: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const loadImage = await fetch(path); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const contentType = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
loadImage?.headers?.get('content-type') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
loadImage?.headers?.get('Content-Type'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const extension = getExtension(contentType)!; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const id = makeId(10); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const params = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Bucket: this._bucketName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Key: `${id}.${extension}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Body: Buffer.from(await loadImage.arrayBuffer()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ContentType: contentType, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ACL: 'public-read', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting ACL to 'public-read' makes uploaded files publicly accessible. Consider if this is intended behavior and whether access should be more restricted based on the file type or user permissions.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const command = new PutObjectCommand({ ...params }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await this._client.send(command); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return this.getUploadUrl(`${id}.${extension}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+39
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve error handling and content type validation The method has potential robustness issues:
async uploadSimple(path: string) {
- const loadImage = await fetch(path);
+ const loadImage = await fetch(path).catch(err => {
+ throw new Error(`Failed to fetch file from ${path}: ${err.message}`);
+ });
const contentType =
loadImage?.headers?.get('content-type') ||
loadImage?.headers?.get('Content-Type');
- const extension = getExtension(contentType)!;
+ const extension = getExtension(contentType);
+ if (!extension) {
+ throw new Error(`Unable to determine file extension for content type: ${contentType}`);
+ }
const id = makeId(10);
const params = {
Bucket: this._bucketName,
Key: `${id}.${extension}`,
Body: Buffer.from(await loadImage.arrayBuffer()),
ContentType: contentType,
ACL: 'public-read',
};
const command = new PutObjectCommand({ ...params });
- await this._client.send(command);
+ await this._client.send(command).catch(err => {
+ throw new Error(`Failed to upload file to S3: ${err.message}`);
+ });
return this.getUploadUrl(`${id}.${extension}`);
} 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async uploadFile(file: Express.Multer.File): Promise<any> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const id = makeId(10); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const extension = mime.extension(file.mimetype) || ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const command = new PutObjectCommand({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Bucket: this._bucketName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Key: `${id}.${extension}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Body: file.buffer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ContentType: file.mimetype, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ACL: 'public-read', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting ACL to 'public-read' makes uploaded files publicly accessible. Consider if this is intended behavior and whether access should be more restricted based on the file type or user permissions.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await this._client.send(command); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const uploadUrl = this.getUploadUrl(`${id}.${extension}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
filename: `${id}.${extension}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
mimetype: file.mimetype, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
size: file.size, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
buffer: file.buffer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
originalname: `${id}.${extension}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fieldname: 'file', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
path: uploadUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
destination: uploadUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
encoding: '7bit', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stream: file.buffer as any, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using 'as any' bypasses type safety. Consider defining a proper type or using a more specific type assertion if the buffer needs to be treated as a stream.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+61
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling and improve type safety Similar issues as
async uploadFile(file: Express.Multer.File): Promise<any> {
const id = makeId(10);
- const extension = mime.extension(file.mimetype) || '';
+ const extension = mime.extension(file.mimetype);
+ if (!extension) {
+ throw new Error(`Unable to determine file extension for mimetype: ${file.mimetype}`);
+ }
const command = new PutObjectCommand({
Bucket: this._bucketName,
Key: `${id}.${extension}`,
Body: file.buffer,
ContentType: file.mimetype,
ACL: 'public-read',
});
- await this._client.send(command);
+ await this._client.send(command).catch(err => {
+ throw new Error(`Failed to upload file to S3: ${err.message}`);
+ });
const uploadUrl = this.getUploadUrl(`${id}.${extension}`);
return {
filename: `${id}.${extension}`,
mimetype: file.mimetype,
size: file.size,
buffer: file.buffer,
originalname: `${id}.${extension}`,
fieldname: 'file',
path: uploadUrl,
destination: uploadUrl,
encoding: '7bit',
stream: file.buffer as any,
};
} 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async removeFile(filePath: string): Promise<void> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const fileName = filePath.split('/').pop(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (fileName) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const command = new DeleteObjectCommand({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Bucket: this._bucketName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Key: fileName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await this._client.send(command); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+90
to
+99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve file path parsing robustness The current implementation of extracting filename from path is fragile and could fail with unexpected path formats. async removeFile(filePath: string): Promise<void> {
- const fileName = filePath.split('/').pop();
+ // Extract filename from URL path, handling both full URLs and relative paths
+ let fileName: string;
+ try {
+ const url = new URL(filePath);
+ fileName = url.pathname.split('/').pop() || '';
+ } catch {
+ // If not a valid URL, treat as relative path
+ fileName = filePath.split('/').pop() || '';
+ }
+
if (fileName) {
const command = new DeleteObjectCommand({
Bucket: this._bucketName,
Key: fileName,
});
- await this._client.send(command);
+ await this._client.send(command).catch(err => {
+ throw new Error(`Failed to delete file from S3: ${err.message}`);
+ });
+ } else {
+ throw new Error(`Unable to extract filename from path: ${filePath}`);
}
} 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export { S3Storage }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export default S3Storage; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,8 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { CloudflareStorage } from './cloudflare.storage'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { IUploadProvider } from './upload.interface'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { LocalStorage } from './local.storage'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { S3Storage } from './s3.storage'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { MinIOStorage } from './minio.storage'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export class UploadFactory { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
static createStorage(): IUploadProvider { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -18,6 +20,22 @@ export class UploadFactory { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.CLOUDFLARE_BUCKETNAME!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.CLOUDFLARE_BUCKET_URL! | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case 's3': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return new S3Storage( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.S3_ACCESS_KEY_ID!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.S3_SECRET_ACCESS_KEY!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.S3_REGION!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.S3_BUCKET_NAME!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.S3_ENDPOINT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
case 'minio': | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return new MinIOStorage( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.MINIO_ACCESS_KEY!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.MINIO_SECRET_KEY!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.MINIO_REGION!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.MINIO_BUCKET_NAME!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
process.env.MINIO_ENDPOINT! | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+23
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider validating environment variables and handling inconsistent endpoint requirements The implementation looks good, but there are a few concerns:
Consider adding validation and making the endpoint handling consistent: case 's3':
+ if (!process.env.S3_ACCESS_KEY_ID || !process.env.S3_SECRET_ACCESS_KEY || !process.env.S3_REGION || !process.env.S3_BUCKET_NAME) {
+ throw new Error('Missing required S3 environment variables');
+ }
return new S3Storage(
process.env.S3_ACCESS_KEY_ID!,
process.env.S3_SECRET_ACCESS_KEY!,
process.env.S3_REGION!,
process.env.S3_BUCKET_NAME!,
process.env.S3_ENDPOINT
);
case 'minio':
+ if (!process.env.MINIO_ACCESS_KEY || !process.env.MINIO_SECRET_KEY || !process.env.MINIO_REGION || !process.env.MINIO_BUCKET_NAME || !process.env.MINIO_ENDPOINT) {
+ throw new Error('Missing required MinIO environment variables');
+ }
return new MinIOStorage(
process.env.MINIO_ACCESS_KEY!,
process.env.MINIO_SECRET_KEY!,
process.env.MINIO_REGION!,
process.env.MINIO_BUCKET_NAME!,
process.env.MINIO_ENDPOINT!
); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
default: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw new Error(`Invalid storage type ${storageProvider}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The S3_UPLOAD_URL environment variable is documented but not used in the S3Storage implementation. Consider removing this unused configuration or implementing its usage.
Copilot uses AI. Check for mistakes.