Skip to content

Commit 09b227f

Browse files
cubapthehabes
andauthored
251 memory reins (#262)
* Add cloneObject helper and replace JSON clones Introduce cloneObject in utils.js that uses globalThis.structuredClone when available and falls back to JSON.parse(JSON.stringify(...)). Replace direct JSON cloning in configureRerumOptions for configuredObject and received_options with cloneObject, and export cloneObject. This centralizes cloning logic and allows better deep-clone behavior when structuredClone is supported. * Add pagination helpers and use cloneObject Introduce MAX_QUERY_LIMIT and MAX_QUERY_SKIP and add clampNonNegativeInt and getPagination to safely parse and clamp limit/skip query params (with sensible defaults and env overrides). Replace ad-hoc deep copies (JSON.parse(JSON.stringify(...))) with utils.cloneObject in idNegotiation and getAllVersions to avoid issues with prototype loss and improve clarity. Export getPagination for reuse. * Use utils.cloneObject and getPagination Replace repeated JSON.parse(JSON.stringify(...)) deep-clones with utils.cloneObject across controllers (crud, patchSet, patchUnset, patchUpdate, putUpdate, gog) and add/get getPagination to standardize parsing of limit/skip in search, gog and crud. Adjust imports accordingly to centralize cloning and pagination logic, improving readability and consistency. * Update history.js * Use structuredClone for deep cloning Replace numerous JSON.parse(JSON.stringify(...)) and utils.cloneObject calls with native structuredClone across controllers and utils for safer, more reliable deep copies. Remove the now-unused cloneObject helper from utils.js and update configureRerumOptions to use structuredClone. Also tweak clampNonNegativeInt to treat non-positive values (<= 0) as fallback, switch history HEAD handling to res.status(200).end(), and add safeBody usage in delete to avoid re-parsing request body. * better head check * Convert tests to todos, fix router & delete.js Replace many end-to-end route tests with it.todo placeholders to temporarily stub numerous route test cases (files under routes/__tests__ and __tests__/routes_mounted.test.js). Update routes_mounted.test.js to safely access Express router stack via app._router?.stack ?? [] for compatibility. Resolve a merge/conflict in controllers/delete.js by removing logic that attempted to read an id from the request body and enforce that DELETE requires the id in the URL (returns a 400 if missing). * Update package.json * activating tests * Convert route mount TODOs to real supertest checks Replace placeholder todos with active integration tests that probe mounted routes via supertest. Removed the unused api_routes import and the routeExists helper; tests now perform actual HTTP requests against app for top-level paths (/v1, /client/register, /v1/id/:id, /v1/since/:id, /v1/history/:id) and for /v1/api/* endpoints using appropriate methods and content types, asserting endpoints are mounted (not 404) or return expected 404 for unknown IDs. This validates routing mounts across multiple HTTP verbs instead of relying on stack inspection. * changes during review --------- Co-authored-by: Bryan Haberberger <bryan.j.haberberger@slu.edu>
1 parent f5e72c0 commit 09b227f

31 files changed

Lines changed: 585 additions & 328 deletions

__tests__/routes_mounted.test.js

Lines changed: 91 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -6,111 +6,139 @@
66
*/
77

88
import request from "supertest"
9-
import api_routes from "../routes/api-routes.js"
109
import app from "../app.js"
1110
import fs from "fs"
1211

13-
let app_stack = app.router.stack
14-
let api_stack = api_routes.stack
15-
16-
/**
17-
* Check if a route exists in the Express app
18-
* @param {Array} stack - The router stack to search
19-
* @param {string} testPath - The path to test for
20-
* @returns {boolean} - True if the route exists
21-
*/
22-
function routeExists(stack, testPath) {
23-
for (const layer of stack) {
24-
// Check if layer has matchers (Express 5)
25-
if (layer.matchers && layer.matchers.length > 0) {
26-
const matcher = layer.matchers[0]
27-
const match = matcher(testPath)
28-
if (match && match.path) return true
29-
}
30-
// Also check route.path directly if it exists
31-
if (layer.route && layer.route.path) {
32-
if (layer.route.path === testPath || layer.route.path.includes(testPath)) return true
33-
}
34-
}
35-
return false
36-
}
37-
3812
describe('Check to see that all expected top level route patterns exist.', () => {
3913

40-
it('/v1 -- mounted ', () => {
41-
expect(routeExists(app_stack, '/v1')).toBe(true)
14+
it('/v1 -- mounted ', async () => {
15+
const response = await request(app).get('/v1')
16+
expect(response.statusCode).not.toBe(404)
4217
})
4318

44-
it('/client -- mounted ', () => {
45-
expect(routeExists(app_stack, '/client')).toBe(true)
19+
it('/client -- mounted ', async () => {
20+
const response = await request(app).get('/client/register')
21+
expect(response.statusCode).not.toBe(404)
4622
})
4723

48-
it('/v1/id/{_id} -- mounted', () => {
49-
expect(routeExists(api_stack, '/id')).toBe(true)
24+
it('/v1/id/{_id} -- mounted', async () => {
25+
const response = await request(app).get('/v1/id/test-mounted-id')
26+
// Mounted route with unknown id should 404 (not an unmapped endpoint 404)
27+
expect(response.statusCode).toBe(404)
5028
})
5129

52-
it('/v1/since/{_id} -- mounted', () => {
53-
expect(routeExists(api_stack, '/since')).toBe(true)
30+
it('/v1/since/{_id} -- mounted', async () => {
31+
const response = await request(app).get('/v1/since/test-mounted-id')
32+
// Mounted route with unknown id should 404
33+
expect(response.statusCode).toBe(404)
5434
})
5535

56-
it('/v1/history/{_id} -- mounted', () => {
57-
expect(routeExists(api_stack, '/history')).toBe(true)
36+
it('/v1/history/{_id} -- mounted', async () => {
37+
const response = await request(app).get('/v1/history/test-mounted-id')
38+
// Mounted route with unknown id should 404
39+
expect(response.statusCode).toBe(404)
5840
})
5941

6042
})
6143

6244
describe('Check to see that all /v1/api/ route patterns exist.', () => {
6345

64-
it('/v1/api/query -- mounted ', () => {
65-
expect(routeExists(api_stack, '/api/query')).toBe(true)
46+
it('/v1/api/query -- mounted ', async () => {
47+
const response = await request(app)
48+
.post('/v1/api/query')
49+
.set('Content-Type', 'application/json')
50+
.send({ mounted: true })
51+
expect(response.statusCode).not.toBe(404)
6652
})
6753

68-
it('/v1/api/create -- mounted ', () => {
69-
expect(routeExists(api_stack, '/api/create')).toBe(true)
54+
it('/v1/api/create -- mounted ', async () => {
55+
const response = await request(app)
56+
.post('/v1/api/create')
57+
.set('Content-Type', 'application/json')
58+
.send({ mounted: true })
59+
expect(response.statusCode).not.toBe(404)
7060
})
7161

72-
it('/v1/api/bulkCreate -- mounted ', () => {
73-
expect(routeExists(api_stack, '/api/bulkCreate')).toBe(true)
62+
it('/v1/api/bulkCreate -- mounted ', async () => {
63+
const response = await request(app)
64+
.post('/v1/api/bulkCreate')
65+
.set('Content-Type', 'application/json')
66+
.send([{ mounted: true }])
67+
expect(response.statusCode).not.toBe(404)
7468
})
7569

76-
it('/v1/api/update -- mounted ', () => {
77-
expect(routeExists(api_stack, '/api/update')).toBe(true)
70+
it('/v1/api/update -- mounted ', async () => {
71+
const response = await request(app)
72+
.put('/v1/api/update')
73+
.set('Content-Type', 'application/json')
74+
.send({ mounted: true })
75+
expect(response.statusCode).not.toBe(404)
7876
})
7977

80-
it('/v1/api/bulkUpdate -- mounted ', () => {
81-
expect(routeExists(api_stack, '/api/bulkUpdate')).toBe(true)
78+
it('/v1/api/bulkUpdate -- mounted ', async () => {
79+
const response = await request(app)
80+
.put('/v1/api/bulkUpdate')
81+
.set('Content-Type', 'application/json')
82+
.send([{ mounted: true }])
83+
expect(response.statusCode).not.toBe(404)
8284
})
8385

84-
it('/v1/api/overwrite -- mounted ', () => {
85-
expect(routeExists(api_stack, '/api/overwrite')).toBe(true)
86+
it('/v1/api/overwrite -- mounted ', async () => {
87+
const response = await request(app)
88+
.post('/v1/api/overwrite')
89+
.set('Content-Type', 'application/json')
90+
.send({ mounted: true })
91+
expect(response.statusCode).not.toBe(404)
8692
})
8793

88-
it('/v1/api/patch -- mounted ', () => {
89-
expect(routeExists(api_stack, '/api/patch')).toBe(true)
94+
it('/v1/api/patch -- mounted ', async () => {
95+
const response = await request(app)
96+
.patch('/v1/api/patch')
97+
.set('Content-Type', 'application/json')
98+
.send({ mounted: true })
99+
expect(response.statusCode).not.toBe(404)
90100
})
91101

92-
it('/v1/api/set -- mounted ', () => {
93-
expect(routeExists(api_stack, '/api/set')).toBe(true)
102+
it('/v1/api/set -- mounted ', async () => {
103+
const response = await request(app)
104+
.patch('/v1/api/set')
105+
.set('Content-Type', 'application/json')
106+
.send({ mounted: true })
107+
expect(response.statusCode).not.toBe(404)
94108
})
95109

96-
it('/v1/api/unset -- mounted ', () => {
97-
expect(routeExists(api_stack, '/api/unset')).toBe(true)
110+
it('/v1/api/unset -- mounted ', async () => {
111+
const response = await request(app)
112+
.patch('/v1/api/unset')
113+
.set('Content-Type', 'application/json')
114+
.send({ mounted: true })
115+
expect(response.statusCode).not.toBe(404)
98116
})
99117

100-
it('/v1/api/delete/{id} -- mounted ', () => {
101-
expect(routeExists(api_stack, '/api/delete')).toBe(true)
118+
it('/v1/api/delete/{id} -- mounted ', async () => {
119+
const response = await request(app).delete('/v1/api/delete/test-mounted-id')
120+
expect(response.statusCode).not.toBe(404)
102121
})
103122

104-
it('/v1/api/release/{id} -- mounted ', () => {
105-
expect(routeExists(api_stack, '/api/release')).toBe(true)
123+
it('/v1/api/release/{id} -- mounted ', async () => {
124+
const response = await request(app).patch('/v1/api/release/test-mounted-id')
125+
expect(response.statusCode).not.toBe(404)
106126
})
107127

108-
it('/v1/api/search -- mounted ', () => {
109-
expect(routeExists(api_stack, '/api/search')).toBe(true)
128+
it('/v1/api/search -- mounted ', async () => {
129+
const response = await request(app)
130+
.post('/v1/api/search')
131+
.set('Content-Type', 'text/plain')
132+
.send('mounted search')
133+
expect(response.statusCode).not.toBe(404)
110134
})
111135

112-
it('/v1/api/search/phrase -- mounted ', () => {
113-
expect(routeExists(api_stack, '/api/search/phrase')).toBe(true)
136+
it('/v1/api/search/phrase -- mounted ', async () => {
137+
const response = await request(app)
138+
.post('/v1/api/search/phrase')
139+
.set('Content-Type', 'text/plain')
140+
.send('mounted phrase search')
141+
expect(response.statusCode).not.toBe(404)
114142
})
115143

116144
})
@@ -142,4 +170,4 @@ describe('Check to see that critical repo files are present', () => {
142170
expect(fs.existsSync(filePath+"jest.config.js")).toBeTruthy()
143171
expect(fs.existsSync(filePath+"package.json")).toBeTruthy()
144172
})
145-
})
173+
})

controllers/bulk.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ const bulkCreate = async function (req, res, next) {
3333
// Each item must be valid JSON, but can't be an array.
3434
if(Array.isArray(d) || typeof d !== "object") return d
3535
try {
36-
JSON.parse(JSON.stringify(d))
36+
// Validate that the entry is structured-cloneable; result is intentionally discarded.
37+
structuredClone(d)
3738
} catch (err) {
3839
return d
3940
}
@@ -120,7 +121,8 @@ const bulkUpdate = async function (req, res, next) {
120121
// Each item must be valid JSON, but can't be an array.
121122
if(Array.isArray(d) || typeof d !== "object") return d
122123
try {
123-
JSON.parse(JSON.stringify(d))
124+
// Validate that the entry is structured-cloneable; result is intentionally discarded.
125+
structuredClone(d)
124126
} catch (err) {
125127
return d
126128
}

controllers/crud.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*/
77
import { newID, isValidID, db } from '../database/index.js'
88
import utils from '../utils.js'
9-
import { _contextid, idNegotiation, generateSlugId, ObjectID, getAgentClaim, parseDocumentID } from './utils.js'
9+
import { _contextid, idNegotiation, getPagination, generateSlugId, ObjectID, getAgentClaim, parseDocumentID } from './utils.js'
1010

1111
/**
1212
* Create a new Linked Open Data object in RERUM v1.
@@ -37,7 +37,7 @@ const create = async function (req, res, next) {
3737
let generatorAgent = getAgentClaim(req, next)
3838
if (!generatorAgent) return
3939
let context = req.body["@context"] ? { "@context": req.body["@context"] } : {}
40-
let provided = JSON.parse(JSON.stringify(req.body))
40+
let provided = structuredClone(req.body)
4141
let rerumProp = { "__rerum": utils.configureRerumOptions(generatorAgent, provided, false, false)["__rerum"] }
4242
if(slug){
4343
rerumProp.__rerum.slug = slug
@@ -55,7 +55,7 @@ const create = async function (req, res, next) {
5555
let result = await db.insertOne(newObject)
5656
res.set(utils.configureWebAnnoHeadersFor(newObject))
5757
newObject = idNegotiation(newObject)
58-
newObject.new_obj_state = JSON.parse(JSON.stringify(newObject))
58+
newObject.new_obj_state = structuredClone(newObject)
5959
res.location(newObject[_contextid(newObject["@context"]) ? "id":"@id"])
6060
res.status(201)
6161
res.json(newObject)
@@ -74,8 +74,7 @@ const create = async function (req, res, next) {
7474
const query = async function (req, res, next) {
7575
res.set("Content-Type", "application/json; charset=utf-8")
7676
let props = req.body
77-
const limit = parseInt(req.query.limit ?? 100)
78-
const skip = parseInt(req.query.skip ?? 0)
77+
const { limit, skip } = getPagination(req.query, 100)
7978
if (!props || Object.keys(props).length === 0) {
8079
//Hey now, don't ask for everything...this can happen by accident. Don't allow it.
8180
let err = {

controllers/delete.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const deleteObj = async function(req, res, next) {
3333
return next(utils.createExpressError(error))
3434
}
3535
if (null !== originalObject) {
36-
let safe_original = JSON.parse(JSON.stringify(originalObject))
36+
let safe_original = structuredClone(originalObject)
3737
if (utils.isDeleted(safe_original)) {
3838
err = Object.assign(err, {
3939
message: `The object you are trying to delete is already deleted. ${err.message}`,
@@ -57,7 +57,7 @@ const deleteObj = async function(req, res, next) {
5757
}
5858
let preserveID = safe_original["@id"]
5959
let deletedFlag = {} //The __deleted flag is a JSONObject
60-
deletedFlag["object"] = JSON.parse(JSON.stringify(originalObject))
60+
deletedFlag["object"] = structuredClone(originalObject)
6161
deletedFlag["deletor"] = agentRequestingDelete
6262
deletedFlag["time"] = new Date(Date.now()).toISOString().replace("Z", "")
6363
let deletedObject = {
@@ -121,7 +121,7 @@ async function healHistoryTree(obj) {
121121
const nextIdForQuery = parseDocumentID(nextID)
122122
const objToUpdate = await db.findOne({"$or":[{"_id": nextIdForQuery}, {"__rerum.slug": nextIdForQuery}]})
123123
if (null !== objToUpdate) {
124-
let fixHistory = JSON.parse(JSON.stringify(objToUpdate))
124+
let fixHistory = structuredClone(objToUpdate)
125125
if (objToDeleteisRoot) {
126126
//This means this next object must become root.
127127
//Strictly, all history trees must have num(root) > 0.
@@ -154,7 +154,7 @@ async function healHistoryTree(obj) {
154154
let previousIdForQuery = parseDocumentID(previous_id)
155155
const objToUpdate2 = await db.findOne({"$or":[{"_id": previousIdForQuery}, {"__rerum.slug": previousIdForQuery}]})
156156
if (null !== objToUpdate2) {
157-
let fixHistory2 = JSON.parse(JSON.stringify(objToUpdate2))
157+
let fixHistory2 = structuredClone(objToUpdate2)
158158
let origNextArray = fixHistory2["__rerum"]["history"]["next"]
159159
let newNextArray = [...origNextArray]
160160
newNextArray = newNextArray.filter(id => id !== obj["@id"])
@@ -193,7 +193,7 @@ async function newTreePrime(obj) {
193193
// fail silently
194194
}
195195
for (const d of descendants) {
196-
let objWithUpdate = JSON.parse(JSON.stringify(d))
196+
let objWithUpdate = structuredClone(d)
197197
objWithUpdate["__rerum"]["history"]["prime"] = primeID
198198
let result = await db.replaceOne({ "_id": d["_id"] }, objWithUpdate)
199199
if (result.modifiedCount === 0) {

controllers/gog.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import { newID, isValidID, db } from '../database/index.js'
1010
import utils from '../utils.js'
11-
import { _contextid, ObjectID, getAgentClaim, parseDocumentID, idNegotiation } from './utils.js'
11+
import { _contextid, ObjectID, getAgentClaim, getPagination, parseDocumentID, idNegotiation } from './utils.js'
1212

1313
/**
1414
* THIS IS SPECIFICALLY FOR 'Gallery of Glosses'
@@ -27,8 +27,7 @@ const _gog_fragments_from_manuscript = async function (req, res, next) {
2727
if (!agent) return
2828
const agentID = agent.split("/").pop()
2929
const manID = req.body["ManuscriptWitness"]
30-
const limit = parseInt(req.query.limit ?? 50)
31-
const skip = parseInt(req.query.skip ?? 0)
30+
const { limit, skip } = getPagination(req.query, 50)
3231
let err = { message: `` }
3332
// This request can only be made my Gallery of Glosses production apps.
3433
if (agentID !== "61043ad4ffce846a83e700dd") {
@@ -159,8 +158,7 @@ const _gog_glosses_from_manuscript = async function (req, res, next) {
159158
if (!agent) return
160159
const agentID = agent.split("/").pop()
161160
const manID = req.body["ManuscriptWitness"]
162-
const limit = parseInt(req.query.limit ?? 50)
163-
const skip = parseInt(req.query.skip ?? 0)
161+
const { limit, skip } = getPagination(req.query, 50)
164162
let err = { message: `` }
165163
// This request can only be made my Gallery of Glosses production apps.
166164
if (agentID !== "61043ad4ffce846a83e700dd") {
@@ -389,7 +387,7 @@ const expand = async function(primitiveEntity, GENERATOR=undefined, CREATOR=unde
389387
})
390388

391389
// Combine the Annotation bodies with the primitive object
392-
let expandedEntity = JSON.parse(JSON.stringify(primitiveEntity))
390+
let expandedEntity = structuredClone(primitiveEntity)
393391
for(const anno of matches){
394392
const body = anno.body
395393
let keys = Object.keys(body)

0 commit comments

Comments
 (0)