Feedback changes from PR 40 and 41

Author: joeljoby02

README.md

@@ -32,7 +32,7 @@ functions are exported – everything else lives in internal modules and is inte
 kept private.  Example:
 
 ```js
-import { app, createServer, start } from 'rerum_server'
+import { app, createServer, start } from 'rerum_server_nodejs'
 
 // `app` is the configured Express application; you can pass it to Supertest or reuse it
 // inside another HTTP stack.

__tests__/public_api.test.js

@@ -18,5 +18,6 @@ describe('public API entry point', () => {
     server.on('listening', () => {
       server.close(() => done())
     })
+    server.on('error', (err) => done(err))
   })
 })

index.js

@@ -23,7 +23,7 @@ export default app
  * additional listeners or configure timeouts before calling
  * `server.listen(...)`.
  *
- * @param {number|string} [port=process.env.PORT||3001] port to assign to
+ * @param {number|string} [port=process.env.PORT??3001] port to assign to
  *        the express app and eventually listen on
  * @returns {import('http').Server} http server instance
  */
@@ -51,5 +51,18 @@ export function start(port) {
   server.on('listening', () => {
     console.log('LISTENING ON ' + p)
   })
+  server.on('error', (error) => {
+    if (error.syscall !== 'listen') throw error
+    switch (error.code) {
+      case 'EACCES':
+        console.error(`Port ${p} requires elevated privileges`)
+        process.exit(1)
+      case 'EADDRINUSE':
+        console.error(`Port ${p} is already in use`)
+        process.exit(1)
+      default:
+        throw error
+    }
+  })
   return server
 }

rest.js

@@ -86,6 +86,7 @@ Token: ${token}`
 You are Forbidden from performing this action. The request does not contain an "Authorization" header.
 Make sure you have registered at ${config.RERUM_PREFIX}. `
             }
+            break
         case 404:
             error.message += `
 The requested web page or resource could not be found.`

routes/__tests__/create.test.js

@@ -3,8 +3,9 @@ import express from "express"
 import request from "supertest"
 import { db } from '../../database/client.js'
 import controller from '../../db-controller.js'
+import config from '../../config/index.js'
 
-const rerum_uri = `${process.env.RERUM_ID_PREFIX}123456`
+const rerum_uri = `${config.RERUM_ID_PREFIX}123456`
 
 // Here is the auth mock so we get a req.user and the controller can function without a NPE.
 const addAuth = (req, res, next) => {

routes/__tests__/idNegotiation.test.js

@@ -1,30 +1,30 @@
 import { jest } from "@jest/globals"
-import dotenv from "dotenv"
 import controller from '../../db-controller.js'
+import config from '../../config/index.js'
 
 it("Functional '@id-id' negotiation on objects returned.", async () => {
   let negotiate = {
     "@context": "http://iiif.io/api/presentation/3/context.json",
     "_id": "example",
-    "@id": `${process.env.RERUM_ID_PREFIX}example`,
+    "@id": `${config.RERUM_ID_PREFIX}example`,
     "test": "item"
   }
   negotiate = controller.idNegotiation(negotiate)
   expect(negotiate._id).toBeUndefined()
   expect(negotiate["@id"]).toBeUndefined()
-  expect(negotiate.id).toBe(`${process.env.RERUM_ID_PREFIX}example`)
+  expect(negotiate.id).toBe(`${config.RERUM_ID_PREFIX}example`)
   expect(negotiate.test).toBe("item")
 
   let nonegotiate = {
     "@context":"http://example.org/context.json",
     "_id": "example",
-    "@id": `${process.env.RERUM_ID_PREFIX}example`,
+    "@id": `${config.RERUM_ID_PREFIX}example`,
     "id": "test_example",
     "test":"item"
   }
   nonegotiate = controller.idNegotiation(nonegotiate)
   expect(nonegotiate._id).toBeUndefined()
-  expect(nonegotiate["@id"]).toBe(`${process.env.RERUM_ID_PREFIX}example`)
+  expect(nonegotiate["@id"]).toBe(`${config.RERUM_ID_PREFIX}example`)
   expect(nonegotiate.id).toBe("test_example")
   expect(nonegotiate.test).toBe("item")
 })

routes/__tests__/patch.test.js

@@ -1,10 +1,10 @@
 import { jest } from "@jest/globals"
-import dotenv from "dotenv"
-dotenv.config()
+// dotenv.config() is no longer needed; config module handles environment loading
 // Only real way to test an express route is to mount it and call it so that we can use the req, res, next.
 import express from "express"
 import request from "supertest"
 import controller from '../../db-controller.js'
+import config from '../../config/index.js'
 
 // Here is the auth mock so we get a req.user and the controller can function without a NPE.
 const addAuth = (req, res, next) => {
@@ -23,7 +23,7 @@ const unique = new Date(Date.now()).toISOString().replace("Z", "")
 it("'/patch' route functions", async () => {
   const response = await request(routeTester)
     .patch('/patch')
-    .send({"@id":`${process.env.RERUM_ID_PREFIX}11111`, "RERUM Update Test":unique})
+    .send({"@id":`${config.RERUM_ID_PREFIX}11111`, "RERUM Update Test":unique})
     .set("Content-Type", "application/json")
     .then(resp => resp)
     .catch(err => err)

routes/__tests__/set.test.js

@@ -1,11 +1,11 @@
 import { jest } from "@jest/globals"
-import dotenv from "dotenv"
-dotenv.config()
-
-// Only real way to test an express route is to mount it and call it so that we can use the req, res, next.
+// dotenv.config() is no longer needed; config module handles environment loading
 import express from "express"
 import request from "supertest"
 import controller from '../../db-controller.js'
+import config from '../../config/index.js'
+
+// Only real way to test an express route is to mount it and call it so that we can use the req, res, next.
 
 // Here is the auth mock so we get a req.user and the controller can function without a NPE.
 const addAuth = (req, res, next) => {
@@ -24,12 +24,12 @@ const unique = new Date(Date.now()).toISOString().replace("Z", "")
 it("'/set' route functions", async () => {
   const response = await request(routeTester)
     .patch("/set")
-    .send({"@id":`${process.env.RERUM_ID_PREFIX}11111`, "test_set":unique})
+    .send({"@id":`${config.RERUM_ID_PREFIX}11111`, "test_set":unique})
     .set('Content-Type', 'application/json; charset=utf-8')
     .then(resp => resp)
     .catch(err => err)
       expect(response.header.location).toBe(response.body["@id"])
-      expect(response.headers["location"]).not.toBe(`${process.env.RERUM_ID_PREFIX}11111`)
+      expect(response.headers["location"]).not.toBe(`${config.RERUM_ID_PREFIX}11111`)
       expect(response.statusCode).toBe(200)
       expect(response.body._id).toBeUndefined()
       expect(response.body["test_set"]).toBe(unique)

routes/__tests__/unset.test.js

@@ -1,11 +1,11 @@
 import { jest } from "@jest/globals"
-import dotenv from "dotenv"
-dotenv.config()
+// dotenv.config() is no longer needed; config module handles environment loading
 
 // Only real way to test an express route is to mount it and call it so that we can use the req, res, next.
 import express from "express"
 import request from "supertest"
 import controller from '../../db-controller.js'
+import config from '../../config/index.js'
 
 // Here is the auth mock so we get a req.user so controller.create can function without a NPE.
 const addAuth = (req, res, next) => {
@@ -23,7 +23,7 @@ routeTester.use("/unset", [addAuth, controller.patchUnset])
 it("'/unset' route functions", async () => {
   const response = await request(routeTester)
     .patch("/unset")
-    .send({"@id":`${process.env.RERUM_ID_PREFIX}11111`, "test_obj":null})
+    .send({"@id":`${config.RERUM_ID_PREFIX}11111`, "test_obj":null})
     .set('Content-Type', 'application/json; charset=utf-8')
     .then(resp => resp)
     .catch(err => err)

routes/__tests__/update.test.js

@@ -1,10 +1,9 @@
 import { jest } from "@jest/globals"
-import dotenv from "dotenv"
-dotenv.config()
-// Only real way to test an express route is to mount it and call it so that we can use the req, res, next.
 import express from "express"
 import request from "supertest"
 import controller from '../../db-controller.js'
+import config from '../../config/index.js'
+// Only real way to test an express route is to mount it and call it so that we can use the req, res, next.
 
 // Here is the auth mock so we get a req.user so controller.create can function without a NPE.
 const addAuth = (req, res, next) => {
@@ -24,12 +23,12 @@ it("'/update' route functions", async () => {
 
   const response = await request(routeTester)
     .put('/update')
-    .send({"@id":`${process.env.RERUM_ID_PREFIX}11111`, "RERUM Update Test":unique})
+    .send({"@id":`${config.RERUM_ID_PREFIX}11111`, "RERUM Update Test":unique})
     .set("Content-Type", "application/json")
     .then(resp => resp)
     .catch(err => err)
   expect(response.header.location).toBe(response.body["@id"])
-  expect(response.headers["location"]).not.toBe(`${process.env.RERUM_ID_PREFIX}11111`)
+  expect(response.headers["location"]).not.toBe(`${config.RERUM_ID_PREFIX}11111`)
   expect(response.statusCode).toBe(200)
   expect(response.body._id).toBeUndefined()
   expect(response.body["RERUM Update Test"]).toBe(unique)