Use ActiveSupport::Cache.expand_cache_key for cache key expansions

[markiz]

Purpose

Allows using non-primitive objects as object cache key.
For example, we might want to return an array of objects that define #cache_key as our serialized object's #cache_key

Changes

join("/") changed to ActiveSupport::Cache.expand_cache_key

Caveats

Dependency on ActiveSupport (which we already have)

Additional helpful information

The test is a bit contrived, sorry about that.
#expand_cache_key can be redefined to change the actual implementation.

Previous version of AMS did use ActiveSupport::Cache.expand_cache_key

[mention-bot]

@markiz, thanks for your PR! By analyzing the annotation information on this pull request, we identified @bf4, @kevintyll and @beauby to be potential reviewers

[NullVoxPopuli]

what are the benefits of this? / what's the use case?

[markiz]

@NullVoxPopuli

We use non-primitive object as cache keys, another contrived example:

class User
  def cache_key
    [super, latest_post, latest_newsfeed_item, latest_friend_request] # where latest_post, latest_newsfeed_item, latest_friend_request are ActiveRecord models, possibly nil
  end
end

There wouldn't be any problem if AMS didn't try to join those.
In 0.8 it did the right thing with expand_cache_key
We can probably live with overridable expand_cache_key too (and override it to do what we want). But using join here breaks compat for us.

[bf4]

@markiz lgtm,

1. would you mind adding a change log?
2. would you mind looking at the object_cache_key methods in the Caching mixin?

[markiz]

@bf4:

1. Sure (4ce7dc0)
2. I'm not sure I follow. object_cache_key duplicates some of the functionality in the expand_cache_key, but it also has some differing behavior, so I don't see any immediate need to change it.

[NullVoxPopuli]

@markiz thanks for that. I didn't know this was an old feature, and I had no idea people even did this. Certainly looks looks better than building your own cache key string. :-)

[bf4]

@NullVoxPopuli yeah.. I'm not sure why it was removed. We use ActiveSupport::Cache which uses expand_cache_key but ¯_(ツ)_/¯

[bf4]

@markiz my only real concern is that these classes are really weird-looking. Would you mind adding some comments why you designed it this way? Be nice to future devs :)

[bf4]

see https://github.com/rails-api/active_model_serializers/pull/1878/files#r74627820