coldbox-modules/cborm

Dynamic Finders Incorrectly Identifying Properties in compileHQLFromDynamicMethod()

Closed this issue · 5 comments

In BaseORMService.cfc around line 1055 is this chunk of code:

var methodGrammars = REMatchNoCase( "((?!(and|or|$))\w)+(#ALL_CONDITIONALS_REGEX#)?(and|or|$)", method );

Let's say you have an entity with properties of handle and sport and name and you attempt a Dynamic Finder like this findAllByHandleAndSportAndName('a','b','c');
Then it will split it up like this:

Array
1 Hand
2 leAnd
3 Spor
4 tAnd
5 Name

I think it should have done this:

Array
1 HandleAnd
2 SportAnd
3 Name

I haven't been able to fix that RegEx yet with the negative lookahead and would appreciate some assistance. @sigmaprojects did some related work in #4. Also there was some related discussion in the ColdBox Google Group a while back.

It's been a while since I looked into this, but unless the dynamic method
name starts to be cAsE sensitive to its easy to find "And" & "Or", to get
around this edge case use the workaround here:
https://ortussolutions.atlassian.net/browse/CCM-8
My only other idea would be to implement a new chaining method like so:
findAllBy('handle','a').and('sport','b').and('name','a')

On Mon, Aug 24, 2015 at 1:33 PM, Wesley Hampton notifications@github.com
wrote:

In BaseORMService.cfc around line 1055 is this chunk of code:

var methodGrammars = REMatchNoCase( "((?!(and|or|$))\w)+(#ALL_CONDITIONALS_REGEX#)?(and|or|$)", method );

Let's say you have an entity with properties of handle and sport and
name and you attempt a Dynamic Finder like this
findAllByHandleAndSportAndName('a','b','c');
Then it will split it up like this:
Array 1 Hand 2 leAnd 3 Spor 4 tAnd 5 Name

I think it should have done this:
Array 1 HandleAnd 2 SportAnd 3 Name

I haven't been able to fix that RegEx yet with the negative lookahead and
would appreciate some assistance. @sigmaprojects
https://github.com/sigmaprojects did some related work in #4
#4. Also there was some related
discussion in the ColdBox Google Group
https://groups.google.com/d/msg/coldbox/gP2d1K2Ce9Q/HdrxsczQSFEJ a
while back.


Reply to this email directly or view it on GitHub
#7.

Yes, those are defintely edge cases and the problem is that it is a continous line that get’s parsed.  So eiterh we make them case sensitive or we expand the DSL for this.  However, the nice .and() DSL, is nice, but looks more like the criteria queries we have already. How about adding _ separators to it.  Like this:

findAllByHandle_and_Sport_and_Name('a','b','c’);

Luis Majano
CEO
Ortus Solutions, Corp
www.ortussolutions.comP/F: 1-888-557-8057

Direct: (909) 248-3408

Linked In: http://www.linkedin.com/pub/3/731/483

Social: twitter.com/ortussolutions | twitter.com/coldbox | twitter.com/lmajano | twitter.com/gocontentbox

On Mon, Aug 24, 2015 at 3:44 PM, Don Quist notifications@github.com
wrote:

It's been a while since I looked into this, but unless the dynamic method
name starts to be cAsE sensitive to its easy to find "And" & "Or", to get
around this edge case use the workaround here:
https://ortussolutions.atlassian.net/browse/CCM-8
My only other idea would be to implement a new chaining method like so:
findAllBy('handle','a').and('sport','b').and('name','a')
On Mon, Aug 24, 2015 at 1:33 PM, Wesley Hampton notifications@github.com
wrote:

In BaseORMService.cfc around line 1055 is this chunk of code:

var methodGrammars = REMatchNoCase( "((?!(and|or|$))\w)+(#ALL_CONDITIONALS_REGEX#)?(and|or|$)", method );

Let's say you have an entity with properties of handle and sport and
name and you attempt a Dynamic Finder like this
findAllByHandleAndSportAndName('a','b','c');
Then it will split it up like this:
Array 1 Hand 2 leAnd 3 Spor 4 tAnd 5 Name

I think it should have done this:
Array 1 HandleAnd 2 SportAnd 3 Name

I haven't been able to fix that RegEx yet with the negative lookahead and
would appreciate some assistance. @sigmaprojects
https://github.com/sigmaprojects did some related work in #4
#4. Also there was some related
discussion in the ColdBox Google Group
https://groups.google.com/d/msg/coldbox/gP2d1K2Ce9Q/HdrxsczQSFEJ a
while back.


Reply to this email directly or view it on GitHub
#7.

I think if we change from regex to parsing the string one token at a time, we can easily work around this. It's just a matter of detecting if the next token in the string is a property name or an operator. Give property names preference and that should solve it.

Would #8 resolve this issue? I'm not confident of all the ways this method gets used but it seemed to resolve the problem I was having. I figured since we already know all the property names for the entity why not just use them in the regex?

I think this was resolved already, can you verify @wphampton