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

Skip Validation #109

Merged
merged 2 commits into from
Mar 2, 2024
Merged

Skip Validation #109

merged 2 commits into from
Mar 2, 2024

Conversation

rmzaoo
Copy link
Contributor

@rmzaoo rmzaoo commented Dec 18, 2023

I modified the code to skip the parse validation function and allow unknown parameters in the request body. This was done because we couldn’t find any other solution.
Did I miss something in the documentation? Is there a way, with the current implementation, to allow unknown parameters in the request body structure without skipping the parse validation function?

Copy link

github-actions bot commented Dec 18, 2023

🧪 A prerelease is available for testing 🧪

You can install this latest build in your project with:

npm install --save https://prerelease-registry.devprod.cloudflare.dev/itty-router-openapi/runs/8122549266/npm-package-itty-router-openapi-109

Or you can immediately run this with npx:

npx https://prerelease-registry.devprod.cloudflare.dev/itty-router-openapi/runs/8122549266/npm-package-itty-router-openapi-109

@G4brym
Copy link
Member

G4brym commented Dec 18, 2023

Hey @rmzaoo
if you are just trying to receive unknown properties in your endpoint request body, you should be able to do so by turning the property raiseUnknownParameters to false when creating your router, like this

    const router = OpenAPIRouter({
        raiseUnknownParameters: false,
    });

Did i understand your question right?

@rmzaoo
Copy link
Contributor Author

rmzaoo commented Dec 18, 2023

@G4brym I tried that but it didn't work.
I tried to debug why and i found out that that property is commented on the handler creation (src/openapi.ts:190).
Also, even when using that property, validation still runs (safeParse function on src/route.ts:252) and unknown properties are still removed from the body data

@rmzaoo
Copy link
Contributor Author

rmzaoo commented Dec 20, 2023

@G4brym Any news about it?

@G4brym
Copy link
Member

G4brym commented Dec 20, 2023

@rmzaoo for you to receive any parameters in the reques body, you will need to use Zod Catchall
Here's an example endpoint to showcase this

export class TestBody extends OpenAPIRoute {
  static schema: OpenAPIRouteSchema = {
    requestBody: z.object({}).catchall(z.any()),
  }

  async handle(
    request: Request,
    env: any,
    context: any,
    data: any,
  ) {
    return data
  }
}

router.post('/test', TestBody)

Then you will be able to send any parameters inside the body like this:
image

@rmzaoo
Copy link
Contributor Author

rmzaoo commented Dec 20, 2023

@G4brym Okey perfect, that's good.
Another thing, for some reason in route.ts:210 the if (queryParams) is true and an empty object is added to the unvalidatedData['query']. Further down it conflicts with the validationSchema.
I solved this by putting only schema.request?.query in the if, could you take a look at this?
(i used your example to test)

@rmzaoo
Copy link
Contributor Author

rmzaoo commented Dec 21, 2023

@G4brym Again about skipvalidation, after a few tests the parameters passed in such as path, query and header also do their validation.
A little bit more context about the project: This was implemented itty-router originally, and I'm currently migrating to itty-router-openapi to have propper documentation for the API. I already do my own validations in the handlers, that's why I don't want to be limited to the validation errors that zod presents before reaching the request handler. For these validations to be mandatory would result in validating my request 2 times and skipvalidation allows me to proceed without validating the entire schema again.

I would be ok with not merging this change, but, do you have any suggestion to skip it not only for body but for path, query and headers too?

@rmzaoo rmzaoo closed this Dec 21, 2023
@rmzaoo rmzaoo reopened this Dec 21, 2023
@G4brym
Copy link
Member

G4brym commented Dec 21, 2023

@rmzaoo sorry for the delay, I'm currently out of the office, I will only be able to merge this early January, meanwhile you can use this branch preview build by running this command npm install --save https://prerelease-registry.devprod.cloudflare.dev/itty-router-openapi/runs/7290903179/npm-package-itty-router-openapi-109

@danbars
Copy link

danbars commented Feb 28, 2024

Is this PR going to be merged?

@G4brym G4brym merged commit 978c412 into cloudflare:main Mar 2, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

3 participants