diegohaz / bodymen

Body parser middleware for MongoDB, Express and Nodejs (MEN)
Other
47 stars 15 forks source link

Error with nested properties #14

Open chemitaxis opened 7 years ago

chemitaxis commented 7 years ago

Hi, I'm trying to use bodymen with nested properties like this:

{
  'Name': 'Marc',
  'Address' : {
          'Street' : 'My Street',
          'City' : 'My City'      
   }
}

But I have an error in the middleware:

ValidationError: Address: Cast to Object failed for value "[object Object]" at path "Address"

Any ideas? Thanks.

chemitaxis commented 7 years ago

Hi @diegohaz , the problem is in the validator method, BUT the problem is not in Bodymen, is your other module rich-param, it can't work fine with objects.

If I change in the validator method this:

/**
   * Validate values of the schema params.
   * @param {Object} [values] - Object with {param: value} pairs to validate.
   * @param {Function} [next] - Callback to be called with error
   * @return {boolean} Result of the validation.
   */
  validate (values = {}, next = error => !error) {
    let error
    if (_.isFunction(values)) {
      next = values
      values = {}
    }

    _.forIn(this.params, param => {
      const value = values[this._getBodyParamName(param.name)]

      if (!_.isNil(value)) {
        // THIS LINE ADDED TO CHECK IF OBJECT
        if (typeof value === 'object') {
          param._value = value
        } else {
          param.value(value)
        }
      }
    })

    for (let i in this.params) {
      if (error) break
      const param = this.params[i]
      param.validate(err => {
        error = err
      })
    }

    return next(error)
  }

What is the best way to solve this problem? Thanks

chemitaxis commented 7 years ago

But... with this, the nested property is not checked... :(

chemitaxis commented 7 years ago

Ok, more information...

If I configure my schema like this in the body:

{
  'Name': String,
  'Address' : {
          type: Object 
   }
}

It works... but the validation of Address properties is not triggered... thanks.

diegohaz commented 7 years ago

@chemitaxis Thank you for reporting. This is a known problem, but not very easy to fix.

FYI, I'm working on a new library to replace querymen, bodymen and rich-param. That solves issues like https://github.com/diegohaz/querymen/issues/30 and this one.

https://github.com/diegohaz/schm

Currently, it lacks validation (https://github.com/diegohaz/schm/issues/1 - almost done on dev branch), express and mongoose support, but these probably will be implemented in the next few weeks. Any help is welcome.

chemitaxis commented 7 years ago

Hi @diegohaz, perfect and thanks. I will check this new repo, and I will try to contribute. My workaround is validate with express-validator. One question, I did not found problems with the issue that you mentioned, https://github.com/diegohaz/querymen/issues/30, but can you please confirm me if I need to be worried about it? ;)

chemitaxis commented 7 years ago

I have check the code, it would be easier to contribute if you document a litte the code ;) For example, how walk method works in utils? Thank you so much!

khelkun commented 7 years ago

Hi I'm using the @diegohaz's rest project generator. I think my problem is the similar to @chemitaxis's problem. I have this Schema :

const contextSchema = new Schema({
  user: {
    type: Schema.ObjectId,
    ref: 'User',
    required: true
  },
  rootview: {
    type: String
  },
  directoryview: {
    type: String
  },
  mimetypes: [ { mime: String, viewer: String } ]
}, {
  timestamps: true
});

The index.test.js of the api is sends this:

test('POST /contexts 201 (user)', async () => {
  const { status, body } = await request(app())
    .post('/')
    .send({
      access_token: userSession,
      rootview: 'UvCardRootView',
      directoryview: 'UvDiffusionDirectoryView',
      mimetypes: [
        { mime: 'image/jpeg', viewer: 'UvDiffusionImageViewer' },
        { mime: 'application/pdf', viewer: 'UvEmbeddedImagePdfViewer' },
        { mime: 'video/mp4', viewer: 'UvBrowserVideoViewer' }
      ]
    });

On the api side it seems that bodymen cannot parse the mimetypes field. Indeed the create method of my api controller receives this body object:

body = Object
   directoryview = "UvDiffusionDirectoryView"
   mimetypes = "[object Object],[object Object],[object Object]"
   rootview = "UvCardRootView"

So body.mimetypes is not an array but a toString of it. I debugged in bodymen and indeed the following line in the middleware function doesn't give me the source req.body.mimetypes array but an "[object Object],[object Object],[object Object]": req.bodymen = { body: _schema.parse(), schema: _schema };

I'm really new to all this validation, mongoose, schema, nested schema ... I did read Mongoose doc, took the time to understand what I could in @diegohaz's work but I don't know how to workaround this. Can you help me ? How I could at least disable the bodymen validation for this api? And so forth directly send the json parsed req.body to the create method of my api controller? I mean what I should doin from the context/index.js of my api? here:

import { Router } from 'express';
import { middleware as query } from 'querymen';
import { middleware as body } from 'bodymen';
import { token } from '../../services/passport';
import { create, index, show, update, destroy } from './controller';
import { schema } from './model';
export Context, { schema } from './model';

const router = new Router();
const { rootview, directoryview, mimetypes } = schema.tree;

router.post('/',
  token({ required: true }),
  body({ rootview, directoryview, mimetypes }),
  create);
chemitaxis commented 7 years ago

Hi @khelkun, my workaround is something like this:

const contextSchema = new Schema({
  user: {
    type: Schema.ObjectId,
    ref: 'User',
    required: true
  },
  rootview: {
    type: String
  },
  directoryview: {
    type: String
  },
  mimetypes: { type: Object }
}, {
  timestamps: true
});

I have changed mimetypes to Object, you can validate after, what you want.

Thanks!

khelkun commented 7 years ago

@chemitaxis thanks it actually works! it also works with this schema:

const contextSchema = new Schema({
  user: {
    type: Schema.ObjectId,
    ref: 'User',
    required: true
  },
  rootview: {
    type: String
  },
  directoryview: {
    type: String
  },
  mimetypes: [ { type: Object } ]
}, {
  timestamps: true
});

Which is probably a bit more accurate since mimetypes field is supposed to be an Array of Objects. Still I wish I could have specified in the schema that each Object in the mimetypes Array has 2 String fields: mime and viewer. But apparently I'll not be able to validate this.

khelkun commented 7 years ago

Also note that bodymen succeeds to validate a body containing [ { mime: "test", viewer: "test"} ] for a schema definition like this: mimetypes: { type: Object, required:true }

But bodymen fails to validate the same body with a schema definition like this: mimetypes: [ { type: Object, required:true } ]

chemitaxis commented 7 years ago

Interesting... you can validate an array ob objects?

khelkun commented 7 years ago

My bad it's not bodymen issue. The validation error comes from mongoose:

Failed: Context validation failed: mimetypes: Cast to Array failed for value "[ { mime: 'image/jpeg', viewer: 'UvMessidorImageViewer' },
  { mime: 'application/pdf', viewer: 'UvEmbeddedImagePdfViewer' } ]" at path "mimetypes"
ValidationError: Context validation failed: mimetypes: Cast to Array failed for value "[ { mime: 'image/jpeg', viewer: 'UvMessidorImageViewer' },
  { mime: 'application/pdf', viewer: 'UvEmbeddedImagePdfViewer' } ]" at path "mimetypes"
    at MongooseError.ValidationError (/home/me/myprojectnode_modules/mongoose/lib/error/validation.js:28:11)
    at model.Object.<anonymous>.Document.invalidate (/home/me/myprojectnode_modules/mongoose/lib/document.js:1612:32)
    at model.Object.<anonymous>.Document.set (/home/me/myprojectnode_modules/mongoose/lib/document.js:761:10)
    at model._handleIndex (/home/me/myprojectnode_modules/mongoose/lib/document.js:594:14)
    at model.Object.<anonymous>.Document.set (/home/me/myprojectnode_modules/mongoose/lib/document.js:554:24)
    at model.Document (/home/me/myprojectnode_modules/mongoose/lib/document.js:73:12)
    at model.Model (/home/me/myprojectnode_modules/mongoose/lib/model.js:47:12)
    at new model (/home/me/myprojectnode_modules/mongoose/lib/model.js:3688:13)
    at /home/me/myprojectnode_modules/mongoose/lib/model.js:1942:51
    at /home/me/myprojectnode_modules/async/internal/parallel.js:27:9
    at eachOfArrayLike (/home/me/myprojectnode_modules/async/eachOf.js:57:9)
    at Object.<anonymous>.exports.default (/home/me/myprojectnode_modules/async/eachOf.js:9:5)
    at _parallel (/home/me/myprojectnode_modules/async/internal/parallel.js:26:5)
    at parallelLimit (/home/me/myprojectnode_modules/async/parallel.js:85:26)
    at /home/me/myprojectnode_modules/mongoose/lib/model.js:1963:5
    at Promise._execute (/home/me/myprojectnode_modules/bluebird/js/release/debuggability.js:300:9)
    at Promise._resolveFromExecutor (/home/me/myprojectnode_modules/bluebird/js/release/promise.js:483:18)
    at new Promise (/home/me/myprojectnode_modules/bluebird/js/release/promise.js:79:10)
    at Function.create (/home/me/myprojectnode_modules/mongoose/lib/model.js:1926:17)
    at _callee$ (/home/me/myprojectsrc/api/context/index.test.js:16:49)
    at tryCatch (/home/me/myprojectnode_modules/regenerator-runtime/runtime.js:65:40)
    at GeneratorFunctionPrototype.invoke [as _invoke] (/home/me/myprojectnode_modules/regenerator-runtime/runtime.js:299:22)

So I guess mimetypes: [ { type: Object, required:true } ] is not a valid schema definition... To be honnest I barely know what I'm doing

chemitaxis commented 7 years ago

Perfect! tell me more later ;)

El mié., 30 ago. 2017 a las 13:14, khelkun (notifications@github.com) escribió:

I think I got something! Here is the mimetypes field definition in my schema:

mimetypes: [{ mime: { type: String, required: true }, viewer: { type: String, required: true }, }]

And in bodymen package I added the following code in the file body-schema.js and the _parseParamOptions function (at the end of the file):

  else if (_lodash2.default.isObject(options)) {
    options = { type: Object, default: options };
  }

Just after the:

  } else if (_lodash2.default.isFunction(options)) {
    options = { type: options };
  }

And this seems to actually work. The only thing is that each object in my mimetypes array get a _id field in my MongoDB collection. Once again I barely know what I'm doing so I can't tell if the code I added in bodymen-schema.js is relevant or stupid.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/diegohaz/bodymen/issues/14#issuecomment-325959831, or mute the thread https://github.com/notifications/unsubscribe-auth/ABWseVNhTbTSKkRReRptI5sN8MTZGArkks5sdUQQgaJpZM4OX8hp .

khelkun commented 7 years ago

Sorry I deleted the message cause the change I was talking about actually breaks bodymen. It works for my schema but the tests fails on the schemas of the others api (auth, password, users).