From ceadbe6e1ee8356d609db3e56ee8dc371c7889ec Mon Sep 17 00:00:00 2001 From: FerretCode Date: Mon, 17 Aug 2026 16:15:27 -0700 Subject: [PATCH 1/8] feat: add KW COCO CSV pre-processing annotation mapping in project settings Co-authored-by: multica-agent --- routes/api.js | 3 + routes/projects/mapKwCocoCsv.js | 118 +++++++++++++++++++++ tests/integration/mapKwCocoCsv.test.js | 94 +++++++++++++++++ tests/unit/parseKwCocoCsv.test.js | 52 +++++++++ utils/parseKwCocoCsv.js | 141 +++++++++++++++++++++++++ views/settings/projSettings.ejs | 60 +++++++++++ 6 files changed, 468 insertions(+) create mode 100644 routes/projects/mapKwCocoCsv.js create mode 100644 tests/integration/mapKwCocoCsv.test.js create mode 100644 tests/unit/parseKwCocoCsv.test.js create mode 100644 utils/parseKwCocoCsv.js diff --git a/routes/api.js b/routes/api.js index 111e6915..72f9bd42 100755 --- a/routes/api.js +++ b/routes/api.js @@ -59,6 +59,7 @@ const importDataset = require("./projects/importDataset"); const importYolo = require("./projects/importYolo"); const importKwCoco = require("./projects/importKwCoco"); const importIfcb = require("./projects/importIfcb"); +const mapKwCocoCsv = require("./projects/mapKwCocoCsv"); const mergeLocal = require("./projects/mergeLocal"); const removeAccess = require("./projects/removeAccess"); const transferAdmin = require("./projects/transferAdmin"); @@ -169,6 +170,8 @@ api.post("/import", importProject); api.post("/api/projects/import-dataset", importDataset); api.post("/api/projects/import-yolo", importYolo); api.post("/api/projects/import-kwcoco", importKwCoco); +api.post("/api/projects/map-kwcoco-csv", mapKwCocoCsv); +api.post("/mapKwCocoCsv", mapKwCocoCsv); api.post("/api/projects/import-ifcb", importIfcb); api.post("/mergeLocal", mergeLocal); api.post("/removeAccess", removeAccess); diff --git a/routes/projects/mapKwCocoCsv.js b/routes/projects/mapKwCocoCsv.js new file mode 100644 index 00000000..3e28a447 --- /dev/null +++ b/routes/projects/mapKwCocoCsv.js @@ -0,0 +1,118 @@ +const path = require('path'); +const fs = require('fs'); +const parseKwCocoCsv = require('../../utils/parseKwCocoCsv'); +const queries = require('../../queries/queries'); +const { Client } = require('../../queries/client'); + +async function mapKwCocoCsv(req, res) { + try { + const projectName = req.body.PName || req.body.project_name || req.body.projectName; + const admin = req.body.Admin || req.cookies?.Username || 'admin'; + + if (!projectName) { + return res.status(400).json({ success: false, message: 'Project name is required.' }); + } + + if (!req.files || Object.keys(req.files).length === 0) { + return res.status(400).json({ success: false, message: 'No CSV file was uploaded.' }); + } + + const uploadedFile = req.files.kwcoco_csv || req.files.csv_file || req.files.upload_csv || Object.values(req.files)[0]; + if (!uploadedFile) { + return res.status(400).json({ success: false, message: 'Invalid file upload payload.' }); + } + + const csvContent = uploadedFile.data ? uploadedFile.data.toString('utf8') : fs.readFileSync(uploadedFile.tempFilePath, 'utf8'); + + const parsedAnnotations = parseKwCocoCsv(csvContent); + if (parsedAnnotations.length === 0) { + return res.status(400).json({ success: false, message: 'No valid KW COCO CSV annotations found in file.' }); + } + + const mainPath = path.join(__dirname, '..', '..', 'public', 'projects'); + const projectPath = path.join(mainPath, `${admin}-${projectName}`); + + if (!fs.existsSync(projectPath)) { + return res.status(404).json({ success: false, message: `Project path not found: ${admin}-${projectName}` }); + } + + const dbPath = path.join(projectPath, `${projectName}.db`); + if (!global.projectDbClients[projectPath]) { + global.projectDbClients[projectPath] = new Client(dbPath); + const client = global.projectDbClients[projectPath]; + if (typeof client.open === 'function') { + client.open(); + } + } + + // Migrate DB if needed + await queries.project.migrateProjectDb(projectPath); + + // 1. Ensure all referenced classes exist in Classes table + const existingClassRows = await queries.project.getAllClasses(projectPath) || []; + const existingClassSet = new Set(existingClassRows.map(c => c.CName)); + + const uniqueClasses = new Set(parsedAnnotations.map(a => a.className)); + let classesAdded = 0; + + for (const cname of uniqueClasses) { + if (!existingClassSet.has(cname)) { + await queries.project.createClass(projectPath, cname); + existingClassSet.add(cname); + classesAdded++; + } + } + + // 2. Ensure referenced images exist in Images table + const existingImageRows = await queries.project.getAllImages(projectPath) || []; + const existingImageSet = new Set(existingImageRows.map(i => i.IName)); + + const uniqueImages = new Set(parsedAnnotations.map(a => a.filename)); + let imagesRegistered = 0; + + for (const iname of uniqueImages) { + if (!existingImageSet.has(iname)) { + await queries.project.sql(projectPath, "INSERT OR IGNORE INTO Images (IName, reviewImage, validateImage) VALUES (?, 0, 0)", [iname]); + existingImageSet.add(iname); + imagesRegistered++; + } + } + + // 3. Get current max LID in Labels table + const maxLidResult = await queries.project.getMaxLabelId(projectPath); + let nextLid = 1; + if (maxLidResult && maxLidResult.length > 0 && maxLidResult[0].LID) { + nextLid = maxLidResult[0].LID + 1; + } + + // 4. Insert labels + let labelsInserted = 0; + for (const ann of parsedAnnotations) { + await queries.project.createLabel( + projectPath, + nextLid++, + ann.className, + ann.x, + ann.y, + ann.w, + ann.h, + ann.filename + ); + labelsInserted++; + } + + return res.json({ + success: true, + message: `Successfully mapped ${labelsInserted} KW COCO annotations.`, + labelsInserted, + classesAdded, + imagesRegistered + }); + + } catch (err) { + console.error('Error mapping KW COCO CSV annotations:', err); + return res.status(500).json({ success: false, message: err.message || 'Internal server error mapping KW COCO CSV.' }); + } +} + +module.exports = mapKwCocoCsv; diff --git a/tests/integration/mapKwCocoCsv.test.js b/tests/integration/mapKwCocoCsv.test.js new file mode 100644 index 00000000..fdf16f2a --- /dev/null +++ b/tests/integration/mapKwCocoCsv.test.js @@ -0,0 +1,94 @@ +const request = require('supertest'); +const fs = require('fs'); +const path = require('path'); +const os = require('os'); +const app = require('../../app'); +const queries = require('../../queries/queries'); +const { Client } = require('../../queries/client'); + +describe('POST /api/projects/map-kwcoco-csv', () => { + let tmpDir; + let projectDir; + let originalProjectsPath; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'map-kwcoco-test-')); + projectDir = path.join(__dirname, '..', '..', 'public', 'projects', 'admin-testproj'); + fs.mkdirSync(projectDir, { recursive: true }); + + const mockClient = { + open: jest.fn(), + all: jest.fn().mockImplementation((sql) => { + if (sql.includes('Classes')) return Promise.resolve([]); + if (sql.includes('Images')) return Promise.resolve([]); + if (sql.includes('Labels')) return Promise.resolve([]); + return Promise.resolve([]); + }), + get: jest.fn().mockResolvedValue(null), + run: jest.fn().mockResolvedValue({ changes: 1 }), + }; + + global.projectDbClients = { + [projectDir]: mockClient + }; + }); + + afterEach(() => { + fs.rmSync(projectDir, { recursive: true, force: true }); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test('returns 400 when project name is missing', async () => { + const res = await request(app) + .post('/api/projects/map-kwcoco-csv') + .set('Cookie', ['Username=admin']); + + expect(res.statusCode).toBe(400); + expect(res.body.success).toBe(false); + expect(res.body.message).toMatch(/Project name is required/i); + }); + + test('returns 400 when no CSV file is uploaded', async () => { + const res = await request(app) + .post('/api/projects/map-kwcoco-csv') + .field('PName', 'testproj') + .field('Admin', 'admin'); + + expect(res.statusCode).toBe(400); + expect(res.body.success).toBe(false); + expect(res.body.message).toMatch(/No CSV file was uploaded/i); + }); + + test('returns 404 when project path does not exist', async () => { + const csvContent = 'filename,class,xmin,ymin,xmax,ymax\nimg1.jpg,dolphin,10,10,50,50'; + const csvPath = path.join(tmpDir, 'test.csv'); + fs.writeFileSync(csvPath, csvContent); + + const res = await request(app) + .post('/api/projects/map-kwcoco-csv') + .field('PName', 'nonexistent_project') + .field('Admin', 'admin') + .attach('kwcoco_csv', csvPath); + + expect(res.statusCode).toBe(404); + expect(res.body.success).toBe(false); + }); + + test('successfully maps KW COCO CSV annotations', async () => { + const csvContent = `filename,class,xmin,ymin,xmax,ymax +img1.jpg,dolphin,10,20,100,150 +img2.jpg,shark,30,40,80,120`; + const csvPath = path.join(tmpDir, 'test.csv'); + fs.writeFileSync(csvPath, csvContent); + + const res = await request(app) + .post('/api/projects/map-kwcoco-csv') + .field('PName', 'testproj') + .field('Admin', 'admin') + .attach('kwcoco_csv', csvPath); + + expect(res.statusCode).toBe(200); + expect(res.body.success).toBe(true); + expect(res.body.labelsInserted).toBe(2); + }); +}); diff --git a/tests/unit/parseKwCocoCsv.test.js b/tests/unit/parseKwCocoCsv.test.js new file mode 100644 index 00000000..3f91e614 --- /dev/null +++ b/tests/unit/parseKwCocoCsv.test.js @@ -0,0 +1,52 @@ +const parseKwCocoCsv = require('../../utils/parseKwCocoCsv'); + +describe('parseKwCocoCsv', () => { + test('returns empty array for invalid or empty input', () => { + expect(parseKwCocoCsv(null)).toEqual([]); + expect(parseKwCocoCsv('')).toEqual([]); + expect(parseKwCocoCsv(' ')).toEqual([]); + }); + + test('parses CSV with header containing filename, class, xmin, ymin, xmax, ymax', () => { + const csv = `filename,class,xmin,ymin,xmax,ymax +image1.jpg,dolphin,10,20,100,150 +image2.png,blue whale,50,60,200,260`; + + const result = parseKwCocoCsv(csv); + expect(result).toEqual([ + { filename: 'image1.jpg', className: 'dolphin', x: 10, y: 20, w: 90, h: 130 }, + { filename: 'image2.png', className: 'blue_whale', x: 50, y: 60, w: 150, h: 200 } + ]); + }); + + test('parses CSV with header containing x, y, w, h', () => { + const csv = `file_name,category,x,y,w,h +path/to/img3.jpg,sea turtle,15,25,80,95`; + + const result = parseKwCocoCsv(csv); + expect(result).toEqual([ + { filename: 'img3.jpg', className: 'sea_turtle', x: 15, y: 25, w: 80, h: 95 } + ]); + }); + + test('parses positional CSV without header', () => { + const csv = `img4.jpg,fish,5,10,45,60`; + + const result = parseKwCocoCsv(csv); + expect(result).toEqual([ + { filename: 'img4.jpg', className: 'fish', x: 5, y: 10, w: 40, h: 50 } + ]); + }); + + test('ignores invalid rows and negative width/height', () => { + const csv = `filename,class,xmin,ymin,xmax,ymax +bad1.jpg,fish,100,100,50,50 +bad2.jpg,fish,invalid,10,20,30 +good.jpg,shark,10,10,30,30`; + + const result = parseKwCocoCsv(csv); + expect(result).toEqual([ + { filename: 'good.jpg', className: 'shark', x: 10, y: 10, w: 20, h: 20 } + ]); + }); +}); diff --git a/utils/parseKwCocoCsv.js b/utils/parseKwCocoCsv.js new file mode 100644 index 00000000..b09bf5a3 --- /dev/null +++ b/utils/parseKwCocoCsv.js @@ -0,0 +1,141 @@ +const path = require('path'); + +/** + * Parses KW COCO CSV annotation strings or file contents into structured bounding box records. + * + * Supports header-based CSVs (filename/file_name, class/category/label, xmin/x/bbox_x, ymin/y/bbox_y, xmax/w/bbox_w, ymax/h/bbox_h) + * and positional/VIAME CSVs (filename, class, xmin/x, ymin/y, xmax/w, ymax/h). + * + * @param {string} csvContent - Raw CSV string content + * @returns {Array<{filename: string, className: string, x: number, y: number, w: number, h: number}>} Parsed annotation objects + */ +function parseKwCocoCsv(csvContent) { + if (!csvContent || typeof csvContent !== 'string') { + return []; + } + + const lines = csvContent + .split(/\r?\n/) + .map(line => line.trim()) + .filter(line => line.length > 0 && !line.startsWith('#')); + + if (lines.length === 0) { + return []; + } + + const firstLine = lines[0]; + const rawTokens = firstLine.split(',').map(t => t.trim()); + const lowerTokens = rawTokens.map(t => t.toLowerCase()); + + const hasHeader = lowerTokens.some(t => + ['filename', 'file_name', 'file', 'image', 'class', 'category', 'label', 'xmin', 'x', 'ymin', 'y', 'xmax', 'w', 'ymax', 'h', 'bbox_x'].includes(t) + ); + + let filenameIdx = 0; + let classIdx = 1; + let xIdx = 2; + let yIdx = 3; + let wOrXmaxIdx = 4; + let hOrYmaxIdx = 5; + let isWidthHeight = false; + + let startLine = 0; + + if (hasHeader) { + startLine = 1; + + filenameIdx = lowerTokens.findIndex(t => ['filename', 'file_name', 'file', 'image', 'iname', 'image_name'].includes(t)); + if (filenameIdx === -1) filenameIdx = 0; + + classIdx = lowerTokens.findIndex(t => ['class', 'category', 'label', 'cname', 'class_name'].includes(t)); + if (classIdx === -1) classIdx = 1; + + xIdx = lowerTokens.findIndex(t => ['xmin', 'x', 'left_x', 'left', 'bbox_x', 'tl_x'].includes(t)); + if (xIdx === -1) xIdx = 2; + + yIdx = lowerTokens.findIndex(t => ['ymin', 'y', 'top_y', 'top', 'bbox_y', 'tl_y'].includes(t)); + if (yIdx === -1) yIdx = 3; + + const wIdx = lowerTokens.findIndex(t => ['w', 'width', 'box_w', 'bbox_w'].includes(t)); + const xmaxIdx = lowerTokens.findIndex(t => ['xmax', 'right_x', 'right', 'br_x'].includes(t)); + + if (wIdx !== -1) { + wOrXmaxIdx = wIdx; + isWidthHeight = true; + } else if (xmaxIdx !== -1) { + wOrXmaxIdx = xmaxIdx; + isWidthHeight = false; + } else { + wOrXmaxIdx = 4; + } + + const hIdx = lowerTokens.findIndex(t => ['h', 'height', 'box_h', 'bbox_h'].includes(t)); + const ymaxIdx = lowerTokens.findIndex(t => ['ymax', 'bottom_y', 'bottom', 'br_y'].includes(t)); + + if (hIdx !== -1) { + hOrYmaxIdx = hIdx; + } else if (ymaxIdx !== -1) { + hOrYmaxIdx = ymaxIdx; + } else { + hOrYmaxIdx = 5; + } + } + + const results = []; + + for (let i = startLine; i < lines.length; i++) { + const row = lines[i].split(',').map(col => col.trim()); + if (row.length <= Math.max(filenameIdx, classIdx, xIdx, yIdx, wOrXmaxIdx, hOrYmaxIdx)) { + continue; + } + + const rawFilename = row[filenameIdx]; + const rawClass = row[classIdx]; + + if (!rawFilename || !rawClass) { + continue; + } + + const filename = path.basename(rawFilename.replace(/\\/g, '/')); + const className = rawClass.replace(/\s+/g, '_'); + + const xVal = parseFloat(row[xIdx]); + const yVal = parseFloat(row[yIdx]); + const val4 = parseFloat(row[wOrXmaxIdx]); + const val5 = parseFloat(row[hOrYmaxIdx]); + + if (isNaN(xVal) || isNaN(yVal) || isNaN(val4) || isNaN(val5)) { + continue; + } + + let x = Math.round(xVal); + let y = Math.round(yVal); + let w = 0; + let h = 0; + + if (isWidthHeight) { + w = Math.round(val4); + h = Math.round(val5); + } else { + w = Math.round(val4 - xVal); + h = Math.round(val5 - yVal); + } + + if (w <= 0 || h <= 0) { + continue; + } + + results.push({ + filename, + className, + x, + y, + w, + h + }); + } + + return results; +} + +module.exports = parseKwCocoCsv; diff --git a/views/settings/projSettings.ejs b/views/settings/projSettings.ejs index 8f9d1f3c..9fed6207 100644 --- a/views/settings/projSettings.ejs +++ b/views/settings/projSettings.ejs @@ -93,6 +93,31 @@
+
+

+ Pre-Processing: Map KW COCO CSV Annotations +

+

+ Upload a `.csv` annotation file (KW COCO format) to map bounding box labels directly to project images. +

+
+ + + +
+ + +
+
+ <% if(user == Admin) { %> + + <% } else { %> + Save + <% } %> +
+
+
+

Delete Images Without Label

@@ -237,4 +262,39 @@ request.send(formData); console.log(formData); }); + + if (document.forms['mapKwCocoCsvForm']) { + document.forms['mapKwCocoCsvForm'].addEventListener('submit', (event) => { + event.preventDefault(); + var formData = new FormData(); + const csvFileInput = document.getElementById('kwcoco_csv'); + if (!csvFileInput || !csvFileInput.files[0]) { + alert("Please select a .csv file to upload."); + return; + } + formData.append('kwcoco_csv', csvFileInput.files[0]); + formData.append("PName", "<%= PName %>"); + formData.append("Admin", "<%= Admin %>"); + formData.append("IDX", "<%= IDX %>"); + + var request = new XMLHttpRequest(); + request.open("post", event.target.action, true); + request.onreadystatechange = function receiveResponse() { + if (this.readyState == 4) { + if (this.status == 200) { + try { + var res = JSON.parse(this.response); + alert(res.message || "Annotations successfully mapped!"); + } catch (e) { + alert("Annotations mapped successfully."); + } + window.location.reload(); + } else { + alert("Failed to map annotations: " + this.response); + } + } + } + request.send(formData); + }); + } From 5a5cff03137d16aa74675ab2ea8ff946910cf8a0 Mon Sep 17 00:00:00 2001 From: FerretCode Date: Mon, 17 Aug 2026 16:38:26 -0700 Subject: [PATCH 2/8] feat: support KW COCO JSON alongside CSV in annotation pre-processing Adds a JSON parser (images/annotations/categories -> bbox records) and dispatches by file extension/content sniffing in the existing map-kwcoco-csv route so uploads of either .csv or .json are handled by the same endpoint. Co-authored-by: multica-agent --- routes/projects/mapKwCocoCsv.js | 16 +++-- tests/integration/mapKwCocoCsv.test.js | 33 ++++++++- tests/unit/parseKwCocoJson.test.js | 64 ++++++++++++++++++ utils/parseKwCocoJson.js | 92 ++++++++++++++++++++++++++ views/settings/projSettings.ejs | 8 +-- 5 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 tests/unit/parseKwCocoJson.test.js create mode 100644 utils/parseKwCocoJson.js diff --git a/routes/projects/mapKwCocoCsv.js b/routes/projects/mapKwCocoCsv.js index 3e28a447..929d7996 100644 --- a/routes/projects/mapKwCocoCsv.js +++ b/routes/projects/mapKwCocoCsv.js @@ -1,6 +1,7 @@ const path = require('path'); const fs = require('fs'); const parseKwCocoCsv = require('../../utils/parseKwCocoCsv'); +const parseKwCocoJson = require('../../utils/parseKwCocoJson'); const queries = require('../../queries/queries'); const { Client } = require('../../queries/client'); @@ -14,19 +15,24 @@ async function mapKwCocoCsv(req, res) { } if (!req.files || Object.keys(req.files).length === 0) { - return res.status(400).json({ success: false, message: 'No CSV file was uploaded.' }); + return res.status(400).json({ success: false, message: 'No annotation file was uploaded.' }); } - const uploadedFile = req.files.kwcoco_csv || req.files.csv_file || req.files.upload_csv || Object.values(req.files)[0]; + const uploadedFile = req.files.kwcoco_csv || req.files.kwcoco_json || req.files.csv_file + || req.files.json_file || req.files.upload_csv || req.files.upload_json || Object.values(req.files)[0]; if (!uploadedFile) { return res.status(400).json({ success: false, message: 'Invalid file upload payload.' }); } - const csvContent = uploadedFile.data ? uploadedFile.data.toString('utf8') : fs.readFileSync(uploadedFile.tempFilePath, 'utf8'); + const fileContent = uploadedFile.data ? uploadedFile.data.toString('utf8') : fs.readFileSync(uploadedFile.tempFilePath, 'utf8'); - const parsedAnnotations = parseKwCocoCsv(csvContent); + const ext = path.extname(uploadedFile.name || '').toLowerCase(); + const trimmedContent = fileContent.trim(); + const isJson = ext === '.json' || (ext !== '.csv' && (trimmedContent.startsWith('{') || trimmedContent.startsWith('['))); + + const parsedAnnotations = isJson ? parseKwCocoJson(fileContent) : parseKwCocoCsv(fileContent); if (parsedAnnotations.length === 0) { - return res.status(400).json({ success: false, message: 'No valid KW COCO CSV annotations found in file.' }); + return res.status(400).json({ success: false, message: 'No valid KW COCO annotations found in file.' }); } const mainPath = path.join(__dirname, '..', '..', 'public', 'projects'); diff --git a/tests/integration/mapKwCocoCsv.test.js b/tests/integration/mapKwCocoCsv.test.js index fdf16f2a..c592f842 100644 --- a/tests/integration/mapKwCocoCsv.test.js +++ b/tests/integration/mapKwCocoCsv.test.js @@ -48,7 +48,7 @@ describe('POST /api/projects/map-kwcoco-csv', () => { expect(res.body.message).toMatch(/Project name is required/i); }); - test('returns 400 when no CSV file is uploaded', async () => { + test('returns 400 when no annotation file is uploaded', async () => { const res = await request(app) .post('/api/projects/map-kwcoco-csv') .field('PName', 'testproj') @@ -56,7 +56,7 @@ describe('POST /api/projects/map-kwcoco-csv', () => { expect(res.statusCode).toBe(400); expect(res.body.success).toBe(false); - expect(res.body.message).toMatch(/No CSV file was uploaded/i); + expect(res.body.message).toMatch(/No annotation file was uploaded/i); }); test('returns 404 when project path does not exist', async () => { @@ -91,4 +91,33 @@ img2.jpg,shark,30,40,80,120`; expect(res.body.success).toBe(true); expect(res.body.labelsInserted).toBe(2); }); + + test('successfully maps KW COCO JSON annotations', async () => { + const jsonContent = JSON.stringify({ + images: [ + { id: 1, file_name: 'img1.jpg' }, + { id: 2, file_name: 'img2.jpg' } + ], + annotations: [ + { id: 1, image_id: 1, category_id: 1, bbox: [10, 20, 90, 130] }, + { id: 2, image_id: 2, category_id: 2, bbox: [30, 40, 50, 80] } + ], + categories: [ + { id: 1, name: 'dolphin' }, + { id: 2, name: 'shark' } + ] + }); + const jsonPath = path.join(tmpDir, 'test.json'); + fs.writeFileSync(jsonPath, jsonContent); + + const res = await request(app) + .post('/api/projects/map-kwcoco-csv') + .field('PName', 'testproj') + .field('Admin', 'admin') + .attach('kwcoco_json', jsonPath); + + expect(res.statusCode).toBe(200); + expect(res.body.success).toBe(true); + expect(res.body.labelsInserted).toBe(2); + }); }); diff --git a/tests/unit/parseKwCocoJson.test.js b/tests/unit/parseKwCocoJson.test.js new file mode 100644 index 00000000..cbb6b0de --- /dev/null +++ b/tests/unit/parseKwCocoJson.test.js @@ -0,0 +1,64 @@ +const parseKwCocoJson = require('../../utils/parseKwCocoJson'); + +describe('parseKwCocoJson', () => { + test('returns empty array for invalid or empty input', () => { + expect(parseKwCocoJson(null)).toEqual([]); + expect(parseKwCocoJson('')).toEqual([]); + expect(parseKwCocoJson('not json')).toEqual([]); + expect(parseKwCocoJson('{}')).toEqual([]); + }); + + test('parses standard COCO-style JSON with images/annotations/categories', () => { + const json = JSON.stringify({ + images: [ + { id: 1, file_name: 'image1.jpg' }, + { id: 2, file_name: 'path/to/image2.png' } + ], + annotations: [ + { id: 1, image_id: 1, category_id: 10, bbox: [10, 20, 90, 130] }, + { id: 2, image_id: 2, category_id: 11, bbox: [50, 60, 150, 200] } + ], + categories: [ + { id: 10, name: 'dolphin' }, + { id: 11, name: 'blue whale' } + ] + }); + + const result = parseKwCocoJson(json); + expect(result).toEqual([ + { filename: 'image1.jpg', className: 'dolphin', x: 10, y: 20, w: 90, h: 130 }, + { filename: 'image2.png', className: 'blue_whale', x: 50, y: 60, w: 150, h: 200 } + ]); + }); + + test('skips annotations referencing unknown images or categories', () => { + const json = JSON.stringify({ + images: [{ id: 1, file_name: 'good.jpg' }], + annotations: [ + { id: 1, image_id: 1, category_id: 5, bbox: [10, 10, 20, 20] }, + { id: 2, image_id: 999, category_id: 5, bbox: [10, 10, 20, 20] }, + { id: 3, image_id: 1, category_id: 999, bbox: [10, 10, 20, 20] } + ], + categories: [{ id: 5, name: 'fish' }] + }); + + const result = parseKwCocoJson(json); + expect(result).toEqual([ + { filename: 'good.jpg', className: 'fish', x: 10, y: 10, w: 20, h: 20 } + ]); + }); + + test('ignores annotations with invalid or non-positive bbox dimensions', () => { + const json = JSON.stringify({ + images: [{ id: 1, file_name: 'img.jpg' }], + annotations: [ + { id: 1, image_id: 1, category_id: 1, bbox: [0, 0, 0, 0] }, + { id: 2, image_id: 1, category_id: 1, bbox: ['a', 'b', 'c', 'd'] }, + { id: 3, image_id: 1, category_id: 1, bbox: [1, 2, 3] } + ], + categories: [{ id: 1, name: 'shark' }] + }); + + expect(parseKwCocoJson(json)).toEqual([]); + }); +}); diff --git a/utils/parseKwCocoJson.js b/utils/parseKwCocoJson.js new file mode 100644 index 00000000..f81d0b06 --- /dev/null +++ b/utils/parseKwCocoJson.js @@ -0,0 +1,92 @@ +const path = require('path'); + +/** + * Parses KW COCO / COCO-style JSON annotation content into the same + * {filename, className, x, y, w, h} shape produced by parseKwCocoCsv, so both + * formats can feed the same downstream label-import pipeline. + * + * Expects the standard COCO structure: {images: [...], annotations: [...], categories: [...]} + * with annotations referencing images/categories by id and bbox as [x, y, w, h]. + * + * @param {string} jsonContent - Raw JSON string content + * @returns {Array<{filename: string, className: string, x: number, y: number, w: number, h: number}>} Parsed annotation objects + */ +function parseKwCocoJson(jsonContent) { + if (!jsonContent || typeof jsonContent !== 'string') { + return []; + } + + let data; + try { + data = JSON.parse(jsonContent); + } catch (err) { + return []; + } + + if (!data || typeof data !== 'object') { + return []; + } + + const images = Array.isArray(data.images) ? data.images : []; + const annotations = Array.isArray(data.annotations) ? data.annotations : []; + const categories = Array.isArray(data.categories) ? data.categories : []; + + if (images.length === 0 || annotations.length === 0) { + return []; + } + + const imageIdToFilename = new Map(); + for (const img of images) { + if (!img || img.id == null) continue; + const rawName = img.file_name || img.filename || img.name; + if (!rawName) continue; + imageIdToFilename.set(img.id, path.basename(String(rawName).replace(/\\/g, '/'))); + } + + const categoryIdToName = new Map(); + for (const cat of categories) { + if (!cat || cat.id == null) continue; + const rawName = cat.name || cat.category_name; + if (!rawName) continue; + categoryIdToName.set(cat.id, String(rawName).replace(/\s+/g, '_')); + } + + const results = []; + + for (const ann of annotations) { + if (!ann) continue; + + const filename = imageIdToFilename.get(ann.image_id); + if (!filename) continue; + + const className = categoryIdToName.get(ann.category_id); + if (!className) continue; + + const bbox = ann.bbox; + if (!Array.isArray(bbox) || bbox.length < 4) continue; + + const xVal = parseFloat(bbox[0]); + const yVal = parseFloat(bbox[1]); + const wVal = parseFloat(bbox[2]); + const hVal = parseFloat(bbox[3]); + + if (isNaN(xVal) || isNaN(yVal) || isNaN(wVal) || isNaN(hVal)) { + continue; + } + + const x = Math.round(xVal); + const y = Math.round(yVal); + const w = Math.round(wVal); + const h = Math.round(hVal); + + if (w <= 0 || h <= 0) { + continue; + } + + results.push({ filename, className, x, y, w, h }); + } + + return results; +} + +module.exports = parseKwCocoJson; diff --git a/views/settings/projSettings.ejs b/views/settings/projSettings.ejs index 9fed6207..1f791f8f 100644 --- a/views/settings/projSettings.ejs +++ b/views/settings/projSettings.ejs @@ -95,18 +95,18 @@

- Pre-Processing: Map KW COCO CSV Annotations + Pre-Processing: Map KW COCO Annotations

- Upload a `.csv` annotation file (KW COCO format) to map bounding box labels directly to project images. + Upload a `.csv` or `.json` annotation file (KW COCO format) to map bounding box labels directly to project images.

- - + +
<% if(user == Admin) { %> From 2c6a523e9799e6dd8dd2540f62acef56af241f58 Mon Sep 17 00:00:00 2001 From: FerretCode Date: Mon, 17 Aug 2026 16:54:58 -0700 Subject: [PATCH 3/8] fix: use .rows when reading Client.all() results in mapKwCocoCsv getAllClasses/getAllImages/getMaxLabelId resolve to {success, rows} (matching queries/client.js's db.all()/db.get() contract used elsewhere, e.g. routes/inference/yoloInference.js), not a bare array, so existingClassRows.map threw at runtime. The integration test mock returned plain arrays instead of the real {rows: [...]} shape, which is why 14/14 passed despite the bug; the mock now matches the real client contract. Co-authored-by: multica-agent --- routes/projects/mapKwCocoCsv.js | 11 +++++++---- tests/integration/mapKwCocoCsv.test.js | 12 ++++++------ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/routes/projects/mapKwCocoCsv.js b/routes/projects/mapKwCocoCsv.js index 929d7996..6f09b827 100644 --- a/routes/projects/mapKwCocoCsv.js +++ b/routes/projects/mapKwCocoCsv.js @@ -55,7 +55,8 @@ async function mapKwCocoCsv(req, res) { await queries.project.migrateProjectDb(projectPath); // 1. Ensure all referenced classes exist in Classes table - const existingClassRows = await queries.project.getAllClasses(projectPath) || []; + const existingClassResult = await queries.project.getAllClasses(projectPath); + const existingClassRows = existingClassResult?.rows || []; const existingClassSet = new Set(existingClassRows.map(c => c.CName)); const uniqueClasses = new Set(parsedAnnotations.map(a => a.className)); @@ -70,7 +71,8 @@ async function mapKwCocoCsv(req, res) { } // 2. Ensure referenced images exist in Images table - const existingImageRows = await queries.project.getAllImages(projectPath) || []; + const existingImageResult = await queries.project.getAllImages(projectPath); + const existingImageRows = existingImageResult?.rows || []; const existingImageSet = new Set(existingImageRows.map(i => i.IName)); const uniqueImages = new Set(parsedAnnotations.map(a => a.filename)); @@ -86,9 +88,10 @@ async function mapKwCocoCsv(req, res) { // 3. Get current max LID in Labels table const maxLidResult = await queries.project.getMaxLabelId(projectPath); + const maxLidRows = maxLidResult?.rows || []; let nextLid = 1; - if (maxLidResult && maxLidResult.length > 0 && maxLidResult[0].LID) { - nextLid = maxLidResult[0].LID + 1; + if (maxLidRows.length > 0 && maxLidRows[0].LID) { + nextLid = maxLidRows[0].LID + 1; } // 4. Insert labels diff --git a/tests/integration/mapKwCocoCsv.test.js b/tests/integration/mapKwCocoCsv.test.js index c592f842..5005498f 100644 --- a/tests/integration/mapKwCocoCsv.test.js +++ b/tests/integration/mapKwCocoCsv.test.js @@ -19,13 +19,13 @@ describe('POST /api/projects/map-kwcoco-csv', () => { const mockClient = { open: jest.fn(), all: jest.fn().mockImplementation((sql) => { - if (sql.includes('Classes')) return Promise.resolve([]); - if (sql.includes('Images')) return Promise.resolve([]); - if (sql.includes('Labels')) return Promise.resolve([]); - return Promise.resolve([]); + if (sql.includes('Classes')) return Promise.resolve({ success: true, rows: [] }); + if (sql.includes('Images')) return Promise.resolve({ success: true, rows: [] }); + if (sql.includes('Labels')) return Promise.resolve({ success: true, rows: [] }); + return Promise.resolve({ success: true, rows: [] }); }), - get: jest.fn().mockResolvedValue(null), - run: jest.fn().mockResolvedValue({ changes: 1 }), + get: jest.fn().mockResolvedValue({ success: true, row: null }), + run: jest.fn().mockResolvedValue({ success: true, changes: 1, lastID: 1 }), }; global.projectDbClients = { From 2f443ca7e1ac8e3a08f8494e9a7975b6ffd3c9aa Mon Sep 17 00:00:00 2001 From: FerretCode Date: Tue, 18 Aug 2026 10:28:56 -0700 Subject: [PATCH 4/8] Add nullable Images.Source column for future S3-backed image storage Lays the groundwork for serving images from S3 on demand instead of requiring a full local sync: an image row can now record where its bytes live. Existing projects are backfilled via a PRAGMA-guarded ALTER TABLE in migrateProjectDb, since CREATE TABLE IF NOT EXISTS is a no-op on tables that already exist. Co-authored-by: multica-agent --- queries/projects/projects.js | 16 +++++- server.js | 7 +++ tests/integration/imageSourceColumn.test.js | 58 +++++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 tests/integration/imageSourceColumn.test.js diff --git a/queries/projects/projects.js b/queries/projects/projects.js index 1c5851ef..929b29ee 100644 --- a/queries/projects/projects.js +++ b/queries/projects/projects.js @@ -202,7 +202,7 @@ module.exports = { "CREATE TABLE IF NOT EXISTS Classes (CName VARCHAR NOT NULL PRIMARY KEY)", ); await db.run( - "CREATE TABLE IF NOT EXISTS Images (IName VARCHAR NOT NULL PRIMARY KEY, reviewImage INTEGER NOT NULL DEFAULT 0, validateImage INTEGER NOT NULL DEFAULT 0)", + "CREATE TABLE IF NOT EXISTS Images (IName VARCHAR NOT NULL PRIMARY KEY, reviewImage INTEGER NOT NULL DEFAULT 0, validateImage INTEGER NOT NULL DEFAULT 0, Source VARCHAR DEFAULT NULL)", ); await db.run( "CREATE TABLE IF NOT EXISTS Labels (LID INTEGER PRIMARY KEY, CName VARCHAR NOT NULL, X VARCHAR NOT NULL, Y VARCHAR NOT NULL, W INTEGER NOT NULL, H INTEGER NOT NULL, IName VARCHAR NOT NULL, FOREIGN KEY(CName) REFERENCES Classes(CName), FOREIGN KEY(IName) REFERENCES Images(IName))", @@ -210,6 +210,20 @@ module.exports = { await db.run( "CREATE TABLE IF NOT EXISTS Validation (Confidence INTEGER NOT NULL, LID INTEGER NOT NULL PRIMARY KEY, CName VARCHAR NOT NULL, IName VARCHAR NOT NULL, FOREIGN KEY(LID) REFERENCES Labels(LID), FOREIGN KEY(IName) REFERENCES Images(IName), FOREIGN KEY(CName) REFERENCES Classes(CName))", ); + + // Images predates the Source column, so CREATE TABLE IF NOT EXISTS above is a + // no-op on any project database created before this change. Back-fill it here, + // guarded by a PRAGMA check since SQLite has no ADD COLUMN IF NOT EXISTS. + const imageColumns = await db.all("PRAGMA table_info(Images)"); + const hasSourceColumn = (imageColumns.rows || []).some( + (column) => column.name === "Source", + ); + + if (!hasSourceColumn) { + await db.run( + "ALTER TABLE Images ADD COLUMN Source VARCHAR DEFAULT NULL", + ); + } }, addImages: async function( projectPath, diff --git a/server.js b/server.js index ed37eab1..37b0fbfa 100644 --- a/server.js +++ b/server.js @@ -1,6 +1,7 @@ global.logger = require('./utils/logger'); const app = require('./app'); const { Client } = require("./queries/client"); +const queries = require("./queries/queries"); global.configFile = require("./utils/config"); @@ -49,6 +50,12 @@ for (const project of fs.readdirSync(allProjectsPath)) { const dbFile = path.join(projectPath, file); global.projectDbClients[projectPath] = new Client(dbFile); + + queries.project.migrateProjectDb(projectPath).catch((err) => { + global.logger.error( + `Failed to migrate project database at ${projectPath}: ${err}`, + ); + }); } } } diff --git a/tests/integration/imageSourceColumn.test.js b/tests/integration/imageSourceColumn.test.js new file mode 100644 index 00000000..9e2f4fde --- /dev/null +++ b/tests/integration/imageSourceColumn.test.js @@ -0,0 +1,58 @@ +// Unit tests for queries/projects/projects.js#migrateProjectDb, covering the nullable +// Images.Source column added so an image's row can record where its bytes live (e.g. +// local disk vs. an S3 bucket) once storage backends other than local disk are supported. +// Project databases created before this column existed only get it via the PRAGMA-guarded +// ALTER TABLE below, since CREATE TABLE IF NOT EXISTS is a no-op on an existing table. + +jest.mock('../../queries/getDbClient'); + +const getDbClient = require('../../queries/getDbClient'); +const projects = require('../../queries/projects/projects'); + +describe('queries/projects migrateProjectDb Source column backfill', () => { + let mockRun; + let mockAll; + + beforeEach(() => { + jest.clearAllMocks(); + mockRun = jest.fn().mockResolvedValue({ success: true, changes: 0, lastID: 0 }); + mockAll = jest.fn(); + getDbClient.mockReturnValue({ run: mockRun, all: mockAll }); + }); + + it('adds the Source column when an existing Images table predates it', async () => { + mockAll.mockResolvedValue({ + success: true, + rows: [ + { name: 'IName' }, + { name: 'reviewImage' }, + { name: 'validateImage' }, + ], + }); + + await projects.project.migrateProjectDb('/projects/testuser-test-project'); + + expect(mockAll).toHaveBeenCalledWith('PRAGMA table_info(Images)'); + expect(mockRun).toHaveBeenCalledWith( + 'ALTER TABLE Images ADD COLUMN Source VARCHAR DEFAULT NULL', + ); + }); + + it('does not re-add the Source column when it is already present', async () => { + mockAll.mockResolvedValue({ + success: true, + rows: [ + { name: 'IName' }, + { name: 'reviewImage' }, + { name: 'validateImage' }, + { name: 'Source' }, + ], + }); + + await projects.project.migrateProjectDb('/projects/testuser-test-project'); + + expect(mockRun).not.toHaveBeenCalledWith( + 'ALTER TABLE Images ADD COLUMN Source VARCHAR DEFAULT NULL', + ); + }); +}); From 6eafb1a49df08b78d9b29106088c0e3eac722ddc Mon Sep 17 00:00:00 2001 From: FerretCode Date: Tue, 18 Aug 2026 11:42:36 -0700 Subject: [PATCH 5/8] Fix S3 sync silently dropping images that share a basename syncS3Bucket tracked "already synced" purely by sanitized basename, so two distinct keys under different prefixes (e.g. 2024/img.jpg and 2025/img.jpg) collided and the second was silently skipped. Adds a nullable Images.SourceKey column (parallel to Source, same PRAGMA-guarded backfill) to record the literal S3 key an image came from, decoupled from its display IName. syncS3Bucket now treats "already synced" as "we've recorded this exact key" rather than "a file with this name exists," and only disambiguates (via a deterministic hash of the key) when two keys from the same bucket listing genuinely collide on basename - a name that already existed on disk before the run is still skipped as before. Co-authored-by: multica-agent --- queries/projects/projects.js | 34 +++++--- routes/api/v2/s3Buckets.js | 58 ++++++++++++-- tests/integration/imageSourceColumn.test.js | 76 ++++++++++++++++-- tests/integration/s3Buckets.test.js | 89 +++++++++++++++++++++ 4 files changed, 234 insertions(+), 23 deletions(-) diff --git a/queries/projects/projects.js b/queries/projects/projects.js index 929b29ee..8e6f27ec 100644 --- a/queries/projects/projects.js +++ b/queries/projects/projects.js @@ -202,7 +202,7 @@ module.exports = { "CREATE TABLE IF NOT EXISTS Classes (CName VARCHAR NOT NULL PRIMARY KEY)", ); await db.run( - "CREATE TABLE IF NOT EXISTS Images (IName VARCHAR NOT NULL PRIMARY KEY, reviewImage INTEGER NOT NULL DEFAULT 0, validateImage INTEGER NOT NULL DEFAULT 0, Source VARCHAR DEFAULT NULL)", + "CREATE TABLE IF NOT EXISTS Images (IName VARCHAR NOT NULL PRIMARY KEY, reviewImage INTEGER NOT NULL DEFAULT 0, validateImage INTEGER NOT NULL DEFAULT 0, Source VARCHAR DEFAULT NULL, SourceKey VARCHAR DEFAULT NULL)", ); await db.run( "CREATE TABLE IF NOT EXISTS Labels (LID INTEGER PRIMARY KEY, CName VARCHAR NOT NULL, X VARCHAR NOT NULL, Y VARCHAR NOT NULL, W INTEGER NOT NULL, H INTEGER NOT NULL, IName VARCHAR NOT NULL, FOREIGN KEY(CName) REFERENCES Classes(CName), FOREIGN KEY(IName) REFERENCES Images(IName))", @@ -211,18 +211,26 @@ module.exports = { "CREATE TABLE IF NOT EXISTS Validation (Confidence INTEGER NOT NULL, LID INTEGER NOT NULL PRIMARY KEY, CName VARCHAR NOT NULL, IName VARCHAR NOT NULL, FOREIGN KEY(LID) REFERENCES Labels(LID), FOREIGN KEY(IName) REFERENCES Images(IName), FOREIGN KEY(CName) REFERENCES Classes(CName))", ); - // Images predates the Source column, so CREATE TABLE IF NOT EXISTS above is a - // no-op on any project database created before this change. Back-fill it here, - // guarded by a PRAGMA check since SQLite has no ADD COLUMN IF NOT EXISTS. + // Images predates the Source/SourceKey columns, so CREATE TABLE IF NOT EXISTS + // above is a no-op on any project database created before this change. + // Back-fill them here, guarded by a PRAGMA check since SQLite has no + // ADD COLUMN IF NOT EXISTS. SourceKey holds the literal S3 object key (which + // may differ from IName once collisions are disambiguated), decoupled from the + // display name. const imageColumns = await db.all("PRAGMA table_info(Images)"); - const hasSourceColumn = (imageColumns.rows || []).some( - (column) => column.name === "Source", + const existingColumnNames = new Set( + (imageColumns.rows || []).map((column) => column.name), ); - if (!hasSourceColumn) { - await db.run( - "ALTER TABLE Images ADD COLUMN Source VARCHAR DEFAULT NULL", - ); + const backfillColumns = [ + { name: "Source", ddl: "Source VARCHAR DEFAULT NULL" }, + { name: "SourceKey", ddl: "SourceKey VARCHAR DEFAULT NULL" }, + ]; + + for (const column of backfillColumns) { + if (!existingColumnNames.has(column.name)) { + await db.run(`ALTER TABLE Images ADD COLUMN ${column.ddl}`); + } } }, addImages: async function( @@ -230,14 +238,18 @@ module.exports = { imageName, reviewImage, validateImage, + source = null, + sourceKey = null, ) { const db = getDbClient(projectPath); const query = - "INSERT OR IGNORE INTO Images (IName, reviewImage, validateImage) VALUES (?, ?, ?)"; + "INSERT OR IGNORE INTO Images (IName, reviewImage, validateImage, Source, SourceKey) VALUES (?, ?, ?, ?, ?)"; const results = await db.run(query, [ imageName, reviewImage, validateImage, + source, + sourceKey, ]); return results; diff --git a/routes/api/v2/s3Buckets.js b/routes/api/v2/s3Buckets.js index 525fa438..53b49d25 100644 --- a/routes/api/v2/s3Buckets.js +++ b/routes/api/v2/s3Buckets.js @@ -1,5 +1,6 @@ const fs = require("fs"); const path = require("path"); +const crypto = require("crypto"); const queries = require("../../../queries/queries"); const { buildS3Client, @@ -25,6 +26,18 @@ function sanitizeFileName(name) { .join("_"); } +// Two distinct S3 keys can share a basename (e.g. "2024/img.jpg" and +// "2025/img.jpg"), so a plain basename can't be assumed unique across a +// bucket. Disambiguate deterministically from the full key so re-running a +// sync never renames or duplicates a file that was already given a suffix. +function disambiguateFileName(fileName, key) { + const ext = path.extname(fileName); + const base = fileName.slice(0, fileName.length - ext.length); + const hash = crypto.createHash("sha1").update(key).digest("hex").slice(0, 8); + + return `${base}_${hash}${ext}`; +} + function trimOrUndefined(value) { return typeof value === "string" ? value.trim() : value; } @@ -167,24 +180,59 @@ async function syncS3Bucket(req, res) { }); const objectKeys = await listImageObjects(s3Client, bucket.BucketName, bucket.Prefix); - const existingImages = new Set(await global.readdirAsync(imagesPath)); + + // Snapshot of what's on disk *before* this run - unrelated to what this run + // itself assigns. A basename already present here (a prior sync from before + // SourceKey existed, or an unrelated local file) is treated as already covered, + // same as before. `assignedNames` starts from this snapshot and grows as this + // run hands out names, so it also catches two keys *from this same bucket + // listing* colliding with each other. + const preExistingImages = new Set(await global.readdirAsync(imagesPath)); + const assignedNames = new Set(preExistingImages); + + const existingImageRows = await queries.project.getAllImages(projectPath); + const existingSourceKeys = new Set( + (existingImageRows.rows || []) + .map((row) => row.SourceKey) + .filter(Boolean), + ); const syncedImages = []; let skippedCount = 0; for (const key of objectKeys) { - const fileName = sanitizeFileName(path.basename(key)); + // Already synced this exact object in a prior run - never re-download or + // re-disambiguate a key we've already assigned a name to. + if (existingSourceKeys.has(key)) { + skippedCount += 1; + continue; + } + + const baseName = sanitizeFileName(path.basename(key)); - if (!fileName || existingImages.has(fileName)) { + if (!baseName) { skippedCount += 1; continue; } + if (preExistingImages.has(baseName)) { + skippedCount += 1; + continue; + } + + // A different key from this same bucket listing already claimed this + // basename - these are genuinely distinct objects, so disambiguate instead + // of dropping this one. + const fileName = assignedNames.has(baseName) + ? disambiguateFileName(baseName, key) + : baseName; + const destPath = path.join(imagesPath, fileName); await downloadObjectToFile(s3Client, bucket.BucketName, key, destPath); - await queries.project.addImages(projectPath, fileName, 0, 0); + await queries.project.addImages(projectPath, fileName, 0, 0, "s3", key); - existingImages.add(fileName); + assignedNames.add(fileName); + existingSourceKeys.add(key); syncedImages.push(fileName); } diff --git a/tests/integration/imageSourceColumn.test.js b/tests/integration/imageSourceColumn.test.js index 9e2f4fde..cd0aebc6 100644 --- a/tests/integration/imageSourceColumn.test.js +++ b/tests/integration/imageSourceColumn.test.js @@ -1,15 +1,17 @@ // Unit tests for queries/projects/projects.js#migrateProjectDb, covering the nullable -// Images.Source column added so an image's row can record where its bytes live (e.g. -// local disk vs. an S3 bucket) once storage backends other than local disk are supported. -// Project databases created before this column existed only get it via the PRAGMA-guarded -// ALTER TABLE below, since CREATE TABLE IF NOT EXISTS is a no-op on an existing table. +// Images.Source/SourceKey columns added so an image's row can record where its bytes +// live (e.g. local disk vs. an S3 bucket) and, for S3-backed images, the literal object +// key (decoupled from IName, since two different keys can sanitize to the same display +// name and need disambiguating - see s3Buckets.test.js). Project databases created +// before these columns existed only get them via the PRAGMA-guarded ALTER TABLE below, +// since CREATE TABLE IF NOT EXISTS is a no-op on an existing table. jest.mock('../../queries/getDbClient'); const getDbClient = require('../../queries/getDbClient'); const projects = require('../../queries/projects/projects'); -describe('queries/projects migrateProjectDb Source column backfill', () => { +describe('queries/projects migrateProjectDb Source/SourceKey column backfill', () => { let mockRun; let mockAll; @@ -20,7 +22,7 @@ describe('queries/projects migrateProjectDb Source column backfill', () => { getDbClient.mockReturnValue({ run: mockRun, all: mockAll }); }); - it('adds the Source column when an existing Images table predates it', async () => { + it('adds both columns when an existing Images table predates them', async () => { mockAll.mockResolvedValue({ success: true, rows: [ @@ -36,9 +38,12 @@ describe('queries/projects migrateProjectDb Source column backfill', () => { expect(mockRun).toHaveBeenCalledWith( 'ALTER TABLE Images ADD COLUMN Source VARCHAR DEFAULT NULL', ); + expect(mockRun).toHaveBeenCalledWith( + 'ALTER TABLE Images ADD COLUMN SourceKey VARCHAR DEFAULT NULL', + ); }); - it('does not re-add the Source column when it is already present', async () => { + it('only adds the column that is missing when one already exists', async () => { mockAll.mockResolvedValue({ success: true, rows: [ @@ -54,5 +59,62 @@ describe('queries/projects migrateProjectDb Source column backfill', () => { expect(mockRun).not.toHaveBeenCalledWith( 'ALTER TABLE Images ADD COLUMN Source VARCHAR DEFAULT NULL', ); + expect(mockRun).toHaveBeenCalledWith( + 'ALTER TABLE Images ADD COLUMN SourceKey VARCHAR DEFAULT NULL', + ); + }); + + it('does not re-add either column when both are already present', async () => { + mockAll.mockResolvedValue({ + success: true, + rows: [ + { name: 'IName' }, + { name: 'reviewImage' }, + { name: 'validateImage' }, + { name: 'Source' }, + { name: 'SourceKey' }, + ], + }); + + await projects.project.migrateProjectDb('/projects/testuser-test-project'); + + expect(mockRun).not.toHaveBeenCalledWith( + expect.stringContaining('ALTER TABLE Images ADD COLUMN'), + ); + }); +}); + +describe('queries/projects addImages Source/SourceKey parameters', () => { + let mockRun; + + beforeEach(() => { + jest.clearAllMocks(); + mockRun = jest.fn().mockResolvedValue({ success: true, changes: 1, lastID: 1 }); + getDbClient.mockReturnValue({ run: mockRun }); + }); + + it('defaults Source and SourceKey to null for callers that do not pass them', async () => { + await projects.project.addImages('/projects/testuser-test-project', 'cat.jpg', 0, 0); + + expect(mockRun).toHaveBeenCalledWith( + expect.stringContaining('INSERT OR IGNORE INTO Images'), + ['cat.jpg', 0, 0, null, null], + ); + }); + + it('passes through Source and SourceKey when a caller (e.g. S3 sync) provides them', async () => { + await projects.project.addImages( + '/projects/testuser-test-project', + 'cat.jpg', + 0, + 0, + 's3', + 'images/cat.jpg', + ); + + expect(mockRun).toHaveBeenCalledWith( + expect.stringContaining('INSERT OR IGNORE INTO Images'), + ['cat.jpg', 0, 0, 's3', 'images/cat.jpg'], + ); }); }); diff --git a/tests/integration/s3Buckets.test.js b/tests/integration/s3Buckets.test.js index 94d7caab..fc4bb3b2 100644 --- a/tests/integration/s3Buckets.test.js +++ b/tests/integration/s3Buckets.test.js @@ -11,6 +11,7 @@ jest.mock('../../queries/queries', () => ({ }, project: { addImages: jest.fn().mockResolvedValue({ success: true }), + getAllImages: jest.fn().mockResolvedValue({ success: true, rows: [] }), }, })); @@ -231,10 +232,98 @@ describe('S3 Bucket Routes', () => { 'cat.jpg', 0, 0, + 's3', + 'images/cat.jpg', ); expect(queries.managed.touchBucketSyncedAt).toHaveBeenCalled(); }); + it('skips an object already synced by key, even if its name is no longer the only file on disk', async () => { + queries.managed.getBucket.mockResolvedValueOnce({ + row: { + BucketName: 'my-bucket', + Region: 'us-east-1', + Prefix: 'images/', + AccessKeyId: 'AKIA...', + SecretAccessKey: 'secret', + }, + }); + s3Client.listImageObjects.mockResolvedValueOnce(['images/cat.jpg']); + global.readdirAsync.mockResolvedValueOnce(['cat.jpg']); + queries.project.getAllImages.mockResolvedValueOnce({ + success: true, + rows: [{ IName: 'cat.jpg', Source: 's3', SourceKey: 'images/cat.jpg' }], + }); + + const res = await request(app) + .post('/api/v2/projects/testuser/test-project/s3-bucket/sync') + .set('Cookie', ['Username=testuser']); + + expect(res.statusCode).toBe(200); + expect(res.body.syncedCount).toBe(0); + expect(res.body.skippedCount).toBe(1); + expect(s3Client.downloadObjectToFile).not.toHaveBeenCalled(); + expect(queries.project.addImages).not.toHaveBeenCalled(); + }); + + it('disambiguates two different keys that sanitize to the same basename instead of dropping one', async () => { + queries.managed.getBucket.mockResolvedValueOnce({ + row: { + BucketName: 'my-bucket', + Region: 'us-east-1', + Prefix: 'images/', + AccessKeyId: 'AKIA...', + SecretAccessKey: 'secret', + }, + }); + s3Client.listImageObjects.mockResolvedValueOnce([ + '2024/img.jpg', + '2025/img.jpg', + ]); + global.readdirAsync.mockResolvedValueOnce([]); + queries.project.getAllImages.mockResolvedValueOnce({ success: true, rows: [] }); + + const res = await request(app) + .post('/api/v2/projects/testuser/test-project/s3-bucket/sync') + .set('Cookie', ['Username=testuser']); + + expect(res.statusCode).toBe(200); + expect(res.body.syncedCount).toBe(2); + expect(res.body.skippedCount).toBe(0); + expect(res.body.images[0]).toBe('img.jpg'); + expect(res.body.images[1]).not.toBe('img.jpg'); + expect(res.body.images[1]).toMatch(/^img_[0-9a-f]{8}\.jpg$/); + + expect(queries.project.addImages).toHaveBeenNthCalledWith( + 1, + expect.stringContaining('testuser-test-project'), + 'img.jpg', + 0, + 0, + 's3', + '2024/img.jpg', + ); + expect(queries.project.addImages).toHaveBeenNthCalledWith( + 2, + expect.stringContaining('testuser-test-project'), + res.body.images[1], + 0, + 0, + 's3', + '2025/img.jpg', + ); + + // Deterministic: re-running sync against the same key must reproduce the + // same disambiguated name, not draw a fresh suffix each time. + const crypto = require('crypto'); + const expectedHash = crypto + .createHash('sha1') + .update('2025/img.jpg') + .digest('hex') + .slice(0, 8); + expect(res.body.images[1]).toBe(`img_${expectedHash}.jpg`); + }); + it('returns 404 when the project has no attached bucket', async () => { queries.managed.getBucket.mockResolvedValueOnce({ row: undefined }); From a05d6ccb84a3054974e996ccf4aa53df10f7511c Mon Sep 17 00:00:00 2001 From: FerretCode Date: Tue, 18 Aug 2026 11:49:54 -0700 Subject: [PATCH 6/8] Downgrade SQLITE_READONLY to a warning during startup project migration migrateProjectDb now runs for every existing project at server boot (added alongside the Source/SourceKey columns), and every statement in it is DDL - even a no-op CREATE TABLE IF NOT EXISTS still requires SQLite to open the file for write. A read-only project database can never be migrated regardless of the reason it's read-only, so treat that specific, expected outcome as a warning instead of an error-level log repeated on every single server start. Co-authored-by: multica-agent --- server.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/server.js b/server.js index 37b0fbfa..0b213fb3 100644 --- a/server.js +++ b/server.js @@ -52,6 +52,20 @@ for (const project of fs.readdirSync(allProjectsPath)) { global.projectDbClients[projectPath] = new Client(dbFile); queries.project.migrateProjectDb(projectPath).catch((err) => { + // Every statement in migrateProjectDb is DDL (even a no-op + // CREATE TABLE IF NOT EXISTS), so SQLite opens the file for + // write regardless of whether a change is actually needed. + // A read-only project database - by design, or a permissions + // quirk of wherever it's deployed - can't be migrated, but + // that's expected and not a failure worth an error-level log + // on every server start. + if (err && err.error && err.error.code === "SQLITE_READONLY") { + global.logger.warn( + `Project database at ${projectPath} is read-only; skipping schema migration.`, + ); + return; + } + global.logger.error( `Failed to migrate project database at ${projectPath}: ${err}`, ); From 28be9501ca4483d3593bf26073c7c7ac2ae0474f Mon Sep 17 00:00:00 2001 From: FerretCode Date: Tue, 18 Aug 2026 12:57:56 -0700 Subject: [PATCH 7/8] Map container UID/GID to the host user in docker-compose Root-caused the SQLITE_READONLY errors on startup migration: the app image has no USER directive, so the container writes to the bind-mounted public/projects (and db, data) directories as root. Anything created while running via docker-compose then can't be written by a normal host user running the app outside Docker (`node .`), which is exactly what surfaced as SQLITE_READONLY for test-gcp-test/test-gcp-test3. Mapping the container to the host UID/GID prevents this going forward; already-affected directories still need a one-time chown. Co-authored-by: multica-agent --- docker-compose.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index b32c102f..220ce1ff 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -6,6 +6,14 @@ services: image: ghcr.io/${GITHUB_REPOSITORY:-njobvu-ai}/app:latest container_name: njobvu-ai restart: unless-stopped + # The image has no USER directive, so without this the container writes + # to the bind-mounted host directories below as root. Anything it + # creates (e.g. project databases/images) then can't be written by a + # normal host user running the app outside Docker (`node .`), failing + # with SQLITE_READONLY. Export UID/GID before `docker compose up` + # (most shells: `export UID GID` - `UID` is a shell built-in that isn't + # exported by default) so files it creates are owned by you instead. + user: "${UID:-1000}:${GID:-1000}" ports: - "3000:3000" environment: From d5f80b410709f93769527f004280a16b0dca070b Mon Sep 17 00:00:00 2001 From: FerretCode Date: Tue, 18 Aug 2026 12:58:08 -0700 Subject: [PATCH 8/8] Add a download-vs-stream choice for S3 bucket sync Adds a SyncMode ("download" | "stream") on S3Buckets, chosen via a new checkbox in the S3 project-creation form. "download" keeps today's behavior unchanged. "stream" registers each object as a project image without copying it to local disk, and adds an on-demand image proxy (GET /api/v2/projects/:admin/:projectName/images/:imageName) that serves a local file if present, or otherwise fetches the object live from S3 for that one request - never persisting it. getAnnotatePage (the per-image labeling/annotation view) now falls back to that proxy and probes the object's dimensions over the same live fetch (aborted early once the header is read, via probe-image-size's stream support) instead of 404ing when an image was never downloaded. Co-authored-by: multica-agent --- db/migrations.sql | 1 + queries/s3/s3.js | 11 +- routes/api.js | 2 + routes/api/v2/s3Buckets.js | 82 ++++++++- routes/pages/getAnnotatePage.js | 70 +++++--- tests/integration/s3Buckets.test.js | 149 +++++++++++++++++ tests/unit/getAnnotatePageS3Streaming.test.js | 155 ++++++++++++++++++ utils/s3Client.js | 16 ++ views/create.ejs | 13 +- 9 files changed, 472 insertions(+), 27 deletions(-) create mode 100644 tests/unit/getAnnotatePageS3Streaming.test.js diff --git a/db/migrations.sql b/db/migrations.sql index 864395ac..8a0431a5 100644 --- a/db/migrations.sql +++ b/db/migrations.sql @@ -3,3 +3,4 @@ INSERT INTO Users (Username, Password, FirstName, LastName, Email) VALUES ('Zero CREATE TABLE Projects (PName VARCHAR NOT NULL, PDescription VARCHAR NOT NULL, AutoSave INTEGER NOT NULL DEFAULT 0, Admin TEXT NOT NULL DEFAULT 'ZeroUser', Validate VARCHAR NOT NULL DEFAULT 0, FOREIGN KEY(Admin) REFERENCES Users(Username), PRIMARY KEY(PName, Admin)); CREATE TABLE Access (Username TEXT NOT NULL, PName VARCHAR NOT NULL, Admin TEXT NOT NULL, FOREIGN KEY(Username) REFERENCES Users(Username), FOREIGN KEY(PName) REFERENCES Projects(PName), FOREIGN KEY(Admin) REFERENCES Projects(Admin)); CREATE TABLE S3Buckets (PName VARCHAR NOT NULL, Admin TEXT NOT NULL, BucketName VARCHAR NOT NULL, Region VARCHAR NOT NULL, Prefix VARCHAR NOT NULL DEFAULT '', AccessKeyId VARCHAR, SecretAccessKey VARCHAR, LastSyncedAt TEXT, Endpoint TEXT, PRIMARY KEY(PName, Admin), FOREIGN KEY(PName) REFERENCES Projects(PName), FOREIGN KEY(Admin) REFERENCES Projects(Admin)); +ALTER TABLE S3Buckets ADD COLUMN SyncMode VARCHAR NOT NULL DEFAULT 'download'; diff --git a/queries/s3/s3.js b/queries/s3/s3.js index 5c5fe857..88d0e182 100644 --- a/queries/s3/s3.js +++ b/queries/s3/s3.js @@ -8,14 +8,16 @@ module.exports = { prefix, accessKeyId, secretAccessKey, - endpoint + endpoint, + syncMode = "download", ) { const query = - "INSERT INTO S3Buckets (PName, Admin, BucketName, Region, Prefix, AccessKeyId, SecretAccessKey, Endpoint) " + - "VALUES (?, ?, ?, ?, ?, ?, ?, ?) " + + "INSERT INTO S3Buckets (PName, Admin, BucketName, Region, Prefix, AccessKeyId, SecretAccessKey, Endpoint, SyncMode) " + + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) " + "ON CONFLICT(PName, Admin) DO UPDATE SET " + "BucketName = excluded.BucketName, Region = excluded.Region, Prefix = excluded.Prefix, " + - "AccessKeyId = excluded.AccessKeyId, SecretAccessKey = excluded.SecretAccessKey, Endpoint = excluded.Endpoint"; + "AccessKeyId = excluded.AccessKeyId, SecretAccessKey = excluded.SecretAccessKey, Endpoint = excluded.Endpoint, " + + "SyncMode = excluded.SyncMode"; const result = await global.managedDbClient.run(query, [ projectName, @@ -26,6 +28,7 @@ module.exports = { accessKeyId || null, secretAccessKey || null, endpoint || "", + syncMode, ]); return result; diff --git a/routes/api.js b/routes/api.js index 72f9bd42..31c3a221 100755 --- a/routes/api.js +++ b/routes/api.js @@ -71,6 +71,7 @@ const { getS3Bucket, deleteS3Bucket, syncS3Bucket, + getProjectImage, } = require("./api/v2/s3Buckets"); const updateLabels = require("./labelling/updateLabels"); @@ -188,6 +189,7 @@ api.post("/api/v2/projects/:admin/:projectName/s3-bucket", attachS3Bucket); api.get("/api/v2/projects/:admin/:projectName/s3-bucket", getS3Bucket); api.delete("/api/v2/projects/:admin/:projectName/s3-bucket", deleteS3Bucket); api.post("/api/v2/projects/:admin/:projectName/s3-bucket/sync", syncS3Bucket); +api.get("/api/v2/projects/:admin/:projectName/images/:imageName", getProjectImage); // LABELLING ROUTES api.post("/updateLabels", updateLabels); diff --git a/routes/api/v2/s3Buckets.js b/routes/api/v2/s3Buckets.js index 53b49d25..bc724f70 100644 --- a/routes/api/v2/s3Buckets.js +++ b/routes/api/v2/s3Buckets.js @@ -7,8 +7,11 @@ const { verifyBucketAccess, listImageObjects, downloadObjectToFile, + getObjectStream, } = require("../../../utils/s3Client"); +const SYNC_MODES = new Set(["download", "stream"]); + function getProjectPath(admin, projectName) { return path.join(currentPath, "public", "projects", `${admin}-${projectName}`); } @@ -51,6 +54,7 @@ async function attachS3Bucket(req, res) { const AccessKeyId = trimOrUndefined(body.AccessKeyId); const SecretAccessKey = trimOrUndefined(body.SecretAccessKey); const Endpoint = trimOrUndefined(body.Endpoint); + const SyncMode = SYNC_MODES.has(body.SyncMode) ? body.SyncMode : "download"; if (!isOwner(req, admin)) { return res.status(403).json({ success: false, error: "Not authorized for this project" }); @@ -83,7 +87,8 @@ async function attachS3Bucket(req, res) { Prefix || "", AccessKeyId, SecretAccessKey, - ...(Endpoint ? [Endpoint] : []), + Endpoint || "", + SyncMode, ); return res.status(200).json({ success: true }); @@ -123,6 +128,7 @@ async function getS3Bucket(req, res) { Prefix: row.Prefix, Endpoint: row.Endpoint, LastSyncedAt: row.LastSyncedAt, + SyncMode: row.SyncMode || "download", hasCredentials: !!row.AccessKeyId, }, }); @@ -227,8 +233,14 @@ async function syncS3Bucket(req, res) { ? disambiguateFileName(baseName, key) : baseName; - const destPath = path.join(imagesPath, fileName); - await downloadObjectToFile(s3Client, bucket.BucketName, key, destPath); + // "stream" mode registers the image (so it's browsable/labelable) without + // pulling its bytes to disk - GET .../images/:imageName fetches them from + // the bucket live, on the rare request that actually needs them. + if (bucket.SyncMode !== "stream") { + const destPath = path.join(imagesPath, fileName); + await downloadObjectToFile(s3Client, bucket.BucketName, key, destPath); + } + await queries.project.addImages(projectPath, fileName, 0, 0, "s3", key); assignedNames.add(fileName); @@ -254,9 +266,73 @@ async function syncS3Bucket(req, res) { } } +// Serves a single project image, transparently covering both storage modes: +// a locally-present file (the common case - local imports, or "download"-mode +// S3 sync) is sent straight from disk; an image registered from S3 that was +// never downloaded ("stream" mode) is fetched from the bucket live, for this +// request only, and never written to disk. Any logged-in user can view it - +// this mirrors the existing (also unauthenticated) static file serving for +// local images, rather than introducing a stricter, inconsistent check here. +async function getProjectImage(req, res) { + const { admin, projectName, imageName } = req.params; + + if (!req.cookies || !req.cookies.Username) { + return res.status(403).json({ success: false, error: "Not authorized" }); + } + + const projectPath = getProjectPath(admin, projectName); + const localPath = path.join(projectPath, "images", imageName); + + if (fs.existsSync(localPath)) { + return res.sendFile(localPath); + } + + try { + const imageResult = await queries.project.getImage(projectPath, imageName); + const image = imageResult && imageResult.row; + + if (!image || image.Source !== "s3" || !image.SourceKey) { + return res.status(404).json({ success: false, error: "Image not found" }); + } + + const bucketResult = await queries.managed.getBucket(projectName, admin); + const bucket = bucketResult && bucketResult.row; + + if (!bucket) { + return res.status(404).json({ success: false, error: "No S3 bucket attached to this project" }); + } + + const s3Client = buildS3Client({ + region: bucket.Region, + accessKeyId: bucket.AccessKeyId, + secretAccessKey: bucket.SecretAccessKey, + endpoint: bucket.Endpoint, + }); + + const { body, contentType } = await getObjectStream(s3Client, bucket.BucketName, image.SourceKey); + + res.setHeader("Content-Type", contentType || "application/octet-stream"); + body.on("error", (err) => { + global.logger.error(err); + if (!res.headersSent) { + res.status(500).end(); + } + }); + body.pipe(res); + } catch (err) { + global.logger.error(err, { + httpStatusCode: err.$metadata?.httpStatusCode, + code: err.Code || err.name, + requestId: err.$metadata?.requestId, + }); + return res.status(500).json({ success: false, error: "Error fetching image" }); + } +} + module.exports = { attachS3Bucket, getS3Bucket, deleteS3Bucket, syncS3Bucket, + getProjectImage, }; diff --git a/routes/pages/getAnnotatePage.js b/routes/pages/getAnnotatePage.js index 0bd35b6f..f043e512 100644 --- a/routes/pages/getAnnotatePage.js +++ b/routes/pages/getAnnotatePage.js @@ -1,3 +1,6 @@ +const queries = require("../../queries/queries"); +const { buildS3Client, getObjectStream } = require("../../utils/s3Client"); + async function getAnnotatePage(req, res) { var IDX = parseInt(req.query.IDX), IName = String(req.query.IName), @@ -129,30 +132,59 @@ async function getAnnotatePage(req, res) { } var abs_image_path = project_path + "/images/" + IName; - - if (!results4 || results4.length === 0 || !fs.existsSync(abs_image_path)) { + var imageExistsLocally = fs.existsSync(abs_image_path); + var imageRow = results4 && results4[0]; + + // A "download"-mode (or local-import) image is a real file at + // abs_image_path, same as always. A "stream"-mode S3 image never has + // one - it's only ever fetched live, on view, via the on-demand proxy + // below - so only 404 here if neither a local file nor an S3-backed row + // exists for this name. + if (!imageRow || (!imageExistsLocally && imageRow.Source !== "s3")) { res.render("404", { title: "404", user: req.cookies.Username, }); } else { - var rel_image_path = rel_project_path + "/images/" + results4[0].IName; - var img = fs.readFileSync( - project_path + "/images/" + results4[0].IName, - (err) => { - if (err) { - res.render("404", { - title: "404", - user: req.cookies.Username, - }); - } - }, - ), - img_data = probe.sync(img), - img_w = img_data.width, - img_h = img_data.height, - image_ratio = img_h / img_w, - image_width = img_w, + var rel_image_path; + var img_w, img_h; + + if (imageExistsLocally) { + rel_image_path = rel_project_path + "/images/" + imageRow.IName; + var img = fs.readFileSync(project_path + "/images/" + imageRow.IName); + var img_data = probe.sync(img); + img_w = img_data.width; + img_h = img_data.height; + } else { + // Point the browser at the on-demand proxy instead of a static + // path that doesn't exist. Probe just enough of the object's + // header (over that same live fetch, aborted by probe() once it + // has what it needs) to lay out the page, rather than pulling + // the whole image server-side just to measure it. + rel_image_path = `api/v2/projects/${admin}/${PName}/images/${imageRow.IName}`; + + try { + var bucketResult = await queries.managed.getBucket(PName, admin); + var bucket = bucketResult && bucketResult.row; + var s3Client = buildS3Client({ + region: bucket.Region, + accessKeyId: bucket.AccessKeyId, + secretAccessKey: bucket.SecretAccessKey, + endpoint: bucket.Endpoint, + }); + var objectStream = await getObjectStream(s3Client, bucket.BucketName, imageRow.SourceKey); + var probed = await probe(objectStream.body); + img_w = probed.width; + img_h = probed.height; + } catch (err) { + global.logger.error(err); + img_w = 0; + img_h = 0; + } + } + + var image_ratio = img_w ? img_h / img_w : 1, + image_width = img_w || 0, image_height = image_ratio * image_width, prev_IName = (next_IName = -1); var curr_index = 1; diff --git a/tests/integration/s3Buckets.test.js b/tests/integration/s3Buckets.test.js index fc4bb3b2..6ca28e93 100644 --- a/tests/integration/s3Buckets.test.js +++ b/tests/integration/s3Buckets.test.js @@ -12,6 +12,7 @@ jest.mock('../../queries/queries', () => ({ project: { addImages: jest.fn().mockResolvedValue({ success: true }), getAllImages: jest.fn().mockResolvedValue({ success: true, rows: [] }), + getImage: jest.fn(), }, })); @@ -20,8 +21,10 @@ jest.mock('../../utils/s3Client', () => ({ verifyBucketAccess: jest.fn().mockResolvedValue(undefined), listImageObjects: jest.fn().mockResolvedValue([]), downloadObjectToFile: jest.fn().mockResolvedValue(undefined), + getObjectStream: jest.fn(), })); +const { Readable } = require('stream'); const request = require('supertest'); const express = require('express'); const cookieParser = require('cookie-parser'); @@ -33,6 +36,7 @@ const { getS3Bucket, deleteS3Bucket, syncS3Bucket, + getProjectImage, } = require('../../routes/api/v2/s3Buckets'); // These handlers are exercised directly against a minimal app (rather than @@ -49,6 +53,17 @@ function buildTestApp() { app.get('/api/v2/projects/:admin/:projectName/s3-bucket', getS3Bucket); app.delete('/api/v2/projects/:admin/:projectName/s3-bucket', deleteS3Bucket); app.post('/api/v2/projects/:admin/:projectName/s3-bucket/sync', syncS3Bucket); + // `fs` is mocked module-wide down to just existsSync (below), so stub + // res.sendFile here rather than letting Express's real implementation + // reach for fs.stat/createReadStream, which don't exist on the mock. + app.get( + '/api/v2/projects/:admin/:projectName/images/:imageName', + (req, res, next) => { + res.sendFile = jest.fn((filePath) => res.status(200).send(`local-file:${filePath}`)); + next(); + }, + getProjectImage, + ); return app; } @@ -97,6 +112,30 @@ describe('S3 Bucket Routes', () => { 'images/', 'AKIA...', 'secret', + '', + 'download', + ); + }); + + it('defaults SyncMode to "download" when omitted or invalid, and passes through "stream" when requested', async () => { + await request(app) + .post('/api/v2/projects/testuser/test-project/s3-bucket') + .set('Cookie', ['Username=testuser']) + .send({ ...validBody, SyncMode: 'not-a-real-mode' }); + + expect(queries.managed.attachBucket).toHaveBeenNthCalledWith( + 1, + 'test-project', 'testuser', 'my-bucket', 'us-east-1', 'images/', 'AKIA...', 'secret', '', 'download', + ); + + await request(app) + .post('/api/v2/projects/testuser/test-project/s3-bucket') + .set('Cookie', ['Username=testuser']) + .send({ ...validBody, SyncMode: 'stream' }); + + expect(queries.managed.attachBucket).toHaveBeenNthCalledWith( + 2, + 'test-project', 'testuser', 'my-bucket', 'us-east-1', 'images/', 'AKIA...', 'secret', '', 'stream', ); }); @@ -155,6 +194,7 @@ describe('S3 Bucket Routes', () => { LastSyncedAt: null, AccessKeyId: 'AKIA...', SecretAccessKey: 'super-secret', + SyncMode: 'stream', }, }); @@ -164,11 +204,24 @@ describe('S3 Bucket Routes', () => { expect(res.statusCode).toBe(200); expect(res.body.bucket.BucketName).toBe('my-bucket'); + expect(res.body.bucket.SyncMode).toBe('stream'); expect(res.body.bucket.hasCredentials).toBe(true); expect(res.body.bucket.SecretAccessKey).toBeUndefined(); expect(res.body.bucket.AccessKeyId).toBeUndefined(); }); + it('defaults SyncMode to "download" for buckets attached before the column existed', async () => { + queries.managed.getBucket.mockResolvedValueOnce({ + row: { BucketName: 'my-bucket', Region: 'us-east-1', Prefix: '', SyncMode: null }, + }); + + const res = await request(app) + .get('/api/v2/projects/testuser/test-project/s3-bucket') + .set('Cookie', ['Username=testuser']); + + expect(res.body.bucket.SyncMode).toBe('download'); + }); + it('returns 404 when no bucket is attached', async () => { queries.managed.getBucket.mockResolvedValueOnce({ row: undefined }); @@ -345,5 +398,101 @@ describe('S3 Bucket Routes', () => { expect(res.statusCode).toBe(404); expect(queries.managed.getBucket).not.toHaveBeenCalled(); }); + + it('registers images without downloading them when SyncMode is "stream"', async () => { + queries.managed.getBucket.mockResolvedValueOnce({ + row: { + BucketName: 'my-bucket', + Region: 'us-east-1', + Prefix: 'images/', + AccessKeyId: 'AKIA...', + SecretAccessKey: 'secret', + SyncMode: 'stream', + }, + }); + s3Client.listImageObjects.mockResolvedValueOnce(['images/cat.jpg']); + global.readdirAsync.mockResolvedValueOnce([]); + + const res = await request(app) + .post('/api/v2/projects/testuser/test-project/s3-bucket/sync') + .set('Cookie', ['Username=testuser']); + + expect(res.statusCode).toBe(200); + expect(res.body.syncedCount).toBe(1); + expect(s3Client.downloadObjectToFile).not.toHaveBeenCalled(); + expect(queries.project.addImages).toHaveBeenCalledWith( + expect.stringContaining('testuser-test-project'), + 'cat.jpg', + 0, + 0, + 's3', + 'images/cat.jpg', + ); + }); + }); + + describe('GET /api/v2/projects/:admin/:projectName/images/:imageName', () => { + it('serves a locally-present file from disk without touching S3', async () => { + const res = await request(app) + .get('/api/v2/projects/testuser/test-project/images/cat.jpg') + .set('Cookie', ['Username=testuser']); + + expect(res.statusCode).toBe(200); + expect(res.text).toContain('local-file:'); + expect(res.text).toContain('cat.jpg'); + expect(s3Client.getObjectStream).not.toHaveBeenCalled(); + }); + + it('streams a "stream"-mode image live from S3 when no local file exists', async () => { + fs.existsSync.mockReturnValue(false); + queries.project.getImage.mockResolvedValueOnce({ + row: { IName: 'cat.jpg', Source: 's3', SourceKey: 'images/cat.jpg' }, + }); + queries.managed.getBucket.mockResolvedValueOnce({ + row: { + BucketName: 'my-bucket', + Region: 'us-east-1', + AccessKeyId: 'AKIA...', + SecretAccessKey: 'secret', + SyncMode: 'stream', + }, + }); + s3Client.getObjectStream.mockResolvedValueOnce({ + body: Readable.from([Buffer.from('fake-image-bytes')]), + contentType: 'image/jpeg', + }); + + const res = await request(app) + .get('/api/v2/projects/testuser/test-project/images/cat.jpg') + .set('Cookie', ['Username=testuser']); + + expect(res.statusCode).toBe(200); + expect(res.headers['content-type']).toBe('image/jpeg'); + expect(Buffer.from(res.body).toString()).toBe('fake-image-bytes'); + expect(s3Client.getObjectStream).toHaveBeenCalledWith( + { fakeClient: true }, + 'my-bucket', + 'images/cat.jpg', + ); + }); + + it('returns 404 when neither a local file nor an S3-backed row exists', async () => { + fs.existsSync.mockReturnValue(false); + queries.project.getImage.mockResolvedValueOnce({ row: undefined }); + + const res = await request(app) + .get('/api/v2/projects/testuser/test-project/images/missing.jpg') + .set('Cookie', ['Username=testuser']); + + expect(res.statusCode).toBe(404); + }); + + it('rejects requests from a logged-out client', async () => { + const res = await request(app).get( + '/api/v2/projects/testuser/test-project/images/cat.jpg', + ); + + expect(res.statusCode).toBe(403); + }); }); }); diff --git a/tests/unit/getAnnotatePageS3Streaming.test.js b/tests/unit/getAnnotatePageS3Streaming.test.js new file mode 100644 index 00000000..a1f3931f --- /dev/null +++ b/tests/unit/getAnnotatePageS3Streaming.test.js @@ -0,0 +1,155 @@ +// Unit tests for routes/pages/getAnnotatePage.js's S3 "stream" mode handling. +// +// Calls the handler directly (bypassing the full Express app/static +// middleware) since that stack is heavily mocked elsewhere for unrelated +// tests in a way that intercepts every route with an empty static response - +// exercising the real branching logic here needs a narrower harness. + +jest.mock('../../queries/queries', () => ({ + managed: { + getBucket: jest.fn(), + }, +})); + +jest.mock('../../utils/s3Client', () => ({ + buildS3Client: jest.fn(() => ({ fakeClient: true })), + getObjectStream: jest.fn(), +})); + +const { Readable } = require('stream'); +const queries = require('../../queries/queries'); +const s3Client = require('../../utils/s3Client'); +const getAnnotatePage = require('../../routes/pages/getAnnotatePage'); + +// Mirrors the real sqlite3 driver's callback style, since getAnnotatePage.js +// wraps `this.get`/`this.all` in its own Promise-returning getAsync/allAsync +// right after construction - only the raw callback methods are ever called. +function makeFakeProjectDb({ classesRows = [], labelsRows = [], imagesRows = [], displayRow }) { + return { + get: jest.fn((sql, cb) => { + if (sql.includes('display_id')) return cb(null, displayRow); + return cb(null, undefined); + }), + all: jest.fn((sql, cb) => { + if (sql.includes('Classes')) return cb(null, classesRows); + if (sql.includes('Labels')) return cb(null, labelsRows); + if (sql.includes('Images')) return cb(null, imagesRows); + return cb(null, []); + }), + each: jest.fn((sql, cb) => cb(null, undefined)), + close: jest.fn((cb) => cb && cb(null)), + }; +} + +describe('getAnnotatePage - S3-backed image serving', () => { + let res; + let req; + + beforeEach(() => { + jest.clearAllMocks(); + + global.logger = { debug: jest.fn(), error: jest.fn(), info: jest.fn() }; + global.currentPath = '/app/'; + global.colorsJSON = [{ value: '#FF0000' }]; + global.db = { + allAsync: jest.fn().mockResolvedValue([{ PName: 'test-project', Admin: 'testuser' }]), + getAsync: jest.fn().mockResolvedValue({ AutoSave: 1 }), + }; + + req = { + query: { IDX: '0', IName: 'image1.jpg', curr_class: 'class1' }, + cookies: { Username: 'testuser' }, + }; + res = { redirect: jest.fn(), render: jest.fn() }; + }); + + it('serves a "stream"-mode image with no local file via the on-demand S3 proxy, without touching disk', async () => { + const imageRow = { IName: 'image1.jpg', reviewImage: 0, Source: 's3', SourceKey: 'images/image1.jpg' }; + + global.fs = { existsSync: jest.fn().mockReturnValue(false), readFileSync: jest.fn() }; + global.sqlite3 = { + Database: jest.fn((dbPath, cb) => { + cb && cb(null); + return makeFakeProjectDb({ + imagesRows: [imageRow], + displayRow: { IName: 'image1.jpg', display_id: 1 }, + }); + }), + }; + global.probe = jest.fn().mockResolvedValue({ width: 400, height: 300 }); + global.probe.sync = jest.fn(); + + queries.managed.getBucket.mockResolvedValueOnce({ + row: { BucketName: 'my-bucket', Region: 'us-east-1', AccessKeyId: 'AKIA...', SecretAccessKey: 'secret' }, + }); + s3Client.getObjectStream.mockResolvedValueOnce({ + body: Readable.from([Buffer.from('fake-image-bytes')]), + contentType: 'image/jpeg', + }); + + await getAnnotatePage(req, res); + + expect(global.fs.readFileSync).not.toHaveBeenCalled(); + expect(global.probe.sync).not.toHaveBeenCalled(); + expect(s3Client.getObjectStream).toHaveBeenCalledWith( + { fakeClient: true }, + 'my-bucket', + 'images/image1.jpg', + ); + expect(res.render).toHaveBeenCalledWith('annotate', expect.objectContaining({ + image_path: 'api/v2/projects/testuser/test-project/images/image1.jpg', + image_width: 400, + image_height: 300, + })); + }); + + it('still reads a locally-present file straight from disk, unaffected by the S3 changes', async () => { + const imageRow = { IName: 'image1.jpg', reviewImage: 0, Source: null, SourceKey: null }; + + global.fs = { + existsSync: jest.fn().mockReturnValue(true), + readFileSync: jest.fn().mockReturnValue(Buffer.from('img-bytes')), + }; + global.sqlite3 = { + Database: jest.fn((dbPath, cb) => { + cb && cb(null); + return makeFakeProjectDb({ + imagesRows: [imageRow], + displayRow: { IName: 'image1.jpg', display_id: 1 }, + }); + }), + }; + global.probe = jest.fn(); + global.probe.sync = jest.fn().mockReturnValue({ width: 800, height: 600 }); + + await getAnnotatePage(req, res); + + expect(s3Client.getObjectStream).not.toHaveBeenCalled(); + expect(queries.managed.getBucket).not.toHaveBeenCalled(); + expect(res.render).toHaveBeenCalledWith('annotate', expect.objectContaining({ + image_path: 'projects/testuser-test-project/images/image1.jpg', + image_width: 800, + image_height: 600, + })); + }); + + it('renders 404 when there is no local file and the image is not S3-backed', async () => { + const imageRow = { IName: 'image1.jpg', reviewImage: 0, Source: null, SourceKey: null }; + + global.fs = { existsSync: jest.fn().mockReturnValue(false), readFileSync: jest.fn() }; + global.sqlite3 = { + Database: jest.fn((dbPath, cb) => { + cb && cb(null); + return makeFakeProjectDb({ + imagesRows: [imageRow], + displayRow: { IName: 'image1.jpg', display_id: 1 }, + }); + }), + }; + + await getAnnotatePage(req, res); + + expect(queries.managed.getBucket).not.toHaveBeenCalled(); + expect(res.render).toHaveBeenCalledWith('404', expect.any(Object)); + }); +}); diff --git a/utils/s3Client.js b/utils/s3Client.js index 93e90951..3668fe5e 100644 --- a/utils/s3Client.js +++ b/utils/s3Client.js @@ -131,9 +131,25 @@ async function downloadObjectToFile(s3Client, bucketName, key, destPath) { }); } +// For "stream" sync mode: fetch an object's bytes live for a single request +// instead of persisting them to disk. Caller is responsible for piping +// response.Body to the outgoing HTTP response. +async function getObjectStream(s3Client, bucketName, key) { + const response = await s3Client.send( + new GetObjectCommand({ Bucket: bucketName, Key: key }), + ); + + return { + body: response.Body, + contentType: response.ContentType, + contentLength: response.ContentLength, + }; +} + module.exports = { buildS3Client, verifyBucketAccess, listImageObjects, downloadObjectToFile, + getObjectStream, }; diff --git a/views/create.ejs b/views/create.ejs index 7562b8bc..e5d890b5 100644 --- a/views/create.ejs +++ b/views/create.ejs @@ -184,7 +184,7 @@
-

Add Project From S3 Bucket ?

+

Add Project From S3 Bucket ?

@@ -224,6 +224,15 @@

Leave the Access Key fields blank to use the server's default AWS credentials/role.

+
+
+ + +
+

@@ -289,6 +298,7 @@ const accessKeyId = document.getElementById('s3_access_key_id').value; const secretAccessKey = document.getElementById('s3_secret_access_key').value; const endpoint = document.getElementById("s3_endpoint").value; + const syncMode = document.getElementById("s3_sync_mode").value; if (!bucketName || !region) { alert("Bucket Name and Region are required."); @@ -327,6 +337,7 @@ AccessKeyId: accessKeyId, SecretAccessKey: secretAccessKey, Endpoint: endpoint, + SyncMode: syncMode, }), });