coerceComponents feature for middleware in order to cast data

[pilerou]

This pull request adds an attribute to middleware configuration : coerceComponents.
It allows to cast :

• some string to objects (Date, MongoDB ObjectId) on request
• objects to string (or other type) on response

I added an example project and a suggestion for README.MD

[pilerou]

This pull request could close #353 #465 #288 #246

[electrotype]

Sadly, doesn't seem to work for me for a date-time field...

In your tests, please add a case where you validate a response consisting of an object with a property of type Date and it passes the validation as a date-time field in the Open API spec.

[cdimascio]

@pilerou it will be great to add test cases that at least cover the three examples you highlight in the example packages

here is an example test file to help get you started. add a similar test file for these use cases

[pilerou]

Sorry for the first version. It still had a bug. I just fixed it, I modified the example to send a date.
I also added a test file for this feature that send a response with :

• an ObjectID id,
• a creationDate in yyyy-mm-dd
• a creationDateTime in ISO date format

[electrotype]

Thank you so much for this PR @pilerou . Merci beaucoup!

I think I understand how your code works. You can tell ajv to run a specific code for a given schema, is that correct?

I've been able to make your code work in my project, but with a change on my side.

In my spec file, this doesn't work:

components:
  schemas:
    User:
      properties:
        creationDate:
          type: string
          format: date-time

But this does work:

components:
  schemas:
    DateTime:
      type: string
      format: date-time
    User:
      properties:
        creationDate:
          $ref: '#/components/schemas/DateTime'

In other words, you need to define a schema for the components you want to tweak, is that correct?

Do you see any way to make your new feature work with my first exemple too? Don't get me wrong, I think your PR should be merged now, and I will definitively use its coerceComponents feature! But being able to validate an inline string/date-time type would be "la crème de la crème"!

[pilerou]

I agree with you about the perfect solution that would be to use the format or pattern attribute.
Unfortunatly, AJV doesn't allow to coerce on this field.
I didn't manage to override format keyword.

At the end, I convinced myself that it's finally a good practice to have a generic component type definition for Date, Date-time or ObjectId... :)

The feature can solve many issues but I do agree that it's not very intuitive for beginner on this module. I didn't find a better way to do it easier. :S

[electrotype]

I think this is an acceptable solution (maybe even the only one)! I would suggest that there is a small section in the documentation to explain it, though.

In issue #288 I said I would give a bounty for a fix so, as soon as the PR is merged, let me know what you want me to do with the money. Send me your Paypal address if you want. Thanks again!

[pilerou]

I didn't see for the bounty purposal.
It's a pleasure if someone can use this feature.
Thanks for the money but I prefer to keep the opportunity to ask your help on this module when I'll need to :)

[electrotype]

The money will go to the project itself then ("Sponsor" link). Cheers!

[cdimascio]

@electrotype thank you :) very generous.

once it's merged in, i will plan to reroute the full bounty to @pilerou. he did the work and deserves it.

@pilerou DM me on twitter or linkedin, so we can sort out how to get it to you.

[cdimascio]

@pilerou, we have serialize/deserialize backwards

serialize should convert the in-memory object e.g. Date to a string

deserialize should take a string and convert it to Date

in these scenarios, deserialization occurs on the value provided in the request e.g. a string representing a date or mongo object id, etc. serialization will occur on a response when converting an object e.g. Date or ObjectId to a string

[cdimascio]

review notes:

• rename coerceComponents to schemaObjectMapper
• serialize should have form object to string
• deserialize should have form string to object

[pilerou]

Thanks for the review. I change it and I push.

[pilerou]

I did the changes :

• Changes coerceComponents to schemaObjectMapper
• Fix function names serialize and deserialize => To be clear for everybody on which functions are executed on requests and responses, it's now deserializeRequestComponent and serializeResponseComponent
  => Unit tests seem OK and I also changed documentation, files name... I tested at home the example. It's also OK "on my computer"

[cdimascio]

@pilerou i made modifed the types to use serialize deserialize and updated the README to be more description about the usage. let me know if it seems clear or if i should add more detail to ensure folks know when to use serialize vs deserialize

[cdimascio]

while making these modifications, another question arose.

what is the behavior if a user sets validateResponses to false, but defines schemaObjectMappers?

[pilerou]

@cdimascio All modifications are good to me. Your english is also much better than mine 👍
I tried with validateResponses to false.
As I thought, It's not working. serialize function is called on response schema validation process. If there's no schema validation on response, there's no serialization.

I see 2 options :

• When schemaObjectMapper is set and validateResponses or validateRequests aren't, we force those attributes to true at middleware creation
• We add a comment explaining validateResponses must be set to true.

I prefer the first solution. What do you prefer ?

[cdimascio]

@pilerou gotcha. in that case, it's best to just throw an error right away and let the user sort out how to proceed.

To throw this error, the validation check can be done here.

All in all, if either validateRequests or validateResponses is not enabled, throw an Error with a message like "schemaObjectMapper requires that the validateRequests and validateResponses options are both enabled."

Also, do you see any way that we could provide this feature without requring validateResponses to be enabled?

There are many users who enable validateResponses only in their dev environments, then disable it in prod. This is different from validateRequests, where users always have it enabled in all environments.

[pilerou]

I understand.
I think about it and I try to find a solution for validateReponses. No time this week to work on it but I hope for the end of 2020 😁

[cdimascio]

Thanks. I'll think on it as well.

[cdimascio]

@all-contributors add @pilerou for code, test, and ideas

[allcontributors]

@cdimascio

I've put up a pull request to add @pilerou! 🎉

[cdimascio]

@pilerou @electrotype
i was thinking about this a bit more. the issues only arises only when validateResponses is enabled. The reaseon for this is because repsonse validation occurs on the in-memory representation of the response object prior to that object being sent to the client via e.g. res.json.

Thus, when the response validation runs, the Date object is still a Date object, thus the validator sees an object, yet expects a string, thus it returns a validation error. This is the core of the problem.

On the other hand, when validateResponses is disabled, the schema is never validated, thus e.g. res.json is called on the response object, the normal JSON serialization kicks in and Date object becomes a string.

An easy way to solve this, is to serialize the response object prior to validation e.g.

body = JSON.parse(JSON.stringify(body));
const valid = validator({
  response: body,
});

This solves the problem. However, when validateResponses is enabled, it requires serialization of response object twice. Once before validation, then when res.json is invoked.

This approache does not provide any control over the string representation. You basically get whatever new Date().toJSON() returns. i suppose one could monkey patch it if they really wanted to e.g.

Date.prototype.toJSON = function () {
  return 'my date';
};

Here is the code: #498. This doens't require mappers but has the side effect i noted above

It would be nice to handle this with something like the above, but somehow avoid the double serialization. It's simpler, doesn't complicate the API/options, and the response is returned whether or not validateResponses is enabled

@pilerou's solution is more robust, but there are behavior differences in the way respnses are handled depending on whether validateResponses is enabled. Perhaps, if there is a way to overcome this limitation, we can consider proceding with his solution

[electrotype]

body = JSON.parse(JSON.stringify(body));
This solves the problem. However, when validateResponses is enabled, it requires serialization of response object twice. Once before validation, then when res.json is invoked.

@cdimascio This is exactly what we are trying to avoid! :-) See #288 (comment) . Yes it works, but you then se/deserialize every response, with or without dates, simply because it is required for this validation to work.

The other "workaround" is for the application code itself to know that it has to convert the date properties before sending the responses. It is faster because it knows the entities so can convert specific properties without se/deserializing everything. I really do not like this solution because the application would do this simply because express-openapi-validator is used. This is way too tightly coupled!

From what I understand of the code, the real solution would be the make a PR to ajd for it to not validate the type (string/object) of a property so early and in a so hardcoded way.

If you are able to hack ajd to prevent that type validation to be done so early, that would be impressive and is clearly a good challenge.

[cdimascio]

@pilerou i refactored the schema preprocessor to traverse the request and response schema in one pass. it effectively traverses one schema, but allows updates to both along the way.

What's improved?

This improvement makes it possible to associate a custom keyword with any schema in the spec (rather than only those we register via property like schemaObjectMapper). By hooking into the preprocessor, we can inject the keyword and associated function onto any schema in the spec. This means a user can simply write the following:

type: string
format: date-time

When the preprocessor visits a schema, we can check... "does type === 'string' && format === 'date-time'? If so, inject the keyword and associate the function.

####Benefit
It's idiomatic. Just use OpenAPI as you expect.
This is the behavior that @electrotype was expecting and likely the behavior users will first expect)

The PR code

PR #499 implements this improved preprocessor.
(Note, the PR is large, but all you need to be concerned with is one method, handleSerDes.

I'm hoping that you might fork the PR branch and integrate something similar to what you've done. One difference being that the user can define the types as usual (just specify type and format.

What's Next? - How can @pilerou help?

It would be awesome, if you could take the essence of what you've done and incorporate it into the preprocessor. Here's what I'm thinking

For formats defined in the OpenAPI spec (date-time, full-date), do the following:

1. Implement the keyword in a manner similar to what you've done in ajv/index.ts
   Note: this code is not needed

2. In handleSerDes, check if e.g. type: string format: date-time is present, if so, assign the keyword + function,
   Note: The argument state.kind will have a value req or res. This tells you whether you are modifying the request's or response's api doc

3. If we can do this with a serializer only. (see my note at the bottom of this comment) then implment the serializer for Date. It should be automatically used anytime type: string format: date-time is encounted. We could also create our own EovDate that implements the appropriate toJSON()/toString() to ensure the proper date format is serialized by express.

   If we require a serializer and deserializer then, we'll require the user to provide both function via an option like schemaObjectMapper

For formats not defined in the OpenAPI spec e.g. MongoDb ObjectId

1. An optoin is required. Something like schemaObjectMapper might be used to enable users to specify the custom functions. Note: if we can do this with a serializer only (see my note at the bottom of this comment), then we don't need the user to provide a deserializer.
   It will also be interesting to consider the unknownFormats property. Currently, it takes a list of strings specifying the formats for AJV to ignore. Perhaps, this could take a list of objects, where each object contains the name and the serializer function. (just a thought)

2. In handleSerDes, check if e.g. type: string format: my-mongo-objectid is present, if so, assign the keyword + function.

   A user might specify a mongo object id custom format like so e.g.
   Note: this is nice as its idiomatic

   type: string
   format: mongo-objectid

   and we can trigger the custom ser/des by matching type: string format: mongo-objectid (or whatever the custom format name is)

Thoughts on specifying serializer only

If we can require only a serializer be provided, we can simply the whole experience.

As i noted above, we may be able to do this using only a serializer (no deserializer). AJV validaties JSON-schema. In AJV, I believe we can validate a Date of type object. For example, if we get Date object today. It fails validation because the schema states type: string. With the new preprocessor, we could change type: string to type: object. In theory, this would validate the Date object, assuming it passes, it would serialize using its toJSON() method (just as it does without validation). The nice thing here, is that we get the same serialization whether or not we usevalidateReponses. To implement different Date formats (rather than what Date.toJSON() returns, we could have the keyword function store the Date in custom Object and attach toJSON() and toString() method that serializes the Date using the appropriate representation (defined by format

Finally

@pilerou, Continue on this and the bounty is certainly yours. I will add to it as well :)
@pilerous, @electrotype, please provide thoughts/feedback if you have any

[cdimascio]

Good news! we can definitely do this with a single serializer. Example here using date-time format (which ultimately will be baked in)

@pilerou, i'll work to get the date-time, full-date formats added as built-ins. Feel free to take a stab at mongo's objectid> This will require a user option, perhaps a good place to put these options is validateResponses.serializers? thoughts?

Note: there is a backward compatibility issue for dates that were provided as string... will think on how to ensure both can work

[electrotype]

Feedback left on #499
Thanks to both of you, again.

[cdimascio]

@electrotype have a look at #499
I'm thinking this may be the final form. (See the README changes in this PR for the usage description). Note that the new behavior is opt-in. This will ensure existing scenarios continue to work as is.

I'm curious to get your thoughts for the next major version. Should this be opt-in? or default behavior?

Note that its essentially either or. if the serializer is enabled, then you MUST specify Date objects in your responses if the serializer is enable for date and/or date-time. if the serializer is not enabled, then you MUST specify a date as a string. One could enable the serializers, then specify date strings as long as format is not specified. In any case, i'm interested to get your thoughts.

Happy Holidays!

[electrotype]

Happy Holidays to you too.

First, excuse me because I know I'm not as deep into the code itself as you (both) are. I currently do not have the time to invest in understanding everything involved in this issue, as much as I would like. Sorry about that, I may try to contribute more to this project one day.

I understand your question and why you are not sure about the best way to go. If I enable the Date serializers, then I could not send a Date as a string anymore:

res.send({
  created_at: '2020-12-25T03:36:34.706Z';
});

And I agree this is not the ideal. It is confusing. I think maybe the root issue of all that is that the validation is not made on what is actually sent to the client (a Date will always result in a string at the end!), but on the raw response your controllers (or whatever) are sending in the first place...

My random ideas/notes:

1. Would it be possible to make the validation run on the serialized response instead of on the raw response? So later in the process of sending a response? I guess this would require an extra deserialization though.

2. Instead of converting the type of a Date from string to object as the main trick for the validation to work, couln't you, in a way or another, use some kind of oneOf (also: https://swagger.io/docs/specification/data-models/data-types/#mixed-type ), in order to specify that a raw Date property can be both a string or an object?

3. In addition to converting the type of a Date from string to object as the main trick for the validation to work, could you also create a new schema on the fly and use this schema to allow both string and object?

4. Question: Does the original solution (from @pilerou) work with strings too? If I create a DateTime schema instead of defining an inline format: date-time, could I send Dates as strings? I didn't check.

Dates really are a tricky type in Javascript/JSON!

[cdimascio]

1. is difficult to get correct, particularly given response are streamed.
2. yes! we can use the mixed variant that OpenAPI notes as incorrect. It's validate JSON schema thus will work
3. 2 above will do it
4. no, same problem (but it could work with 2 above)

all in all, we can do 2. are remove the serializer option. Note that option will likely return if @pilerou adds support for custom serializers, so that folks can handle e.g. Mongo ObjectIds and more

[cdimascio]

@all-contributors add @electrotype for ideas

[allcontributors]

@cdimascio

I've put up a pull request to add @electrotype! 🎉

[electrotype]

Can't wait to see the final PR! You guys are doing an incredible job... Listening to your users and such. Cheers!

And don't worry about the bounty and where it will go, just make sure the solution is the best one. I will make sure everybody involved here has my official "thank you very, very much" appreciation... The hard part is sometimes simply to make things move. :-)

[cdimascio]

v4.10.0 is live and provides a solution for date-time and date. It does not require any user configuration.

@pilerou it will be fantastic if you can add the functionality for ObjectId once you're time frees up again.

A path toward a solution might look something like this:

1. Create a new option validateResponses.serializers. It's shape will look similar to the original schemaObjectMapper (perhaps a list?) e.g.

   validateResponses: {
     serializers: [{
        format: 'mongodb-objectid',
        serializer: (o) => o.toString(),
     }]
   }

2. Modify handleSerDes to handle custom serializers for a registered format whose input is a JavaScript object, not a string e.g. MongoDb ObjectId

[electrotype]

I'll be trying 4.10.0 very soon. Thank you!

[pilerou]

Hi.
Things are moving forward. That's great.
I'm looking to changes you made.
I'm not sure the new mecanism allows the same. Could you confirm ?

Questions

• Does it also deserialize fields on request ? When I get a string in request with date-time format, does it cast the string to Date object ?
• For ObjectId components, I don't describe it with format description field. I use pattern field. New PR code use format field only. We could add other field (such as pattern) each time we need to but it will get heavier and heavier.

**Implementation examples : **

• For ObjectId :
  • I'll need to add an unknownFormats for mongodb-objectid ?
  • Then I'll need to add a new option to validateResponses as described in your solution ?
  • Do I need to configure something on request validation ?
• I call a request with objectId in url : /user/5fe65b0dc4a82ae0cb4d914a
  ==> With no deserialize function, I'll have a string in my route function or I'll have (and don't forget) to cast it to ObjectId in my route function. That's what I wanted to avoid.

Suggestion

• I think we should configure Serialize/deserialize as a Map instead of an Array. We can only add on serialize function for each format. With the "Array" conception, it would allow users to add many configuration for a format. I began to make some changes that make it more generic. The module would configure basic serializers for date and date-time and we could add new serializers in ValidateResponseOpts. Thos serializers would be added or could override basic serializers.

Here how I changed it in schema.preprocessor.ts (I also changed types.ts to make it compile) :

export class SchemaPreprocessor {
  private ajv: Ajv;
  private apiDoc: OpenAPIV3.Document;
  private apiDocRes: OpenAPIV3.Document;
  private responseOpts: ValidateResponseOpts;
  private serializers: { [p: string]: Serializer; };
  constructor(
    apiDoc: OpenAPIV3.Document,
    ajvOptions: ajv.Options,
    validateResponsesOpts: ValidateResponseOpts,
  ) {
    this.ajv = createRequestAjv(apiDoc, ajvOptions);
    this.apiDoc = apiDoc;
    this.responseOpts = validateResponsesOpts;
    if(this.responseOpts && this.responseOpts.serializers)
    this.serializers = {
      ...basicSerializer,
      ...this.responseOpts.serializers
    }
  }

const basicSerializer = {
  'date-time': {
    serialize: (d: Date) => {
      return d && d.toISOString();
    },
  },
  'date': {
    format: 'date',
    serialize: (d: Date) => {
      return d && d.toISOString().split('T')[0];
    },
  }
};

private handleSerDes(
    parent: SchemaObject,
    schema: SchemaObject,
    state: TraversalState,
  ) {
    if (state.kind === 'res') {
      if (schema.type === 'string' && !!schema.format && this.serializers[schema.format]) {
        schema['x-eov-serializer'] = this.serializers[schema.format];
      }
    }
  }

[cdimascio]

Ki@pilerou

Does it also deserialize fields on request ? When I get a string in request with date-time format, does it cast the string to Date object ?

The new feature does not affect the request at all, thus

• it does not require a user option (and remains backward compatible)
• it does not attempt to deserialize any value in a request
• it also does not block the addition of deserializers. this is still possible.

For ObjectId components, I don't describe it with format description field. I use pattern field. New PR code use format field only. We could add other field (such as pattern) each time we need to but it will get heavier and heavier.

• The current code only checks format, however, pattern could also be checked. Note that the new implementation did not add or change any user-facing options or APIs. Thus, the shape of the new options / API can be decided here. I'm certainly open to ideas.

**Implementation examples : **

For ObjectId :

• I'll need to add an unknownFormats for mongodb-objectid ?
• Then I'll need to add a new option to validateResponses as described in your solution ?
• Do I need to configure something on request validation ?

I call a request with objectId in url : /user/5fe65b0dc4a82ae0cb4d914a
==> With no deserialize function, I'll have a string in my route function or I'll have (and don't forget) to cast it to ObjectId in > my route function. That's what I wanted to avoid.

This is currently the case.
The first bullet could be solved by automatically adding any new format to the unknownFormats list.
The second is true, given my proposed solution above.
The third, would require some registering a deserializer on the request.

The options are basically

validateRequests: [{
  format: 'mongodb-objectid':
  deserializer: (s) => ObjectId(s)
}]

vs

serdes: [{
  format: 'mongodb-objectid',
  deserializer: (s) => ObjectId(s),
}]

for a user with both request and response validation enabled

validateRequests: [{
  format: 'mongodb-objectid',
  deserializer: (o) => o.toString()
}]

validateReponses: [{
  format: 'mongodb-objectid',
  serializer: (o) => o.toString()
}]

vs

serdes: [{
  format: 'mongodb-objectid',
  serializer: (o) => o.toString(),
  deserializer: (s) => ObjectId(s),
}]

Okay, @pilerou I'm with you, option 2 is better :)
Also, I'm not ruling out a map, would be nice to see an example of what that might look like

[cdimascio]

@pilerou @electrotype Another thought to consider...

for requests:

express does not perform any special deserialization of requests. For example, if you provide a string representation of a Date or ObjectId, a string is received in the express request handler.

for responses:

similarly, express does not perform any complex serialization, it just calls toJSON()/toString()

Next steps

Perhaps, we should start by solving this problem. That is, when enabling validateResponses, we provide the base express behavior described above. This, at the least, provides a consistent experience without requiring user options

This would be a great first step.
Once in place, we could start the work to add custom (de)serializers.

As we've dive deep into these discussions, to me, it seems the custom (de)serializers are a nice-to-have, while serializing an e.g. ObjectId to its default string representation is a must-have. The reason I feel this way is because this is the behavior express exhibits. It will be great to provide that out of the box (without registering a serializer). A user would, however, have to declare something in options i.e. a format. If this is in place, I wonder if the need for a custom serializer is as necessary?

By implementing this as a first step, a user would just list the format, then e.g. ObjectIds would serialize according to its toString(). This would meet one of your two remaining needs. Request deserialization could be considered next.

Implementation thoughts

Spittballing here, but this first step could be implemented by adding the type, object, to any declared unknown types. For example,

ObjectId could be declared in the spec as

type: string
format: mongodb-objectid

Then, the preprocessor would transform the schema to

type: ['string', 'object']
format: mongodb-objectid

The serialization could be applied anytime the object variant is seen by the validator.

This benefit here is that no new options are necessary.

Thoughts?

[electrotype]

@pilerou I think the decision made by @cdimascio is the good one. Modifications were quite simple and they address one use case perfectly. The scope of what you suggest is larger and, probably for some, more complete.

Anyway, I'm very happy with the developments you both made, thank you again.

Since you do not want the bounty (or do you? Let me know if you changed your mind), can I ask you to pick a charity? I will send them the money and will post a proof here.

Good luck with your own use cases and with the continuation of those features!

[pilerou]

Thanks for your responses. I also think you made the good solution. I work on request mecanism and other custom formats and I make a new proposition.
I don't use to contribute on github project :

• Should I delete my fork and do a new fork from Master?
• Then should I create a new PR or continue on this one?

[cdimascio]

It may be simplest to start a new fork. That said, the PR that I merged in is based on your original work, thus, you could merge master into this PR's branch. It likely won't run into conflicts. Either way will work. Up to u.

[electrotype]

@pilerou Without a suggestion from you, I'm going to give the money to some charity that saves giraffes in Alaska or something.

[pilerou]

Thanks @cdimascio. I do that.
@electrotype It's very sympathetic. I really appreciate. Actually, I would feel guilty to accept money. Carmine does a wonderful job and I did a very little thing. I let you decide if you give to the last giraffes in Alaska :) or if you give the money to Carmine or if you keep the money for the next time you come in Brest (France) to drink a beer with me :) It would be a pleasure.

[electrotype]

https://www.buymeacoffee.com/m97tA5c/c/631986
@cdimascio , please take care of the giraffes.
@pilerou Je te fais signe si je vais en France! Merci encore pour ton travail!