update to activesupport 4.2 #94
Conversation
|
There's an issue with MysqlAesNew search on 4.2. It might be related to changes in scope, but I'm not sure. I'll investigate. |
|
Should be good now, I switched the |
| @@ -36,7 +36,7 @@ def decrypt(value) | |||
| # | |||
| # Returns an Enumerable | |||
| def search(records, field, criteria) | |||
| records.where(field.to_sym => encrypt(criteria)) | |||
| records.select { |record| record[field] == criteria } | |||
There was a problem hiding this comment.
This change requires every single record in the table to be loaded by active record, which isn't ideal. Any idea on the root cause?
There was a problem hiding this comment.
The query builder has changed a bit in 4.2, and .where(field.to_sym => encrypt(criteria)) doesn't get turned into the same query as it did in 4.1.x. The tests are passing around an array of classes and not specifically an AR collection, so it may behave differently.
I tried a variety of queries and the select is the only one that seems to pass. Any ideas @jmazzi? I'm happy to keep hacking at it.
|
👍 |
|
Can anyone figure out a better way to write this line? |
|
any progress on this fellas? In the process of upgrading an app and just realized this is a blocker for me. Thanks. |
|
@stve It's a blocker for my team as well, but I/we haven't had time to figure it out yet. The only issue is the Mysql aes search discussed above. If you have any thoughts, we can probably get it knocked out soon. (As long as the problem isn't with ActiveRecord or the query builder) |
|
I took a look at this and it seems that the encryption is executed automatically by ActiveRelation when it is constructing the query. Dumping out the SQL generated seemed to confirm this. The following should work fine: |
|
@Ch4s3 nice! can you squash it into one commit and document the change? I will release it right after merging. |
Ch4s3
force-pushed
the
support_rails_4_2
branch
from
8cbfebb
to
9b0b8f9
Compare
Ch4s3
force-pushed
the
support_rails_4_2
branch
from
9b0b8f9
to
3f3c803
Compare
|
@jmazzi Its squashed and I added a bit to the Readme that says tested against 4.2, I wasn't sure what/where else to document the changes. I also added MRI 2.2 to the travis matrix for AR 4.2, since I feel like most people using 4.2 will want to use 2.2. |
| gem.add_runtime_dependency 'activerecord', '>= 3.1', '< 4.2' | ||
| gem.add_runtime_dependency 'activesupport', '>= 3.1', '< 4.2' | ||
| gem.add_runtime_dependency 'activerecord', '>= 3.1', '<= 4.2' | ||
| gem.add_runtime_dependency 'activesupport', '>= 3.1', '<= 4.2' |
There was a problem hiding this comment.
@Ch4s3 is there a specific reason you went with <= over < 4.3? If not, I'm going to change that in master after merging.
There was a problem hiding this comment.
No specific reason, but I'm not sure if 4.3 will be a thing or if they're jumping straight to 5.0, I haven't been following AR/Rails too closely lately.
|
This fix was released as 0.19.0
|
No description provided.