feat: Add support for private files to file module (#8169)
This commit is contained in:
@@ -7,6 +7,7 @@ type UploadFilesStepInput = {
|
||||
filename: string
|
||||
mimeType: string
|
||||
content: string
|
||||
access: "public" | "private"
|
||||
}[]
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ type WorkflowInput = {
|
||||
filename: string
|
||||
mimeType: string
|
||||
content: string
|
||||
access: "public" | "private"
|
||||
}[]
|
||||
}
|
||||
|
||||
|
||||
@@ -16,4 +16,9 @@ export interface CreateFileDTO {
|
||||
* The file content as a binary-encoded string
|
||||
*/
|
||||
content: string
|
||||
|
||||
/**
|
||||
* The access level of the file. Defaults to private if not passed
|
||||
*/
|
||||
access?: "public" | "private"
|
||||
}
|
||||
|
||||
@@ -24,10 +24,6 @@ export type ProviderGetFileDTO = {
|
||||
* The file's key.
|
||||
*/
|
||||
fileKey: string
|
||||
/**
|
||||
* Whether the file is private.
|
||||
*/
|
||||
isPrivate?: boolean
|
||||
[x: string]: unknown
|
||||
}
|
||||
|
||||
@@ -65,6 +61,11 @@ export type ProviderUploadFileDTO = {
|
||||
* The file content as a binary-encoded string
|
||||
*/
|
||||
content: string
|
||||
|
||||
/**
|
||||
* The access level of the file. Defaults to private if not passed
|
||||
*/
|
||||
access?: "public" | "private"
|
||||
}
|
||||
|
||||
export interface IFileProvider {
|
||||
@@ -85,6 +86,7 @@ export interface IFileProvider {
|
||||
*
|
||||
*/
|
||||
delete(fileData: ProviderDeleteFileDTO): Promise<void>
|
||||
|
||||
/**
|
||||
* This method is used to retrieve a download URL of the file. For some file services, such as S3, a presigned URL indicates a temporary URL to get access to a file.
|
||||
*
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
export interface LocalFileServiceOptions {
|
||||
upload_dir?: string
|
||||
private_upload_dir?: string
|
||||
backend_url?: string
|
||||
}
|
||||
|
||||
@@ -25,6 +25,7 @@ export const POST = async (
|
||||
filename: f.originalname,
|
||||
mimeType: f.mimetype,
|
||||
content: f.buffer.toString("binary"),
|
||||
access: "public",
|
||||
})),
|
||||
},
|
||||
})
|
||||
|
||||
@@ -6,11 +6,14 @@ import path from "path"
|
||||
export class LocalFileService extends AbstractFileProviderService {
|
||||
static identifier = "localfs"
|
||||
protected uploadDir_: string
|
||||
protected privateUploadDir_: string
|
||||
protected backendUrl_: string
|
||||
|
||||
constructor(_, options: LocalFileServiceOptions) {
|
||||
super()
|
||||
this.uploadDir_ = options?.upload_dir || path.join(process.cwd(), "static")
|
||||
this.privateUploadDir_ =
|
||||
options?.private_upload_dir || path.join(process.cwd(), "private")
|
||||
this.backendUrl_ = options?.backend_url || "http://localhost:9000/static"
|
||||
}
|
||||
|
||||
@@ -29,14 +32,19 @@ export class LocalFileService extends AbstractFileProviderService {
|
||||
}
|
||||
|
||||
const parsedFilename = path.parse(file.filename)
|
||||
await this.ensureDirExists(parsedFilename.dir)
|
||||
const baseDir =
|
||||
file.access === "public" ? this.uploadDir_ : this.privateUploadDir_
|
||||
const dir = await this.ensureDirExists(baseDir, parsedFilename.dir)
|
||||
|
||||
const fileKey = path.join(
|
||||
parsedFilename.dir,
|
||||
`${Date.now()}-${parsedFilename.base}`
|
||||
// We append "private" to the file key so deletions and presigned URLs can know which folder to look into
|
||||
`${Date.now()}-${parsedFilename.base}${
|
||||
file.access === "public" ? "" : "-private"
|
||||
}`
|
||||
)
|
||||
|
||||
const filePath = this.getUploadFilePath(fileKey)
|
||||
const filePath = this.getUploadFilePath(baseDir, fileKey)
|
||||
const fileUrl = this.getUploadFileUrl(fileKey)
|
||||
|
||||
const content = Buffer.from(file.content as string, "binary")
|
||||
@@ -49,7 +57,11 @@ export class LocalFileService extends AbstractFileProviderService {
|
||||
}
|
||||
|
||||
async delete(file: FileTypes.ProviderDeleteFileDTO): Promise<void> {
|
||||
const filePath = this.getUploadFilePath(file.fileKey)
|
||||
const baseDir = file.fileKey.endsWith("-private")
|
||||
? this.privateUploadDir_
|
||||
: this.uploadDir_
|
||||
|
||||
const filePath = this.getUploadFilePath(baseDir, file.fileKey)
|
||||
try {
|
||||
await fs.access(filePath, fs.constants.F_OK)
|
||||
await fs.unlink(filePath)
|
||||
@@ -60,26 +72,34 @@ export class LocalFileService extends AbstractFileProviderService {
|
||||
return
|
||||
}
|
||||
|
||||
// For private files, we simply return the file path, which can then be loaded manually by the backend.
|
||||
// The local file provider doesn't support presigned URLs for private files.
|
||||
async getPresignedDownloadUrl(
|
||||
fileData: FileTypes.ProviderGetFileDTO
|
||||
file: FileTypes.ProviderGetFileDTO
|
||||
): Promise<string> {
|
||||
const isPrivate = file.fileKey.endsWith("-private")
|
||||
const baseDir = isPrivate ? this.privateUploadDir_ : this.uploadDir_
|
||||
|
||||
const filePath = this.getUploadFilePath(baseDir, file.fileKey)
|
||||
|
||||
try {
|
||||
await fs.access(
|
||||
this.getUploadFilePath(fileData.fileKey),
|
||||
fs.constants.F_OK
|
||||
)
|
||||
await fs.access(filePath, fs.constants.F_OK)
|
||||
} catch {
|
||||
throw new MedusaError(
|
||||
MedusaError.Types.NOT_FOUND,
|
||||
`File with key ${fileData.fileKey} not found`
|
||||
`File with key ${file.fileKey} not found`
|
||||
)
|
||||
}
|
||||
|
||||
return this.getUploadFileUrl(fileData.fileKey)
|
||||
if (isPrivate) {
|
||||
return filePath
|
||||
}
|
||||
|
||||
return this.getUploadFileUrl(file.fileKey)
|
||||
}
|
||||
|
||||
private getUploadFilePath = (fileKey: string) => {
|
||||
return path.join(this.uploadDir_, fileKey)
|
||||
private getUploadFilePath = (baseDir: string, fileKey: string) => {
|
||||
return path.join(baseDir, fileKey)
|
||||
}
|
||||
|
||||
private getUploadFileUrl = (fileKey: string) => {
|
||||
@@ -88,12 +108,14 @@ export class LocalFileService extends AbstractFileProviderService {
|
||||
return baseUrl.href
|
||||
}
|
||||
|
||||
private async ensureDirExists(dirPath: string) {
|
||||
const relativePath = path.join(this.uploadDir_, dirPath)
|
||||
private async ensureDirExists(baseDir: string, dirPath: string) {
|
||||
const relativePath = path.join(baseDir, dirPath)
|
||||
try {
|
||||
await fs.access(relativePath, fs.constants.F_OK)
|
||||
} catch (e) {
|
||||
await fs.mkdir(relativePath, { recursive: true })
|
||||
}
|
||||
|
||||
return relativePath
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,45 +35,50 @@ describe.skip("S3 File Plugin", () => {
|
||||
}
|
||||
)
|
||||
})
|
||||
;(["public", "private"] as const).forEach((access) => {
|
||||
it("uploads, reads, and then deletes a file successfully", async () => {
|
||||
const fileContent = await fs.readFile(fixtureImagePath)
|
||||
const fixtureAsBinary = fileContent.toString("binary")
|
||||
|
||||
it("uploads, reads, and then deletes a file successfully", async () => {
|
||||
const fileContent = await fs.readFile(fixtureImagePath)
|
||||
const fixtureAsBinary = fileContent.toString("binary")
|
||||
const resp = await s3Service.upload({
|
||||
filename: "catphoto.jpg",
|
||||
mimeType: "image/jpeg",
|
||||
content: fixtureAsBinary,
|
||||
access,
|
||||
})
|
||||
|
||||
const resp = await s3Service.upload({
|
||||
filename: "catphoto.jpg",
|
||||
mimeType: "image/jpeg",
|
||||
content: fixtureAsBinary,
|
||||
expect(resp).toEqual({
|
||||
key: expect.stringMatching(/tests\/catphoto.*\.jpg/),
|
||||
url: expect.stringMatching(/https:\/\/.*\.jpg/),
|
||||
})
|
||||
|
||||
const urlResp = await axios.get(resp.url).catch((e) => e.response)
|
||||
expect(urlResp.status).toEqual(access === "public" ? 200 : 403)
|
||||
|
||||
const signedUrl = await s3Service.getPresignedDownloadUrl({
|
||||
fileKey: resp.key,
|
||||
})
|
||||
|
||||
const signedUrlFile = Buffer.from(
|
||||
await axios
|
||||
.get(signedUrl, { responseType: "arraybuffer" })
|
||||
.then((r) => r.data)
|
||||
)
|
||||
|
||||
expect(signedUrlFile.toString("binary")).toEqual(fixtureAsBinary)
|
||||
|
||||
await s3Service.delete({ fileKey: resp.key })
|
||||
|
||||
// TODO: Currently the presignedURL will be returned even if the file doesn't exist. Should we check for existence first?
|
||||
const deletedFileUrl = await s3Service.getPresignedDownloadUrl({
|
||||
fileKey: resp.key,
|
||||
})
|
||||
|
||||
const { response } = await axios
|
||||
.get(deletedFileUrl, { responseType: "arraybuffer" })
|
||||
.catch((e) => e)
|
||||
|
||||
expect(response.status).toEqual(404)
|
||||
})
|
||||
|
||||
expect(resp).toEqual({
|
||||
key: expect.stringMatching(/tests\/catphoto.*\.jpg/),
|
||||
url: expect.stringMatching(/https:\/\/.*\.jpg/),
|
||||
})
|
||||
|
||||
const signedUrl = await s3Service.getPresignedDownloadUrl({
|
||||
fileKey: resp.key,
|
||||
})
|
||||
|
||||
const signedUrlFile = Buffer.from(
|
||||
await axios
|
||||
.get(signedUrl, { responseType: "arraybuffer" })
|
||||
.then((r) => r.data)
|
||||
)
|
||||
|
||||
expect(signedUrlFile.toString("binary")).toEqual(fixtureAsBinary)
|
||||
|
||||
await s3Service.delete({ fileKey: resp.key })
|
||||
|
||||
// TODO: Currently the presignedURL will be returned even if the file doesn't exist. Should we check for existence first?
|
||||
const deletedFileUrl = await s3Service.getPresignedDownloadUrl({
|
||||
fileKey: resp.key,
|
||||
})
|
||||
|
||||
const { response } = await axios
|
||||
.get(deletedFileUrl, { responseType: "arraybuffer" })
|
||||
.catch((e) => e)
|
||||
|
||||
expect(response.status).toEqual(404)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -92,14 +92,12 @@ export class S3FileService extends AbstractFileProviderService {
|
||||
|
||||
const content = Buffer.from(file.content, "binary")
|
||||
const command = new PutObjectCommand({
|
||||
// TODO: Add support for private files
|
||||
// We probably also want to support a separate bucket altogether for private files
|
||||
// protected private_bucket_: string
|
||||
// protected private_access_key_id_: string
|
||||
// protected private_secret_access_key_: string
|
||||
|
||||
// ACL: options.acl ?? (options.isProtected ? "private" : "public-read"),
|
||||
ACL: "public-read",
|
||||
ACL: file.access === "public" ? "public-read" : "private",
|
||||
Bucket: this.config_.bucket,
|
||||
Body: content,
|
||||
Key: fileKey,
|
||||
|
||||
@@ -377,7 +377,7 @@ moduleIntegrationTestRunner<IWorkflowEngineService>({
|
||||
asFunction(() => "test")
|
||||
)
|
||||
|
||||
const spy = await createScheduled("remove-scheduled", {
|
||||
const spy = await createScheduled("shared-container-job", {
|
||||
cron: "* * * * * *",
|
||||
})
|
||||
await jest.runOnlyPendingTimersAsync()
|
||||
|
||||
@@ -74,12 +74,11 @@ describe("Workflow Orchestrator module", function () {
|
||||
},
|
||||
})
|
||||
|
||||
await onApplicationStart()
|
||||
|
||||
query = remoteQuery
|
||||
sharedContainer_ = sharedContainer!
|
||||
|
||||
await runMigrations()
|
||||
await onApplicationStart()
|
||||
|
||||
workflowOrcModule = modules.workflows as unknown as IWorkflowEngineService
|
||||
})
|
||||
@@ -391,7 +390,7 @@ describe("Workflow Orchestrator module", function () {
|
||||
asFunction(() => "test")
|
||||
)
|
||||
|
||||
const spy = await createScheduled("remove-scheduled", {
|
||||
const spy = await createScheduled("shared-container-job", {
|
||||
cron: "* * * * * *",
|
||||
})
|
||||
await setTimeout(1100)
|
||||
|
||||
Reference in New Issue
Block a user