Skip to content

Replace Bad Instance of databaseTiny.find() #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thehabes merged 2 commits into from
Mar 12, 2026
Merged

Replace Bad Instance of databaseTiny.find() #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c259f72
Fix smoke test for development. Fix issue with bad usage of database…
thehabes Mar 12, 2026
47ba68a
Changes during review
thehabes Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions __tests__/smoke.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ async function runSmokeChecks() {
if (!(hasHeading || hasWelcome)) throw new Error('Expected TPEN3 Services index HTML not found in response')
}))

// Protected endpoint requires authentication (400 when Authorization header is missing)
// Protected endpoint requires authentication (401 when Authorization header is missing)
checks.push(await runCheck('Protected endpoint requires authentication', async () => {
const res = await request('/my/profile')
if (res.status !== 400) throw new Error(`Expected 400, got ${res.status}`)
if (res.status !== 401) throw new Error(`Expected 401, got ${res.status}`)
}))

// CORS headers present for allowed origin
Expand Down
25 changes: 23 additions & 2 deletions utilities/shared.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,29 @@ export const updateLayerAndProject = async (layer, project, userId) => {
const pageOverwrites = updatedLayer.pages
.filter(page => page.id.startsWith(process.env.RERUMIDPREFIX))
.map(async page => {
const oldPage = await databaseTiny.find({_id: page.id.split("/").pop()})
if(!oldPage) throw new Error(`Page with ID ${page.id} not found in RERUM`)
const oldPage = await fetch(page.id).then(async (resp) => {
if (resp.ok) return resp.json()
Comment on lines 84 to +88
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new RERUM page overwrite path (fetching the page by URL, handling non-OK responses/network failures, then overwriting with updated partOf) isn’t covered by tests. Since utilities/shared.js already has unit tests, add targeted unit tests for updateLayerAndProject() that mock fetch() and databaseTiny.overwrite() to cover success, 404/non-OK responses, and network error handling.

Copilot uses AI. Check for mistakes.
let rerumErrorMessage
try {
rerumErrorMessage = `${resp.status ?? 500}: ${page.id} - ${await resp.text()}`
} catch (e) {
rerumErrorMessage = `500: ${page.id} - A RERUM error occurred`
}
const err = new Error(rerumErrorMessage)
err.status = 502
throw err
})
.catch(err => {
if (err.status === 502) throw err
const genericRerumNetworkError = new Error(`500: ${page.id} - A RERUM error occurred`)
genericRerumNetworkError.status = 502
throw genericRerumNetworkError
})
if (!(oldPage?.id || oldPage?.["@id"])) {
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oldPage is assumed to be a valid RERUM document object, but unlike similar RERUM fetch patterns elsewhere (e.g., Page/Layer/Line loaders), this code doesn’t validate the fetched JSON shape (e.g., presence of id/@id) before mutating and overwriting. If RERUM returns a 200 with unexpected/garbled JSON, this can silently overwrite bad data or throw when setting partOf. Add a guard to verify the fetched payload is a document with an identifier (and fail with a 502) before calling databaseTiny.overwrite().

Suggested change
if (!(oldPage?.id || oldPage?.["@id"])) {
if (
!oldPage ||
typeof oldPage !== "object" ||
Array.isArray(oldPage) ||
!(oldPage.id || oldPage["@id"])
) {

Copilot uses AI. Check for mistakes.
const err = new Error(`500: ${page.id} - A RERUM error occurred`)
err.status = 502
throw err
}
oldPage.partOf = [{ id: updatedLayer.id, type: "AnnotationCollection" }]
return databaseTiny.overwrite(oldPage)
})
Expand Down
Loading