Skip to content
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.

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

Take advantage of Middleware / route typings in separate files #708

Open
cyrilchapon opened this issue Apr 5, 2023 · 6 comments
Open

Take advantage of Middleware / route typings in separate files #708

cyrilchapon opened this issue Apr 5, 2023 · 6 comments

Comments

@cyrilchapon
Copy link

cyrilchapon commented Apr 5, 2023

Hey !

Hacking around; I'm facing a challenge with middleware typings. Here's my need expressed :

I would like to "split" the construction of my routes in several parts, so that I have a file for the router, and another file for each route. Basic.

I came up with the following theorical implementation, which would be super sexy :

Router

const v1Router = () => {
  const publicRoute = route

  // This adds request.session or 401
  const restrictedRoute = route.use(verifySessionMiddleware)

  // Bind routes
  const _v1Router = router(
    // "SelfOrAdmin" route
    // The goal is to check if (
    //   request.session.user.id === path.id ||
    //   request.session.user.isAdmin === true
    //  )
    findUserController(
      restrictedRoute
        .get('/user/:id')
        .use(isSelfOrAdmin)
    ),

    // "Admin only" route
    // The goal is to check if (
    //   request.session.user.isAdmin === true
    // )
    listUserController(
      restrictedRoute
        .get('/user')
        .use(isAdmin)
    ),
  )

  return _v1Router.handler()
}

verifySessionMiddleware

This is a wrapped one I made over Supertokens. Basically it writes request.session or throws 401

import { verifySession as _verifySession } from 'supertokens-node/recipe/session/framework/express/index.js'
import {
  SessionContainer,
  VerifySessionOptions,
} from 'supertokens-node/recipe/session/index.js'
import { wrapNative } from 'typera-express/middleware.js'
import { SessionRequest } from 'supertokens-node/framework/express/index.js'
import { Middleware, Response } from 'typera-express'
import { ApiSupertokensErrorPayloadDTO } from '@@dtos/error.js'

export type SessionWrapper = { session: SessionContainer }
export const getVerifySession = (options?: VerifySessionOptions | undefined) =>
  wrapNative<SessionWrapper>(_verifySession(options), (_request) => {
    const request = _request.req as SessionRequest

    return {
      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
      session: request.session!,
    }
  }) as VerifySessionMiddleware

export type VerifySessionMiddleware = Middleware.Middleware<
  SessionWrapper,
  Response.Unauthorized<ApiSupertokensErrorPayloadDTO>
>

And this is where I begin struggling.

My end goal is to provide strong typings

  • For the isSelfOrAdminMiddleware
  • For the findUserController himself

I'll begin with the controller

Example "controller" : findUserController

import { Response, Route } from 'typera-express'
import { UserPayloadDTO } from '@@dtos/user.js'
import { ApiHttpErrorPayloadDTO } from '@@dtos/error.js'
import { prismaClient } from '@@lib/prisma.js'

// ##### => Here I want to grab back the type
// of restrictedRoute.get('/user/:id').use(isSelfOrAdmin)
// ( WITHOUT repeating the path ideally)
type BaseRouteCtor = ???????

const findUserController = (
  // #### => RouteCtor currified here
  baseRoute: BaseRouteCtor,
): Route<
  Response.Ok<UserPayloadDTO> | Response.NotFound<ApiHttpErrorPayloadDTO>
> =>
  baseRoute.handler(async (request) => {
    // ##### => ... to benefit this typing from the baseRoute
    const { id } = request.routeParams

    const foundUser = await prismaClient.user.findUnique({ where: { id } })

    if (foundUser === null) {
      return Response.notFound<ApiHttpErrorPayloadDTO>({
        type: 'http',
        code: 404,
      })
    }

    return Response.ok(foundUser)
  })

export default findUserController

isSelfOrAdminMiddleware

import { ApiHttpErrorPayloadDTO } from '@@dtos/error.js'
import { Middleware, Response } from 'typera-express'
import { SessionWrapper } from './verify-session.js'

export type IsSelfOrAdminMiddleware = Middleware.ChainedMiddleware<
  // #### => HERE I want to smoothly grab the type
  // of the "Request" inside my routeCtor,
  // notably with routeParams
  SessionWrapper & ???????????,
  unknown,
  Response.Forbidden<ApiHttpErrorPayloadDTO>
>

export const isSelfOrAdminMiddleware: IsSelfOrAdminMiddleware = async (request) => {
    const { session, headers } = request

    // #### => HERE I benefit from the typings of routeParams
    const { params } = request

    const sessionUser = session.user
    const paramsUserId = params.id

    // Do my checks in database
    const paramsUser = ... // find user from params in db
    // Check if users are the same, or if sessionUser is admin
    // ...

    // Maybe populate "request.paramsUser"
    // Maybe populate "request.isAdmin" + "request.isSelf"

    return Middleware.next()
  }

To sumup, my whole point is roughly to benefit the typings from route.[method]('/path/:id') both in a further middleware and in the final handler.

Any idea ? :)
Thanks !

@cyrilchapon cyrilchapon changed the title Take advantage of Middleware pipeline typings Take advantage of Middleware / route typings in separate files Apr 5, 2023
@akheron
Copy link
Owner

akheron commented Apr 6, 2023

There's probably something I don't understand here. Why would you want to pass a "partial" route to findUserController, just to add the handler there?

If the idea is to use the findUserController logic for multiple routes, then I would advise you to extract the stuff you want to do there in a standalone function instead, and call that function from all the route handlers that need the common functionality. Something like this:

const restrictedRoute = route.use(verifySessionMiddleware)

const route1 = restrictedRoute
  .get('/user/:id')
  .use(isSelfOrAdmin)
  .handler(async (req) => {
    return findUserController(req.foo, somethingElse)
  })

const route2 = restrictedRoute
  .get('/other/path')
  .use(isSelfOrAdmin)
  .handler(async (req) => {
    return findUserController(req.bar, otherStuff)
  })

What comes to IsSelfOrAdminMiddleware, it seems to me that the middleware expects a certain params object in the request, so why not just:

export type IsSelfOrAdminMiddleware = Middleware.ChainedMiddleware<
  SessionWrapper & { params: { foo: string, bar: number } },
  unknown,
  Response.Forbidden<ApiHttpErrorPayloadDTO>
>

@cyrilchapon
Copy link
Author

There's probably something I don't understand here. Why would you want to pass a "partial" route to findUserController, just to add the handler there?

Mostly because in a real world application, having the route paths scattered in 50 files is a pretty bad idea. So I'd want to centralise them right in the router code. Thus the "partial route" (RouteConstructor) seems the way to go.

What comes to IsSelfOrAdminMiddleware, it seems to me that the middleware expects a certain params object in the request, so why not just

Good point there.

@cyrilchapon
Copy link
Author

cyrilchapon commented Apr 6, 2023

After digging, I think exporting publicly RouteConstructor would perfectly fit my need.

Do you mind if I PR this ?


Another solution would be to export JUST the handler from the controller file. But that would require a great type definition for it. I need to explorer this one

@akheron
Copy link
Owner

akheron commented Apr 7, 2023

Mostly because in a real world application, having the route paths scattered in 50 files is a pretty bad idea. So I'd want to centralise them right in the router code. Thus the "partial route" (RouteConstructor) seems the way to go.

Centralising paths, middleware, etc. makes a lot of sense! But you could achieve this by making your route handlers small so that they only pass the data from the request to one (or a few) functions, and then create a response based on what those functions return. So instead of moving the route handler elsewhere, move the logic to separate functions and keep the route handlers small.

@cyrilchapon
Copy link
Author

@akheron thanks for the suggestion. Though, I'd still have the trouble to have to "receive" the route typings (and so, re-construct it) in the route handler.

Here is my current solution :
(for example here, receiving the user retrieved from a session)

export const baseRoute = route.useParamConversions(customConversions)
export type BaseRouteFn = typeof baseRoute
export type BaseRouteConstructor<
  Path extends string = string,
  TAdditionalRequestProps extends object = object,
> = MakeRoute<RequestBase & TAdditionalRequestProps, CustomConversions, Path>
export const _sessionRoute = (appContext: AppContext) =>
  baseRoute
    // Check account with supertokens
    .use(verifySessionMiddleware())
    // Fetch user by account
    .use(_sessionUserMiddleware(appContext))
export type SessionRouteFn = ReturnType<typeof _sessionRoute>
export type SessionRouteConstructor<
  Path extends string = string,
  TAdditionalRequestProps extends object = object,
> = BaseRouteConstructor<
  Path,
  SessionWrapper & SessionUser & TAdditionalRequestProps
>
const v1Router = (appContext: AppContext) => {
  const sessionRoute = _sessionRoute(appContext)

  // Bind routes
  const _v1Router = router(
    // Signed-in User
    findUserMeController(appContext)(sessionRoute.get('/user/me'))
  )
}
const findUserMeController: Controller<
  SessionRouteConstructor,
  Response.Ok<UserPayloadDTO> | Response.NotFound<ApiHttpErrorPayloadDTO>
> = () => (baseRoute) =>
  baseRoute.handler(async (request) => {
    // User already extracted from session
    // by sessionUserMiddleware above.
    // It would SELECT it or throw 404
    const { sessionUser } = request

    return Response.ok(sessionUser)
  })

This works like a charm, but when I want to retrieve some typed request params; I have to re-specify the path in the handler.
Like so :

const findTenantController: Controller<
  SessionRouteConstructor<'/:id(uuidv4)'>, // <= HERE
  Response.Ok<TenantPayloadDTO> | Response.NotFound<ApiHttpErrorPayloadDTO>
> = (appContext) => (baseRoute) =>
  baseRoute.handler(async (request) => {
    //...
  })

I think exporting internal RouteConstructor would satisfy this need.
(currently only available when returned by MakeRoute

@ajmnz
Copy link
Contributor

ajmnz commented May 5, 2023

I'd still have the trouble to have to "receive" the route typings (and so, re-construct it) in the route handler.

I'd like to chime in here, because I once faced the same issue. What I ended up doing was following @akheron 's advice of making your route handlers small so that they only pass the data from the request to one (or a few) functions.

What I did was create a generic RequestCtx<T> type that you can construct based on the different middleware your functions depend on.

For example, based on the following middleware, I purposely abstract their type, so it can be imported later on:

export type AuthCtx = { user: Pick<User, "id"> };

const authMiddleware: Middleware.Middleware<AuthCtx, never> = () => {
  // ...
  return Middleware.next({ user: { id: 3 } });
};

export type BodyCtx<T extends z.ZodTypeAny> = { body: z.infer<T> };

const bodyParserMiddleware = <T extends z.ZodTypeAny>(schema: T): Middleware.Middleware<BodyCtx<T>, never> =>
  async (ctx) => {
    try {
      const body = await schema.parseAsync(ctx.req.body);
      return Middleware.next({ body });
    } catch {
      throw new HttpBadRequestError();
    }
  };

Then I construct my route, which quickly delegates all heavy work to the product service layer and passes down the request context.

export const bodyShape = z.object({ name: z.string() })

const myRoute = route
  .post("/products")
  .use(authMiddleware, bodyParserMiddleware(bodyShape))
  .handler(async (ctx) => {
    const product = await productService.create(ctx);
    return Response.created(product);
  });

And finally, the create function takes a specific type-safe request context that must be satisfied when calling it

function create(ctx: RequestCtx<AuthCtx | BodyCtx<typeof bodyShape>>) {
  // ...
  return db.product.create({ userId: ctx.user.id, ...ctx.body });
}

This is how the RequestCtx<T> type looks like. It transforms the union of ctxes into a single object, which mirrors the context created by typera.

import type { RequestBase } from "typera-express";

type UnionToIntersectionHelper<U> = (U extends any ? (k: U) => void : never) extends (
  k: infer I
) => void
  ? I
  : never;

type UnionToIntersection<U> = UnionToIntersectionHelper<U> extends infer O
  ? { [K in keyof O]: O[K] }
  : never;

export type RequestCtx<T = {}> = RequestBase & UnionToIntersection<T>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants