Skip to content

Commit 304d201

Browse files
committed
fix: scope BucketNameResolver to (databaseId, bucketKey)
Update the presigned URL plugin's BucketNameResolver signature from (databaseId) to (databaseId, bucketKey) so each logical bucket resolves to its own S3 bucket. Previously all uploads for a database went to a single S3 bucket regardless of bucketKey, creating a mismatch with the bucket provisioner plugin which creates per-bucketKey S3 buckets. Changes: - types.ts: BucketNameResolver now takes (databaseId, bucketKey) - plugin.ts: resolveS3ForDatabase accepts bucketKey, passes it through to the resolver in upload, bulk upload, and delete paths - download-url-field.ts: fetches bucket_id from file row, looks up bucketKey from the buckets table, passes it to resolveS3ForDatabase - presigned-url-resolver.ts: createBucketNameResolver returns ${prefix}-${bucketKey}-${databaseId} instead of ${prefix}-${databaseId} Breaking change: custom resolveBucketName callbacks must now accept (databaseId: string, bucketKey: string) instead of (databaseId: string).
1 parent a8c2647 commit 304d201

4 files changed

Lines changed: 48 additions & 23 deletions

File tree

graphile/graphile-presigned-url-plugin/src/download-url-field.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,11 @@ function resolveS3ForDatabase(
6060
options: PresignedUrlPluginOptions,
6161
storageConfig: StorageModuleConfig,
6262
databaseId: string,
63+
bucketKey: string,
6364
): S3Config {
6465
const globalS3 = resolveS3(options);
6566
const bucket = options.resolveBucketName
66-
? options.resolveBucketName(databaseId)
67+
? options.resolveBucketName(databaseId, bucketKey)
6768
: globalS3.bucket;
6869
const publicUrlPrefix = storageConfig.publicUrlPrefix ?? globalS3.publicUrlPrefix;
6970

@@ -127,6 +128,7 @@ export function createDownloadUrlPlugin(
127128
const $key = $parent.get('key');
128129
const $isPublic = $parent.get('is_public');
129130
const $filename = $parent.get('filename');
131+
const $bucketId = $parent.get('bucket_id');
130132

131133
const $withPgClient = (grafastContext() as any).get('withPgClient');
132134
const $pgSettings = (grafastContext() as any).get('pgSettings');
@@ -135,11 +137,12 @@ export function createDownloadUrlPlugin(
135137
key: $key,
136138
isPublic: $isPublic,
137139
filename: $filename,
140+
bucketId: $bucketId,
138141
withPgClient: $withPgClient,
139142
pgSettings: $pgSettings,
140143
});
141144

142-
return lambda($combined, async ({ key, isPublic, filename, withPgClient, pgSettings }: any) => {
145+
return lambda($combined, async ({ key, isPublic, filename, bucketId, withPgClient, pgSettings }: any) => {
143146
if (!key) return null;
144147

145148
let s3ForDb = resolveS3(options);
@@ -155,11 +158,24 @@ export function createDownloadUrlPlugin(
155158
const allConfigs = await loadAllStorageModules(pgClient, databaseId);
156159
const config = resolveStorageConfigFromCodec(capturedCodec, allConfigs);
157160
if (!config) return null;
158-
return { config, databaseId };
161+
162+
// Look up the bucket key for scoped S3 resolution
163+
let bucketKey = 'public';
164+
if (bucketId) {
165+
const bucketResult = await pgClient.query({
166+
text: `SELECT key FROM ${config.bucketsQualifiedName} WHERE id = $1 LIMIT 1`,
167+
values: [bucketId],
168+
});
169+
if (bucketResult.rows[0]?.key) {
170+
bucketKey = bucketResult.rows[0].key;
171+
}
172+
}
173+
174+
return { config, databaseId, bucketKey };
159175
});
160176
if (resolved) {
161177
downloadUrlExpirySeconds = resolved.config.downloadUrlExpirySeconds;
162-
s3ForDb = resolveS3ForDatabase(options, resolved.config, resolved.databaseId);
178+
s3ForDb = resolveS3ForDatabase(options, resolved.config, resolved.databaseId, resolved.bucketKey);
163179
}
164180
}
165181
} catch {

graphile/graphile-presigned-url-plugin/src/plugin.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,11 @@ function resolveS3ForDatabase(
9292
options: PresignedUrlPluginOptions,
9393
storageConfig: StorageModuleConfig,
9494
databaseId: string,
95+
bucketKey: string,
9596
): S3Config {
9697
const globalS3 = resolveS3(options);
9798
const bucket = options.resolveBucketName
98-
? options.resolveBucketName(databaseId)
99+
? options.resolveBucketName(databaseId, bucketKey)
99100
: globalS3.bucket;
100101
const publicUrlPrefix = storageConfig.publicUrlPrefix ?? globalS3.publicUrlPrefix;
101102

@@ -282,7 +283,7 @@ export function createPresignedUrlPlugin(
282283
);
283284
if (!bucket) throw new Error('BUCKET_NOT_FOUND');
284285

285-
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId);
286+
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId, bucket.key);
286287
await ensureS3BucketExists(options, s3ForDb.bucket, bucket, databaseId, storageConfig.allowedOrigins);
287288

288289
return processSingleFile(options, txClient, storageConfig, databaseId, bucket, s3ForDb, {
@@ -397,7 +398,7 @@ export function createPresignedUrlPlugin(
397398
);
398399
}
399400

400-
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId);
401+
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId, bucket.key);
401402
await ensureS3BucketExists(options, s3ForDb.bucket, bucket, databaseId, storageConfig.allowedOrigins);
402403

403404
const results = [];
@@ -533,7 +534,17 @@ export function createPresignedUrlPlugin(
533534
}
534535

535536
// No other references — attempt sync S3 delete
536-
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId);
537+
// Look up the bucket key for scoped S3 resolution
538+
const bucketResult = await pgClient.query({
539+
text: `SELECT key FROM ${storageConfig.bucketsQualifiedName} WHERE id = $1 LIMIT 1`,
540+
values: [fileRow!.bucket_id],
541+
});
542+
const bucketKey = bucketResult.rows[0]?.key;
543+
if (!bucketKey) {
544+
log.warn(`Bucket not found for bucket_id=${fileRow!.bucket_id}; skipping S3 delete`);
545+
return;
546+
}
547+
const s3ForDb = resolveS3ForDatabase(options, storageConfig, databaseId, bucketKey);
537548
await deleteS3Object(s3ForDb, fileRow!.key);
538549
log.info(`Sync S3 delete succeeded for key=${fileRow!.key}`);
539550
});

graphile/graphile-presigned-url-plugin/src/types.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,16 +149,17 @@ export interface S3Config {
149149
export type S3ConfigOrGetter = S3Config | (() => S3Config);
150150

151151
/**
152-
* Function to derive the actual S3 bucket name for a given database.
152+
* Function to derive the actual S3 bucket name for a given database and bucket key.
153153
*
154154
* When provided, the presigned URL plugin calls this on every request
155-
* to determine which S3 bucket to use — enabling per-database bucket
155+
* to determine which S3 bucket to use — enabling per-(database, bucketKey)
156156
* isolation. If not provided, falls back to `s3Config.bucket` (global).
157157
*
158158
* @param databaseId - The metaschema database UUID
159-
* @returns The S3 bucket name for this database
159+
* @param bucketKey - The logical bucket key (e.g., "public", "private")
160+
* @returns The S3 bucket name for this database + bucket key
160161
*/
161-
export type BucketNameResolver = (databaseId: string) => string;
162+
export type BucketNameResolver = (databaseId: string, bucketKey: string) => string;
162163

163164
/**
164165
* Callback to lazily provision an S3 bucket on first use.

graphile/graphile-settings/src/presigned-url-resolver.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,24 +86,21 @@ export function getPresignedUrlS3Config(): S3Config {
8686
}
8787

8888
/**
89-
* Create a per-database bucket name resolver.
89+
* Create a per-(database, bucketKey) bucket name resolver.
9090
*
91-
* Uses the BUCKET_NAME env var as a prefix. For each database, the S3 bucket
92-
* name becomes `{prefix}-{databaseId}` (e.g., "myapp-abc123def456").
91+
* Uses the BUCKET_NAME env var as a prefix. For each (database, bucketKey)
92+
* pair, the S3 bucket name becomes `{prefix}-{bucketKey}-{databaseId}`
93+
* (e.g., "myapp-public-abc123def456").
9394
*
94-
* In local development with MinIO (default BUCKET_NAME="test-bucket"),
95-
* all databases share the same bucket for simplicity — the resolver
96-
* returns the prefix as-is when it looks like a local dev bucket.
97-
*
98-
* In production, set BUCKET_NAME to your org prefix (e.g., "myapp")
99-
* and each database gets its own isolated S3 bucket.
95+
* This aligns with the bucket provisioner plugin which creates separate
96+
* S3 buckets per logical bucket key.
10097
*/
10198
export function createBucketNameResolver(): BucketNameResolver {
10299
const { cdn } = getEnvOptions();
103100
const prefix = cdn?.bucketName || 'test-bucket';
104101

105-
return (databaseId: string): string => {
106-
return `${prefix}-${databaseId}`;
102+
return (databaseId: string, bucketKey: string): string => {
103+
return `${prefix}-${bucketKey}-${databaseId}`;
107104
};
108105
}
109106

0 commit comments

Comments
 (0)