Attempting to fix issue with @Page decorator that didn't preserve contex + some work to make the @Controller more useful etc...

src/decorators/Controller.ts

@@ -1,12 +1,12 @@
-import type { Application, Request, Response, NextFunction } from 'express';
+import type { Application, Request, Response, NextFunction, RequestHandler } from 'express';
 
 import { BaseController } from '../controllers/BaseController.js';
 
 import { CHILD_CONTROLLER_METADATA_KEY } from './ChildController.js';
-import { GET_METADATA_KEY } from './GET.js';
+import { GET_METADATA_KEY, GET_FORCE_PREFIX_METADATA_KEY } from './GET.js';
-import { POST_METADATA_KEY } from './POST.js';
+import { POST_METADATA_KEY, POST_FORCE_PREFIX_METADATA_KEY } from './POST.js';
-import { PUT_METADATA_KEY } from './PUT.js';
+import { PUT_METADATA_KEY, PUT_FORCE_PREFIX_METADATA_KEY } from './PUT.js';
-import { DELETE_METADATA_KEY } from './DELETE.js';
+import { DELETE_METADATA_KEY, DELETE_FORCE_PREFIX_METADATA_KEY } from './DELETE.js';
 
 /**
  * Class decorator to "fill in" the setup method which is called on server startup and connects the methods/functions and their desired paths in the express app.
@@ -35,19 +35,74 @@ import { DELETE_METADATA_KEY } from './DELETE.js';
  *    private myDeleteMethod(req: Request, res: Response) {}
  * }
  * ```
+ * 
+ * @example
+ * The following example shows how to use the Controller decorator with a base path, middleware and a child controller.
+ * ```ts
+ * import { Request, Response } from 'express';
+ * 
+ * import { Controller, ChildController, GET, BaseController } from '@BridgemanAccessible/ba-web-framework';
+ * 
+ * @Controller('/api', myMiddlewareFunction)
+ * @ChildController(APIV1Controller)
+ * export class ApiController extends BaseController {
+ *     @GET('/status')
+ *     private getStatus(req: Request, res: Response) { /* ... * / }
+ * }
+ * 
+ * @Controller('/v1')
+ * export class APIV1Controller extends BaseController {
+ *     // Full path will be /api/v1/route because of the parent controller and this controller's base paths
+ *     @GET('/route')
+ *     private getV1Route(req: Request, res: Response) { /* ... * / }
+ * }
+ * ```
+ * 
+ * @param basePath The base path for the controller. Defaults to `'/'`.
+ * @param middlewares Optional middlewares to apply to all routes in the controller.
  */
-export function Controller<T extends { new (...args: any[]): BaseController }>() {
+export function Controller<T extends { new (...args: any[]): BaseController }>(basePath: string = '/', ...middlewares: RequestHandler[]) {
     // Technically the function we return here with the target parameter is the actual decorator itself but we wrap it in case we ever want to add parameters to the decorator
     return function(target: T){
+        Reflect.defineMetadata('basePath', basePath, target);
+        Reflect.defineMetadata('middlewares', middlewares, target);
         Reflect.defineMetadata('originalClass', target, target);
+
         // We extend the class that is decorated and override the setup method to automatically setup the routes
         return class extends target {
+            // Helper function to join paths cleanly
+            static joinPaths(base: string, path: string): string {
+                // Combine the base path and the specific route path
+                let fullPath = base + path;
+                
+                // Remove double slashes (e.g., //user -> /user)
+                fullPath = fullPath.replace(/\/\//g, '/');
+                
+                // Ensure it starts with / (in case base was empty and path was empty)
+                if (!fullPath.startsWith('/')) {
+                    fullPath = '/' + fullPath;
+                }
+                
+                // Remove trailing slash (optional, unless it's just root '/')
+                if (fullPath.length > 1 && fullPath.endsWith('/')) {
+                    fullPath = fullPath.slice(0, -1);
+                }
+                
+                return fullPath;
+            }
+
             /**
              * Setup the routes for the controller.
              * 
              * @param app The express application to setup the routes on.
              */
-            static setup(app: Application) {
+            static setup(app: Application, pathPrefix: string = '') {
+                // Calculate the current context's absolute prefix
+                // Order matters: Incoming Prefix + Current Controller Base Path
+                // Ex: "" + "/api" = "/api"
+                // Ex: "/api" + "/v1" = "/api/v1"
+                const currentPrefix = this.joinPaths(pathPrefix, Reflect.getMetadata('basePath', target) as string);
+                
                 // If the decorated class is also decorated with the `@ChildController` decorator, 
                 // then we call the child controller's setup method as well.
                 // 
@@ -57,10 +112,11 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                 if(typeof childControllers !== 'undefined') {
                     if(Array.isArray(childControllers)) {
                         childControllers.forEach((childController) => {
-                            childController.setup(app);
+                            childController.setup(app, currentPrefix);
                         });
-                    } else {
+                    }
-                        childControllers.setup(app);
+                    else {
+                        childControllers.setup(app, currentPrefix);
                     }
                 }
 
@@ -77,16 +133,30 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                         // Get the path
                         const path = Reflect.getMetadata(GET_METADATA_KEY, target.prototype, method);
 
+                        const fullPath = Reflect.getMetadata(GET_FORCE_PREFIX_METADATA_KEY, target.prototype, method) ? this.joinPaths(currentPrefix, path) : path;
+
                         // Bind the method to the class instance
-                        app.get(path, async (req, res, next) => {
+                        app.get(fullPath, ...[
+                            // Middleware defined at the controller level
+                            ...(Reflect.getMetadata('middlewares', target) as RequestHandler[]), 
+
+                            // Route handler
+                            async (req: Request, res: Response, next: NextFunction) => {
                                 //console.log('[Controller.setup.<GET request>] Request:', req);
                                 //console.log('[Controller.setup.<GET request>] Response:', res);
                                 //console.log('[Controller.setup.<GET request>] Next:', next);
 
                                 try {
-                                await Promise.all([fn.bind(controller)(req, res, next)])
+                                    await Promise.all([
+                                        fn.bind(controller)(req, res, next)
+                                    ])
                                         .catch((error) => { 
                                             //console.log('[Controller.setup.<GET request>] Error in promise:', error);
+                                            
+                                            if(error instanceof Error) {
+                                                throw error;
+                                            }
+
                                             throw new Error(error); 
                                         });
                                         
@@ -109,7 +179,8 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                                 }
 
                                 //console.log('[Controller.setup.<GET request>] Finished processing GET request for method:', method);
-                        });
+                            }
+                        ]);
                     });
                 
                 // Loop over all the methods in the decorated class looking for methods that use the POST decorator
@@ -125,8 +196,18 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                         const path = postRoute.path;
                         const middleware: NextFunction[] = postRoute.middleware;
 
+                        const fullPath = Reflect.getMetadata(POST_FORCE_PREFIX_METADATA_KEY, target.prototype, method) ? this.joinPaths(currentPrefix, path) : path;
+
                         // Bind the method to the class instance
-                        app.post(path, ...middleware, async (req, res, next) => {
+                        app.post(fullPath, ...[
+                            // Middleware defined at the controller level
+                            ...(Reflect.getMetadata('middlewares', target) as RequestHandler[]),
+                            
+                            // Middleware defined at the route level
+                            ...middleware, 
+                            
+                            // Route handler
+                            async (req: Request, res: Response, next: NextFunction) => {
                                 //console.log('[Controller.setup.<POST request>] Request:', req);
                                 //console.log('[Controller.setup.<POST request>] Response:', res);
                                 //console.log('[Controller.setup.<POST request>] Next:', next);
@@ -150,7 +231,8 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                                         return Promise.reject(error);
                                     }
                                 }
-                        });
+                            }
+                        ]);
                     });
 
                 // Loop over all the methods in the decorated class looking for methods that use the `@PUT` decorator
@@ -166,8 +248,13 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                         const path = putRoute.path;
                         const middleware = putRoute.middleware;
 
+                        const fullPath = Reflect.getMetadata(PUT_FORCE_PREFIX_METADATA_KEY, target.prototype, method) ? this.joinPaths(currentPrefix, path) : path;
+                        
                         // Bind the method to the class instance
-                        app.put(path, ...middleware, async (req, res, next) => {
+                        app.put(fullPath, ...[
+                            ...(Reflect.getMetadata('middlewares', target) as RequestHandler[]),
+                            ...middleware, 
+                            async (req: Request, res: Response, next: NextFunction) => {
                                 //console.log('[Controller.setup.<PUT request>] Request:', req);
                                 //console.log('[Controller.setup.<PUT request>] Response:', res);
                                 //console.log('[Controller.setup.<PUT request>] Next:', next);
@@ -191,7 +278,8 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                                         return Promise.reject(error);
                                     }
                                 }
-                        });
+                            }
+                        ]);
                     });
                 
                 // Loop over all the methods in the decorated class looking for methods that use the `@DELETE` decorator
@@ -207,8 +295,13 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                         const path = deleteRoute.path;
                         const middleware = deleteRoute.middleware;
 
+                        const fullPath = Reflect.getMetadata(DELETE_FORCE_PREFIX_METADATA_KEY, target.prototype, method) ? this.joinPaths(currentPrefix, path) : path;
+                        
                         // Bind the method to the class instance
-                        app.delete(path, ...middleware, async (req, res, next) => {
+                        app.delete(fullPath, ...[
+                            ...(Reflect.getMetadata('middlewares', target) as RequestHandler[]),
+                            ...middleware, 
+                            async (req: Request, res: Response, next: NextFunction) => {
                                 //console.log('[Controller.setup.<DELETE request>] Request:', req);
                                 //console.log('[Controller.setup.<DELETE request>] Response:', res);
                                 //console.log('[Controller.setup.<DELETE request>] Next:', next);
@@ -231,8 +324,9 @@ export function Controller<T extends { new (...args: any[]): BaseController }>()
                                         // Because next is undefined and we still want to have Express handle the error we reject the promise
                                         return Promise.reject(error);
                                     }
-                            };
+                                }
-                        });
+                            }
+                        ]);
                     });
             }
         }

src/decorators/DELETE.ts

@@ -2,6 +2,7 @@ import type { NextFunction } from 'express';
 import type { NextHandleFunction } from 'connect';
 
 export const DELETE_METADATA_KEY = 'Delete';
+export const DELETE_FORCE_PREFIX_METADATA_KEY = 'DeleteForcePrefix';
 
 /**
  * Method decorator intended to be used on methods of a class decorated with the `@Controller` decorator.
@@ -11,11 +12,13 @@ export const DELETE_METADATA_KEY = 'Delete';
  * Controller authors can also specify middleware to be used with the DELETE route (because a middleware is commonly required to parse the body of a DELETE request).
  * 
  * @param path The path for the DELETE route.
+ * @param forcePrefix Whether to force the appropriate prefix to be applied to the route's path. Default is true.
  * @param middleware The middleware to use with the DELETE route.
  */
-export function DELETE(path: string, ...middleware: (NextHandleFunction | NextFunction)[]) {
+export function DELETE(path: string = '', forcePrefix: boolean = true, ...middleware: (NextHandleFunction | NextFunction)[]) {
     return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) {
         // Define a metadata key with the path and middleware as the value on the target's propertyKey
         Reflect.defineMetadata(DELETE_METADATA_KEY, { path, middleware }, target, propertyKey);
+        Reflect.defineMetadata(DELETE_FORCE_PREFIX_METADATA_KEY, forcePrefix, target, propertyKey);
     };
 }

src/decorators/GET.ts

@@ -1,4 +1,5 @@
 export const GET_METADATA_KEY = 'Get';
+export const GET_FORCE_PREFIX_METADATA_KEY = 'GetForcePrefix';
 
 /**
  * Method decorator intended to be used on methods of a class decorated with the `@Controller` decorator.
@@ -7,10 +8,12 @@ export const GET_METADATA_KEY = 'Get';
  * The specific path for the route is defined by the path parameter.
  * 
  * @param path The path for the GET route.
+ * @param forcePrefix Whether to force the appropriate prefix to be applied to the route's path. Default is true.
  */
-export function GET(path: string) {
+export function GET(path: string = '', forcePrefix: boolean = true) {
     return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) {
         // Define a `Get` metadata key with the path as the value on the target's propertyKey
         Reflect.defineMetadata(GET_METADATA_KEY, path, target, propertyKey);
+        Reflect.defineMetadata(GET_FORCE_PREFIX_METADATA_KEY, forcePrefix, target, propertyKey);
     };
 }

src/decorators/POST.ts

@@ -1,7 +1,8 @@
 import type { NextFunction } from 'express';
 import type { NextHandleFunction } from 'connect';
 
-export const POST_METADATA_KEY = 'Put';
+export const POST_METADATA_KEY = 'Post';
+export const POST_FORCE_PREFIX_METADATA_KEY = 'PostForcePrefix';
 
 /**
  * Method decorator intended to be used on methods of a class decorated with the `@Controller` decorator.
@@ -11,11 +12,13 @@ export const POST_METADATA_KEY = 'Put';
  * Controller authors can also specify middleware to be used with the POST route (because a middleware is commonly required to parse the body of a POST request).
  * 
  * @param path The path for the GET route.
+ * @param forcePrefix Whether to force the appropriate prefix to be applied to the route's path.
  * @param middleware The middleware to use with the POST route.
  */
-export function POST(path: string, ...middleware: (NextHandleFunction | NextFunction)[]) {
+export function POST(path: string = '', forcePrefix: boolean = true,  ...middleware: (NextHandleFunction | NextFunction)[]) {
     return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) {
         // Define a `Get` metadata key with the path as the value on the target's propertyKey
         Reflect.defineMetadata(POST_METADATA_KEY, { path, middleware }, target, propertyKey);
+        Reflect.defineMetadata(POST_FORCE_PREFIX_METADATA_KEY, forcePrefix, target, propertyKey);
     };
 }

src/decorators/PUT.ts

@@ -2,6 +2,7 @@ import type { NextFunction } from 'express';
 import type { NextHandleFunction } from 'connect';
 
 export const PUT_METADATA_KEY = 'Put';
+export const PUT_FORCE_PREFIX_METADATA_KEY = 'PutForcePrefix';
 
 /**
  * Method decorator intended to be used on methods of a class decorated with the `@Controller` decorator.
@@ -11,11 +12,13 @@ export const PUT_METADATA_KEY = 'Put';
  * Controller authors can also specify middleware to be used with the PUT route (because a middleware is commonly required to parse the body of a PUT request).
  * 
  * @param path The path for the PUT route.
+ * @param forcePrefix Whether to force the appropriate prefix to be applied to the route's path. Default is true.
  * @param middleware The middleware to use with the PUT route.
  */
-export function PUT(path: string, ...middleware: (NextHandleFunction | NextFunction)[]) {
+export function PUT(path: string = '', forcePrefix: boolean = true, ...middleware: (NextHandleFunction | NextFunction)[]) {
     return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) {
         // Define a metadata key with the path and middleware as the value on the target's propertyKey
         Reflect.defineMetadata(PUT_METADATA_KEY, { path, middleware }, target, propertyKey);
+        Reflect.defineMetadata(PUT_FORCE_PREFIX_METADATA_KEY, forcePrefix, target, propertyKey);
     };
 }

src/decorators/Page.ts

@@ -1,4 +1,4 @@
-import type { Request, Response, NextFunction } from 'express';
+import type { Response } from 'express';
  
 // Response type guard
 function isResponse(obj) {
@@ -36,12 +36,34 @@ function isResponse(obj) {
  * @param otherParams Any other parameters to pass to the page
  */
 export function Page(title: string, page: string, extraScripts: (string | { script: string, defer: boolean })[] = [], extraStyles: string[] = [], ...otherParams: any[]) {
+    /** Extract the response object from the arguments */
+    const getResponseFromArgs = (propertyKey: string, ...args: any[]) => {
+        // Find the response object in the arguments
+        const responseArgs = args.filter(arg => isResponse(arg));
+        
+        // Verify there is only one response object
+        if (responseArgs.length !== 1) {
+            console.warn(`Page decorator: Incorrect number of response objects found in arguments for ${propertyKey}. Should ONLY be one.`);
+            return null;
+        }
+        
+        const res = responseArgs[0] as Response;
+
+        return res;
+    }
+    
+    /** Render the page with the given parameters */
     const doRender = (propertyKey: string, output: any, ...args: any[]) => {
-        if (!args.some(arg => isResponse(arg)) || args.filter(arg => isResponse(arg)).length > 1) {
+        // Get the response object from the arguments
-            console.warn(`Page decorator: Incorrect number of response objects found in arguments for ${propertyKey}. Should ONLY be one response object.`);
+        const res = getResponseFromArgs(propertyKey, ...args);
+        
+        // Verify there is a response object
+        if (res === null) {
+            console.warn(`Page decorator: Can't find response object for ${propertyKey}. Aborting render.`);
             return;
         }
 
+        // Parameters from the decorator to pass as rendering parameters
         const renderParams: { [key: string]: any } = {
             title: title,
             page: page,
@@ -50,39 +72,38 @@ export function Page(title: string, page: string, extraScripts: (string | { scri
             ...otherParams
         };
 
-        // If the decorated method's output is an object, we want to merge it with the renderParams
+        // If the decorated method's output is an object, 
-        if(typeof output === 'object') {
+        // we want to merge it with the parameters from the decorator
-            Object.entries(output).forEach((entry) => {
+        // Note, this does allow overriding of the decorator parameters from function output
-                renderParams[entry[0]] = entry[1];
+        if(typeof output === 'object' && output !== null) {
-            });
+            Object.assign(renderParams, output);
         }
 
-        args.find(arg => isResponse(arg)).render('base', renderParams);
+        res.render('base', renderParams);
     }
 
     return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) {
+        // The "original" (decorated) method
         const original = descriptor.value;
 
-        // If the original function has more than 3 parameters, we can assume it is an error handler
+        descriptor.value = async function (...args: any[]) {
-        // An error handler starts with the error object, then the request, response and next functions.
+            // Extract the `next` function from the arguments (if it exists)
-        if(original.length > 3) {
+            // This handles both (req, res, next) and (err, req, res, next) signatures dynamically
-            descriptor.value = async function (err: Error, req: Request, res: Response, next: NextFunction, ...args: any[]) {
+            const nextIndex = args.findIndex(arg => typeof arg === 'function');
-                //console.log('[Page Decorator] Error:', err);
+            const next = nextIndex !== -1 ? args[nextIndex] : undefined;
-                //console.log('[Page Decorator] Request:', req);
+            
-                //console.log('[Page Decorator] Response:', res);
+            //if(args.length > 3) {    
+            //    console.log('[Page Decorator] Error:', args[0]);
+            //}
+            //console.log('[Page Decorator] Request:', args.length > 3 ? args[1] : args[0]);
+            //console.log('[Page Decorator] Response:', args.length > 3 ? args[2] : args[1]);
             //console.log('[Page Decorator] Next:', next);
                 
             try {
-                    // We run the original here so that if the decorated method has specific checks it needs to make (ex. if the ID of whatever actually exists) it can make them before rendering the page
+                // We run the original (decorated) function here. 
-                    // 
+                // This is so that it can pass any needed parameters to the rendered page and/or stop rendering (return `false`)
-                    // !!!IMPORTANT!!!
+                // Note, we `.apply` to `this` preserving instance context
-                    // This currently is kind of a hacky solution, because we bind to the target, which is the prototype rather than the instance.
+                const output = await original.apply(this, args);
-                    // This means instance data is not properly available/accessible within the decorated method.
-                    // However, you can use the `this` keyword to access other methods on the class, etc...
-                    //
-                    // The above (this approach) is necessary at time of writing because the `this` (which should be bound to for the instance) seems to be undefined within the context of the decorator function here for some reason
-                    // Ideally, would dive into this and do it properly but this seems to be good enough for now.
-                    const output = await original.bind(target)(err, req, res, next, ...args);
                 
                 // If the output is false, we don't want to render the page
                 // 
@@ -92,55 +113,13 @@ export function Page(title: string, page: string, extraScripts: (string | { scri
                     return;
                 }
 
-                    doRender(propertyKey, output, err, req, res, next, ...args);
+                doRender(propertyKey, output, ...args);
             }
             catch(error) {
-                    //console.error('[Page Decorator] Error:', error);
+                //console.error(`[Page Decorator] Error while rendering page: ${error}`);
 
                 if(typeof next !== 'undefined') {
-                        next(error);
+                    //console.log(`[Page Decorator] Calling next with error: ${error}`);
-                        return;
-                    }
-                    else {
-                        // Because next is undefined and we still want to have Express handle the error we reject the promise
-                        return Promise.reject(error);
-                    }
-                }
-            }
-        }
-        else {
-            descriptor.value = async function (req: Request, res: Response, next: NextFunction) {
-                //console.log('[Page Decorator] Request:', req);
-                //console.log('[Page Decorator] Response:', res);
-                //console.log('[Page Decorator] Next:', next);
-                
-                try {
-                    // We run the original here so that if the decorated method has specific checks it needs to make (ex. if the ID of whatever actually exists) it can make them before rendering the page
-                    // 
-                    // !!!IMPORTANT!!!
-                    // This currently is kind of a hacky solution, because we bind to the target, which is the prototype rather than the instance.
-                    // This means instance data is not properly available/accessible within the decorated method.
-                    // However, you can use the `this` keyword to access other methods on the class, etc...
-                    //
-                    // The above (this approach) is necessary at time of writing because the `this` (which should be bound to for the instance) seems to be undefined within the context of the decorator function here for some reason
-                    // Ideally, would dive into this and do it properly but this seems to be good enough for now.
-                    const output = await original.bind(target)(req, res, next);
-                    
-                    // If the output is false, we don't want to render the page
-                    // 
-                    // This allows the decorated method to handle the response itself
-                    // Ex. Logging in, if the user is already logged in we want to redirect them, not render the login page again
-                    if(typeof output === 'boolean' && !output) {
-                        return;
-                    }
-
-                    doRender(propertyKey, output, req, res, next);
-                }
-                catch(error) {
-                    //console.error('[Page Decorator] Error:', error);
-
-                    if(typeof next !== 'undefined') {
-                        //console.log('[Page Decorator] Calling next with error:', error);
                         
                     next(error);
                     return;
@@ -155,4 +134,3 @@ export function Page(title: string, page: string, extraScripts: (string | { scri
         }
     }
 }
-}