Skip to content
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
15 changes: 13 additions & 2 deletions apps/backend/lambdas/reports/controllers/reports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
objectUrlFor,
keyFromObjectUrl,
reportKeyPrefix,
getObjectSize,
} from '../report-service';

const s3 = new S3Client({ region: process.env.AWS_REGION ?? 'us-east-2' });
Expand Down Expand Up @@ -88,6 +89,11 @@ export const generateReport: RouteHandler = async ({ event }) => {
return json(400, { message: `report_type must be one of: ${REPORT_TYPES.join(', ')}` });
}

// Optional. Falls back to the auto-generated "<project> — <date>" title
// below when omitted or blank, so this is fully backward compatible with
// any caller that never sends it.
const customTitle = typeof body.title === 'string' ? body.title.trim() : '';

const reportData = await fetchReportData(projectId);
if (!reportData) {
return json(404, { message: 'Project not found' });
Expand All @@ -107,7 +113,7 @@ export const generateReport: RouteHandler = async ({ event }) => {
return serverError(err, 'Failed to upload report');
}

const title = `${reportData.project.name} — ${new Date().toLocaleDateString('en-US', { year: 'numeric', month: 'long', day: 'numeric' })}`;
const title = customTitle || `${reportData.project.name} — ${new Date().toLocaleDateString('en-US', { year: 'numeric', month: 'long', day: 'numeric' })}`;
const record = await saveReportRecord(projectId, objectUrl, title, reportType);

return json(201, {
Expand Down Expand Up @@ -168,7 +174,7 @@ export const listReports: RouteHandler = async ({ event }) => {
const totalPages = Math.ceil(totalItems / limit);

return json(200, {
data: reports,
data: await withSizes(reports),
pagination: { page, limit, totalItems, totalPages },
});
}
Expand Down Expand Up @@ -312,3 +318,8 @@ export const deleteReport: RouteHandler = async ({ params, path, method }) => {

return json(200, { ok: true, route: 'DELETE /reports/{id}', pathParams: { id }, fileDeleted });
};

async function withSizes<T extends { object_url: string }>(rows: T[]) {
const sizes = await Promise.all(rows.map((r) => getObjectSize(r.object_url)));
return rows.map((r, i) => ({ ...r, file_size: sizes[i] }));
}
14 changes: 13 additions & 1 deletion apps/backend/lambdas/reports/report-service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import db from './db';
import { S3Client, PutObjectCommand } from '@aws-sdk/client-s3';
import { S3Client, PutObjectCommand, HeadObjectCommand } from '@aws-sdk/client-s3';
import type { TDocumentDefinitions, Content, TableCell } from 'pdfmake/interfaces';
import {
Document,
Expand Down Expand Up @@ -548,3 +548,15 @@ export async function saveReportRecord(

return { report_id: row.report_id, object_url: row.object_url, report_type: row.report_type };
}

export async function getObjectSize(objectUrl: string): Promise<number | null> {
const key = keyFromObjectUrl(objectUrl);
if (!key) return null;
try {
const head = await s3.send(new HeadObjectCommand({ Bucket: getBucketName(), Key: key }));
return head.ContentLength ?? null;
} catch (err) {
console.error('Failed to read object size', key, err);
return null;
}
}
57 changes: 56 additions & 1 deletion apps/backend/lambdas/reports/test/report-service.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Pool } from 'pg';
import { ensureSchema, resetData } from '../../../db/testkit';

import db from '../db';
import { fetchReportData } from '../report-service';
import { fetchReportData, keyFromObjectUrl, objectUrlFor, reportKeyPrefix } from '../report-service';

const pool = new Pool({
host: 'localhost',
Expand Down Expand Up @@ -66,3 +66,58 @@ describe('fetchReportData', () => {
expect(total).toBe(1000);
});
});

// keyFromObjectUrl/objectUrlFor are pure functions but depend on
// REPORTS_BUCKET_NAME and AWS_REGION at call time, so each test sets its own
// env rather than relying on a shared beforeAll value.
describe('objectUrlFor / keyFromObjectUrl', () => {
const ORIGINAL_BUCKET = process.env.REPORTS_BUCKET_NAME;
const ORIGINAL_REGION = process.env.AWS_REGION;

beforeEach(() => {
process.env.REPORTS_BUCKET_NAME = 'bucket';
process.env.AWS_REGION = 'us-east-2';
});

afterAll(() => {
process.env.REPORTS_BUCKET_NAME = ORIGINAL_BUCKET;
process.env.AWS_REGION = ORIGINAL_REGION;
});

test('objectUrlFor and keyFromObjectUrl round-trip a key', () => {
const key = `${reportKeyPrefix(1)}report.pdf`;
const url = objectUrlFor(key);
expect(keyFromObjectUrl(url)).toBe(key);
});

test('a region-less legacy host still resolves the key, for rows written before objectUrlFor', () => {
const legacyUrl = 'https://bucket.s3.amazonaws.com/reports/1/old-report.pdf';
expect(keyFromObjectUrl(legacyUrl)).toBe('reports/1/old-report.pdf');
});

test('a non-https URL is rejected', () => {
const httpUrl = 'http://bucket.s3.us-east-2.amazonaws.com/reports/1/report.pdf';
expect(keyFromObjectUrl(httpUrl)).toBeNull();
});

test('a URL pointed at a different bucket entirely is rejected', () => {
const otherBucketUrl = 'https://someone-elses-bucket.s3.us-east-2.amazonaws.com/reports/1/report.pdf';
expect(keyFromObjectUrl(otherBucketUrl)).toBeNull();
});

test('a malformed percent-encoded path is caught and returns null rather than throwing', () => {
const malformedUrl = 'https://bucket.s3.us-east-2.amazonaws.com/reports/1/%ZZbad.pdf';
expect(() => keyFromObjectUrl(malformedUrl)).not.toThrow();
expect(keyFromObjectUrl(malformedUrl)).toBeNull();
});

test('a completely invalid URL string returns null rather than throwing', () => {
expect(() => keyFromObjectUrl('not a url at all')).not.toThrow();
expect(keyFromObjectUrl('not a url at all')).toBeNull();
});

test('an empty path (just the bucket root) returns null', () => {
const rootUrl = 'https://bucket.s3.us-east-2.amazonaws.com/';
expect(keyFromObjectUrl(rootUrl)).toBeNull();
});
});
70 changes: 69 additions & 1 deletion apps/backend/lambdas/reports/test/reports.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jest.mock('@aws-sdk/client-s3', () => ({
send: jest.fn().mockReturnValue({} as any),
})),
PutObjectCommand: jest.fn().mockImplementation((params: unknown) => params),
GetObjectCommand: jest.fn().mockImplementation((params: unknown) => params),
}));
jest.mock('@aws-sdk/s3-request-presigner', () => ({
getSignedUrl: jest.fn().mockReturnValue('https://presigned.example.com/upload' as any),
Expand Down Expand Up @@ -405,6 +406,56 @@ describe('Reports e2e tests', () => {
});
});

describe('GET /reports/{id}/download', () => {
function downloadEvent(id: string | number) {
return {
rawPath: `/reports/${id}/download`,
requestContext: { http: { method: 'GET' } },
headers: { Authorization: 'Bearer fake-token' },
};
}

test('200: returns a signed downloadUrl and expiresIn for a report stored under its project prefix', async () => {
// Seed rows aren't guaranteed to sit under reports/<projectId>/ for the
// current REPORTS_BUCKET_NAME, so create a report with a known-good
// objectUrl first (same pattern as the POST /reports suite) and
// download that one rather than assuming anything about seed data.
const fakeObjectUrl = 'https://bucket.s3.us-east-2.amazonaws.com/reports/1/dl-test-report.pdf';
const createRes = await handler({
rawPath: '/reports',
requestContext: { http: { method: 'POST' } },
headers: { Authorization: 'Bearer fake-token' },
queryStringParameters: {},
body: JSON.stringify({ title: 'Download Test', projectId: 1, objectUrl: fakeObjectUrl }),
});
expect(createRes.statusCode).toBe(201);
const createdId = JSON.parse(createRes.body).report_id;

const res = await handler(downloadEvent(createdId));
expect(res.statusCode).toBe(200);
const body = JSON.parse(res.body);
expect(body.downloadUrl).toBe('https://presigned.example.com/upload');
expect(body.expiresIn).toBe(900);
});

test('404: non-numeric id falls through to catch-all', async () => {
const res = await handler(downloadEvent('abc'));
expect(res.statusCode).toBe(404);
});

test('404: unknown id returns 404', async () => {
const res = await handler(downloadEvent(99999));
expect(res.statusCode).toBe(404);
expect(JSON.parse(res.body).message).toBe('Report not found');
});

test('401: unauthenticated request is rejected', async () => {
mockAuthenticateRequest.mockResolvedValue({ isAuthenticated: false });
const res = await handler(downloadEvent(1));
expect(res.statusCode).toBe(401);
});
});

describe('DELETE /reports/{id}', () => {
function idEvent(method: 'GET' | 'DELETE', id: string | number) {
return {
Expand Down Expand Up @@ -472,5 +523,22 @@ describe('Reports e2e tests', () => {
client2.release();
}
});

test('200: deleting a report whose object_url is not under the reports bucket still deletes the row, fileDeleted false', async () => {
// Seed report 5's object_url ('https://s3.amazonaws.com/reports/b.pdf' or
// similar legacy-style URL) may not resolve via keyFromObjectUrl against
// the current REPORTS_BUCKET_NAME; either way the row must go.
const res = await handler(idEvent('DELETE', 2));
expect(res.statusCode).toBe(200);
expect(typeof JSON.parse(res.body).fileDeleted).toBe('boolean');

const client = await pool.connect();
try {
const result = await client.query('SELECT * FROM branch.reports WHERE report_id = 2');
expect(result.rows.length).toBe(0);
} finally {
client.release();
}
});
});
});
});
Loading
Loading