Option to disable the type coercion about openapi-backend HOT 9 CLOSED

Would you like this to be a constructor parameter? Right now you can just:

const api = new OpenAPIBackend(opts);
await api.init();
api.validator.ajvOpts.coerceTypes = false;

EDIT: this was my intention, but the example above actually doesn't work because the validation functions are built during .init() and that's when the ajvOpts are read. You can't set ajvOpts any earlier either because the validator object is created on .init(). Sorry!

from openapi-backend.

Thank you for your time. I did not notice the above was possible, but since there is already an option object, it feels like it would be cleaner to add it to the constructor.

I thought a bit more about this, and I think there is several approach possible for this, some of them requiring more work, but offers greater flexibility:

1. Keep a simple boolean for the type coercion and add it as a constructor parameter.

2. Allow a function to be passed in order to enable or disable the coercion per operation.
   I imagine using it like this: { coerce: function (operationId, operation) { [...]; return true;}}

3. Allow a function to be passed in order to enable or disable the coercion per operation and per "in" type to be sanitized. ("query", "header", "path", "cookie" in the openApi spec and "body"?)
   For instance, I would like to have type coercion on my path/query parameters, but I would like to avoid having it in the body.
   I imagine using it like this: { coerce: function (operationId, in, operation) { [...]; return true;}}
   This actually requires more work because currently only one function is compiled and checks everything in one go, but that would have to be split into 5 sanitization functions. Since the objects to sanitize will be smaller, I don't expect this to have a noticeable performance impact.

I think that the 3rd option is the best, but I would love to hear your thought on this.

I'd be glad to work on a PR for any of the above :).

from openapi-backend.

I like the third option as well for the flexibility. It definitely makes sense to coerce types for path params, query and headers, but not request body. Unfortunately that means we need multiple validation functions, but I'm sure there should be almost no performance hit on that. Just slightly more complexity.

Here's what I'm thinking. Let's add a global ajvOpts option in the constructor. This will be the default ajv opts for all validations.

By default, in: path, header, query, cookie should all have type coercion enabled (as an override for the default ajvOpts) whereas for requestBody, coercion should be whatever what was given in the ajvOpts constructor opt (default: false).

What do you think? I'm a little hesitant to add constructor options to allow setting coercion opts for each operation and parameter type individually due to the complexity.

from openapi-backend.

Rather than just control the coerce option for all operations and parameters with a function, it would maybe make sense to do a function similar to your suggestion, which can be used to override the default behavior in buildSchemaForOperation.

It would receive the operation object and should return a list of compiled Ajv validation functions that should be run for all requests in the operation.

Here's a simplified example:

const api = new OpenAPIBackend({
  definition,
  customValidation: (operation, Ajv, defaults) => {
     // perhaps you only want custom validation for a single endpoint
     const shouldUseCustom = operation.operationId === 'myCustomEndpoint';
     if (!shouldUseCustom) {
       // just use the default validation functions
       return defaults;
     }

     // ... build your own schemas (schema1, schema2) from the operation
     
     const ajv1 = new Ajv({ coerceTypes: true });
     const validate1 = avj1.compile(schema1);

     const ajv2 = new Ajv({ coerceTypes: false });
     const validate2 = avj2.compile(schema2);

     return [validate1, validate2];
  },
});

from openapi-backend.

I like the third option as well for the flexibility. It definitely makes sense to coerce types for path params, query and headers, but not request body. Unfortunately that means we need multiple validation functions, but I'm sure there should be almost no performance hit on that. Just slightly more complexity.

Here's what I'm thinking. Let's add a global ajvOpts option in the constructor. This will be the default ajv opts for all validations.

By default, in: path, header, query, cookie should all have type coercion enabled (as an override for the default ajvOpts) whereas for requestBody, coercion should be whatever what was given in the ajvOpts constructor opt (default: false).

What do you think? I'm a little hesitant to add constructor options to allow setting coercion opts for each operation and parameter type individually due to the complexity.

With that option I guess most use cases are covered. I really like the fact that it would be super simple to use.

Rather than just control the coerce option for all operations and parameters with a function, it would maybe make sense to do a function similar to your suggestion, which can be used to override the default behavior in buildSchemaForOperation.

It would receive the operation object and should return a list of compiled Ajv validation functions that should be run for all requests in the operation.

Here's a simplified example:

const api = new OpenAPIBackend({
  definition,
  customValidation: (operation, Ajv, defaults) => {
     // perhaps you only want custom validation for a single endpoint
     const shouldUseCustom = operation.operationId === 'myCustomEndpoint';
     if (!shouldUseCustom) {
       // just use the default validation functions
       return defaults;
     }

     // ... build your own schemas (schema1, schema2) from the operation
     
     const ajv1 = new Ajv({ coerceTypes: true });
     const validate1 = avj1.compile(schema1);

     const ajv2 = new Ajv({ coerceTypes: false });
     const validate2 = avj2.compile(schema2);

     return [validate1, validate2];
  },
});

I guess you're just going to call all validations functions in sequence?
This is nice because it gives complete control, but as a lib user I don't really fancy rewriting the logic to extract the schema from the operation. Adding InputValidationSchema to the list of parameters would make it easier.
For example in order to disable the coercion on the body, one could do that:

const api = new OpenAPIBackend({
  definition,
  customValidation: (operation, inputValidationSchema, Ajv, defaults) => {
     // perhaps you only want custom validation for a single endpoint
     const shouldUseCustom = operation.operationId === 'myCustomEndpoint';
     if (!shouldUseCustom) {
       // just use the default validation functions
       return defaults;
     }

     // ... build your own schemas (schema1, schema2) from the operation
     const originalRequestBodySchema = inputValidationSchema.properties.requestBody;

     const ajv1 = new Ajv({ coerceTypes: true });
     inputValidationSchema.properties.requestBody = undefined;
     inputValidationSchema.properties.required = inputValidationSchema.properties.required.filter((e) => e !== 'requestBody');
     const validate1 = ajv1.compile(inputValidationSchema);

     const ajv2 = new Ajv({ coerceTypes: false });
     inputValidationSchema.properties = { requestBody: originalRequestBodySchema };
     inputValidationSchema.properties.required = ['requestBody']);
     const validate2 = ajv2.compile(inputValidationSchema);

     return [validate1, validate2];
  },
});

I feel having one of these solutions would be enough, even though having both would make for nicer default in the second one.

from openapi-backend.

I agree. It's not super great having to build the schemas from scratch every time.

I've now implemented the first part of this in #8. Right now the default is split to two schemas, one for the params and another for the payload.

I like your idea of building just one big input schema and omitting properties for each use case. Maybe that's the way to go.

from openapi-backend.

Thank you very much !
If you want any help let me know.

from openapi-backend.

I'll try to publish a minor version bump with these changes today. I'll try to get a couple more related issues (mainly #9) in there as well :)

from openapi-backend.

@akatsukle This fix is now part of [email protected] !

from openapi-backend.