-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add unique index requirement for restrictions. #53
Conversation
lib/restrictions.js
Outdated
'restiction.methodOptions.limit': 1, | ||
'restiction.methodOptions.duration': 1 | ||
}, | ||
options: {unique: false, background: false} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not unique
(it is set to false
). Also, this can't be done with uniqueness as restriction.id
is unique as well -- without disabling sharding. We should consider alternative fixes to whatever the problem is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dlongley what we are trying to prevent is someone being able to add the same restriction to the system over and over again, which they are now able to do if they use a different id each time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment.
If we don't have another way to prevent that from happening, we should generate an ID deterministically from the values we want to be considered unique, i.e., create an object or array (in a well-defined order) of the parameters we want to be considered unique, convert it to canonical JSON, and hash it -- and then express the result as a multihash value as part of the ID. Anyone following this process will get a duplicate error if trying to insert the same record, all without having to modify the database at all. |
Closing this PR, issue with required changes opened here: #54 |
No description provided.