balderdashy / sails

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

MongoDB adapter doesn't recognise FKs to mysql table correctly #4367

Open mp3por opened 6 years ago

mp3por commented 6 years ago

Sails version: sails@1.0.1 Node version: 8.9.4 NPM version: 5.6.0 DB adapter name: sails-mongo DB adapter version: sails-mongo@1.0.0 Operating system: macOS 10.13.3


Hello,

I have a reference from my mongo model ( Client ) to a mysql model via user: { model: 'User', required: true } and User is in mysql database. I then want to create a Client and supply {user: 1} the adapter throws

Error: Invalid replacement foreign key value provided for association (user). Cannot interpret1as a Mongo id.

REPO: git@bitbucket.org:mp3por/sails-bug-2.git

Please advice :)

sailsbot commented 6 years ago

@mp3por Thanks for posting, we'll take a look as soon as possible.


For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

mp3por commented 6 years ago

This also doesn't work if the id of the linked mysql model is UUID.

I managed to hack it for UUID via user : { type:'string'} and for ID via user : { type: 'number' }.

mikermcneil commented 6 years ago

@mp3por thanks for the heads up- I think I see why this is happening

image

Will look into it ASAP. Your workaround makes sense in the mean time (of course it means you won't be able to rely on .populate())

mikermcneil commented 6 years ago

Here's the code in sails-mongo that's specifically wrong: https://github.com/balderdashy/sails-mongo/blob/18d0f31df0da0659c93bd5f59cdc4d422ab596a3/lib/private/machines/private/reify-values-to-set.js#L88-L99

@mp3por Also in the mean time, if you have a moment to take a pass at a PR, I'd be happy to review and hopefully merge. I think what we need there is to add another disjunct checking if the foreign key is pointed at another model powered by the current sails-mongo adapter, and if so, skip the check. Please just comment here with that PR if you get to it

(Also, you'll notice in that code that there's another workaround hack you could use involving meta-- though you shouldn't have to -- this is definitely something that should work out of the box)

maheshwarishivam commented 6 years ago

Another possible workaround without changing the Model Structure (NOT a fix):

Setting dontUseObjectIds: true for the Model (which is stored in MySQL) seems to fix the issue.

In the example in OP, set this for User model.

Bonus: .populate() will work too!

mikermcneil commented 6 years ago

@maheshwarishivam :+1: thanks for the follow-up!

@streleck this is related to the other issue we were talking about this morning. (If you've still got that link handy, would make sense to backlink here) (Never mind, just noticed @maheshwarishivam already did :+1:)

ghost commented 5 years ago

@maheshwarishivam I'm using sails@1.0.2 and I have the same issue. Does that label closed means it is already available in new version?

node v8.12.0 npm 6.4.1 sails-mongo@1.0.1 sails-mysql@1.0.0 ubuntu 16.04 LTS

johnabrams7 commented 5 years ago

@ghost - The referenced issue was closed because for being relatively duplicate of this one -however it has a lot of useful info so it's definitely worth checking out. @maheshwarishivam Thanks for the workaround!