middleware icon indicating copy to clipboard operation
middleware copied to clipboard

Alternative OpenAPI middleware - thoughts on including it in this repo?

Open paolostyle opened this issue 1 year ago • 10 comments

Hey!

First of all, I'm not trying to promote my library. I'd like to include it in this repo as an official... not sure, either replacement or alternative to the existing zod-openapi middleware.

While I really love Hono and the ecosystem around it, I can't quite say the same about the OpenAPI middleware (zod-openapi). Perhaps I'm alone in this but I developed my apps with just zod-validator (which works great) and I was quite surprised that creating an OpenAPI documentation would require me to refactor pretty much the entire Hono code - you no longer can use the standard Hono class, .get/.post etc. methods (well I guess you can but they wouldn't be documented), I would have to use z object from the library, even gradual migration is difficult. I did not like that and I ended up not implementing OpenAPI in my app at all because of that.

Instead I spent some time to try and create a middleware that would be much simpler, something that would work similarly to zod-validator middleware and would be easy to gradually implement. I believe I succeeded in that and although it's a bit rough around the edges at the moment and I still need to test some edge cases, I do have my apps documented with that library and it works quite well.

⚠EDIT: The API is a bit different now. Please visit the README to see the proper examples there or at least to this post in this thread.

A very simple example of how my middleware works:

⚠Old code / outdated info warning, no longer relevant, keeping it just for context⚠
import { Hono } from 'hono';
import { z } from 'zod';
import { createOpenApi, openApi } from 'hono-zod-openapi';

export const app = new Hono().get(
  '/user',
  openApi(
    // simple response type
    z.object({ hi: z.string() }),
   // request validators, uses zod-validator internally
    {
      query: z.object({ id: z.string() }),
    },
  ),
  (c) => {
    // this still works exactly as with `zod-validator`
    const { id } = c.req.valid('query');
    return c.json({ hi: id }, 200);
  },
)

// this will add a `GET /doc` route to the `app` router
// it's possible to do it manually, also possible to manually adjust the OpenAPI document
createOpenApi(app, {
  title: 'Example API',
  version: '1.0.0',
});

You can pass the response type also as a more complex object or array of them:

[
  {
    status: 200,
    description: '200 OK',
    schema: z
      .object({
        message: z.string(),
      })
      .openapi({
        examples: [
          {
            message: 'Hello, World!',
          },
        ],
      }),
  },
]

There is also support for validating the response but I'm on the fence whether it's actually useful. I think it would be more useful if c.json in the handler would be able to pick up that type and scream at you if it didn't match the schema.

OpenAPI functionality is handled by zod-openapi (so not the same library the official middleware is using, I found this one to be nicer to work with).

You can check out the repo here: https://github.com/paolostyle/hono-zod-openapi.

Now the main question - is there a place for a library like this here, in this repo, considering an official one already exists? Or should I just keep maintaining it in my repo? If it would be possible to include, how should I go about it? Just create a PR and we'd talk about the implementation details etc. there?

paolostyle avatar Oct 01 '24 00:10 paolostyle

@paolostyle I appreciate your work on zod-openapi, especially the use of middleware instead of wrapping routes.

However, I prefer a more explicit OpenAPI schema definition without excessive abstraction. A potential solution could be allowing middleware to take a specific zod-openapi object per route while providing a helper function for easy migration. For example:

// Abstract (with helper function 'fromValidators()')
export const app = new Hono().get(
  '/user',
  openApi(
    fromValidators([zValidator('json', JsonSchema), zValidator('query', QuerySchema)])
  ),
  (c) => {
    const { id } = c.req.valid('query');
    return c.json({ hi: id }, 200);
  },
);

// Specific (raw zod-openapi object)
export const app2 = new Hono().get(
  '/user',
  openApi({
    requestParams: { query: QuerySchema },
    requestBody: { content: { 'application/json': JsonSchema } },
  }),
  (c) => {
    const { id } = c.req.valid('query');
    return c.json({ hi: id }, 200);
  },
);

The idea is to let openApi() accept raw zod-openapi objects, with abstractions added via helpers like fromValidators(). This simplifies migration from zod-validator while offering a more explicit API for those who prefer less abstraction.

This flexibility would make me seriously consider switching to your library over defining OpenAPI schemas first and using my openapi-router for typesafe Hono routes.

cheers :)

bennobuilder avatar Oct 01 '24 05:10 bennobuilder

@bennobuilder Thanks for the comment. I'm not sure if I'm following, though. So just to be clear, the second argument of my middleware defined like this: { json: someZodSchema, header: otherZodSchema } is equivalent to passing two zodValidator middlewares like this: zValidator('json', someZodSchema), zValidator('header', otherZodSchema). The added value of the middleware is that you get the OpenAPI spec for free, OpenAPI specific options are handled through .openapi method on the zod schema, just like in the current version.

For response schemas it is indeed slightly more abstracted but at the end of the day I wanted to do it mostly for myself and I hate the verbosity of the spec, I'd much rather pass a flat list of possible responses than defining a 6 levels deep json/yaml structure. I understand that the spec is somewhat of an industry standard, but my goal was to get an accurate Swagger docs page without needing to deal with yamls. The goal of my middleware is that the developer should spend as little time as possible on the OpenAPI quirks and get reasonable quality docs almost for free while writing idiomatic Hono code. createOpenApi function (the one that actually creates the document) accepts an overrides method where you can modify the doc however you want, and it receives a generated paths object which can be modified as required. This is obviously not the recommended way, I haven't figured out yet if there are things that are impossible to do in the middleware API, things like servers definitely need to be specified there but it's not really something that can be reliably inferred from the Hono object.

If my library was to replace the current one I would definitely have to support the existing createRoute API at least as a migration path, if that's what you meant by "specific zod-openapi object".

paolostyle avatar Oct 01 '24 07:10 paolostyle

Does this have support for the security field in the createRoute yet?

iatomic1 avatar Oct 01 '24 21:10 iatomic1

I mostly agree with your point that, as long as we avoid over-abstraction, less code usually leads to a better DX. I would prefer using an API similar to the one below. I might contribute to your library and use it, or perhaps build my own 😅.

new Hono().get(
  "/user",
  oapOperation({
    summary: "Get user",
    tags: ["User"],
  }),
  oapRequest("header", requestHeaderSchema),
  oapRequest("json", requestBodySchema),
  oapResponse("default", defaultResponseSchema),
  oapResponse(403, forbiddenResponseSchema),
  async (c) => {
    const { authorization } = c.req.valid("header")
    if (!authorization) {
      return c.text("Unauthorized", 403)
    }
    const { id } = c.req.valid("json")
    return c.json({ hi: id }, 200)
  },
)

I believe this would cover 95% of use cases, and for other scenarios, we could provide a usage pattern closer to the native implementation. The example below should be equivalent to the one above.


new Hono().get(
  "/user",
  oapOperation({
    summary: "Get user",
    tags: ["User"],
  }),
  oapRequest("header", [
    {
      name: "authorization",
      required: true,
      schema: {
        type: "string",
      },
    },
  ]),
  oapRequest("json", {
    description: "body description",
    required: true,
    content: {
      "application/json": {
        schema: requestBodySchema,
      },
    },
  }),
  oapResponse("default", {
    description: "default response description",
    content: {
      "application/json": {
        schema: defaultResponseSchema,
      },
    },
  }),
  oapResponse(403, {
    description: "forbidden response description",
    content: {
      "text/plain": {
        schema: {
          type: "string",
          example: "Forbidden",
        },
      },
    },
  }),
  async (c) => {
    const { authorization } = c.req.valid("header") // 
    if (!authorization) {
      return c.text("Unauthorized", 403)
    }
    const { id } = c.req.valid("json")
    return c.json({ hi: id }, 200)
  },
)

maou-shonen avatar Oct 02 '24 05:10 maou-shonen

@adeyemialameen04 It is possible, openApi middleware accepts 3rd argument where you can pass any operation-level properties, you'd have to add the security component to createOpenApiDocs overrides (this name is probably not ideal) field, though. You can pass tags, summary etc. there, too. However for now I would not recommend using my library in production as it's still evolving and there might be breaking changes between the versions at least until 1.x version is released.

@maou-shonen Thanks for the feedback, I like this API a lot, just a couple of concerns, but I think they're all solvable:

  • we wouldn't be able to enforce usage of the oapResponse (I would change that name, oap is a strange abbreviation, it would be probably one of openApiResponse/oapiResponse/openApiRes/oapiRes) middleware and if it's missing, we won't be able to generate the document. We could probably just console.error it when calling createOpenApiDocs, though.
  • description is a required field for responses, so we'd have to provide default ones based on the status code when used like this: oapResponse(200, defaultResponseSchema). Not a huge issue, but I think description, headers, links and summary fields (all of them are part of the OpenAPI spec for response object type) should be passable through a 3rd argument.
  • In your 403 example the content type is text/plain. It is not possible to provide the media type just through a zod schema. Currently I'm assuming it's always application/json unless specified through mediaType property. Even though it would be non-standard, I'd lean towards providing it through 3rd argument, like e.g. description, I think this would solve the issue for 95% of the cases and for the remaining 5% users would have the fallback to the standard spec definition.

paolostyle avatar Oct 02 '24 12:10 paolostyle

@adeyemialameen04 It is possible, openApi middleware accepts 3rd argument where you can pass any operation-level properties, you'd have to add the security component to createOpenApiDocs overrides (this name is probably not ideal) field, though. You can pass tags, summary etc. there, too. However for now I would not recommend using my library in production as it's still evolving and there might be breaking changes between the versions at least until 1.x version is released.

@maou-shonen Thanks for the feedback, I like this API a lot, just a couple of concerns, but I think they're all solvable:

* we wouldn't be able to enforce usage of the `oapResponse` (I would change that name, `oap` is a strange abbreviation, it would be probably one of `openApiResponse`/`oapiResponse`/`openApiRes`/`oapiRes`) middleware and if it's missing, we won't be able to generate the document. We could probably just `console.error` it when calling `createOpenApiDocs`, though.

* `description` is a required field for responses, so we'd have to provide default ones based on the status code when used like this: `oapResponse(200, defaultResponseSchema)`. Not a huge issue, but I think `description`, `headers`, `links` and `summary` fields (all of them are part of the OpenAPI spec for response object type) should be passable through a 3rd argument.

* In your `403` example the content type is `text/plain`. It is not possible to provide the media type just through a zod schema. Currently I'm assuming it's always `application/json` unless specified through `mediaType` property. Even though it would be non-standard, I'd lean towards providing it through 3rd argument, like e.g. `description`, I think this would solve the issue for 95% of the cases and for the remaining 5% users would have the fallback to the standard spec definition.

Thanks for your feedback @paolostyle Anticipating v1

iatomic1 avatar Oct 02 '24 13:10 iatomic1

@paolostyle

  • I'm not a native English speaker, so feel free to modify any names if you think they don’t sound quite right.

  • In my opinion, oapiResponse shouldn’t be a required field. If a developer doesn’t provide an oapiResponse, I take it as a sign that the developer considers the response unimportant. In that case, we can insert a placeholder value to comply with the OpenAPI specification. However, we can't be 99% sure this is what the developer intended (it could also just be an oversight). As you mentioned, I think it would be useful to add a defaultResponse option in createOpenAPI to enable or disable this behavior. As an example, in one of the projects I manage, we use a CQRS-style API, which makes about 80%-90% of the responses unimportant, as they only check if the status is 2xx.

  • description, similar to oapiResponse, in my mind, should be provided as an option in the third parameter and be optional.

type StatusLike = 'default' | '2xx' | '3xx' | '4xx' | '5xx' | number
type OapiResponsePropertySpec = {
  type: ?,
  schema: ?,
  ...etc
}
type Options = {
  description?: string, // maybe default ""
  examples?: any[],
  ...etc
}
type oapiResponse = (status: StatusLike, schema: z.ZodTypeAny | OapiResponsePropertySpec, options?: Options) => Middleware
  • 403 example, you're right, a string schema shouldn't default to text/plain, but an object could default to JSON.

I haven’t thoroughly read every detail of the OpenAPI spec, so my example doesn’t fully account for how to comply with the spec. If you find any issues, you’re probably correct.

maou-shonen avatar Oct 02 '24 15:10 maou-shonen

I just released v0.2.0 which is a pretty large API change. I considered @maou-shonen suggestions, but I felt like defining everything in a single object would be simpler and flexible enough. I don't really have an issue with createRoute API, although like I mentioned I don't like the verbosity of it. I also added fairly solid test suite though it omits basically the entire logic handled by zod-openapi.

See the README for a full documentation of the new API and release notes for a breakdown of changes.

The new API looks more or less like this:

// defineOpenApiOperation is just a TS helper - you can just inline an object in the middleware, too
const operation = defineOpenApiOperation({
  // all operation OpenAPI fields are supported here
  tags: ['User'],
  // all of these are supported!
  responses: {
    // shortest form - recommended whenever possible
    200: z.object({ name: z.string() }),
    // flattened form - recommended if more details are required
    400: {
      schema: z.object({ message: z.string() }),
      description: 'Custom description', // optional
      mediaType: 'application/xml', // also optional, mediaType is 'application/json' by default or text/plain when schema is a z.string()
    },
    // zod-openapi form - if you want to be as close to spec as possible
    401: {
      description: 'Required description',
      content: {
        'application/json': {
          schema: z.object({ message: z.string() }),
        },
      },
    },
    // just a regular OpenAPI spec - I guess you can use it if you just hate zod and your life
    404: {
      description: 'Not found',
      content: {
        'application/json': {
          schema: {
            type: 'object',
            properties: {
              message: { type: 'string' },
            },
            required: ['message'],
          },
        },
      },
    },
  },
  request: {
    // takes care of `requestBody` and validates like zod-validator
    json: z.object({ email: z.string() }),
    // just takes care of the docs, i.e. it will end up in the `parameters` field and won't validate
    cookie: {
      schema: z.object({ session: z.string() }),
      validate: false,
    },
  },
});

const app = new Hono().post('/user', openApi(operation), async (c) => {
  const { name } = c.req.valid('json');

  return c.json({ name }, 200);
});
This will result in this JSON document:
{
  "info": {
    "title": "Example API",
    "version": "1.0.0"
  },
  "openapi": "3.1.0",
  "paths": {
    "/user": {
      "get": {
        "parameters": [
          {
            "in": "cookie",
            "name": "session",
            "required": true,
            "schema": {
              "type": "string"
            }
          }
        ],
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "properties": {
                  "email": {
                    "type": "string"
                  }
                },
                "required": ["email"],
                "type": "object"
              }
            }
          }
        },
        "responses": {
          "200": {
            "content": {
              "application/json": {
                "schema": {
                  "properties": {
                    "name": {
                      "type": "string"
                    }
                  },
                  "required": ["name"],
                  "type": "object"
                }
              }
            },
            "description": "200 OK"
          },
          "400": {
            "content": {
              "application/xml": {
                "schema": {
                  "properties": {
                    "message": {
                      "type": "string"
                    }
                  },
                  "required": ["message"],
                  "type": "object"
                }
              }
            },
            "description": "Custom description"
          },
          "401": {
            "content": {
              "application/json": {
                "schema": {
                  "properties": {
                    "message": {
                      "type": "string"
                    }
                  },
                  "required": ["message"],
                  "type": "object"
                }
              }
            },
            "description": "Required description"
          },
          "404": {
            "content": {
              "application/json": {
                "schema": {
                  "properties": {
                    "message": {
                      "type": "string"
                    }
                  },
                  "required": ["message"],
                  "type": "object"
                }
              }
            },
            "description": "Not found"
          }
        },
        "tags": ["User"]
      }
    }
  }
}

Obviously my question from the original post still stands.

paolostyle avatar Oct 06 '24 21:10 paolostyle

Hey @paolostyle thanks for using my library! I stumbled upon this thread after many a user from here have reported bugs with @hono/zod-openapi due to the underlying issues in zod-to-openapi.

If you have a look at the contributors for zod-to-openapi, I used to contribute alot to that repo, but I felt it was very difficult to convince them to change how it was implemented so I re-wrote my own version from scratch 😄. There are many fundamental issues in that library which I have attempted to solve in my own implementation.

samchungy avatar Nov 04 '24 03:11 samchungy

Ha, thank you for creating it! 😄 I definitely felt like your library would work better for the problem I was trying to solve as it completely eliminates the need for having some custom class that I'd likely have to manage internally considering the DX improvements I wanted to include. It gives much more control to the end user. So really all the heavy lifting in my library is done by zod-openapi, I just glued things together so it's nice to use and as compatible as possible with the "vanilla Hono/non-OpenAPI router.

On another topic, considering I already invested quite a bit of time into setting up CI, release pipelines, etc. and there was no response in this thread, I think I'll continue maintaining the library in my own repo. However, I'm obviously open to discussing things. Ultimately I'd like everyone to have the best experience possible.

paolostyle avatar Nov 04 '24 09:11 paolostyle