fix(presigned-url): no downloadUrl for a file that is not uploaded

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

@@ -27,6 +27,7 @@ import type { GraphileConfig } from 'graphile-config';
 import { withRequestPgClient } from 'graphile-plugin-utils';
 import { DOWNLOAD_URL_FIELD } from 'graphile-storage-registry';
 
+import { isLiveFileRow } from './file-lifecycle';
 import { resolveS3ForDatabase } from './physical-bucket';
 import { generatePresignedGetUrl } from './s3-signer';
 import { loadAllStorageModules, readBucketPhysicalName, resolveStorageConfigFromCodec } from './storage-module-cache';
@@ -86,13 +87,15 @@ export function createDownloadUrlPlugin(
                 {
                   description:
                     'URL to download this file. For public files, returns the public URL. ' +
-                    'For private files, returns a time-limited presigned URL.',
+                    'For private files, returns a time-limited presigned URL. ' +
+                    'Null until the upload is confirmed (status uploaded or processed), and for a rejected file.',
                   type: GraphQLString,
                   plan($parent: any) {
                     const $key = $parent.get('key');
                     const $isPublic = $parent.get('is_public');
                     const $filename = $parent.get('filename');
                     const $bucketId = $parent.get('bucket_id');
+                    const $status = $parent.get('status');
 
                     const $withPgClient = (grafastContext() as any).get('withPgClient');
                     const $pgSettings = (grafastContext() as any).get('pgSettings');
@@ -102,12 +105,14 @@ export function createDownloadUrlPlugin(
                       isPublic: $isPublic,
                       filename: $filename,
                       bucketId: $bucketId,
+                      status: $status,
                       withPgClient: $withPgClient,
                       pgSettings: $pgSettings,
                     });
 
-                    return lambda($combined, async ({ key, isPublic, filename, bucketId, withPgClient, pgSettings }: any) => {
+                    return lambda($combined, async ({ key, isPublic, filename, bucketId, status, withPgClient, pgSettings }: any) => {
                       if (!key) return null;
+                      if (!isLiveFileRow({ status })) return null;
 
                       if (!withPgClient || !pgSettings) return null;
 

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

@@ -6,8 +6,9 @@
 export const LIVE_FILE_STATUSES = ['uploaded', 'processed'];
 
 /**
- * Whether an existing row may be handed back as a dedup hit: only once the bytes
- * it names are in S3.
+ * Whether a row is live (constructive-db file lifecycle invariant S2): a dedup
+ * hit, and a row `downloadUrl` signs a URL for, only once the bytes it names
+ * are in S3.
  *
  * The row is written before the PUT; the confirm-upload job moves it out of
  * `requested` only after inspecting the object, so its status is the answer.

graphql/server-test/__fixtures__/seed/simple-seed-storage/schema.sql

@@ -118,6 +118,21 @@ BEGIN
   EXECUTE format(
     'GRANT EXECUTE ON FUNCTION %I.app_files_record_file(uuid, text, text, text, bigint, text, jsonb, text, uuid) TO administrator, authenticated, anonymous',
     schema_name || '-private');
+
+  -- The generator's bucket physical-name reader, which downloadUrl calls.
+  EXECUTE format($sql$
+    CREATE FUNCTION %I.app_buckets_physical_name(bucket_id uuid) RETURNS text
+    LANGUAGE sql
+    SECURITY DEFINER
+    STABLE
+    AS $function$
+      SELECT b.physical_name FROM %I.app_buckets b WHERE b.id = app_buckets_physical_name.bucket_id
+    $function$;
+  $sql$, schema_name || '-private', schema_name);
+
+  EXECUTE format(
+    'GRANT EXECUTE ON FUNCTION %I.app_buckets_physical_name(uuid) TO administrator, authenticated, anonymous',
+    schema_name || '-private');
 END;
 $$;
 

graphql/server-test/__tests__/download-url-status.integration.test.ts

@@ -0,0 +1,118 @@
+/**
+ * Integration tests: `downloadUrl` is handed out only for a file whose bytes a
+ * reader can GET.
+ *
+ * A files row is written before its presigned PUT runs, and the confirm-upload
+ * job settles its `status` after inspecting the object. A `requested` row may
+ * name an object that never arrives, and a `rejected` row is a settled
+ * failure, so neither may carry a download URL.
+ *
+ *   pnpm test -- --testPathPattern=download-url-status
+ */
+
+import path from 'path';
+import type { PgTestClient } from 'pgsql-test';
+import type supertest from 'supertest';
+
+import { getConnections, seed } from '../src';
+
+jest.setTimeout(120000);
+
+const localSeedRoot = path.join(__dirname, '..', '__fixtures__', 'seed');
+const sharedSeedRoot = path.join(__dirname, '..', '..', '..', '__fixtures__', 'seed');
+const sql = (seedDir: string, file: string) => path.join(localSeedRoot, seedDir, file);
+const pgpmWorkspace = path.join(sharedSeedRoot, '..', '..');
+
+const aliceDatabaseId = '80a2eaaf-f77e-4bfe-8506-df929ef1b8d9';
+const aliceSchema = 'simple-storage-public';
+const alicePrivateBucketId = 'd0000001-0000-0000-0000-000000000002';
+
+const metaSchemas = [
+  'catalog_private',
+  'routing_public',
+  'apps_public',
+  'metaschema_public',
+  'metaschema_modules_public'
+];
+
+const STATUSES = ['requested', 'uploaded', 'processed', 'rejected'];
+
+const APP_FILES = `
+  query AppFiles {
+    appFiles {
+      nodes {
+        key
+        downloadUrl
+      }
+    }
+  }
+`;
+
+describe('downloadUrl by file status', () => {
+  let request: supertest.Agent;
+  let pg: PgTestClient;
+  let teardown: () => Promise<void>;
+  let urlByKey: Record<string, string | null>;
+
+  beforeAll(async () => {
+    ({ request, pg, teardown } = await getConnections(
+      {
+        schemas: [aliceSchema],
+        authRole: 'anonymous',
+        server: {
+          useRouting: true,
+          api: {
+            isPublic: false,
+            metaSchemas
+          }
+        }
+      },
+      [
+        seed.pgpm(pgpmWorkspace),
+        seed.sqlfile([
+          sql('simple-seed-storage', 'schema.sql'),
+          sql('simple-seed-storage', 'test-data.sql'),
+          sql('simple-seed-storage', 'auth-provision.sql')
+        ])
+      ]
+    ));
+
+    for (const status of STATUSES) {
+      await pg.query(
+        `INSERT INTO "${aliceSchema}".app_files
+           (bucket_id, key, content_hash, mime_type, filename, status)
+         VALUES ($1, $2, $2, 'text/plain', $3, $4)`,
+        [alicePrivateBucketId, `status-${status}`, `${status}.txt`, status]
+      );
+    }
+
+    const res = await request
+      .post('/graphql')
+      .set('X-Database-Id', aliceDatabaseId)
+      .set('X-Schemata', aliceSchema)
+      .send({ query: APP_FILES });
+    expect(res.status).toBe(200);
+    expect(res.body.errors).toBeUndefined();
+    urlByKey = Object.fromEntries(
+      res.body.data.appFiles.nodes.map((n: { key: string; downloadUrl: string | null }) => [
+        n.key,
+        n.downloadUrl
+      ])
+    );
+  });
+
+  afterAll(async () => {
+    if (teardown) await teardown();
+  });
+
+  it.each(['uploaded', 'processed'])('signs a download URL for a %s file', (status) => {
+    const url = urlByKey[`status-${status}`];
+    expect(url).toEqual(expect.any(String));
+    expect(new URL(url).pathname).toContain(`/status-${status}`);
+  });
+
+  it.each(['requested', 'rejected'])('returns no download URL for a %s file', (status) => {
+    expect(urlByKey).toHaveProperty(`status-${status}`);
+    expect(urlByKey[`status-${status}`]).toBeNull();
+  });
+});