Feature Request: Boolean Flag -> Disable Strict Attribute Name Checking for find()

[mdconaway]

Waterline version: 0.13.0-rc9
Node version: 6.2.0
NPM version: 3.6.0
Operating system: Windows/Linux

---

Hey Guys,

Sails/Waterline is pretty much my favorite server framework ever.

I have recently been upgrading an application to Sails 1.0, and Waterline 0.13.x, and have encountered a pretty severe functionality loss due to a single line of code in Waterline.

In Sails/Waterline 0.12.x it was possible to query using the natural find() method using the following syntax, if it was supported by the adapter:

Model.find({
'key.nested.name': 'something'
})...

This has quite a few benefits if the underlying adapter supports this syntax (sails-disk and sails-mongo both support this syntax at the moment), as I can generate arbitrarily complex queries in the client and let all of my normal generic crud actions take care of the rest.

After upgrading to Waterline 0.13.x, this feature is completely disabled due to the stage2 query forge.

I have traced the issue down to the function in this file:

https://github.com/balderdashy/waterline/blob/master/lib/waterline/utils/query/private/is-valid-attribute-name.js

Which invalidates the query key (key.nested.name) name due to the RX_IS_VALID_ECMASCRIPT_5_1_VAR_NAME regular expression.

Would it be possible to add a feature flag (even if it clearly states it is unsafe!) that would allow that regex to be disabled specifically for find operations?

I have managed to get Waterline 0.13.x and deep key searching working in my app with some really clever hacking of the require cache to give waterline a function without the regex, but it would be safer to disable this function only for finds.

I would be willing to make a pull request for this feature if someone could point me at the best spot to add this feature flag.

Thanks! And keep up the great work on Sails/Waterline!

[sailsbot]

Hi @mdconaway! 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:

• Verify "I am experiencing a concrete technical issue with Waterline (ideas and feature proposals should follow the guide for proposing features and enhancements (http://bit.ly/sails-feature-guide), which involves making a pull request). If you're not 100% certain whether it's a bug or not, that's okay--you may continue. The worst that can happen is that the issue will be closed and we'll point you in the right direction."
• Verify "I am not asking a question about how to use Waterline or about whether or not Waterline has a certain feature (please refer to the documentation(http://sailsjs.org), or post on http://stackoverflow.com, our Google Group (http://bit.ly/sails-google-group) or our live chat (https://gitter.im/balderdashy/sails)."
• Verify "I have already searched for related issues, and found none open (if you found a related closed issue, please link to it in your post)."
• Verify "My issue title is concise, on-topic and polite."
• Verify "I can provide steps to reproduce this issue that others can follow."

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]

[mdconaway]

Fixed initial description as per sails-bot recommendation.

[sailsbot]

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

[sailsbot]

@mdconaway,@sailsbot: Hello, I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message and simply close the issue if possible. On the other hand, if you are still waiting on a patch, please post a comment to keep the thread alive (with any new information you can provide).

If no further activity occurs on this thread within the next 3 days, the issue will automatically be closed.

Thanks so much for your help!

[mdconaway]

Still needs input from repo owner

[sailsbot]

@mdconaway,@sailsbot: Hello, I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message and simply close the issue if possible. On the other hand, if you are still waiting on a patch, please post a comment to keep the thread alive (with any new information you can provide).

If no further activity occurs on this thread within the next 3 days, the issue will automatically be closed.

Thanks so much for your help!

[mdconaway]

Bump

[sailsbot]

@mdconaway,@sailsbot: Hello, I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message and simply close the issue if possible. On the other hand, if you are still waiting on a patch, please post a comment to keep the thread alive (with any new information you can provide).

If no further activity occurs on this thread within the next 3 days, the issue will automatically be closed.

Thanks so much for your help!

[mdconaway]

Bump

[mikermcneil]

Hey @mdconaway -- while this was possible prior to WL 0.13, it (and other Mongo-specific usage like it) was never officially supported, and was the source of a lot of issues, questions, and confusion. So I'm pretty against weakening any of the stage 2 query forging stuff by default.

That said, I think it'd be ok for there to be an opt-in .meta() key that allows for skipping constraint normalization. Basically, we'd need to add a check here and have it skip constraint normalization altogether if .meta({skipConstraintNormalization: true}) is enabled.

//...
// If `skipConstraintNormalization` meta key is enabled, then skip constraint
// normalization within "where" clauses altogether.
else if (meta && meta.skipConstraintNormalization) {
  // Bail early.
  return;
}
// Otherwise we can go ahead and normalize the...
else {
  // ...

There are definitely other advantages to constraint normalization (you would have to fully expand some other things that waterline expands for you automatically, such as "in" constraints), making this a use-at-your-own-risk kind of feature. Also, please keep in mind that we'd need to pass through the meta info from the query to the normalizeWhere() utility (it's not currently being passed in). Also, remember that we have to tolerate the non-existence of the meta dictionary when doing the check. Finally (most importantly) note that we'd need to add the new skipConstraintNormalization meta key to the reference docs on the pages for all relevant model methods.

[mdconaway]

Hey @mikermcneil , thanks for the info!

I think it may actually be possible to enable find, findone and sort to allow mongo-style query syntax and also maintain all of the other niceties currently occurring in Waterline. (With the opt-in meta key of course!)

I will experiment with a fork of Waterline this weekend and try to put together a pull request for you guys to review.

Thanks again for making such a great framework!

[mdconaway]

Hey @mikermcneil and @sgress454 ,

What do you think of this batteries-included pull request?

balderdashy/waterline#1519

[mdconaway]

Also, if you guys can point me at the place in the docs that you would like this flag to be summarized, I will gladly update the documentation.

[sailsbot]

@mdconaway,@sailsbot,@mikermcneil: Hello, I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message and simply close the issue if possible. On the other hand, if you are still waiting on a patch, please post a comment to keep the thread alive (with any new information you can provide).

If no further activity occurs on this thread within the next 3 days, the issue will automatically be closed.

Thanks so much for your help!

[mdconaway]

Bump

[sailsbot]

@mdconaway,@sailsbot,@mikermcneil: Hello, I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message and simply close the issue if possible. On the other hand, if you are still waiting on a patch, please post a comment to keep the thread alive (with any new information you can provide).

If no further activity occurs on this thread within the next 3 days, the issue will automatically be closed.

Thanks so much for your help!