Let cache handle marshalling of results.

[rovermicrover]

Also multi_fetch should use the splat operator.
http://api.rubyonrails.org/classes/ActiveSupport/Cache/Store.html#method-i-fetch_multi

[beauby]

Hi @rovermicrover – thanks for the PR. Would you mind splitting this into two? (One with the splat change, and one with the marshalling)

I'll merge the splat one immediately, but I'd like to discuss a bit more about the marshalling part. The idea is that most of the time is spent transforming a hash into a JSON string, hence the JSON fragments I introduced. What's your rationale behind the change?

[rovermicrover]

When we turned on caching the first issue we ran into was that it was thinking the array of cache keys was one giant cache key. Thus the splat operator.

Next once that was done the JSON result had instead of data being an array of objects, it was an array of strings of escaped JSON. This was just a fix to get things working, and thought I should make a PR for feedback.

Will submit a separate PR tomorrow for the splat operator and awaiting your feedback on the other part.

[beauby]

The splat part was clearly a bug.

Next once that was done the JSON result had instead of data being an array of objects, it was an array of strings of escaped JSON. This was just a fix to get things working, and thought I should make a PR for feedback.

I assume you're using rails. After some investigation, it seems ActiveSupport is putting its nasty fingers inside JSON generation. I'll think of a fix.

Edit: Problem actually comes from there.
Edit 2: Easy fix: use JSON.generate instead of #to_json in rails renderers (here).

[beauby]

@rovermicrover See my above edits. Would you mind issuing a PR with the mentioned fix?

[rovermicrover]

Still results in an array of escaped JSON strings.

https://github.com/rails/rails/blob/08754f12e65a9ec79633a605e986d0f1ffa4b251/activesupport/lib/active_support/json/encoding.rb#L79

Rails wraps all strings it turns into JSON in 'EscapedString's. This removes the to_json method on JSONString.

If we make JSONString not inherit from String then it will call as_json. We can't return self because then it end up in an endless loop. We could try tricking rails into thinking JSONString is a Numeric, NilClass, TrueClass, FalseClass but that just seems really hacky.

Also note marshing the objects from cache to hash to JSON takes 400 MS to go through for 250 objects. While cache to JSON takes like 300MS. Going to move forward with this PR version for for now on my project as it saves a lot of processing power even if it's not perfect. Not sure of the right solution for this.

[beauby]

Hi @rovermicrover – the relevant part for ActiveSupport's modification of the JSON gem behavior is there: https://github.com/rails/rails/blob/56c1326abb11ed275f04b6e0592ca66975e37f24/activesupport/lib/active_support/core_ext/object/json.rb#L23

As you can see, calling JSON.generate will prevent the ActiveSupport overrides to kick in. I did test the fix locally and it worked.

it saves a lot of processing power even if it's not perfect.

I'm glad to hear that (:

[beauby]

You may want to give jsonapi-rb/jsonapi-rails#70 a try.

[rovermicrover]

@beauby I think the issue is we are using rails 4

[beauby]

Oh I see. I’ll try to come up with a rails 4 compatible fix – do not hesitate if you have any suggestions in the meantime. Also,I’d like to setup Travis to test against rails 4 on the jsonapi-rails repo. If you have some extra free time and feel like issuing a PR for that, it would be super helpful and would get us started on the caching issue.

On Fri 17 Nov 2017 at 22:40, Andrew Rove (Rover) ***@***.***> wrote: @beauby <https://github.com/beauby> I think the issue is we are using rails 4 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#28 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACHPYp-0dSnlgzLGtJWLypM6GEE0BN7Eks5s3f1TgaJpZM4QfcJ8> .

-- Lucas Hosseini [email protected]