test: end-to-end upload integration test for presigned URL flow#974
Merged
pyramation merged 5 commits intomainfrom Apr 11, 2026
Merged
test: end-to-end upload integration test for presigned URL flow#974pyramation merged 5 commits intomainfrom
pyramation merged 5 commits intomainfrom
Conversation
Adds a new integration test that exercises the full upload pipeline for both public and private files using real MinIO: - requestUploadUrl → PUT to presigned URL → confirmUpload - Tests public bucket (is_public=true) and private bucket (is_public=false) - Tests content-hash deduplication - Uses lazy S3 bucket provisioning (from PR #969) - Uses per-database bucket naming (from PR #968) Includes seed fixtures (simple-seed-storage) that create: - jwt_private schema with current_database_id() function - metaschema tables (database, schema, table, field) - services tables (apis, domains, api_schemas) - storage_module config row - storage tables (buckets, files, upload_requests) - Two buckets: public and private
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…signed URL plugin
The @dataplan/pg adapter wraps pgClient with a custom query method that
expects { text, values } objects, not raw SQL strings. The plugin was
calling pgClient.query('BEGIN') etc. with raw strings, causing 'text'
to be undefined when destructured by the adapter.
Changes:
- Convert all pgClient.query(string, params) calls to
pgClient.query({ text: string, values: params }) format
- Replace manual BEGIN/COMMIT/ROLLBACK with pgClient.withTransaction()
since the adapter already manages transactions when pgSettings are present
- Update storage-module-cache.ts and download-url-field.ts similarly
…that don't support it
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new integration test in
graphql/server-testthat exercises the full presigned URL upload pipeline against real MinIO. This is a prerequisite for the more comprehensive RLS-aware tests inconstructive-db.Test file:
__tests__/upload.integration.test.tsrequestUploadUrl→ PUT to presigned URL →confirmUpload(status → ready)bucketKey: 'private'deduplicated: truewith no upload URLSeed fixtures:
__fixtures__/seed/simple-seed-storage/setup.sql— storage-specific additions only (~70 lines):jwt_privateschema +storage_moduletable definition. Loaded aftersimple-seed-services/setup.sql, which provides all metaschema/services/role infrastructure via pgpm.schema.sql— creates thesimple-storage-publicschema withbuckets,files,upload_requeststables (with@storageBuckets/@storageFilessmart tags)test-data.sql— seeds a database, schema, table entries, services API, storage_module config row, and two buckets (public + private)The test composes seed files from both
simple-seed-servicesandsimple-seed-storageviaseed.sqlfile([...]), so metaschema infrastructure is reused rather than duplicated.The test uses the services API mode (
enableServicesApi: true) withX-Database-Idheader so the middleware setsjwt.claims.database_id, which the presigned URL plugin reads viajwt_private.current_database_id()to resolve per-database storage config.Relies on lazy S3 bucket provisioning (PR #969) and per-database bucket naming (PR #968).
Production bug fixes discovered by the test
1.
pgClient.queryformat mismatch (plugin.ts,storage-module-cache.ts,download-url-field.ts)The
@dataplan/pgadapter wrapspgClientwith a customquery()method that destructures{ text, values }from the argument. The presigned URL plugin was passing raw SQL strings (pgClient.query('BEGIN'),pgClient.query(sql, params)) — when the adapter destructured a string,textbecameundefined, causing"A query must have either text or a name"errors.Fix: Converted all
pgClient.query(string, params)calls topgClient.query({ text, values })format and replaced manualBEGIN/COMMIT/ROLLBACKwithpgClient.withTransaction().2. Bucket provisioner graceful degradation (
provisioner.ts)The
BucketProvisioner.provision()flow callsPutPublicAccessBlock,PutBucketCors, andDeleteBucketPolicy— none of which are supported by older MinIO (edge-cicd). The provisioner threwProvisionerErrorand the entire upload flow failed.Fix:
setPublicAccessBlock,setCors, anddeleteBucketPolicynow catch XML parse / "not implemented" errors and skip gracefully. The bucket is still created and usable; these operations are best-effort for S3-compatible backends.Review & Testing Checklist for Human
BEGIN/COMMIT/ROLLBACKwithpgClient.withTransaction(callback). Verify this method exists on the@dataplan/pg-wrapped client in all execution paths (requestUploadUrl and confirmUpload). IfwithTransactionis not available in some edge case, the mutation will throw a runtime error.err.message?.includes('not well-formed'),err.message?.includes('CORSConfiguration'), etc. If a legitimate AWS S3 error happens to contain these substrings, it would be silently swallowed. Consider whether this is acceptable for production or if the check should be tighter (e.g. also checkingerr.$metadata?.httpStatusCode).STORAGE_MODULE_QUERYinstorage-module-cache.tsjoinsstorage_module → metaschema_public.table → metaschema_public.schema. Verify the seed data UUIDs (buckets_table_id,files_table_id,upload_requests_table_id→ table rows → schema row) form a valid join path. A mismatch here means the plugin getsnullconfig and all mutations fail.downloadUrlverification: The test confirms uploads succeed but does not querydownloadUrlon the file afterward. The public-URL-prefix path and presigned-GET path are untested here.Recommended manual test
pnpm test -- --testPathPattern=upload.integrationingraphql/server-testwith MinIO running locallyrequestUploadUrl→ PUT →confirmUploadworks for both public and private bucketswithTransactionand graceful degradation paths work against a non-MinIO backendNotes
anonymous. RLS-aware upload tests are planned forconstructive-db.describeblock is sequential and state-sharing (let uploadUrl,let fileId). This is consistent with existing tests in this package.Link to Devin session: https://app.devin.ai/sessions/e47513cf8b974ae6985c42c0a657e4d7
Requested by: @pyramation