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

Field validation occurs _before_ beforeCreate() #6890

Open
alxndrsn opened this issue Nov 12, 2019 · 8 comments
Open

Field validation occurs _before_ beforeCreate() #6890

alxndrsn opened this issue Nov 12, 2019 · 8 comments
Labels
needs documentation orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. question resolved

Comments

@alxndrsn
Copy link

Node version: 10
Sails version (sails): 1.2.3


It seems that field validation when calling SomeModel.create() is done before the beforeCreate() lifecycle callback.

This seems surprising, as it means that e.g.

  1. you cannot generate/calculate required values in beforeCreate(), and also
  2. as per Waterline allows NaN as a number field if set in beforeCreate() callback #6889, values set in beforeCreate() skip the sails type-based validation

Is this the intended behaviour? If so, I think it would be helpful to document it on https://sailsjs.com/documentation/concepts/models-and-orm/lifecycle-callbacks; if not, would a PR to move validation to after the beforeCreate() callback be considered?

@sailsbot
Copy link

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

@alxndrsn alxndrsn changed the title Cannot generate IDs in beforeCreate() Field validation occurs _before_ beforeCreate() Nov 12, 2019
@johnabrams7
Copy link
Contributor

johnabrams7 commented Nov 18, 2019

@alxndrsn Thanks for bringing this up as well. Also not entirely sure if this was intentional or just a bug - will have to team examine the logic behind this one as well.

@johnabrams7 johnabrams7 added orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. question labels Nov 18, 2019
@alxndrsn
Copy link
Author

Thanks, I look forward to hearing more 🙂

@rachaelshaw
Copy link
Member

@alxndrsn this is intentional, but it'd definitely be a good idea to update the docs clarifying when these things run.
As for generating required values, I'd recommend (depending on what you're trying to do) either (a) setting defaultsTo for the attribute and not making it required, or (b) using a helper that handles that logic before calling .create(). In our own projects, we pretty much always opt for using helpers rather than relying on lifecycle callbacks; it's a bit easier to keep track of that way, since some lifecycle callbacks (like afterCreate) only run when a record is created under certain conditions.

@danielsharvey
Copy link
Contributor

@rachaelshaw With reference to helpers, this seems to an entirely different mechanism to the lifecycle callbacks queried in this issue, which are designed to capture all operations, both Waterline Model operations and blueprints.

@alxndrsn I've facing a similar issue. If blueprints are what you are targeting, I've been looking at intercepting the process in parseBlueprintOptions(). This allows you to act before field validation.

@alxndrsn
Copy link
Author

alxndrsn commented Apr 7, 2020

@danielsharvey not using blueprints, but thanks for sharing your approach - hopefully helpful to others.

@rachaelshaw I guess I could use helpers 🤷‍♂️

My specific use case is using uuids instead of autoincremented numbers for database IDs, and it makes for a lot of noise in my code. I don't think this could be solved with defaultsTo.

I note that defaultsTo does not allow functions, and the relating documentation is explicitly misleading as per this bug report:

In the `id` attribute of model `thing`:
The `defaultsTo` property can no longer be specified as a function in Sails 1.0.  If you
need to calculate a value for the attribute before creating a record, try wrapping your
`create` logic in a helper (see http://sailsjs.com/docs/concepts/helpers) or using a lifecycle
hook (see https://sailsjs.com/documentation/concepts/models-and-orm/lifecycle-callbacks#callbacks-on-create).

@danielsharvey
Copy link
Contributor

danielsharvey commented Apr 10, 2020

@alxndrsn Understood. I am also using beforeCreate() to generate UUID-based IDs. I am using the value <new> passed in which then gets replaced in beforeCreate() by the generated UUID. It would be nice for this to be cleaner.

@whichking
Copy link
Contributor

Hey, @alxndrsn! Thanks for drawing our attention to that misleading error message; I've gone ahead and updated it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs documentation orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. question resolved
Development

No branches or pull requests

6 participants