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

feat: allow for server uploads #993

Merged
merged 2 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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: 48 additions & 0 deletions core/lib/server/assets.db.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import fs from "fs/promises";

import { beforeAll, describe, expect, it } from "vitest";

import { mockServerCode } from "~/lib/__tests__/utils";
import { env } from "../env/env.mjs";

const { createForEachMockedTransaction } = await mockServerCode();

const { getTrx } = createForEachMockedTransaction();

beforeAll(async () => {
// check if minio is up

if (!env.ASSETS_STORAGE_ENDPOINT) {
throw new Error(
"You should only run this test against a local minio instance, not to prod S3"
);
}

const check = await fetch(env.ASSETS_STORAGE_ENDPOINT, {
method: "OPTIONS",
});

if (!check.ok) {
throw new Error(
"Minio is not running. Please setup the test environment properly by running `pnpm -w test:setup`"
);
}
});

describe("assets upload", () => {
it("should be able to upload a file to the minio bucket from the server", async () => {
const { uploadFileToS3 } = await import("./assets");

const file = await fs.readFile(new URL("./assets.ts", import.meta.url));

const url = await uploadFileToS3("test", "test.ts", file, {
contentType: "text/plain",
});

expect(url).toBeDefined();

const text = await fetch(url).then((res) => res.text());

expect(text).toEqual(file.toString("utf8"));
});
});
Comment on lines +32 to +48
Copy link
Member Author

Choose a reason for hiding this comment

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

do we need more tests of eg larger files?

76 changes: 73 additions & 3 deletions core/lib/server/assets.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
import { PutObjectCommand, S3Client } from "@aws-sdk/client-s3";
import { Upload } from "@aws-sdk/lib-storage";
import { getSignedUrl } from "@aws-sdk/s3-request-presigner";

import type { PubsId } from "db/public";
import { logger } from "logger";

import { env } from "../env/env.mjs";

export const generateSignedAssetUploadUrl = async (pubId: PubsId, fileName: string) => {
let s3Client: S3Client;

export const getS3Client = () => {
if (s3Client) {
return s3Client;
}

const region = env.ASSETS_REGION;
const key = env.ASSETS_UPLOAD_KEY;
const secret = env.ASSETS_UPLOAD_SECRET_KEY;
const bucket = env.ASSETS_BUCKET_NAME;

const client = new S3Client({
s3Client = new S3Client({
endpoint: env.ASSETS_STORAGE_ENDPOINT,
region: region,
credentials: {
Expand All @@ -20,9 +27,72 @@ export const generateSignedAssetUploadUrl = async (pubId: PubsId, fileName: stri
},
forcePathStyle: !!env.ASSETS_STORAGE_ENDPOINT, // Required for MinIO
});

return s3Client;
};

export const generateSignedAssetUploadUrl = async (pubId: PubsId, fileName: string) => {
const client = getS3Client();

const bucket = env.ASSETS_BUCKET_NAME;
const command = new PutObjectCommand({
Bucket: bucket,
Key: `${pubId}/${fileName}`,
});

return await getSignedUrl(client, command, { expiresIn: 3600 });
};

/**
* Uploads a file to the S3 bucket using the S3 client directly
* @param id - id under which the file will be stored. eg for a pub, the pubId. for community assets like the logo, the communityId. for user avatars, the userId.
* @param fileName - name of the file to be stored
* @param fileData - the file data to upload (Buffer or Uint8Array)
* @param contentType - MIME type of the file (e.g., 'image/jpeg')
* @returns the URL of the uploaded file
*/
export const uploadFileToS3 = async (
id: string,
fileName: string,
fileData: Buffer | Uint8Array,
{
contentType,
queueSize,
partSize,
progressCallback,
}: {
contentType: string;
queueSize?: number;
partSize?: number;
progressCallback?: (progress: any) => void;
}
): Promise<string> => {
const client = getS3Client();
const bucket = env.ASSETS_BUCKET_NAME;
const key = `${id}/${fileName}`;

const parallelUploads3 = new Upload({
client,
params: {
Bucket: bucket,
Key: key,
Body: fileData,
ContentType: contentType,
},
queueSize: queueSize ?? 3, // optional concurrency configuration
partSize: partSize ?? 1024 * 1024 * 5, // optional size of each part, in bytes, at least 5MB
leavePartsOnError: false, // optional manually handle dropped parts
});

parallelUploads3.on(
"httpUploadProgress",
progressCallback ??
((progress) => {
logger.debug(progress);
})
);

const result = await parallelUploads3.done();

return result.Location!;
};
Comment on lines +74 to +98
Copy link
Member Author

Choose a reason for hiding this comment

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

basically the same as our legacy implementation

https://github.com/pubpub/pubpub/blob/2dfe92a70d647278e512a91dfb060e4480739df2/server/utils/s3.ts#L93-L121

minus the ACL. I'm not sure what it is and why it would be necessary. we always seem to set it to public-read in legacy, but do make uploads with it? eg here
https://github.com/pubpub/pubpub/blob/2dfe92a70d647278e512a91dfb060e4480739df2/server/utils/s3.ts#L93-L121

upload seemed to work fine without it

1 change: 1 addition & 0 deletions core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
},
"dependencies": {
"@aws-sdk/client-s3": "^3.445.0",
"@aws-sdk/lib-storage": "^3.750.0",
"@aws-sdk/s3-request-presigner": "^3.445.0",
"@dagrejs/dagre": "^1.0.4",
"@dnd-kit/core": "^6.1.0",
Expand Down
2 changes: 2 additions & 0 deletions docker-compose.test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ services:
networks:
- app-network
profiles:
- test
- integration

minio-init:
Expand All @@ -39,6 +40,7 @@ services:
networks:
- app-network
profiles:
- test
- integration

inbucket:
Expand Down
Loading
Loading