Shopify/identity_cache — pull request #132 by dylanahsmith (2014-03-06 21:25:10+00:00)
Title: Avoid preloading non-embedded associations when resolving cache misses.

@fbogsany & @arthurnn for review

## Problem

`fetch_multi` supports an `:includes` option to preloading associations with identity cache which aren't fully embedded. For example:

```ruby
User.fetch_multi(*user_ids, :includes => :friends)
```

would fetch the friends associations for all fetched user's using a single `fetch_multi` call to avoid an N+1.

This is similar to how active record allows associations to be preloaded, so previously we were using active record's preloading when IDC is disabled (i.e. `IdentityCache.should_cache?` is false) by passing the `:includes` option into `find_batch`.

Unfortunately, we were also passing the `:includes` option into `find_batch` when resolving cache misses, even though these associations get ignored when serializing the records to store into memcached, and lost after deserializing these records. This was likely unintentional since we were calling prefetch_associations on the deserialized records to fetch these additional associations.

## Solution

Avoiding passing in options to find_batch when resolving the cache miss solves this problem.

We can also use `prefetch_associations` even when caching is disabled, since it just uses `fetch_multi` to load the additional associations. By using this, we can completely remove the `options` argument from `find_batch`, and remove a lot of extra logic.

We actually had a test which was asserting the buggy behaviour, since it was heavily using mocha to test internals, which missed the high level problem. I added the spy gem to easily assert that fetch_multi was called for the cache_belongs_to association, since the associated class was the same class we are making the fetch_multi on in the test, so I couldn't stub out the Item.fetch_multi method.