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

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

Open
sumitLKpatel opened this issue Jul 11, 2019 · 22 comments
Open

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

sumitLKpatel opened this issue Jul 11, 2019 · 22 comments
Labels
helpful info or workaround mongo Issue only occurs when using MongoDB orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc.

Comments

@sumitLKpatel
Copy link

sumitLKpatel commented Jul 11, 2019

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
Copy link

@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:

  • look for a workaround. (Even if it's just temporary, sharing your solution can save someone else a lot of time and effort.)
  • tell us why this issue is important to you and your team. What are you trying to accomplish? (Submissions with a little bit of human context tend to be easier to understand and faster to resolve.)
  • make sure you've provided clear instructions on how to reproduce the bug from a clean install.
  • double-check that you've provided all of the requested version and dependency information. (Some of this info might seem irrelevant at first, like which database adapter you're using, but we ask that you include it anyway. Oftentimes an issue is caused by a confluence of unexpected factors, and it can save everybody a ton of time to know all the details up front.)
  • read the code of conduct.
  • if appropriate, ask your business to sponsor your issue. (Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)
  • let us know if you are using a 3rd party plugin; whether that's a database adapter, a non-standard view engine, or any other dependency maintained by someone other than our core team. (Besides the name of the 3rd party package, it helps to include the exact version you're using. If you're unsure, check out this list of all the core packages we maintain.)

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 johnabrams7 added mongo Issue only occurs when using MongoDB orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. labels Jul 11, 2019
@johnabrams7
Copy link
Contributor

johnabrams7 commented Jul 11, 2019

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

attributes: {
   id: { type: 'string', columnName: '_id' },
 }

@sumitLKpatel
Copy link
Author

@johnabrams7
Thanks for comment but I have already tried that

@johnabrams7
Copy link
Contributor

@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?

@johnabrams7 johnabrams7 added more info please repro please Could you reproduce this in a repository for us? labels Jul 11, 2019
@sumitLKpatel
Copy link
Author

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

image

@sailsbot sailsbot removed more info please repro please Could you reproduce this in a repository for us? labels Jul 12, 2019
@sumitLKpatel
Copy link
Author

sumitLKpatel commented Jul 12, 2019

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
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Author

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

@whichking
Copy link
Contributor

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.

@whichking whichking added the repro please Could you reproduce this in a repository for us? label Jul 19, 2019
@sumitLKpatel
Copy link
Author

Hey @MadisonHicks

here is repo, you can check it.

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

@sailsbot sailsbot removed the repro please Could you reproduce this in a repository for us? label Jul 19, 2019
@whichking
Copy link
Contributor

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
Copy link
Author

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

@whichking
Copy link
Contributor

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

@whichking
Copy link
Contributor

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
Copy link
Author

sumitLKpatel commented Aug 26, 2019

@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
Copy link
Contributor

@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
Copy link

AmaarHassan commented Sep 9, 2019

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
Copy link

stensrud commented Dec 9, 2019

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
Copy link
Contributor

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!

@whichking whichking added the what do you think? Community feedback requested label Dec 17, 2019
@golojads
Copy link

golojads commented Feb 8, 2020

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

@sailsbot sailsbot removed the what do you think? Community feedback requested label Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
helpful info or workaround mongo Issue only occurs when using MongoDB orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc.
Development

No branches or pull requests

7 participants