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

instance.validate does not do typeValidation #11266

Closed
2 of 7 tasks
simonbrent opened this issue Aug 1, 2019 · 5 comments · Fixed by #14505
Closed
2 of 7 tasks

instance.validate does not do typeValidation #11266

simonbrent opened this issue Aug 1, 2019 · 5 comments · Fixed by #14505
Labels
status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@simonbrent
Copy link

What are you doing?

const sequelize = new Sequelize('database', 'username', 'password', { typeValidation: true });
const model = sequelize.define('foo', {
  bar: { type: Sequelize.INTEGER },
});
model.build({ id: 1, bar: 'bar' }).validate();
model.create({ id: 1, bar: 'baz' });

To Reproduce
Run the code above

What do you expect to happen?

When creating a Sequelize instance with typeValidation turned on, I expect that types are validated when calling the validate method on an instance. I expect output from the above code
to be two SequelizeValidationErrors - one saying '"bar" is not a valid integer' and one saying '"baz" is not a valid integer'.

What is actually happening?

Only one error way thrown - the one saying '"baz" is not a valid integer'.
This is because the InstanceValidator does not do anything with typeValidation, while the QueryGenerator (which isn't being used in the build/validate process) does.

Environment

Dialect:

  • mysql
  • postgres
  • sqlite
  • mssql
  • any
    Dialect library version: 10.5
    Database version: 10
    Sequelize version: 5.10.0
    Node Version: 10.16.0
    OS: Ubuntu 16.04

Tested with latest release:

  • No (but looking at the code on github, nothing seems to have changed with regards to typeValidation since 5.10.0)
  • Yes, specify that version:
@papb papb added status: understood For issues. Applied when the issue is understood / reproducible. type: bug type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference. status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action type: feature For issues and PRs. For new features. Never breaking changes. and removed status: understood For issues. Applied when the issue is understood / reproducible. type: bug type: docs For issues and PRs. Things related to documentation, such as changes in the manuals / API reference. labels Aug 1, 2019
@papb
Copy link
Member

papb commented Aug 1, 2019

Actually, I have just checked the docs, what is says for options.typeValidation is this:

Run built in type validators on insert and update, e.g. validate that arguments passed to integer fields are integer-like.

So apparently I think your example behaves like the docs say it should.

However, I agree this might be a valid change...

@papb papb added status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. and removed status: awaiting response For issues and PRs. OP must respond (or change something, if it is a PR). Maintainers have no action labels Aug 1, 2019
@simonbrent
Copy link
Author

I think the behaviour as it stands is problematic because all the write functions in model.js (save, update, upsert, bulkCreate) start by doing instance.validate if options.validate is true. It should therefore be possible to skip a lot of code if type validation fails, instead of finding out much later that the query is invalid.

@simonbrent
Copy link
Author

simonbrent commented Nov 3, 2020

I have come across another problem relating to this issue.

If you are doing bulkCreate with an array of data containing multiple entries with type validation errors, the way typeValidation works results in a ValidationError being thrown containing only the details of the first validation error, rather than a BulkRecordError like you get for other types of validation.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2021

This issue has been automatically marked as stale because it has been open for 7 days without activity. It will be closed if no further activity occurs. If this is still an issue, just leave a comment or remove the "stale" label. 🙂

@github-actions github-actions bot added the stale label Nov 8, 2021
@simonbrent
Copy link
Author

This is still an issue with version 6.9.0

@github-actions github-actions bot removed the stale label Nov 11, 2021
@ephys ephys mentioned this issue Oct 6, 2022
42 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants