From f96ced6ea18c1a6f2796ee1c26866274c497a720 Mon Sep 17 00:00:00 2001 From: Olasubomi Adekunle Date: Sun, 11 Oct 2026 17:54:16 -0500 Subject: [PATCH 1/2] Keep viewer content usable while scenes are migrated (#469) --- boneset-api/data/bones/axis.json | 1 - .../data/subbones/axis_body_(dens).json | 1 - boneset-api/images.js | 2 +- boneset-api/legacyImport.js | 2 +- boneset-api/migrationValidation.test.js | 7 +- boneset-api/scenes.js | 11 + boneset-api/server.js | 7 +- boneset-api/viewerTransition.test.js | 164 ++++++++++++ templates/boneset.html | 2 + templates/js/description.js | 7 + templates/js/dropdowns.js | 80 +++++- templates/js/sceneLinks.js | 97 +++++++ templates/js/scenes.js | 47 ++++ templates/style.css | 67 +++++ templates/tests/viewerTransition.test.js | 244 ++++++++++++++++++ 15 files changed, 722 insertions(+), 17 deletions(-) delete mode 100644 boneset-api/data/subbones/axis_body_(dens).json create mode 100644 boneset-api/viewerTransition.test.js create mode 100644 templates/js/sceneLinks.js create mode 100644 templates/tests/viewerTransition.test.js diff --git a/boneset-api/data/bones/axis.json b/boneset-api/data/bones/axis.json index 650641bd..3a3ab7a1 100644 --- a/boneset-api/data/bones/axis.json +++ b/boneset-api/data/bones/axis.json @@ -3,7 +3,6 @@ "id": "axis", "subBones": [ "axis_body", - "axis_body_(dens)", "axis_transverse_process", "axis_transverse_foramen", "axis_pedicle", diff --git a/boneset-api/data/subbones/axis_body_(dens).json b/boneset-api/data/subbones/axis_body_(dens).json deleted file mode 100644 index fd6b6925..00000000 --- a/boneset-api/data/subbones/axis_body_(dens).json +++ /dev/null @@ -1 +0,0 @@ -{"id": "axis_body_(dens)", "name": "Axis Body (Dens)"} \ No newline at end of file diff --git a/boneset-api/images.js b/boneset-api/images.js index 4aab5b2f..b6a21818 100644 --- a/boneset-api/images.js +++ b/boneset-api/images.js @@ -13,7 +13,7 @@ const path = require("path"); const IMAGE_EXTENSIONS = new Set([".jpg", ".jpeg", ".png"]); const DESCRIPTION_SUFFIX = /_description\.json$/i; -const BONE_ID_PATTERN = /^[a-z0-9_]+$/i; +const BONE_ID_PATTERN = /^[a-z0-9_-]+$/i; // hyphens allowed for 2nd-5th_metatarsals (Issue #469) const HEADER_BYTES = 256 * 1024; function imageUrl(filename) { diff --git a/boneset-api/legacyImport.js b/boneset-api/legacyImport.js index d9aec8d7..20e88b90 100644 --- a/boneset-api/legacyImport.js +++ b/boneset-api/legacyImport.js @@ -10,7 +10,7 @@ const fs = require("fs").promises; const path = require("path"); const { getImageDimensions, imageUrl } = require("./images"); -const BONE_ID_PATTERN = /^[a-z0-9_]+$/i; +const BONE_ID_PATTERN = /^[a-z0-9_-]+$/i; // hyphens allowed for 2nd-5th_metatarsals (Issue #469) const MAX_IMAGE_SIZE = 480; // longest side of an imported image, in scene units const IMAGE_GAP = 40; // space between imported images, and between import rows diff --git a/boneset-api/migrationValidation.test.js b/boneset-api/migrationValidation.test.js index 682f4ea5..1a25a744 100644 --- a/boneset-api/migrationValidation.test.js +++ b/boneset-api/migrationValidation.test.js @@ -9,6 +9,7 @@ const { buildLegacyAnnotations } = require("./legacyAnnotations"); const { listItems, migrate } = require("./scripts/migration-report"); const DATA_DIR = path.join(__dirname, "data"); +const LONG_TEST = 30000; // going through every item can take longer than Jest's default 5 seconds const items = listItems(); function readJson(file) { @@ -37,7 +38,7 @@ describe("Migrated scene validation - Issue 468", () => { const row = await migrate(item); if (row.status !== "imported") expect(row.note.length).toBeGreaterThan(0); } - }); + }, LONG_TEST); describe("every imported item", () => { let imported; @@ -47,7 +48,7 @@ describe("Migrated scene validation - Issue 468", () => { const result = await importItem(item.id).catch(() => null); if (result) imported.push({ id: item.id, ...result }); } - }); + }, LONG_TEST); it("keeps the viewer's image order", () => { for (const { id, images } of imported) { @@ -120,4 +121,4 @@ describe("Migrated scene validation - Issue 468", () => { } }); }); -}); \ No newline at end of file +}); diff --git a/boneset-api/scenes.js b/boneset-api/scenes.js index 8b4812af..626ae07c 100644 --- a/boneset-api/scenes.js +++ b/boneset-api/scenes.js @@ -162,12 +162,23 @@ function applyImagePatch(existingImage, fields) { return { image: merged }; } +// The bone viewer items a scene's images were imported from (Issue #469), so the +// viewer can link an item to the scenes made from it. +function legacySources(scene) { + const boneIds = (scene.images || []) + .filter((image) => image && image.source && image.source.type === "legacy-image") + .map((image) => image.source.boneId) + .filter((boneId) => typeof boneId === "string"); + return [...new Set(boneIds)]; +} + function toSummary(scene) { return { id: scene.id, name: scene.name, imageCount: (scene.images || []).length, annotationCount: (scene.annotations || []).length, + sources: legacySources(scene), createdAt: scene.createdAt, updatedAt: scene.updatedAt, }; diff --git a/boneset-api/server.js b/boneset-api/server.js index aa83b0aa..9acd5e33 100644 --- a/boneset-api/server.js +++ b/boneset-api/server.js @@ -60,9 +60,10 @@ function isValidBoneId(boneId) { return false; } - // Only allow alphanumeric characters and underscores + // Only allow letters, numbers, underscores, and hyphens. // This prevents path traversal and URL injection attacks - const validBoneIdPattern = /^[a-z0-9_]+$/i; + // (hyphens are needed for ids like 2nd-5th_metatarsals, Issue #469). + const validBoneIdPattern = /^[a-z0-9_-]+$/i; return validBoneIdPattern.test(boneId) && boneId.length > 0 && boneId.length <= 100; } @@ -304,7 +305,7 @@ app.get("/api/bone-data/", async (req, res) => { } if (!isValidBoneId(boneId)) { - return res.status(400).json({ error: "Invalid boneId format. Only alphanumeric characters and underscores are allowed." }); + return res.status(400).json({ error: "Invalid boneId format. Only letters, numbers, underscores, and hyphens are allowed." }); } const descriptionPath = path.join(DESCRIPTIONS_DIR, `${boneId}_description.json`); diff --git a/boneset-api/viewerTransition.test.js b/boneset-api/viewerTransition.test.js new file mode 100644 index 00000000..a53332e4 --- /dev/null +++ b/boneset-api/viewerTransition.test.js @@ -0,0 +1,164 @@ +// Tests for keeping the bone viewer usable while content is moved into scenes. +// Issue #469 (parent #463). +const fs = require("fs"); +const os = require("os"); +const path = require("path"); +const request = require("supertest"); + +// Keep scenes made by these tests out of boneset-api/data/scenes. +// This has to be set before server.js is loaded. +const scenesDir = fs.mkdtempSync(path.join(os.tmpdir(), "bonebox-transition-test-")); +process.env.SCENES_DIR = scenesDir; + +const { app, serverReady } = require("./server"); + +const IMAGES_DIR = path.join(__dirname, "data", "images"); +const LONG_TEST = 30000; // tests that go through every item need more than Jest's default 5 seconds + +// Every boneset, bone, and bone part in the viewer's dropdowns. +let items = []; + +beforeAll(async () => { + await serverReady; + const response = await request(app).get("/combined-data"); + items = [ + ...response.body.bonesets.map((set) => set.id), + ...response.body.bones.map((bone) => bone.id), + ...response.body.subbones.map((subbone) => subbone.id), + ]; +}); + +afterAll(() => { + fs.rmSync(scenesDir, { recursive: true, force: true }); +}); + +const createScene = async (name) => (await request(app).post("/api/scenes").send({ name })).body; +const importInto = (sceneId, boneId) => + request(app).post(`/api/scenes/${sceneId}/import-legacy`).send({ boneId, annotations: { labels: true, regions: true } }); + +// Everything the viewer loads for one item, so we can check it doesn't change. +async function viewerContent(boneId) { + const boneData = await request(app).get(`/api/bone-data/?boneId=${boneId}`); + const description = await request(app).get(`/api/description/?boneId=${boneId}`); + const regions = await request(app).get(`/api/colored-regions?boneId=${boneId}`); + return { + boneData: [boneData.statusCode, boneData.body], + description: [description.statusCode, description.text], + regions: [regions.statusCode, regions.body], + }; +} + +describe("Non-migrated content stays viewable - Issue 469", () => { + it("checks all the items in the dropdowns", () => { + expect(items.length).toBeGreaterThan(150); + }); + + it("gives every item an id the API accepts", async () => { + const rejected = []; + for (const id of items) { + const response = await request(app).get(`/api/bone-data/?boneId=${encodeURIComponent(id)}`); + if (response.statusCode === 400) rejected.push(id); + } + expect(rejected).toEqual([]); + }, LONG_TEST); + + it("never gives a server error for any item", async () => { + for (const id of items) { + const boneData = await request(app).get(`/api/bone-data/?boneId=${encodeURIComponent(id)}`); + // 200 = has content, 404 = no content yet (the viewer shows a message for that) + expect([200, 404]).toContain(boneData.statusCode); + + const description = await request(app).get(`/api/description/?boneId=${encodeURIComponent(id)}`); + expect(description.statusCode).toBe(200); + expect(description.text).not.toContain("Invalid bone ID"); + } + }, LONG_TEST); + + it("has a file for every image the viewer lists", async () => { + const missing = []; + for (const id of items) { + const response = await request(app).get(`/api/bone-data/?boneId=${encodeURIComponent(id)}`); + if (response.statusCode !== 200) continue; + for (const image of response.body.images) { + if (!fs.existsSync(path.join(IMAGES_DIR, image.filename))) missing.push(`${id}: ${image.filename}`); + } + } + expect(missing).toEqual([]); + }, LONG_TEST); + + it("no longer lists the leftover axis_body_(dens) item", () => { + expect(items).not.toContain("axis_body_(dens)"); + expect(items).toContain("axis_body"); + }); + + it("can open 2nd-5th metatarsals now that ids can have hyphens", async () => { + const boneData = await request(app).get("/api/bone-data/?boneId=2nd-5th_metatarsals"); + expect(boneData.statusCode).toBe(200); + expect(boneData.body.images.length).toBeGreaterThan(0); + + const regions = await request(app).get("/api/colored-regions?boneId=2nd-5th_metatarsals"); + expect(regions.statusCode).toBe(200); + }); + + it("still rejects ids that could reach other folders", async () => { + expect((await request(app).get("/api/bone-data/?boneId=../server")).statusCode).toBe(400); + expect((await request(app).get("/api/bone-data/?boneId=a/b")).statusCode).toBe(400); + expect((await request(app).get("/api/colored-regions?boneId=..%2Fserver")).statusCode).toBe(400); + }); +}); + +describe("Migrated and non-migrated content side by side - Issue 469", () => { + it("lists which viewer items each scene was made from", async () => { + const scene = await createScene("Pelvis and Femur"); + expect((await importInto(scene.id, "ilium")).statusCode).toBe(200); + expect((await importInto(scene.id, "femur")).statusCode).toBe(200); + + const list = await request(app).get("/api/scenes"); + const summary = list.body.scenes.find((s) => s.id === scene.id); + expect(summary.sources).toEqual(["ilium", "femur"]); + }); + + it("lists no sources for a scene without imported bone images", async () => { + const scene = await createScene("Empty Scene"); + const list = await request(app).get("/api/scenes"); + expect(list.body.scenes.find((s) => s.id === scene.id).sources).toEqual([]); + }); + + it("doesn't change what the viewer shows for an item after it is migrated", async () => { + const before = await viewerContent("iliac_crest"); + + const scene = await createScene("Iliac Crest Scene"); + expect((await importInto(scene.id, "iliac_crest")).statusCode).toBe(200); + + expect(await viewerContent("iliac_crest")).toEqual(before); + }); + + it("works when moving between migrated and non-migrated items in one session", async () => { + const scene = await createScene("Atlas Scene"); + expect((await importInto(scene.id, "atlas")).statusCode).toBe(200); + const atlasBefore = await viewerContent("atlas"); + + // 1. A migrated item in the viewer + expect(atlasBefore.boneData[0]).toBe(200); + + // 2. The scene made from it, with every image loading + const opened = await request(app).get(`/api/scenes/${scene.id}`); + expect(opened.statusCode).toBe(200); + expect(opened.body.images.length).toBeGreaterThan(0); + for (const image of opened.body.images) { + expect(image.source.boneId).toBe("atlas"); + const file = await request(app).get(image.src); + expect(file.statusCode).toBe(200); + } + + // 3. An item nobody has migrated + const scenes = (await request(app).get("/api/scenes")).body.scenes; + expect(scenes.some((s) => s.sources.includes("humerus_head"))).toBe(false); + const humerusHead = await viewerContent("humerus_head"); + expect(humerusHead.boneData[0]).toBe(200); + expect(humerusHead.boneData[1].images.length).toBeGreaterThan(0); + + // 4. Back to the migrated item, which looks the same as before + expect(await viewerContent("atlas")).toEqual(atlasBefore); + }); +}); diff --git a/templates/boneset.html b/templates/boneset.html index dc840548..62260324 100644 --- a/templates/boneset.html +++ b/templates/boneset.html @@ -140,6 +140,7 @@

Visual Reference

+ @@ -205,6 +206,7 @@

Workspace

+