search

Shopify / identity_cache

IdentityCache is a blob level caching solution to plug into Active Record. Don't #find, #fetch!
http://shopify.github.io/identity_cache
MIT License
1.92k stars 173 forks source link

should inverse_of work? #357

Open roccoblues opened 6 years ago

roccoblues commented 6 years ago

Not sure if it's a misconfiguration on my side or if it's simply not supported.

Rails 5.1.4 identity_cache 0.5.1

Given:

class Person < ActiveRecord::Base
  include IdentityCache
  belongs_to :client, inverse_of: :persons
  cache_belongs_to :client
end

class Client < ActiveRecord::Base
  include IdentityCache
  has_many :persons, -> { where partner_id: nil }, dependent: :destroy, inverse_of: :client
  cache_has_many :persons
end

Without the cache the client isn't fetched again when we fetch persons on a client:

irb(main):002:0> client = Client.find(4)
  Client Load (0.5ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
=> #<Client id: 4, name: .... >
irb(main):003:0> client.persons.map{|p| p.client.object_id}
  Person Load (0.7ms)  SELECT `persons`.* FROM `persons` WHERE `persons`.`deleted_at` IS NULL AND `persons`.`client_id` = 4 AND `persons`.`partner_id` IS NULL
=> [70220559135500, 70220559135500, 70220559135500, 70220559135500, 70220559135500, 70220559135500, 70220559135500, 70220559135500]

With IdentityCache I get a client query for every person and different objects. Shouldn't IdentityCache simply setup the association to the existing client object?

irb(main):002:0> client = Client.find(4)
  Client Load (0.5ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
=> #<Client id: 4, name: .... >
irb(main):002:0> client.fetch_persons.map{|p| p.client.object_id}
   (1.2ms)  SELECT `persons`.`id` FROM `persons` WHERE `persons`.`deleted_at` IS NULL AND `persons`.`client_id` = 4 AND `persons`.`partner_id` IS NULL
Cache cas_multi: greenlight:1:IDC:7:blob:Person:7919784493729940308:9/IDC:7:blob:Person:7919784493729940308:10/IDC:7:blob:Person:7919784493729940308:11/IDC:7:blob:Person:7919784493729940308:3459/IDC:7:blob:Person:79197844937299403:md5:1e1faf6c2624ebf80c00411dbf35dcc9 ({:namespace=>"greenlight:1", :expires_in=>21600})
  Person Load (0.8ms)  SELECT `persons`.* FROM `persons` WHERE `persons`.`deleted_at` IS NULL AND `persons`.`id` IN (9, 10, 11, 3459, 4135, 4147, 4312, 4458)
Cache write: greenlight:1:IDC:7:blob:Person:7919784493729940308:9 ({:namespace=>"greenlight:1", :expires_in=>21600, :unless_exist=>true})
Cache write: greenlight:1:IDC:7:blob:Person:7919784493729940308:10 ({:namespace=>"greenlight:1", :expires_in=>21600, :unless_exist=>true})
Cache write: greenlight:1:IDC:7:blob:Person:7919784493729940308:11 ({:namespace=>"greenlight:1", :expires_in=>21600, :unless_exist=>true})
Cache write: greenlight:1:IDC:7:blob:Person:7919784493729940308:3459 ({:namespace=>"greenlight:1", :expires_in=>21600, :unless_exist=>true})
Cache write: greenlight:1:IDC:7:blob:Person:7919784493729940308:4135 ({:namespace=>"greenlight:1", :expires_in=>21600, :unless_exist=>true})
Cache write: greenlight:1:IDC:7:blob:Person:7919784493729940308:4147 ({:namespace=>"greenlight:1", :expires_in=>21600, :unless_exist=>true})
Cache write: greenlight:1:IDC:7:blob:Person:7919784493729940308:4312 ({:namespace=>"greenlight:1", :expires_in=>21600, :unless_exist=>true})
Cache write: greenlight:1:IDC:7:blob:Person:7919784493729940308:4458 ({:namespace=>"greenlight:1", :expires_in=>21600, :unless_exist=>true})
[IdentityCache] cache miss for IDC:7:blob:Person:7919784493729940308:9 (multi)
[IdentityCache] cache miss for IDC:7:blob:Person:7919784493729940308:10 (multi)
[IdentityCache] cache miss for IDC:7:blob:Person:7919784493729940308:11 (multi)
[IdentityCache] cache miss for IDC:7:blob:Person:7919784493729940308:3459 (multi)
[IdentityCache] cache miss for IDC:7:blob:Person:7919784493729940308:4135 (multi)
[IdentityCache] cache miss for IDC:7:blob:Person:7919784493729940308:4147 (multi)
[IdentityCache] cache miss for IDC:7:blob:Person:7919784493729940308:4312 (multi)
[IdentityCache] cache miss for IDC:7:blob:Person:7919784493729940308:4458 (multi)
  Client Load (0.6ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
  Client Load (0.5ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
  Client Load (0.6ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
  Client Load (0.5ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
  Client Load (0.6ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
  Client Load (0.5ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
  Client Load (0.5ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
  Client Load (0.5ms)  SELECT  `clients`.* FROM `clients` WHERE `clients`.`id` = 4 LIMIT 1
=> [70220565335420, 70220557688780, 70220557994400, 70220558006100, 70220589297320, 70220589903960, 70220589965920, 70220560291960]
dylanahsmith commented 6 years ago

This is working as intended.

irb(main):002:0> client.fetch_persons.map{|p| p.client.object_id}

You need to replace the Person#client call with Person#fetch_client if you want the cached record. Try this instead

irb(main):002:0> client.fetch_persons.map{|p| p.fetch_client.object_id}

roccoblues commented 6 years ago

That fetches the client from cache for every person and results in different client objects:

irb(main):002:0> client.fetch_persons.map{|p| p.fetch_client.object_id}
   (0.7ms)  SELECT `persons`.`id` FROM `persons` WHERE `persons`.`deleted_at` IS NULL AND `persons`.`client_id` = 4 AND `persons`.`partner_id` IS NULL
Cache cas_multi: greenlight:1:IDC:7:blob:Person:14507715566204489110:9/IDC:7:blob:Person:14507715566204489110:10/IDC:7:blob:Person:14507715566204489110:11/IDC:7:blob:Person:14507715566204489110:3459/IDC:7:blob:Person:1450771556620:md5:86dc7956880e3af3985af73a2db3aec2 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (backend) cache hit for IDC:7:blob:Person:14507715566204489110:9 (multi)
[IdentityCache] (backend) cache hit for IDC:7:blob:Person:14507715566204489110:10 (multi)
[IdentityCache] (backend) cache hit for IDC:7:blob:Person:14507715566204489110:11 (multi)
[IdentityCache] (backend) cache hit for IDC:7:blob:Person:14507715566204489110:3459 (multi)
[IdentityCache] (backend) cache hit for IDC:7:blob:Person:14507715566204489110:4135 (multi)
[IdentityCache] (backend) cache hit for IDC:7:blob:Person:14507715566204489110:4147 (multi)
[IdentityCache] (backend) cache hit for IDC:7:blob:Person:14507715566204489110:4312 (multi)
[IdentityCache] (backend) cache hit for IDC:7:blob:Person:14507715566204489110:4458 (multi)
Cache cas: greenlight:1:IDC:7:blob:Client:2431980387103363215:4 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (cache_backend) cache hit for IDC:7:blob:Client:2431980387103363215:4
Cache cas: greenlight:1:IDC:7:blob:Client:2431980387103363215:4 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (cache_backend) cache hit for IDC:7:blob:Client:2431980387103363215:4
Cache cas: greenlight:1:IDC:7:blob:Client:2431980387103363215:4 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (cache_backend) cache hit for IDC:7:blob:Client:2431980387103363215:4
Cache cas: greenlight:1:IDC:7:blob:Client:2431980387103363215:4 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (cache_backend) cache hit for IDC:7:blob:Client:2431980387103363215:4
Cache cas: greenlight:1:IDC:7:blob:Client:2431980387103363215:4 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (cache_backend) cache hit for IDC:7:blob:Client:2431980387103363215:4
Cache cas: greenlight:1:IDC:7:blob:Client:2431980387103363215:4 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (cache_backend) cache hit for IDC:7:blob:Client:2431980387103363215:4
Cache cas: greenlight:1:IDC:7:blob:Client:2431980387103363215:4 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (cache_backend) cache hit for IDC:7:blob:Client:2431980387103363215:4
Cache cas: greenlight:1:IDC:7:blob:Client:2431980387103363215:4 ({:namespace=>"greenlight:1", :expires_in=>21600})
[IdentityCache] (cache_backend) cache hit for IDC:7:blob:Client:2431980387103363215:4
=> [70107689539100, 70107689572200, 70107689434820, 70107689369160, 70107692043900, 70107689250420, 70107689202020, 70107666820400]

My problem is that I have some expensive associations already fetched in the client that I would like to reuse on person.client.[expensive_stuff] calls. That's why I want to have all person.client objects to be the same as the one we fetched the persons on.

I guess I can set that up myself with something like this:

irb(main):003:0> client.fetch_persons.map{|p| p.client = client; p}.map{|p| p.client.object_id}
=> [70107688039420, 70107688039420, 70107688039420, 70107688039420, 70107688039420, 70107688039420, 70107688039420, 70107688039420]

But I'm still wondering if it isn't something that IdentityCache could do. If you're open for it I could try to create a pull-request.

Thanks!

dylanahsmith commented 6 years ago

Oh I see. It looks like cache_hasmany with embed: :ids implements the `fetch#{association}` method by just memoizing a fetch_multi on the associated class.

Yes, it would make sense if the inverse cached association were set on the returned records.

By the way, I think that works for cache_has_many associations with the embed: true option.

roccoblues commented 6 years ago

I've started by adding tests for the inverse_of behaviour here: https://github.com/roccoblues/identity_cache/commit/01219cce14289431c57697a541c6bc4fa271b6e4

Do you agree that the two failing ones should be fixed?

  1) Failure:
PrefetchAssociationsTest#test_fetch_should_setup_association_for_cache_has_many [test/inverse_of_test.rb:38]:
Expected: 70228404239540
  Actual: 70228404877000

  2) Failure:
PrefetchAssociationsTest#test_fetch_should_setup_association_for_cache_has_many_embedded_ids [test/inverse_of_test.rb:60]:
Expected: 70228399614120
  Actual: 70228392031260
dylanahsmith commented 6 years ago

Yes, although you don't need both of those tests, since the only difference is that one tests the default value of the cache_has_many embed keyword argument.