Problem with validation when updating via PUT #26
Comments
|
Can I see your request please? I'll start setting up the automated tests for all the autogenerated endpoints. I just tested manually before, which I'm gradually moving away from. I completely intend to allow a user to update a partial document using JSON PATCH, but I am trying to encourage best practices, and HTTP PUT should really only be used to update an entire resource. Of course, if you need, you can always override any of the autogenerated endpoints to provide that functionality. I'll eventually allow users to override any of the default collection endpoints (for keeping things DRY). That would be super simple, but there's just a list of things I need to get to first. |
|
I set up some automated tests for this and everything seems to work fine on the autogenerated PUT. I was able to update the entire resource, and when I left out a field it was nullified, as expected. I'll definitely need to see your request to help figure this out. I'll include the test code below, which has some sample requests using Meteor's HTTP package. describe 'A collection route', ->
context 'with the default autogenerated endpoints', ->
# Add a new collection route with all the default options
Restivus.addCollection new Mongo.Collection('testautogen')
# The ID of a newly created resource to persist between tests
testId = null
it 'should support a POST on api/collection', (test) ->
result = HTTP.post 'http://localhost:3000/api/v1/testAutogen',
data:
name: 'test name'
description: 'test description'
response = JSON.parse result.content
responseData = response.data
test.equal result.statusCode, 201
test.equal response.status, 'success'
test.equal responseData.name, 'test name'
test.equal responseData.description, 'test description'
# Persist the new resource id
testId = responseData._id
it 'should support a PUT on api/collection/:id', (test) ->
result = HTTP.put "http://localhost:3000/api/v1/testAutogen/#{testId}",
data:
name: 'update name'
description: 'update description'
response = JSON.parse result.content
responseData = response.data
test.equal result.statusCode, 200
test.equal response.status, 'success'
test.equal responseData.name, 'update name'
test.equal responseData.description, 'update description'
result = HTTP.put "http://localhost:3000/api/v1/testAutogen/#{testId}",
data:
name: 'update name with no description'
response = JSON.parse result.content
responseData = response.data
test.equal result.statusCode, 200
test.equal response.status, 'success'
test.equal responseData.name, 'update name with no description'
test.isUndefined responseData.description
All of those tests pass (except the POST returns a 200, when it should be returning a 201, which I will fix in a future update). |
|
Thanks for your reply - I added the request here https://gist.github.com/sschuchlenz/89d608263af51225e253 When I submit this as |
|
Geez. Curl is kinda evil. It's awesome, but you have to be very careful with it. This same thing happened to another user, and it looks like an issue with the API, but it's really just the fact that the Check out this response in Issue #16 for a little more detail and the stack overflow response where I found the answer. I know this is the issue so I'm going to go ahead and close this. Feel free to continue the discussion here if you have any other questions. |
|
In fact my gist is just the curl-representation of my API tests with postman (https://chrome.google.com/webstore/detail/postman-rest-client/fdmmgilgnpjigdojojpjoooidkmcomcm), I only use curl directly on a case by case basis and only for simple things like getting user id and token from the API. But I'll check if there is a postman setting to encapsulate URLs in quotes - thanks for the heads up. |
|
Did you mean to use local.com in your url? Maybe that was just a typo or something Postman does? It seems to be some random website, and I just realized that I was using the url I say this because it looks like you were actually escaping the |
|
Sorry I forgot to mention this - local.com is a local mapping to localhost (so that is identical to http://localhost:3000), we use this for some reasons unrelated to meteor (since some components of ours need a "real" domain and local.com is just a workaround). I'll have a look, maybe this is a weird issue with postman and resolution of our local |
|
Update: Switched local.com with localhost but to no avail. I also tried to remove all the validations and default values from my If everything else yields no success I'll try to overwrite the |
|
I didn't realize you were doing the schema validation. Simple schema validation is currently on the roadmap for Restivus, and is a very important feature for me. Restivus may actually be interfering somehow. I need to look into this further. I'm going to go ahead and reopen this issue, since I misunderstood the problem. Could you please post everything you're doing with the schema validation? The more code, the merrier. |
|
My apologies that I didn't mention this - somehow I wasn't thinking straight ;) I completely removed the simple schema (for testing) and it works, so the problem is indeed rooted in simple schema. Since the app I'm working on is a client project (which I can't share per se due to contract obligations) I'll create a sample app with similar code and concepts, upload this to github and share the link once I'm done, I hope this aids in sorting out the issues with simple schema. Many thanks for your time and effort! |
|
I added a small sample application with restivus and a simple schema enabled collection (which validates some basics) here: https://github.com/sschuchlenz/restivus-simpleschema |
- Add tests for the autogenerated POST and PUT collection endpoints in response to Issue #26
- Add tests for the autogenerated POST and PUT collection endpoints in response to Issue #26
|
After testing this out some more (thanks for the test repo!), and reading through all comments in the issue you referenced in your initial post, I'm almost sure that we'll need to modify simple-schema to get this to work, since it currently doesn't support updating an entire document, which is the behavior we need |
|
I'm very graceful for your effort and if you manage to put in the effort to do a pull request I'll update the demo repo to aid in testing and help with sorting out any issues the best I can. Please let me know if there is anything else I can do to assist. |
|
Other than removing the error that SS throws, you will just have to make sure that it validates as an insert. It may kind of "just work" due to the fact that SS doesn't care about the concepts of insert vs. update, it only thinks in terms of modifier vs. document. So in the case of a whole-document update, it will think of it as an insert, which should be correct. It may be trickier to add collection2 support because that package does have a concept of insert and update, and this is something in between. (People with custom validators may be assuming that Also, I don't think full-doc updates are allowed on the client so if that's still true, this will be a server-side change only. |
|
Thanks for the guidance, Eric! Much appreciated! |
|
Hi @kahmali, First allow me to say "Restivus is an AMAZING Meteor package". Thank you so much for writing it!!! I am experiencing the same situation as @sschuchlenz and just wondering, just a vague thought..., do you think line 164 instead of Would that solves the validation error? |
|
Thank you so much for the kind words @zgudino! My life has been consumed by a startup I'm working on, and I don't have any time to test this out right now. Would it be too much trouble for you to fork Restivus and try testing the update in your development environment? If the update works, just submit a PR and I'll review and merge it in. I hate that I can't be more helpful right now. Sorry about that! As soon as I have some downtime I'm going to give Restivus some TLC. Just an aside, but I'm now using Astronomy for my model layer, in place of simple schema and its related packages. I (and Eric Dobbertin, the author of simple schema himself) would recommend you check it out. It certainly has the potential to become the de facto model layer for Meteor, and it recently hit 1.0 and is in active development. Unfortunately for us, we're still waiting on the relations module before upgrading to 1.0, but hopefully that makes its way in there soon. Thanks again for the suggestion! Let me know what you find if you look into it further. |
|
What is the status with this? |
|
Has this not been fixed? I'm having issues with PUT as well:
|
|
Someone of us should probably try what @zgudino is proposing, using $set :) And send PR if it works. |
|
The issue with what @zgudino is suggesting is that it would change the behavior of PUT from updating an entire document to only updating the specified fields, which is not the intended behavior for a PUT. What's really needed for that is PATCH support. Until PATCH support comes around (at the rate I'm going it'll be sometime in the year 2050) you can use my suggestion here to override the default PUT, or make the changes to simple-schema and collection2 suggested by @aldeed above. I promise to make JSON PATCH support the first major update I make when I have a chance to continue developing Restivus. I'm gonna organize a ZenHub board to begin a roadmap for development. Hopefully that'll jumpstart things. I also need to update the docs to include a note about this issue with validating a PUT using simple schema (just leaving this as a note for myself). |
When I try to update a record via a PUT request (I use the autogenerated endpoint), I get the following error:
Error: When the modifier option is true, all validation object keys must be operators. Did you forget$set?It seems that this has to do with literal document modifiers - how can I solve this since I can't influence the update call restivus executes (or can I?)
More info here: Meteor-Community-Packages/meteor-simple-schema#175
It would also be great if there was a way to only specify those fields in the request that need change (so the client does not have to submit the whole object when updating only one or two fields).
The text was updated successfully, but these errors were encountered: