search

coldbox-modules / quick

A ColdBox ORM Engine
https://quick.ortusbooks.com
MIT License
23 stars 19 forks source link

ACF compatibility: Force generated key to be an integer #201

Closed homestar9 closed 2 years ago

homestar9 commented 2 years ago

See https://github.com/coldbox-modules/quick/issues/200

elpete commented 2 years ago

Does the PR address your concerns raised here?

homestar9 commented 2 years ago

Yes it does. However, there's still a problem for those using big integers and ACF. Both val() and int() don't work for very big numbers. This might be an edge case though and I could see recommending they use a different autoincrement model designed to handle big integers.

elpete commented 2 years ago

What is the approach using Java's BigInteger? Could we do that?

homestar9 commented 2 years ago

What is the approach using Java's BigInteger? Could we do that?

I don't have any experience with java.math.BigInteger, but I'll play around with it to see if I can create something that works. I would be concerned about any possible overhead performance issues, though, since most users will be using normal integers.

homestar9 commented 2 years ago

BigInteger may not be the way to go. precisionEvaluate() may work, but doesn't solve the original intent of using val() to always return a number. Therefore, perhaps the better solution is something like this:

arguments.entity.assignAttribute( keyName,  ( isNumeric( generatedKey ) ? precisionEvaluate( generatedKey ) : val( generatedKey ) ) );

The above code should be tested to ensure it can handle a wide variety of numbers, but the idea here is that it first checks the generatedKey to see if it is numeric. If so, run precisionEvaluate() on it to return an actual numeric object. This should fix the value coming back as a string instead. If the generatedString is not a number, val() will turn it into 0 (or you can just return 0).

Still, I guess that precisionEvaluate() must come at a performance cost, so if it were me, I would probably set up a new AutoBigIncrementingKeyType.cfc that uses precisionEvaluate() to avoid any performance hit for most users. This PR will still work for most reasonably sized integers.

Cffiddle using precisionEvalute() https://cffiddle.org/app/file?filepath=1ee1557f-2357-4650-97bd-ced0fb178891/a16ff988-b7e1-471b-a13a-e4b8007c1101/c460398d-20cd-4dba-9304-5c7db5193498.cfm ( can't use trycf because it blocks precisionEvalute )

homestar9 commented 2 years ago

@elpete just following up on this issue as currently, anyone using ACF will be receiving decimals for primary keys. While CFML is smart enough to treat this as an integer, other languages are not. I had to create a few workarounds in my identifier code to allow other languages to digest primary keys returned by Quick JSON output.