Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PUT (update) does not run mongoose validators #115

Closed
sylvainlap opened this issue Aug 24, 2016 · 6 comments
Closed

PUT (update) does not run mongoose validators #115

sylvainlap opened this issue Aug 24, 2016 · 6 comments

Comments

@sylvainlap
Copy link

Hi,

I created a really simple model, called tag, with only one property, text, which is required.

text: {
    type: String,
    required: true,
    unique: true,
}

When I POST a new tag, text is required, and I got an error if text is not set.

However, if a do a PUT request on /tags/:idOfTheNewTag with an empty body, my tag is replaced, with a new one with no text. So, that PUT request did not thow an error for the "required" validator.

@daffl
Copy link
Member

daffl commented Aug 24, 2016

This problem should be fixed via #87 in version 3.4.0 and later.

@sylvainlap
Copy link
Author

I'm using "feathers-mongoose": "^3.5.2", and I still have the issue.
Seems that runValidators does not work if required attributes are missing.

@sean-nicholas
Copy link

sean-nicholas commented Oct 9, 2016

I have the same issue. It does not validate on patch, too

EDIT: Ahh found the issue. From http://mongoosejs.com/docs/validation.html#update-validator-paths

When using update validators, required validators only fail when you try to explicitly $unset the key.

@ekryski
Copy link
Member

ekryski commented Oct 22, 2016

@sean-nicholas thanks for doing that digging.

Wow! That's pretty crappy. I'm not sure how we can support that.

For an update (ie. PUT) we could potentially inspecting the model to check to see which fields are required in the schema but not included. But that's what mongoose should be doing.

For a patch we'd have no idea because you aren't sending the whole object.


Just brainstorming here, but one thing we might be able to do is wrap the whole data object in a $set here, however this could really mess up if people are using that in their data object already.

I think you probably do that yourself in a before hook if you need to.


Just a little insight that we are working on a common schema definition across all DBs so mongoose will likely go away at some point so I'm not sure we are going to address this as this would just be a hack to monkey patch unexpected mongoose behavior whereas the sooner we can get to a common schema you won't have these sort of inconsistencies.

@sean-nicholas
Copy link

I solved my problem by using https://github.com/kulakowka/feathers-validate-hook/
I like it because it is database agnostic and is-my-json-valid (used in the project) is pretty powerful.
In my service folder there is a schema.js file which looks like this:

const _ = require('lodash');

const schema = {
  required: true,
  type: 'object',
  properties: {
    name: {
      required: true,
      type: 'string',
      minLength: 1
    },
    number: {
      required: true,
      type: 'string',
      minLength: 1
    },
  }
};

exports.create = function() {
  return schema;
};

exports.update = function() {
  return schema;
};

exports.patch = function() {
  let patchSchema = _.cloneDeep(schema);
  patchSchema.properties.name.required = false;
  patchSchema.properties.number.required = false;

  return patchSchema;
};`

As you can see it modifies the schema for a patch and sets required to false so it won't fail if you don't provide name / number in the body. But if you do so it must be a string and have at least of length 1. So something like this in the body won't work:

{
    "name": ""
}

And of course things like name: undefinedor name: null won't work either because they aren't strings.

@daffl
Copy link
Member

daffl commented Aug 14, 2017

Going to close this since it is pretty old and this seems to be a Mongoose restriction which can be circumenvented by the aforementioned validate hook or the validateSchema common hook.

@daffl daffl closed this as completed Aug 14, 2017
mariagregorio added a commit to mariagregorio/fullstack-hy2020.github.io that referenced this issue Jan 11, 2021
Wondering what could be done about this... looked around a bit for an answer and nothing feathersjs-ecosystem/feathers-mongoose#115
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants