Skip to content

Add bucket initialization to SourceService operations #28

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.

Sign up for GitHub

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

Merged
marcelsamyn merged 1 commit into from
Mar 17, 2026
+24 −0
Merged

Add bucket initialization to SourceService operations #28

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 24 additions & 0 deletions src/lib/sources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,34 @@ export interface SourceCreateInput {
* Service for managing sources and raw payload storage.
*/
export class SourceService {
private bucketReady: Promise<void> | null = null;

constructor(
private db: DrizzleDB,
private minioClient: MinioClient,
private bucket: string,
private inlineThreshold = 1024, // bytes
) {}

/** Ensure the S3/MinIO bucket exists, creating it if necessary */
private ensureBucket(): Promise<void> {
if (!this.bucketReady) {
this.bucketReady = this.minioClient
.bucketExists(this.bucket)
.then((exists) => {
if (!exists) {
return this.minioClient.makeBucket(this.bucket);
}
})
.catch((err) => {
// Reset so next call retries
this.bucketReady = null;
throw err;
});
}
return this.bucketReady;
}
Comment on lines +61 to +77
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 17, 2026

Choose a reason for hiding this comment

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

medium

While the current promise-based implementation is correct, using async/await can improve readability and make the logic easier to follow. This is more idiomatic in modern TypeScript/JavaScript. The async IIFE (Immediately Invoked Function Expression) pattern ensures the promise is created and assigned synchronously, preserving the lazy-initialization and race-condition-free behavior of your original implementation.

Suggested change
private ensureBucket(): Promise<void> {
if (!this.bucketReady) {
this.bucketReady = this.minioClient
.bucketExists(this.bucket)
.then((exists) => {
if (!exists) {
return this.minioClient.makeBucket(this.bucket);
}
})
.catch((err) => {
// Reset so next call retries
this.bucketReady = null;
throw err;
});
}
return this.bucketReady;
}
private ensureBucket(): Promise<void> {
if (!this.bucketReady) {
this.bucketReady = (async () => {
try {
const exists = await this.minioClient.bucketExists(this.bucket);
if (!exists) {
await this.minioClient.makeBucket(this.bucket);
}
} catch (err) {
// Reset so next call retries
this.bucketReady = null;
throw err;
}
})();
}
return this.bucketReady;
}


/** Insert multiple sources with optional inline or blob payloads */
async insertMany(inputs: SourceCreateInput[]): Promise<{
successes: TypeId<"source">[];
Expand Down Expand Up @@ -98,6 +119,7 @@ export class SourceService {
.returning();

// 2. Handle payloads
await this.ensureBucket();
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 17, 2026

Choose a reason for hiding this comment

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

medium

For a minor optimization, you could defer calling ensureBucket() until it's certain a blob operation is needed. This avoids the call if all sources are handled as inline content. You could move this call inside the else if (input.fileBuffer) block (line 155) where minioClient.putObject is used. While the performance impact is negligible due to promise caching in ensureBucket, this would make the code slightly more efficient and precise.

for (const row of inserted) {
const lookupKey = makeLookupKey(row.userId, row.type, row.externalId);
const input = inputLookup.get(lookupKey);
Expand Down Expand Up @@ -177,6 +199,7 @@ export class SourceService {

/** Hard delete a source: remove blob then drop the DB row */
async deleteHard(userId: string, sourceId: TypeId<"source">): Promise<void> {
await this.ensureBucket();
const key = `${userId}/${sourceId}`;
// delete blob, ignore errors
try {
Expand All @@ -203,6 +226,7 @@ export class SourceService {
and(eq(src.userId, userId), inArray(src.id, sourceIds)),
});
const results: RawResult[] = [];
await this.ensureBucket();
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 17, 2026

Choose a reason for hiding this comment

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

medium

For a minor optimization, you could defer calling ensureBucket() until it's certain a blob operation is needed. This avoids the call if all requested sources have inline content. You could move this call inside the else block (line 239) where minioClient.getObject is used. While the performance impact is negligible due to promise caching in ensureBucket, this would make the code slightly more efficient and precise.


for (const row of rows) {
const meta = metadataSchema.parse(row.metadata ?? {});
Expand Down
Loading