Skip to content

Code optimization: modernize JavaScript patterns and improve consistency #211

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Code optimization: modernize JavaScript patterns and improve consistency #211

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0df436a
Initial plan
Copilot Aug 18, 2025
cfae428
Optimize utils.js: replace hasOwnProperty with optional chaining, imp…
Copilot Aug 18, 2025
e225989
Optimize controllers: replace let with const, improve conditionals an…
Copilot Aug 18, 2025
35b99ad
Optimize bulk operations and gog controllers, standardize string quotes
Copilot Aug 18, 2025
d8d1bd0
Potential fix for code scanning alert no. 69: Database query built fr…
cubap Aug 18, 2025
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
42 changes: 20 additions & 22 deletions controllers/crud.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,18 @@
res.set("Content-Type", "application/json; charset=utf-8")
let slug = ""
if(req.get("Slug")){
let slug_json = await generateSlugId(req.get("Slug"), next)
const slug_json = await generateSlugId(req.get("Slug"), next)
if(slug_json.code){
next(createExpressError(slug_json))
return
}
else{
slug = slug_json.slug_id
}
slug = slug_json.slug_id
}

let generatorAgent = getAgentClaim(req, next)
let context = req.body["@context"] ? { "@context": req.body["@context"] } : {}
let provided = JSON.parse(JSON.stringify(req.body))
let rerumProp = { "__rerum": utils.configureRerumOptions(generatorAgent, provided, false, false)["__rerum"] }
const generatorAgent = getAgentClaim(req, next)
const context = req.body["@context"] ? { "@context": req.body["@context"] } : {}
const provided = JSON.parse(JSON.stringify(req.body))
const rerumProp = { "__rerum": utils.configureRerumOptions(generatorAgent, provided, false, false)["__rerum"] }
rerumProp.__rerum.slug = slug
const providedID = provided._id
const id = isValidID(providedID) ? providedID : ObjectID()
Expand All @@ -40,16 +38,16 @@
if(_contextid(provided["@context"])) delete provided.id
delete provided["@context"]

let newObject = Object.assign(context, { "@id": process.env.RERUM_ID_PREFIX + id }, provided, rerumProp, { "_id": id })
const newObject = Object.assign(context, { "@id": process.env.RERUM_ID_PREFIX + id }, provided, rerumProp, { "_id": id })
console.log("CREATE")
try {
let result = await db.insertOne(newObject)
const result = await db.insertOne(newObject)
res.set(utils.configureWebAnnoHeadersFor(newObject))
newObject = idNegotiation(newObject)
newObject.new_obj_state = JSON.parse(JSON.stringify(newObject))
res.location(newObject[_contextid(newObject["@context"]) ? "id":"@id"])
const negotiatedObject = idNegotiation(newObject)
negotiatedObject.new_obj_state = JSON.parse(JSON.stringify(negotiatedObject))
res.location(negotiatedObject[_contextid(negotiatedObject["@context"]) ? "id":"@id"])
res.status(201)
res.json(newObject)
res.json(negotiatedObject)
}
catch (error) {
//MongoServerError from the client has the following properties: index, code, keyPattern, keyValue
Expand All @@ -64,12 +62,12 @@
* */
const query = async function (req, res, next) {
res.set("Content-Type", "application/json; charset=utf-8")
let props = req.body
const props = req.body
const limit = parseInt(req.query.limit ?? 100)
const skip = parseInt(req.query.skip ?? 0)
if (Object.keys(props).length === 0) {
//Hey now, don't ask for everything...this can happen by accident. Don't allow it.
let err = {
const err = {
message: "Detected empty JSON object. You must provide at least one property in the /query request body JSON.",
status: 400
}
Expand All @@ -79,7 +77,7 @@
try {
let matches = await db.find(props).limit(limit).skip(skip).toArray()
matches = matches.map(o => idNegotiation(o))
res.set(utils.configureLDHeadersFor(matches))

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources High

This query object depends on a
user-provided value
Loading
.
res.json(matches)
} catch (error) {
next(createExpressError(error))
Expand All @@ -93,9 +91,9 @@
* */
const id = async function (req, res, next) {
res.set("Content-Type", "application/json; charset=utf-8")
let id = req.params["_id"]
const id = req.params["_id"]
try {
let match = await db.findOne({"$or": [{"_id": id}, {"__rerum.slug": id}]})
const match = await db.findOne({"$or": [{"_id": id}, {"__rerum.slug": id}]})
if (match) {
res.set(utils.configureWebAnnoHeadersFor(match))
//Support built in browser caching
Expand All @@ -105,12 +103,12 @@
// Include current version for optimistic locking
const currentVersion = match.__rerum?.isOverwritten ?? ""
res.set('Current-Overwritten-Version', currentVersion)
match = idNegotiation(match)
res.location(_contextid(match["@context"]) ? match.id : match["@id"])
res.json(match)
const negotiatedMatch = idNegotiation(match)
res.location(_contextid(negotiatedMatch["@context"]) ? negotiatedMatch.id : negotiatedMatch["@id"])
res.json(negotiatedMatch)
return
}
let err = {
const err = {
"message": `No RERUM object with id '${id}'`,
"status": 404
}
Expand Down
22 changes: 10 additions & 12 deletions controllers/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@
const _id = resBody._id
delete resBody._id
if(!resBody["@context"]) return resBody
let modifiedResBody = JSON.parse(JSON.stringify(resBody))
const modifiedResBody = JSON.parse(JSON.stringify(resBody))
const context = { "@context": resBody["@context"] }
if(_contextid(resBody["@context"])) {
delete resBody["@id"]
delete resBody["@context"]
modifiedResBody = Object.assign(context, { "id": process.env.RERUM_ID_PREFIX + _id }, resBody)
return Object.assign(context, { "id": process.env.RERUM_ID_PREFIX + _id }, resBody)
}
return modifiedResBody
}
Expand All @@ -71,7 +71,7 @@
*
*/
const generateSlugId = async function(slug_id="", next){
let slug_return = {"slug_id":"", "code":0}
const slug_return = {"slug_id":"", "code":0}
let slug
if(slug_id){
slug_return.slug_id = slug_id
Expand Down Expand Up @@ -100,7 +100,7 @@
}

function createExpressError(err) {
let error = {}
const error = {}
if (err.code) {
switch (err.code) {
case 11000:
Expand Down Expand Up @@ -145,14 +145,13 @@
*/
function getAgentClaim(req, next) {
const claimKeys = [process.env.RERUM_AGENT_CLAIM, "http://devstore.rerum.io/v1/agent", "http://store.rerum.io/agent"]
let agent = ""
for (const claimKey of claimKeys) {
agent = req.user[claimKey]
const agent = req.user[claimKey]
if (agent) {
return agent
}
}
let err = {
const err = {
"message": "Could not get agent from req.user. Have you registered with RERUM?",
"status": 403
}
Expand Down Expand Up @@ -181,7 +180,7 @@
//We can keep this real short if we trust the objects sent into here. I think these are private helper functions, and so we can.
if(objToUpdate.__rerum.history.next.indexOf(newNextID) === -1){
objToUpdate.__rerum.history.next.push(newNextID)
let result = await db.replaceOne({ "_id": objToUpdate["_id"] }, objToUpdate)
const result = await db.replaceOne({ "_id": objToUpdate["_id"] }, objToUpdate)
return result.modifiedCount > 0
}
return true
Expand All @@ -196,9 +195,8 @@
* @throws Exception when a JSONObject with no '__rerum' property is provided.
*/
async function getAllVersions(obj) {
let ls_versions
let primeID = obj?.__rerum.history.prime
let rootObj = ( primeID === "root")
const primeID = obj?.__rerum.history.prime
const rootObj = ( primeID === "root")
? //The obj passed in is root. So it is the rootObj we need.
JSON.parse(JSON.stringify(obj))
: //The obj passed in knows the ID of root, grab it from Mongo
Expand All @@ -209,7 +207,7 @@
* This is the because some of the @ids have different RERUM URL patterns on them.
**/
//All the children of this object will have its @id in __rerum.history.prime
ls_versions = await db.find({ "__rerum.history.prime": rootObj['@id'] }).toArray()
const ls_versions = await db.find({ "__rerum.history.prime": rootObj['@id'] }).toArray()
//The root object is a version, prepend it in
ls_versions.unshift(rootObj)
return ls_versions
Expand Down
47 changes: 18 additions & 29 deletions utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const configureRerumOptions = function(generator, received, update, extUpdate){
}
else{
//We are either updating an existing RERUM object or creating a new one.
if(received_options.hasOwnProperty("history")){
if(received_options?.history){
history = received_options.history
if(update){
//This means we are configuring from the update action and we have passed in a clone of the originating object (with its @id) that contained a __rerum.history
Expand All @@ -68,7 +68,7 @@ const configureRerumOptions = function(generator, received, update, extUpdate){
history_prime = "root"
history_previous = ""
}
if(received_options.hasOwnProperty("releases")){
if(received_options?.releases){
releases = received_options.releases
releases_previous = releases.previous
}
Expand All @@ -86,7 +86,7 @@ const configureRerumOptions = function(generator, received, update, extUpdate){
rerumOptions.alpha = true
rerumOptions.APIversion = process.env.RERUM_API_VERSION
//It is important for the cache workflow that these be properly formatted.
let creationDateTime = new Date(Date.now()).toISOString().replace("Z", "")
const creationDateTime = new Date(Date.now()).toISOString().replace("Z", "")
rerumOptions.createdAt = creationDateTime
rerumOptions.isOverwritten = ""
rerumOptions.isReleased = ""
Expand All @@ -101,18 +101,14 @@ const configureRerumOptions = function(generator, received, update, extUpdate){
* Check this object for deleted status. deleted objects in RERUM look like {"@id":"{some-id}", __deleted:{object properties}}
*/
const isDeleted = function(obj){
return obj.hasOwnProperty("__deleted")
return obj?.__deleted !== undefined
}

/**
* Check this object for released status. Released objects in RERUM look like {"@id":"{some-id}", __rerum:{"isReleased" : "ISO-DATE-TIME"}}
*/
const isReleased = function(obj){
let bool =
(obj.hasOwnProperty("__rerum") &&
obj.__rerum.hasOwnProperty("isReleased") &&
obj.__rerum.isReleased !== "")
return bool
return obj?.__rerum?.isReleased !== undefined && obj.__rerum.isReleased !== ""
Copy link
Member

Choose a reason for hiding this comment

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

since isReleased is gone or a timestamp, we might not need such a specific check

}

/**
Expand All @@ -130,17 +126,17 @@ const isGenerator = function(origObj, changeAgent){
* return a JSON object. keys are header names, values are header values.
*/
const configureWebAnnoHeadersFor = function(obj){
let headers = {}
const headers = {}
if(isLD(obj)){
headers["Content-Type"] = "application/ld+json;charset=utf-8;profile=\"http://www.w3.org/ns/anno.jsonld\""
}
if(isContainerType(obj)){
headers["Link"] = "application/ld+json;charset=utf-8;profile=\"http://www.w3.org/ns/anno.jsonld\""
headers.Link = "application/ld+json;charset=utf-8;profile=\"http://www.w3.org/ns/anno.jsonld\""
}
else{
headers["Link"] = "<http://www.w3.org/ns/ldp#Resource>; rel=\"type\""
headers.Link = "<http://www.w3.org/ns/ldp#Resource>; rel=\"type\""
}
headers["Allow"] = "GET,OPTIONS,HEAD,PUT,PATCH,DELETE,POST"
headers.Allow = "GET,OPTIONS,HEAD,PUT,PATCH,DELETE,POST"
return headers
}

Expand All @@ -154,7 +150,7 @@ const configureLDHeadersFor = function(obj){
//Note that the optimal situation would be to be able to detect the LD-ness of this object
//What we have are the arrays returned from the aformentioned getters (/query, /since, /history)
//We know we want them to be LD and that they likely contain LD things, but the arrays don't have an @context
let headers = {}
const headers = {}
/**
if(isLD(obj)){
headers["Content-Type"] = 'application/ld+json;charset=utf-8;profile="http://www.w3.org/ns/anno.jsonld"'
Expand All @@ -165,9 +161,9 @@ const configureLDHeadersFor = function(obj){
headers["Content-Type"] = "application/json;charset=utf-8;"
}
*/
headers["Allow"] = "GET,OPTIONS,HEAD,PUT,PATCH,DELETE,POST"
headers.Allow = "GET,OPTIONS,HEAD,PUT,PATCH,DELETE,POST"
headers["Content-Type"] = 'application/ld+json;charset=utf-8;profile="http://www.w3.org/ns/anno.jsonld"'
headers["Link"] = '<http://store.rerum.io/v1/context.json>; rel="http://www.w3.org/ns/json-ld#context"; type="application/ld+json"'
headers.Link = '<http://store.rerum.io/v1/context.json>; rel="http://www.w3.org/ns/json-ld#context"; type="application/ld+json"'
return headers
}

Expand All @@ -177,8 +173,7 @@ const configureLDHeadersFor = function(obj){
* return boolean
*/
const isContainerType = function(obj){
let answer = false
let typestring = obj["@type"] ?? obj.type ?? ""
const typestring = obj["@type"] ?? obj.type ?? ""
const knownContainerTypes = [
"ItemList",
"AnnotationPage",
Expand All @@ -194,11 +189,10 @@ const isContainerType = function(obj){
for(const t of knownContainerTypes){
//Dang those pesky prefixes...circumventing exact match for now
if(typestring.includes(t)){
answer = true
break
return true
}
}
return answer
return false
//return knownContainerTypes.includes(typestring)
}

Expand All @@ -220,15 +214,10 @@ const isLD = function(obj){
*/
const configureLastModifiedHeader = function(obj){
let date = ""
if(obj.__rerum){
if(!obj.__rerum.isOverwritten === ""){
date = obj.__rerum.isOverwritten
}
else{
date = obj.__rerum.createdAt
}
if(obj?.__rerum){
date = obj.__rerum.isOverwritten !== "" ? obj.__rerum.isOverwritten : obj.__rerum.createdAt
Copy link
Member

Choose a reason for hiding this comment

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

is this the same as the simpler date = obj.__rerum.isOverwritten ?? obj.__rerum.createdAt

}
else if(obj.__deleted){
else if(obj?.__deleted){
date = obj.__deleted.time
}
//Note that dates like 2021-05-26T10:39:19.328 have been rounded to 2021-05-26T10:39:19 in browser headers. Account for that here.
Expand Down
Loading