Skip to content
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

chore: Uprade aws-sdk to v3 #3989

Merged
merged 9 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 43 additions & 5 deletions api.planx.uk/modules/file/file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,19 @@ describe("File upload", () => {
});

it("should upload file", async () => {
vi.stubEnv("API_URL_EXT", "https://api.editor.planx.dev");
vi.stubEnv("AWS_S3_BUCKET", "myBucketName");

await supertest(app)
.post(ENDPOINT)
.field("filename", "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.txt")
.then((res) => {
expect(res.body).toEqual({
fileType: "text/plain",
fileUrl: expect.stringContaining(
"/file/private/nanoid/modified%20key",
),
// Bucket name stripped from URL
fileUrl:
"https://api.editor.planx.dev/file/private/nanoid/modified%20key",
});
});
expect(mockPutObject).toHaveBeenCalledTimes(1);
Expand All @@ -103,6 +106,26 @@ describe("File upload", () => {
});
expect(mockPutObject).toHaveBeenCalledTimes(1);
});

it("should generate a correct URL on production", async () => {
vi.stubEnv("API_URL_EXT", "https://api.editor.planx.dev");
jessicamcinchak marked this conversation as resolved.
Show resolved Hide resolved
vi.stubEnv("NODE_ENV", "production");

await supertest(app)
.post(ENDPOINT)
.field("filename", "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.txt")
.then((res) => {
expect(res.body).toEqual({
fileType: "text/plain",
fileUrl: expect.stringContaining(
"/file/private/nanoid/modified%20key",
),
});
});
expect(mockPutObject).toHaveBeenCalledTimes(1);
expect(getSignedUrl).toHaveBeenCalledTimes(1);
});
});

describe("Public", () => {
Expand Down Expand Up @@ -237,14 +260,29 @@ describe("File download", () => {

await supertest(app)
.get("/file/public/someKey/someFile.txt")
.field("filename", "some_file.txt")
.attach("file", Buffer.from("some data"), "some_file.txt")
.expect(500)
.then((res) => {
expect(res.body.error).toMatch(/S3 error!/);
});
expect(mockGetObject).toHaveBeenCalledTimes(1);
});

it("should handle an empty file body", async () => {
mockGetObject = vi.fn(() =>
Promise.resolve({
...getObjectResponse,
Body: undefined,
}),
);

await supertest(app)
.get("/file/public/someKey/someFile.txt")
.expect(500)
.then((res) => {
expect(res.body.error).toMatch(/Missing body from S3 file/);
});
expect(mockGetObject).toHaveBeenCalledTimes(1);
});
});

describe("Private", () => {
Expand Down
1 change: 0 additions & 1 deletion api.planx.uk/modules/file/service/getFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export const getFileFromS3 = async (fileId: string) => {

const file = await s3.getObject(params);

// TODO: test this
if (!file.Body) throw Error(`Missing body from S3 file ${fileId}`);

const body = Buffer.from(await file.Body.transformToByteArray());
Expand Down
4 changes: 2 additions & 2 deletions api.planx.uk/modules/file/service/uploadFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ export function generateFileParams(
ACL: "public-read",
Bucket: process.env.AWS_S3_BUCKET,
Key: key,
Body: file?.buffer || JSON.stringify(file),
Body: file.buffer,
ContentDisposition: `inline;filename="${filename}"`,
ContentType: file?.mimetype || "application/json",
Comment on lines -76 to -78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These || statements are unreachable and can be safely removed.

ContentType: file.mimetype,
};

return {
Expand Down
8 changes: 4 additions & 4 deletions api.planx.uk/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ export default defineConfig({
// html reporter required to inspect coverage in Vitest UI dashboard
reporter: ["lcov", "html", "text-summary"],
thresholds: {
statements: 72.03,
branches: 54.92,
functions: 67.62,
lines: 71.84,
statements: 73.28,
branches: 55.27,
functions: 73.59,
lines: 73.42,
autoUpdate: true,
},
},
Expand Down
Loading