Sails: When using sails-disk any attribute with `unique:true` must also have `required:true`

[Noitidart]

Sails version: 1.1.0
Node version: 8.14.0
NPM version: 6.4.1
DB adapter name: sails-disk
DB adapter version: N/A
Operating system: Windows 10

---

I have one-to-one relationship. A task must have a message. Even though I set required to true in the Task.message relationship, I keep getting error "In attribute message of model task: When using sails-disk, any attribute with unique: true must also have required: true". My code:

/api/models/Task.js

message: {
      model: 'message',
      required: true,
      unique: true
},

/api/models/Message.js

task: {
      collection: 'task',
      via: 'message'
},

When using sails-disk any attribute with unique:true must also have required:true.

I asked on gitter and they said:

Did you try with a real database? May be there is a bug in sails-disk and throwing a wrong message. May be sails-disk adapter is looking at the relationship as 1-to-m. I suggest execute your code with a real database and then post your findings

I tried with sails-mysql and no issue. I reported this in gitter to see if they could help more, they said not to use sails-disk.

I think this is a bug, and I don't want to just ignore it if I found it. So reporting it here.

Gitter comments:

sails-disk is very simple implementation based on json file. It is not a DBMS so do not expect much from it. It is made available for quick prototyping. Do not use

It would be nice if we could fix sails-disk so it didn't get a bad rep like this. They tell me simply to not use it.

[sailsbot]

Hi @Noitidart! It looks like you missed a step or two when you created your issue. Please edit your comment (use the pencil icon at the top-right corner of the comment box) and fix the following:

• Provide your Operating system

As soon as those items are rectified, post a new comment (e.g. “Ok, fixed!”) below and we'll take a look. Thanks!

*If you feel this message is in error, or you want to debate the merits of my existence (sniffle), please contact [email protected]

[Noitidart]

Edited.

[sailsbot]

@Noitidart 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.

[raqem]

Hi @Noitidart,
Thank you for reporting your findings! It will take some time but we will look into this issue and see what we come up with.
Thanks again!

[Noitidart]

Thank you!

Just a note - I also tried it with sails-mongo and it is working fine.

[german970814]

I have the same issue, there is not docs about it. Im trying to run tests on memory store but I can't because my model is not defined with required: true and I cannot define like this by bussines logic

[Noitidart]

@german970814 i couldn't fix it, so I had to move off sails-disk to another adapter.

[rained23]

happen to our test too, @Noitidart are you using a different adapter for testing?

[Noitidart]

@rained23 - Yeah I couldn't fix it, I moved to sails-mongo adapter in testing. I found it better to use same adapater in testing as in production, as i would get bugs with sails-memory, that I wouldn't with sails-mongo.

[lucafaggianelli]

Same error for me. As additional info, my associated field (in the example message) was just required and not unique, everything was working, the error appears as soon as i added the uniqueness property... it appears that schema is somehow cached !?!?

[raqem]

Hi @Noitidart, @german970814, @rained23 and @lucafaggianelli
I am happy to repot that this has been fixed with an update about 2 weeks ago.
https://github.com/balderdashy/sails-disk/commit/0cdd3f6fa234171c8cefa32a3111d1b7edda5b8a
If you are still experiencing this issue I would suggest a clean slate with rm -rf node_modules/ && package-lock.json

[Noitidart]

Awesome!!! Thanks team!! I will try to dig up this project and try it out. Its buried pretty deep :(

[anand514]

Hi @Noitidart, @german970814, @rained23 and @lucafaggianelli
I am happy to repot that this has been fixed with an update about 2 weeks ago.
balderdashy/sails-disk@0cdd3f6
If you are still experiencing this issue I would suggest a clean slate with rm -rf node_modules/ && package-lock.json

Hi @raqem , i am still experiencing this issue , getting the same error When using sails-disk, any attribute with unique: true must also have required: true even after pulling the latest sails-disk package.

[rachaelshaw]

@anand514 what does your model definition look like? Are you getting this error when setting unique: true on an association, or a normal attribute? (Just checking because for non-association attributes you still can only use unique: true if you also have required: true.)

[anand514]

@rachaelshaw , thanks for responding , this is normal attribute , so you were saying if i have normal attribute with unique property it should required also have required , is my understanding correct, if so , could you please let me know the reason for this.

FYI, this model is working for me with sails-mysql however when i try to run integration testing using sails-disk it is throwing above error

[mikermcneil]

@anand514 It often seems reasonable to use required: false and unique: true at the same time-- I still sometimes forget myself and get reminded by the same error message. Every time it happens to me, I have to remind myself why that is. (Would be nice if the error message included a link with more information on this in the Sails docs! Might make a good PR when someone gets a sec)

Consider:

await Character.create({ fullName: 'Jack Torrance', uniqueCatchPhrase: 'Heeeeere’s Johnny!' });
await Character.create({ fullName: 'Clubber Lang', uniqueCatchPhrase: 'I pity the fool.' });
await Character.create({ fullName: 'Mary Lazarus' });
// If `uniqueCatchPhrase` is required, we'd never make it here.

await Character.create({ fullName: 'Reverend Mother' });
// ...but if `uniqueCatchPhrase` was optional, what would happen now?

i.e. uniqueCatchPhrase is supposed to be unique, so you can't have two records with empty string ('') as the value. That can be a kinda confusing error to hit at runtime, and it is avoidable: the adapter can just decide not to allow both required:false and unique:true at the same time. And that way, you get informed about the issue at lift time rather than waiting until runtime and potentially not noticing the logic twist-up until after you're in production.

Good question- thanks for following up!

[anand514]

@mikermcneil , thanks for responding, I've understood the scenario you pointing out related to empty string , just curious , is it should be specification irrespective the adapters ?(the reason i am checking for this is that same model is working fine with mysql adapater but with not with sails-disk)

[raqem]

If a particular database considers stuff like null,null,4,null,5 to be a
unique dataset, it could be compatible with the idea of singular
associations in sails, which can be either null or the foreign key id.

Sails-disk doesn’t support this, MySQL does, so waterline allows it for
MySQL.

This usually isn’t a blocker, even if you choose to stay adapter agnostic -
one way to accomplish the same thing: If no two pets can belong to the same
person, you can enforce that when creating pet records, updating pets’
owners, and when deleting pets.

On Wed, Dec 2, 2020 at 01:26 anand [e-mail] wrote:

@mikermcneil https://github.com/mikermcneil , thanks for responding,
I've understood the scenario you pointing out related to empty string ,
just curious , is it should be specification irrespective the adapters
?(the reason i am checking for this is that same model is working fine with
mysql adapater but with not with sails-disk)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/balderdashy/sails/issues/4559#issuecomment-737047452,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AGNDQDITEQDEUL7W5SXMXYTSSXTYPANCNFSM4GJXD4XA
.

>

@mikermcneil
founder / ceo / creator of sails.js

We create cutting-edge digital experiences for new brands and the young at
heart.
https://sailsjs.com/about

[anand514]

yes, it does , thanks for your response