balderdashy / sails

Realtime MVC Framework for Node.js
https://sailsjs.com
MIT License
22.84k stars 1.95k forks source link

ObjectId not working when using object id in nested collection #6808

Open sumitLKpatel opened 5 years ago

sumitLKpatel commented 5 years ago

Node version: v8.16.0 Sails version (sails): 1.2.3 Sails Mongo (sails-mongo): 1.0.1


my app was working in old sails were i used object id in nested collection , and now i did rewrite my app in latest sails, now it's not working with old collection, it's giving error like

AdapterError: Unexpected error from database adapter: object [{"_bsontype":"ObjectID","id":null}] is not a valid ObjectId

this is happening when i try to use objectId in nested collection. please fix it ASAP because it's really hard to me to go live with this code.

sailsbot commented 5 years ago

@sumitLKpatel Thanks for posting! We'll take a look as soon as possible.

In the mean time, there are a few ways you can help speed things along:

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

johnabrams7 commented 5 years ago

@sumitLKpatel - In your config/models.js, have you tried setting:

attributes: {
   id: { type: 'string', columnName: '_id' },
 }
sumitLKpatel commented 5 years ago

@johnabrams7 Thanks for comment but I have already tried that

johnabrams7 commented 5 years ago

@sumitLKpatel Sounds good and thanks for the info so far. If you don't mind, can you reproduce this in a new Sails v1.x app repo with only one model? Also, can you include instructions for creating a mongo record that causes this error?

sumitLKpatel commented 5 years ago

i want something like below image, but it's not working as i mention above.

image

sumitLKpatel commented 5 years ago

i got this type of object in model, in beforUpdate function

image

but befor it comes to Model i print that id and it gives like below but when it's goes to model, something goes wrong with object id

image

whichking commented 5 years ago

Hi, @sumitLKpatel!

Thanks for those screenshots; those are super helpful. I believe that your issue is arising from the use of .create(). Per this bit of the upgrade guide, .create() no longer returns the created record. Instead, you should use .fetch(). One way to do this is to, in config/models.js, set fetchRecordsOnCreate: true. The part of the upgrade guide to which I linked above goes into transitioning from .create() to .fetch() in more detail.

Let me know if this works for you!

sumitLKpatel commented 5 years ago

@madisonhicks thanks for notice, but the issue is not only at .create(), the issue is arise on .update() also. because in my existing collections there are many records which includes nested objectId, and when i'm going to update that collection it's giving below error

AdapterError: Unexpected error from database adapter: object [{"_bsontype":"ObjectID","id":null}] is not a valid ObjectId

so i don't think there is any issue is related to .fetch(), the issue is mongo-adapter thawing error while i'm going to update my those records which have nested objectIds. and when i convert those objectIds in to string, then it works....but i don't wan't to convert because there is lot data available like this in my database colections

whichking commented 5 years ago

Hey, @sumitLKpatel!

Sorry, I should have been more thorough in my last comment—the default result of .create(), .createEach(), .update(), and .destroy() has changed with Sails v1.

Per the upgrade guide:

To encourage better performance and easier scalability, .create() no longer sends back the created record. Similarly, .createEach() no longer sends back an array of created records, .update() no longer sends back an array of updated records, and .destroy() no longer sends back destroyed records. Instead, the second argument to the .exec() callback is now undefined (or the first argument to .then(), if you're using promises).

The .fetch() method is recommended for grabbing those records.

This bit of the upgrade guide has more information.

sumitLKpatel commented 5 years ago

@madisonhicks I think i'm not able to explain my issue exactly... anyways thanks for the comments

whichking commented 5 years ago

Hey, @sumitLKpatel—

Can you create a new Sails app with a single model that reproduces this error, and include a brief description of the action you're taking to trigger the error? I'd like to be able to help, and I think that this might help me better understand your issue.

sumitLKpatel commented 5 years ago

Hey @madisonhicks

here is repo, you can check it.

https://github.com/sumitLKpatel/sails-mongo-issue

whichking commented 5 years ago

Hey, @sumitLKpatel—

I'm getting errors when I sails lift. Are you able to lift with this repo? It might also be something on my end that's bugging out, but I thought I'd check with you first.

sumitLKpatel commented 5 years ago

yes it's working for me perfectly....

whichking commented 5 years ago

@sumitLKpatel—cool, it's just on my end, then. I'll spend some time with this as soon as I get a chance.

whichking commented 5 years ago

Hey, @sumitLKpatel—sorry for the radio silence. I just chatted with the rest of the team, and our recommendation is to use strings instead of object IDs. We think that your problem may be that objectID instances are being passed into nested JSON.

sumitLKpatel commented 5 years ago

@madisonhicks so there is no any solution of this issue...I must need to use string? If I'm going to use string..then what should I do with my existing thousands of records which includes objectIds..

whichking commented 5 years ago

@sumitLKpatel—yeah, that sounds like it would be super inconvenient for you. Can you share a little bit more about your use case? We'd love to help you find a workaround.

AmaarHassan commented 5 years ago

Hi @sumitLKpatel , i experienced the same issue recently. I was working with sails and it has a hook of beforeCreate that is executed before the model document is created in its collection. In other words, it is the final function where you can make any modifications. So as i know that one my addresses field has an account reference as mongodb object id. I simply converted it to string

e.g. vendorStore.address.account = vendorStore.address.account.toString();

it never gave me any trouble afterwards.

If you have some issue in fetching it, perhaps you can convert at the time of retrieval.

Hope it helps

stensrud commented 4 years ago

I'm in a Sails 0.12 to 1.x-transition and have the same problem.

After migrating several projects to Sails 1.x, my honest opinion is that Sails (Waterline) is no longer very well suited for use with MongoDB databases. It doesn't support storing even basic data structures (like arrays) with id's in them, mutates returned objects, doesn't allow geo-queries etc. Also, the sails-mongo adapter hasn't had an update in over a year. Waterline is an ORM, cross-database compatibility will always be more important than supporting native features.

My workaround is to get a native db connection with sails.getDatastore().manager.collection(table), then you can execute any query supported by the database.

whichking commented 4 years ago

Hey, @stensrud! Thanks for the input and workaround.

It seems like using native tools will, indeed, be the best option for functionality not supported by Waterline. Other community input on this topic is welcome!

golojads commented 4 years ago

Nope. Even Native update query ends up with the same error. @AmaarHassan solution works! Just convert to string all ObjectId within updated values.