moleculerjs / moleculer

:rocket: Progressive microservices framework for Node.js
https://moleculer.services/
MIT License
6.16k stars 586 forks source link

Improvement to work with Joi validator #419

Open Embraser01 opened 5 years ago

Embraser01 commented 5 years ago

Prerequisites

Please answer the following questions for yourself before submitting an issue.

Expected Behavior

When I want to use Joi as validator inside a single node containing the API GW and internal nodes, I can't query internal nodes correctly.

Current Behavior

It fails validation

GET http://localhost:3000/api/v2/~node/actions?withServices=true&onlyAvailable=false

HTTP/1.1 422 Unprocessable Entity
X-Powered-By: Express
Content-type: application/json; charset=utf-8
Date: Sun, 18 Nov 2018 18:38:33 GMT
Connection: keep-alive
Transfer-Encoding: chunked

{
  "name": "ValidationError",
  "message": "child \"onlyAvailable\" fails because [\"onlyAvailable\" must be an object]",
  "code": 422,
  "type": "VALIDATION_ERROR",
  "data": [
    {
      "message": "\"onlyAvailable\" must be an object",
      "path": [
        "onlyAvailable"
      ],
      "type": "object.base",
      "context": {
        "key": "onlyAvailable",
        "label": "onlyAvailable"
      }
    }
  ]
}

Response code: 422 (Unprocessable Entity); Time: 49ms; Content length: 423 bytes

Failure Information

This bug happend because the validator for $node.list look like this and isn't a Joi schema (of course):

https://github.com/moleculerjs/moleculer/blob/2071fcf3ff8d54416b351312921dc184e22db58f/src/internals.js#L15-L25

The only solution I see is to create another ServiceBroker with the fastest-validator but I don't want to handle multiple nodes for now (I'm just starting with moleculer, it's awesome 🎉).

Is there a way to override the validator on a per service base?

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

  1. Create a ServiceBroker with a Joi Validator (from the doc example)
  2. Launch an API GW in it
  3. Request to GET http://localhost:3000/api/v2/~node/actions?withServices=true&onlyAvailable=false
  4. You should get an error of validation

Reproduce code snippet

class JoiValidator extends BaseValidator {
  constructor() {
    super();
    // eslint-disable-next-line global-require
    this.validator = require('joi');
  }

  compile(schema) {
    return params => this.validate(params, schema);
  }

  validate(params, schema) {
    const { error } = this.validator.validate(params, schema);
    if (error) throw new ValidationError(error.message, null, error.details);

    return true;
  }
}

const broker = new ServiceBroker({
    logger: console,
    transporter: "NATS",
    validation: true,
    validator: new JoiValidator(),
});

broker.createService({
    name: "test",
    mixins: [ApiGateway],
    settings: {
        middleware: true,

        routes: [{  
          // Will also fail if the validator is strict (icebob/fastest-validator#11) ?
          mergeParams: false,

          // List all routes
          aliases: {
            'REST stock-events': 'stock-events',
          },
        }],
    }
});

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

Failure Logs

See request upper.

icebob commented 5 years ago

Currently, I don't know, how we can resolve it. A workaround:

Embraser01 commented 5 years ago

Ok thanks!

I did manage to create a workaround who will use Joi only if the schema is from Joi:

const Joi = require('joi');
const BaseValidator = require('moleculer').Validator;
const { ValidationError } = require('moleculer').Errors;

class JoiValidator extends BaseValidator {
  compile(schema) {
    if (schema.isJoi) {
      return params => this.validate(params, schema);
    }
    return this.validator.compile(schema);
  }

  // eslint-disable-next-line class-methods-use-this
  validate(params, schema) {
    const { error } = Joi.validate(params, schema);
    if (error) throw new ValidationError(error.message, null, error.details);

    return true;
  }
}

module.exports = JoiValidator;

This add a restriction on the way the params are declared. It must be declared using this syntax:

params: Joi.object({
    paramA: Joi.string().required(),
    paramB: Joi.string(),
}),

// This will not work anymore
params: {
    paramA: Joi.string().required(),
    paramB: Joi.string(),
},

Also the isJoi is not declared in Joi's API except for Errors but do exist in any Joi's object.

There is also the circular problem when listing actions (localhost:3000/api/~node/actions) (cf. #191) because action list contains the Joi schema (which is not safe).

The last problem I saw was the mergeParams problem. Because the params structure changes, but other service don't know that. In this case, your workaround does fix this 😃

As I see it, validators may be on a Service instead of a ServiceBroker. Because in the case I use a service as a library like https://github.com/designtesbrot/moleculer-vault, I will have the same problem except if I launch it in antoher ServiceBroker.

Of course I just started learning all this so I could have missed a lot of concept 😄.

Embraser01 commented 5 years ago

Following problems with Joi, I had to adapt the validator in order to work:

The example in the documentation may not work all the time because

So I had to create a (not so nice) workaround to handle this:

const Joi = require('joi');
const BaseValidator = require('moleculer').Validator;
const { ValidationError } = require('moleculer').Errors;

class JoiValidator extends BaseValidator {
  compile(schema) {
    // Need to use a function because Joi schemas are not cloneable
    // The workaround might be dirty but should works
    if (typeof schema === 'function') {
      // Precompile schema to improve performances
      const compiledSchema = Joi.compile(schema());
      return params => this.validate(params, compiledSchema);
    }
    return this.validator.compile(schema);
  }

  // eslint-disable-next-line class-methods-use-this
  validate(params, schema) {
    const { error } = Joi.validate(params, schema);
    if (error) throw new ValidationError(error.message, null, error.details);

    return true;
  }
}

module.exports = JoiValidator;

Then to use in actions:

module.exports = {
    name: 'service',
    actions: {
        list: {
            // Use Joi validator inside an arrow function
            params: () => Joi.object({page: Joi.number().integer().required()}),
            async handler(ctx) {...}
        },
        get: {
            // Can still use fastest-validator
            params: { id: {type: 'number', optional: false}},
            async handler(ctx) {..}
        }
    }
}

It could be nice to change the joi example in the documentation to handle these problems or put a note explaining the edge cases of the example 😃