Router: validate input parameters

[bajtos]

Overview

In loopback/strong-remoting 3.x, we are validating input parameters against the types/schema defined in remoting metadata to ensure the client is passing valid values. We should offer similar capability in LoopBack.next.

Acceptance Criteria

• Write a validation function using AJV that accepts a JSON object (body) and a JSON schema that:
  • Validates a given object against a given JSON schema specification
  • return HTTP 422 if validation fails and return validation error
  • errors should be batched (please don't force users to resubmit requests over and over to find all of their validation issues!)
• Change parseParams in loopback-rest (not in DefaultSequence! look for the provider/function) to use the new validation code
• Update TSDocs
  • This function should retrieve the appropriate schema object from the OpenAPI v3 spec as provided by the routing decorator/api decorator, not the raw JSON schema metadata generated from model classes!
• Write documentation to describe the new default behaviour of Sequence
• Write documentation to describe how to leverage this new validation (via decorators)
• Write an awesome blog post about this cool stuff!

Notes

If we run into any edge cases that aren't trivial, we should create new issues to handle them and address them after the fact, instead of blocking this task.

[bajtos]

I am proposing to use AJV:

• it's based on JSON Schema, which is used by OpenAPI/Swagger too
• it's optimised for high performance

https://github.com/epoberezkin/ajv#coercing-data-types

[lehni]

I think the use of AJV for validation is a good idea. This then has me wondering if LoopBack.next couldn't optionally allow the definition of models in that format, to avoid redundancy? LoopBack's model.json format already kind of looks like a subset?

[bajtos]

This then has me wondering if LoopBack.next couldn't optionally allow the definition of models in that format, to avoid redundancy? LoopBack's model.json format already kind of looks like a subset?

I had the same idea in my mind too. Why to invent and maintain our own domain-specific language for defining models, when we can leverage an existing standard (JSON Schema)?

However, @raymondfeng, our architect, has a different opinion. IIRC, he is proposing a slightly different implementation, where LoopBack internally uses some custom format, and then allows 3rd party plugins to provide converters from from different DSLs into this internal LoopBack format. In other words, JSON Schema would become only one of the supported ways how to define models. @raymondfeng could you please confirm whether I described your ideas correctly?

[lehni]

That sounds like a good idea. but if AJV becomes the validator for LoopBack.next, what if the internal format was actually JSON Schema, and other formats could be translated to it in the way you described? This would make it easier to provide validation for all formats.

I am actually experimenting with something like this right now for LoopBack 3.8, by patching Registry.prototype.createModel()

[lehni]

@bajtos I just finished a first implementation of this for LB 3.8 by overriding the internal Registry class, and it works surprisingly well. I was also able to add support for the missing json validation properties listed here: https://loopback.io/doc/en/lb3/Model-definition-JSON-file.html#validations

Do you think this is of interest for 3.8? Shall I open a PR for it?

[bajtos]

Interesting. Is your code on GitHub, so that we can take a look before you invest more time on a pull request? IMO, we need to carefully asses impact of such change, for example on loopback-swagger.

[lehni]

It's not yet. The code is fairly modular and short, so I can easily move it to its own package and create a little test case illustrating how it's being used, as a base for further discussions both in LB 3.8 and 4

[lehni]

@bajtos I've finally moved my code into a separate module and put it up on Github:

https://github.com/lehni/loopback-jsonschema-registry

This still needs proper documentation, but for now there's an example in the README that should explain the based functioning of all this. I am happy to explain in more detail here, and curious to hear your thoughts!

[bajtos]

@lehni thank you for making your code public. I read through it and to be honest, I don't know what and how to review ☹️ The code looks pretty reasonable at the first read. The only question that comes to my mind is how to use these new JSON schemas in the output of loopback-swagger?

[lehni]

I meant to write more documentation to explain better what is going on there. Here a quick explanation, perhaps this can turn into proper README content later:

So I decided to keep using the normal LB3 type JSON model files, but offer a couple of ways to extend them, and then use some preprocessing magic in which these new hybrid JSON modes files get translated to two things: 1. normal LB JSON models with only properties and value present that LB can actually understand, and 2. separate JSON schema descriptions for better validation.

Some examples:

• I can use the configureProperty(schema) callback that is passed to the registry in its options as a way to process / configure a model property. In the example in the README I am using this to add support for "type": "integer", which gets translated to "type": "number" for the LB JSON model, and is kept as "type": "integer" for the JSON schema file.

• I can register what i call validators, which are basically either keywords or formats in the JSON schema sense, e.g. a format validator that makes sure the value isn't blank:

  export default {
    format: 'not-empty',
    validate: value => !!value,
    message: 'is required'
  }

  ...and then I can use them like so:

  {
    "name": "Test",
    "base": "PersistedModel",
    "properties": {
      "someValue": {
        "type": "string",
        "format": "not-empty"
      }
    }
  }

The same I can do with keywords, as shown with the "range" example in the README

There are many advantages and benefits for us using this approach, and in order to explain these I need to outline the functioning in more detail. But here some of the top of my head:

• The registry is able to detect properties added by mixins and builds JSON schema validation structures for those too.
• This validation works really well, even recursively, with so called "anonymous" models as used for Array and Object data types that end up being stored as JSON in the database. This is something that we just couldn't get validated properly with the normal LB mechanism, but now we have four nesting anonymous models (each having objects or array referencing the next one), and the JSON schema validation references them properly.
• Modle inheritance is reflected in the created JSON schemas through the use of $merge with, as provided by the ajv-merge-patch package.

There are some potential problems with our approach to improve validation also, mainly:

We had to monkey-patch model.isValid(). And since currently, our validations are always defined as async to easily provide the possibility to define custom validation keywords that actually do run asynchronously, we changed its behaviour to return a promise instead of false if no callback is provided. This doesn't clash with the LB codebase since it's always called with a callback internally, but it is not compatible with old code. It does however look much nicer when using async / await :)

We also needed to perform quite a few tricks to get the AJV validation errors added to the internal errors structure so that ValidationError can pick them up. I think I should work on improving this and perhaps exposing some structures there so that this is less hackish in the future.

But before going into more detail:

I just wanted to share this so you can see how we're currently embedding and using AJV, and for us some really nice things have come out of this.

I'm not sure this should go into LB3, but I would be excited and happy to help if you thought it should! I do think I could maybe inform discussions on LB4, and perhaps we can look into how to smooth things out for LB3 to work better together with this package?

[bajtos]

@kjdelisle and me agreed to remove Validation from MVP scope. The current validation performed by Juggler should be good enough for CRUD applications which are our MVP target.

[jannyHou]

@bajtos see PR #1489, I checked the items finished.
While a big problem is "Dereference the schema in requestBodySpec":

• The parser function doesn't have access to the whole openapi spec, so it doesn't have components.schemas
• A solution would be adding openapiSpec in the context, and expose the context in the parser action.

[raymondfeng]

However, @raymondfeng, our architect, has a different opinion. IIRC, he is proposing a slightly different implementation, where LoopBack internally uses some custom format, and then allows 3rd party plugins to provide converters from from different DSLs into this internal LoopBack format. In other words, JSON Schema would become only one of the supported ways how to define models. @raymondfeng could you please confirm whether I described your ideas correctly?

There are JSON-schema like languages to define models:

• Swagger 2.0 definitions
• OpenAPI 3.0 schemas
• JSON Schema
• GraphQL schema
• LoopBack 3.x DSL

The JSON-schema itself can only be used define the typing perspective of models. LoopBack often requires additional metadata, such as mapping to database schemas and JSON serialization on the wire.

[bajtos]

@raymondfeng your comment seems out of topic of this issue, I cross-posted it in #699 - it seems like a more relevant place for that information, at least to me.

[dhmlau]

Closing as done.